Re: [Qemu-devel] [PATCH 2/2] milkymist-uart: use Device::realize instead of SysBusDevice::init

[Andreas Färber]

Am 31.08.2013 07:33, schrieb Antony Pavlov:
> <quote author="Peter Crosthwaite"
>  url="http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg04748.html";>
>   Use of SysBusDevice::init is deprecated. Please use
>   Device::realize instead of SysBusDevice::init.
>   Check dma/pl330.c for an example of the pattern.
> </quote>
> 
> Signed-off-by: Antony Pavlov <address@hidden>
> CC: Peter Crosthwaite <address@hidden>
> CC: Michael Walle <address@hidden>
> ---
>  hw/char/milkymist-uart.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Thanks for looking into this! Some small issues...

Please replace the XML quote above with a proper textual description,
such as "Use of SysBusDevice::init is deprecated, use Device::realize
instead." You can add a line
Reported-by: Peter Crosthwaite <address@hidden>
before your Sob to attribute that to him. (Subject is perfect, thanks.)

> diff --git a/hw/char/milkymist-uart.c b/hw/char/milkymist-uart.c
> index 6e4bc20..66fba82 100644
> --- a/hw/char/milkymist-uart.c
> +++ b/hw/char/milkymist-uart.c
> @@ -195,22 +195,20 @@ static void milkymist_uart_reset(DeviceState *d)
>      s->regs[R_STAT] = STAT_THRE;
>  }
>  
> -static int milkymist_uart_init(SysBusDevice *dev)
> +static void milkymist_uart_realize(DeviceState *dev, Error **errp)
>  {
>      MilkymistUartState *s = MILKYMIST_UART(dev);
>  
> -    sysbus_init_irq(dev, &s->irq);
> +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>  
>      memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
>                            "milkymist-uart", R_MAX * 4);
> -    sysbus_init_mmio(dev, &s->regs_region);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->regs_region);

Please avoid repeated casts by using a local sbd variable, ordered from
most abstract to concrete. However, since s->irq and R_MAX don't seem to
depend on user-settable properties, please move all of the above -
sysbus_init_irq(), memory_region_init_io() and sysbus_init_mmio() - into
a TypeInfo::instance_init function named milkymist_uart_init(Object
*obj) above or below ..._realize.

Otherwise patch looks good. Please CC me on v2 and I'll queue this one
on the qom-next tree.

Regards,
Andreas

>  
>      s->chr = qemu_char_get_next_serial();
>      if (s->chr) {
>          qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
>      }
> -
> -    return 0;
>  }
>  
>  static const VMStateDescription vmstate_milkymist_uart = {
> @@ -227,9 +225,8 @@ static const VMStateDescription vmstate_milkymist_uart = {
>  static void milkymist_uart_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = milkymist_uart_init;
> +    dc->realize = milkymist_uart_realize;
>      dc->reset = milkymist_uart_reset;
>      dc->vmsd = &vmstate_milkymist_uart;
>  }

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg