On 25 April 2018 at 15:54, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi Peter,
>
> On 04/20/2018 11:52 AM, Peter Maydell wrote:
>> Change all the uses of serial_hds[] to go via the new
>> serial_hd() function. Code change produced with:
>> find hw -name '*.[ch]' | xargs sed -i -e
>> 's/serial_hds\[\([^]]*\)\]/serial_hd(\1)/g'
>>
>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>> --- a/hw/arm/mps2-tz.c
>> +++ b/hw/arm/mps2-tz.c
>> @@ -172,7 +172,7 @@ static MemoryRegion *make_uart(MPS2TZMachineState *mms,
>> void *opaque,
>> {
>> CMSDKAPBUART *uart = opaque;
>> int i = uart - &mms->uart[0];
>> - Chardev *uartchr = i < MAX_SERIAL_PORTS ? serial_hds[i] : NULL;
>> + Chardev *uartchr = i < MAX_SERIAL_PORTS ? serial_hd(i) : NULL;
>
> You can remove uartchr and directly use serial_hd(i).
These kinds of cleanup are in later patches in the series --
this one is kept strictly to the purely-mechanical sed change,
as otherwise it would be a bit awkward to review I thought.
>> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
>> index f68df56b97..75c44adf7d 100644
>> --- a/hw/arm/msf2-soc.c
>> +++ b/hw/arm/msf2-soc.c
>> @@ -138,10 +138,10 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc,
>> Error **errp)
>> system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
>>
>> for (i = 0; i < MSF2_NUM_UARTS; i++) {
>> - if (serial_hds[i]) {
>> + if (serial_hd(i)) {
>
> We can now remove this check, but maybe another series...
>
>> serial_mm_init(get_system_memory(), uart_addr[i], 2,
>> qdev_get_gpio_in(armv7m, uart_irq[i]),
>> - 115200, serial_hds[i], DEVICE_NATIVE_ENDIAN);
>> + 115200, serial_hd(i), DEVICE_NATIVE_ENDIAN);
>> }
I agree that's probably a good change to make, but it's also
a behavioural change (we would go from only creating serial
devices where -serial args are given, to always creating
them). I wanted to generally avoid having any behavioural
changes in this series.
>> @@ -3963,21 +3963,21 @@ struct omap_mpu_state_s
>> *omap310_mpu_init(MemoryRegion *system_memory,
>> omap_findclk(s, "uart1_ck"),
>> s->drq[OMAP_DMA_UART1_TX], s->drq[OMAP_DMA_UART1_RX],
>> "uart1",
>> - serial_hds[0]);
>> + serial_hd(0));
>> s->uart[1] = omap_uart_init(0xfffb0800,
>> qdev_get_gpio_in(s->ih[1], OMAP_INT_UART2),
>> omap_findclk(s, "uart2_ck"),
>> omap_findclk(s, "uart2_ck"),
>> s->drq[OMAP_DMA_UART2_TX], s->drq[OMAP_DMA_UART2_RX],
>> "uart2",
>> - serial_hds[0] ? serial_hds[1] : NULL);
>> + serial_hd(0) ? serial_hd(1) : NULL);
>
> This will need cleanup later...
It's a bit weird, but I don't think it's specifically wrong.
>> --- a/hw/mips/mips_mipssim.c
>> +++ b/hw/mips/mips_mipssim.c
>> @@ -213,8 +213,8 @@ mips_mipssim_init(MachineState *machine)
>>
>> /* A single 16450 sits at offset 0x3f8. It is attached to
>> MIPS CPU INT2, which is interrupt 4. */
>> - if (serial_hds[0])
>> - serial_init(0x3f8, env->irq[4], 115200, serial_hds[0],
>> + if (serial_hd(0))
>
> We will drop this if () later... This now misses brackets :/
Hazard of pure-mechanical-change cleanup is that it
doesn't include coding-style fixes, but it's no
worse than it was before.
thanks
-- PMM
