On Wed, 23 Oct 2019 at 18:34, Marc-André Lureau
<marcandre.lur...@redhat.com> wrote:
>
> Make serial IO a proper sysbus device, similar to serial MM.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> hw/char/serial.c | 58 ++++++++++++++++++++++++++++++++--------
> include/hw/char/serial.h | 13 +++++++--
> 2 files changed, 58 insertions(+), 13 deletions(-)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index a40bc3ab81..84de2740a7 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -986,22 +986,57 @@ const MemoryRegionOps serial_io_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> -SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> - Chardev *chr, MemoryRegion *system_io)
> +static void serial_io_realize(DeviceState *dev, Error **errp)
> {
> - DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
> - SerialState *s = SERIAL(dev);
> + SerialIO *self = SERIAL_IO(dev);
"sio" or something rather than "self".
> + SerialState *s = &self->serial;
>
> - s->irq = irq;
> - qdev_prop_set_uint32(dev, "baudbase", baudbase);
> - qdev_prop_set_chr(dev, "chardev", chr);
> - qdev_prop_set_int32(dev, "instance-id", base);
> - qdev_init_nofail(dev);
> + qdev_init_nofail(DEVICE(s));
>
> memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
> - memory_region_add_subregion(system_io, base, &s->io);
> + sysbus_init_irq(SYS_BUS_DEVICE(self), &self->serial.irq);
You could say '&s->irq' here, since you have the local variable.
> +}
> +
> +static void serial_io_class_init(ObjectClass *klass, void* data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = serial_io_realize;
For class methods where the class has no data that needs
to be migrated it's helpful to have a comment
/* No dc->vmsd: class has no migratable state */
(which lets us know that it's intentional and not a forgotten
thing). Some day I will get round to writing a patch so you
can say "dc->vmsd = no_migratable_state;" ...
> +}
> +
> +static void serial_io_instance_init(Object *o)
> +{
> + SerialIO *self = SERIAL_IO(o);
> +
> + object_initialize_child(o, "serial", &self->serial, sizeof(self->serial),
> + TYPE_SERIAL, &error_abort, NULL);
> +
> + qdev_alias_all_properties(DEVICE(&self->serial), o);
> +}
> +
> +
> +static const TypeInfo serial_io_info = {
> + .name = TYPE_SERIAL_IO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(SerialIO),
> + .instance_init = serial_io_instance_init,
> + .class_init = serial_io_class_init,
> +};
> +
> +SerialIO *serial_init(int base, qemu_irq irq, int baudbase,
> + Chardev *chr, MemoryRegion *system_io)
> +{
> + SerialIO *self = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO));
>
> - return s;
> + qdev_prop_set_uint32(DEVICE(self), "baudbase", baudbase);
> + qdev_prop_set_chr(DEVICE(self), "chardev", chr);
> + qdev_prop_set_int32(DEVICE(self), "instance-id", base);
> + qdev_init_nofail(DEVICE(self));
> +
> + sysbus_connect_irq(SYS_BUS_DEVICE(self), 0, irq);
> + memory_region_add_subregion(system_io, base, &self->serial.io);
> +
> + return self;
> }
thanks
-- PMM
