| Message ID | 20231019104929.16517-1-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/char/mcf_uart: Have mcf_uart_create() return DeviceState | expand |
Am Thu, 19 Oct 2023 12:49:29 +0200 schrieb Philippe Mathieu-Daudé <philmd@linaro.org>: > There is no point in having mcf_uart_init() demote the DeviceState > pointer and return a void one. Directly return the real typedef. > > mcf_uart_init() do both init + realize: rename as mcf_uart_create(). > > Similarly, mcf_uart_mm_init() do init / realize / mmap: rename as > mcf_uart_create_mmap(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/m68k/mcf.h | 4 ++-- > hw/char/mcf_uart.c | 13 +++++++------ > hw/m68k/mcf5206.c | 6 +++--- > hw/m68k/mcf5208.c | 6 +++--- > 4 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h > index 8cbd587bbf..5d9f876ffe 100644 > --- a/include/hw/m68k/mcf.h > +++ b/include/hw/m68k/mcf.h > @@ -10,8 +10,8 @@ uint64_t mcf_uart_read(void *opaque, hwaddr addr, > unsigned size); > void mcf_uart_write(void *opaque, hwaddr addr, > uint64_t val, unsigned size); > -void *mcf_uart_init(qemu_irq irq, Chardev *chr); > -void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chr); > +DeviceState *mcf_uart_create(qemu_irq irq, Chardev *chr); > +DeviceState *mcf_uart_create_mmap(hwaddr base, qemu_irq irq, Chardev *chr); > > /* mcf_intc.c */ > qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem, > diff --git a/hw/char/mcf_uart.c b/hw/char/mcf_uart.c > index 6fa4ac502c..f9cbc9bdc4 100644 > --- a/hw/char/mcf_uart.c > +++ b/hw/char/mcf_uart.c > @@ -342,25 +342,26 @@ static void mcf_uart_register(void) > > type_init(mcf_uart_register) > > -void *mcf_uart_init(qemu_irq irq, Chardev *chrdrv) > +DeviceState *mcf_uart_create(qemu_irq irq, Chardev *chrdrv) > { > - DeviceState *dev; > + DeviceState *dev; > > dev = qdev_new(TYPE_MCF_UART); > if (chrdrv) { > qdev_prop_set_chr(dev, "chardev", chrdrv); > } > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > - > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); > > return dev; > } > > -void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chrdrv) > +DeviceState *mcf_uart_create_mmap(hwaddr base, qemu_irq irq, Chardev *chrdrv) > { > - DeviceState *dev; > + DeviceState *dev; > > - dev = mcf_uart_init(irq, chrdrv); > + dev = mcf_uart_create(irq, chrdrv); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > + > + return dev; > } Changing the return type (and name) of mcf_uart_mm_init() seems to be unnecessary. Could you please drop that change? Thomas > diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c > index 2ab1b4f059..673861574c 100644 > --- a/hw/m68k/mcf5206.c > +++ b/hw/m68k/mcf5206.c > @@ -167,7 +167,7 @@ typedef struct { > MemoryRegion iomem; > qemu_irq *pic; > m5206_timer_state *timer[2]; > - void *uart[2]; > + DeviceState *uart[2]; > uint8_t scr; > uint8_t icr[14]; > uint16_t imr; /* 1 == interrupt is masked. */ > @@ -599,8 +599,8 @@ static void mcf5206_mbar_realize(DeviceState *dev, Error **errp) > s->pic = qemu_allocate_irqs(m5206_mbar_set_irq, s, 14); > s->timer[0] = m5206_timer_init(s->pic[9]); > s->timer[1] = m5206_timer_init(s->pic[10]); > - s->uart[0] = mcf_uart_init(s->pic[12], serial_hd(0)); > - s->uart[1] = mcf_uart_init(s->pic[13], serial_hd(1)); > + s->uart[0] = mcf_uart_create(s->pic[12], serial_hd(0)); > + s->uart[1] = mcf_uart_create(s->pic[13], serial_hd(1)); > s->cpu = M68K_CPU(qemu_get_cpu(0)); > } > > diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c > index be1033f84f..d22d8536db 100644 > --- a/hw/m68k/mcf5208.c > +++ b/hw/m68k/mcf5208.c > @@ -261,9 +261,9 @@ static void mcf5208evb_init(MachineState *machine) > /* Internal peripherals. */ > pic = mcf_intc_init(address_space_mem, 0xfc048000, cpu); > > - mcf_uart_mm_init(0xfc060000, pic[26], serial_hd(0)); > - mcf_uart_mm_init(0xfc064000, pic[27], serial_hd(1)); > - mcf_uart_mm_init(0xfc068000, pic[28], serial_hd(2)); > + mcf_uart_create_mmap(0xfc060000, pic[26], serial_hd(0)); > + mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1)); > + mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h index 8cbd587bbf..5d9f876ffe 100644 --- a/include/hw/m68k/mcf.h +++ b/include/hw/m68k/mcf.h @@ -10,8 +10,8 @@ uint64_t mcf_uart_read(void *opaque, hwaddr addr, unsigned size); void mcf_uart_write(void *opaque, hwaddr addr, uint64_t val, unsigned size); -void *mcf_uart_init(qemu_irq irq, Chardev *chr); -void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chr); +DeviceState *mcf_uart_create(qemu_irq irq, Chardev *chr); +DeviceState *mcf_uart_create_mmap(hwaddr base, qemu_irq irq, Chardev *chr); /* mcf_intc.c */ qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem, diff --git a/hw/char/mcf_uart.c b/hw/char/mcf_uart.c index 6fa4ac502c..f9cbc9bdc4 100644 --- a/hw/char/mcf_uart.c +++ b/hw/char/mcf_uart.c @@ -342,25 +342,26 @@ static void mcf_uart_register(void) type_init(mcf_uart_register) -void *mcf_uart_init(qemu_irq irq, Chardev *chrdrv) +DeviceState *mcf_uart_create(qemu_irq irq, Chardev *chrdrv) { - DeviceState *dev; + DeviceState *dev; dev = qdev_new(TYPE_MCF_UART); if (chrdrv) { qdev_prop_set_chr(dev, "chardev", chrdrv); } sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); return dev; } -void mcf_uart_mm_init(hwaddr base, qemu_irq irq, Chardev *chrdrv) +DeviceState *mcf_uart_create_mmap(hwaddr base, qemu_irq irq, Chardev *chrdrv) { - DeviceState *dev; + DeviceState *dev; - dev = mcf_uart_init(irq, chrdrv); + dev = mcf_uart_create(irq, chrdrv); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); + + return dev; } diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c index 2ab1b4f059..673861574c 100644 --- a/hw/m68k/mcf5206.c +++ b/hw/m68k/mcf5206.c @@ -167,7 +167,7 @@ typedef struct { MemoryRegion iomem; qemu_irq *pic; m5206_timer_state *timer[2]; - void *uart[2]; + DeviceState *uart[2]; uint8_t scr; uint8_t icr[14]; uint16_t imr; /* 1 == interrupt is masked. */ @@ -599,8 +599,8 @@ static void mcf5206_mbar_realize(DeviceState *dev, Error **errp) s->pic = qemu_allocate_irqs(m5206_mbar_set_irq, s, 14); s->timer[0] = m5206_timer_init(s->pic[9]); s->timer[1] = m5206_timer_init(s->pic[10]); - s->uart[0] = mcf_uart_init(s->pic[12], serial_hd(0)); - s->uart[1] = mcf_uart_init(s->pic[13], serial_hd(1)); + s->uart[0] = mcf_uart_create(s->pic[12], serial_hd(0)); + s->uart[1] = mcf_uart_create(s->pic[13], serial_hd(1)); s->cpu = M68K_CPU(qemu_get_cpu(0)); } diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c index be1033f84f..d22d8536db 100644 --- a/hw/m68k/mcf5208.c +++ b/hw/m68k/mcf5208.c @@ -261,9 +261,9 @@ static void mcf5208evb_init(MachineState *machine) /* Internal peripherals. */ pic = mcf_intc_init(address_space_mem, 0xfc048000, cpu); - mcf_uart_mm_init(0xfc060000, pic[26], serial_hd(0)); - mcf_uart_mm_init(0xfc064000, pic[27], serial_hd(1)); - mcf_uart_mm_init(0xfc068000, pic[28], serial_hd(2)); + mcf_uart_create_mmap(0xfc060000, pic[26], serial_hd(0)); + mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1)); + mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2)); mcf5208_sys_init(address_space_mem, pic);
There is no point in having mcf_uart_init() demote the DeviceState pointer and return a void one. Directly return the real typedef. mcf_uart_init() do both init + realize: rename as mcf_uart_create(). Similarly, mcf_uart_mm_init() do init / realize / mmap: rename as mcf_uart_create_mmap(). Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/m68k/mcf.h | 4 ++-- hw/char/mcf_uart.c | 13 +++++++------ hw/m68k/mcf5206.c | 6 +++--- hw/m68k/mcf5208.c | 6 +++--- 4 files changed, 15 insertions(+), 14 deletions(-)