On Fri, Dec 20, 2019 at 5:57 PM Marc-André Lureau <marcandre.lur...@redhat.com> wrote: > > Memory mapped serial device is in fact a sysbus device. The following > patches will make use of sysbus facilities for resource and > registration. In particular, "serial-mm: use sysbus facilities" will > move internal serial realization to serial_mm_realize callback to > follow qdev best practices. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > hw/char/omap_uart.c | 2 +- > hw/char/serial.c | 66 +++++++++++++++++++++++++++++++--------- > hw/mips/boston.c | 2 +- > hw/mips/mips_malta.c | 2 +- > include/hw/char/serial.h | 20 +++++++++--- > 5 files changed, 70 insertions(+), 22 deletions(-) > > diff --git a/hw/char/omap_uart.c b/hw/char/omap_uart.c > index 13e4f43c4c..e8da933378 100644 > --- a/hw/char/omap_uart.c > +++ b/hw/char/omap_uart.c > @@ -27,7 +27,7 @@ > struct omap_uart_s { > MemoryRegion iomem; > hwaddr base; > - SerialState *serial; /* TODO */ > + SerialMM *serial; /* TODO */ > struct omap_target_agent_s *ta; > omap_clk fclk; > qemu_irq irq; > diff --git a/hw/char/serial.c b/hw/char/serial.c > index ec388f3876..905b84c25f 100644 > --- a/hw/char/serial.c > +++ b/hw/char/serial.c > @@ -1032,16 +1032,28 @@ static const TypeInfo serial_info = { > static uint64_t serial_mm_read(void *opaque, hwaddr addr, > unsigned size) > { > - SerialState *s = opaque; > - return serial_ioport_read(s, addr >> s->it_shift, 1); > + SerialMM *s = SERIAL_MM(opaque); > + return serial_ioport_read(&s->serial, addr >> s->it_shift, 1); > } > > static void serial_mm_write(void *opaque, hwaddr addr, > uint64_t value, unsigned size) > { > - SerialState *s = opaque; > + SerialMM *s = SERIAL_MM(opaque); > value &= 255; > - serial_ioport_write(s, addr >> s->it_shift, value, 1); > + serial_ioport_write(&s->serial, addr >> s->it_shift, value, 1); > +} > + > +static void serial_mm_realize(DeviceState *dev, Error **errp) > +{ > + SerialMM *s = SERIAL_MM(dev); > + Error *local_err = NULL; > + > + object_property_set_bool(OBJECT(&s->serial), true, "realized", > &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > } > > static const MemoryRegionOps serial_mm_ops[3] = { > @@ -1068,30 +1080,56 @@ static const MemoryRegionOps serial_mm_ops[3] = { > }, > }; > > -SerialState *serial_mm_init(MemoryRegion *address_space, > +SerialMM *serial_mm_init(MemoryRegion *address_space, > hwaddr base, int it_shift, > qemu_irq irq, int baudbase, > Chardev *chr, enum device_endian end) > { > - DeviceState *dev = DEVICE(object_new(TYPE_SERIAL)); > - SerialState *s = SERIAL(dev); > + SerialMM *smm = SERIAL_MM(qdev_create(NULL, TYPE_SERIAL_MM)); > + SerialState *s = &smm->serial; > > - s->it_shift = it_shift; > + smm->it_shift = it_shift; > s->irq = irq; > - qdev_prop_set_uint32(dev, "baudbase", baudbase); > - qdev_prop_set_chr(dev, "chardev", chr); > - qdev_set_legacy_instance_id(dev, base, 2); > - qdev_init_nofail(dev); > + qdev_prop_set_uint32(DEVICE(s), "baudbase", baudbase); > + qdev_prop_set_chr(DEVICE(s), "chardev", chr); > + qdev_set_legacy_instance_id(DEVICE(s), base, 2); > > - memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s, > + qdev_init_nofail(DEVICE(smm)); > + > + memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], smm, > "serial", 8 << it_shift); > memory_region_add_subregion(address_space, base, &s->io); > - return s; > + > + return smm; > +} > + > +static void serial_mm_instance_init(Object *o) > +{ > + SerialMM *smm = SERIAL_MM(o); > + > + object_initialize_child(o, "serial", &smm->serial, sizeof(smm->serial), > + TYPE_SERIAL, &error_abort, NULL); > } > > +static void serial_mm_class_init(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(oc); > + > + dc->realize = serial_mm_realize; > +} > + > +static const TypeInfo serial_mm_info = { > + .name = TYPE_SERIAL_MM, > + .parent = TYPE_SYS_BUS_DEVICE, > + .class_init = serial_mm_class_init, > + .instance_init = serial_mm_instance_init, > + .instance_size = sizeof(SerialMM), > +}; > + > static void serial_register_types(void) > { > type_register_static(&serial_info); > + type_register_static(&serial_mm_info); > } > > type_init(serial_register_types) > diff --git a/hw/mips/boston.c b/hw/mips/boston.c > index ca7d813a52..23fdd5ec6a 100644 > --- a/hw/mips/boston.c > +++ b/hw/mips/boston.c > @@ -50,7 +50,7 @@ typedef struct { > > MachineState *mach; > MIPSCPSState cps; > - SerialState *uart; > + SerialMM *uart; > > CharBackend lcd_display; > char lcd_content[8]; > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index 783cd99848..ea92e5e27d 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -83,7 +83,7 @@ typedef struct { > uint32_t i2csel; > CharBackend display; > char display_text[9]; > - SerialState *uart; > + SerialMM *uart; > bool display_inited; > } MaltaFPGAState; > > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h > index 548944b06a..730165347c 100644 > --- a/include/hw/char/serial.h > +++ b/include/hw/char/serial.h > @@ -57,7 +57,6 @@ typedef struct SerialState { > qemu_irq irq; > CharBackend chr; > int last_break_enable; > - int it_shift; > uint32_t baudbase; > uint32_t tsr_retry; > guint watch_tag; > @@ -80,6 +79,14 @@ typedef struct SerialState { > MemoryRegion io; > } SerialState; > > +typedef struct SerialMM { > + SysBusDevice parent; > + > + SerialState serial; > + > + int it_shift; > +} SerialMM; > + > extern const VMStateDescription vmstate_serial; > extern const MemoryRegionOps serial_io_ops; > > @@ -88,12 +95,15 @@ void serial_set_frequency(SerialState *s, uint32_t > frequency); > #define TYPE_SERIAL "serial" > #define SERIAL(s) OBJECT_CHECK(SerialState, (s), TYPE_SERIAL) > > +#define TYPE_SERIAL_MM "serial-mm" > +#define SERIAL_MM(s) OBJECT_CHECK(SerialMM, (s), TYPE_SERIAL_MM) > + > SerialState *serial_init(int base, qemu_irq irq, int baudbase, > Chardev *chr, MemoryRegion *system_io); > -SerialState *serial_mm_init(MemoryRegion *address_space, > - hwaddr base, int it_shift, > - qemu_irq irq, int baudbase, > - Chardev *chr, enum device_endian end); > +SerialMM *serial_mm_init(MemoryRegion *address_space, > + hwaddr base, int it_shift, > + qemu_irq irq, int baudbase, > + Chardev *chr, enum device_endian end); > > /* serial-isa.c */ > > -- > 2.24.0.308.g228f53135a > >
ping, thanks -- Marc-André Lureau