[Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Alistair Francis]

SysBusDevice::init is deprecated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index bf0c853..5a22a72 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
     uart_update_status(s);
 }
 
-static int cadence_uart_init(SysBusDevice *dev)
+static void candence_uart_realize(DeviceState *dev, Error **errp)
 {
     UartState *s = CADENCE_UART(dev);
 
-    memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000);
-    sysbus_init_mmio(dev, &s->iomem);
-    sysbus_init_irq(dev, &s->irq);
-
-    s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-            (QEMUTimerCB *)fifo_trigger_update, s);
-
-    s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
-
     s->chr = qemu_char_get_next_serial();
 
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive,
                               uart_event, s);
     }
+}
 
-    return 0;
+static void cadence_uart_init(Object *obj)
+{
+    UartState *s = CADENCE_UART(obj);
+
+    memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+    sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+    s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+            (QEMUTimerCB *)fifo_trigger_update, s);
+
+    s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
 }
 
 static int cadence_uart_post_load(void *opaque, int version_id)
@@ -520,9 +523,8 @@ static const VMStateDescription vmstate_cadence_uart = {
 static void cadence_uart_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
 
-    sdc->init = cadence_uart_init;
+    dc->realize = candence_uart_realize;
     dc->vmsd = &vmstate_cadence_uart;
     dc->reset = cadence_uart_reset;
 }
@@ -531,6 +533,7 @@ static const TypeInfo cadence_uart_info = {
     .name          = TYPE_CADENCE_UART,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(UartState),
+    .instance_init = cadence_uart_init,
     .class_init    = cadence_uart_class_init,
 };
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Peter Crosthwaite]

On Tue, Jun 24, 2014 at 4:06 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> SysBusDevice::init is deprecated. Convert to Object::init and
> Device::realize as prescribed by QOM conventions.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

CC Peter for target-arm.

Regards,
Peter

> ---
>
>  hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>  1 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index bf0c853..5a22a72 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>      uart_update_status(s);
>  }
>
> -static int cadence_uart_init(SysBusDevice *dev)
> +static void candence_uart_realize(DeviceState *dev, Error **errp)
>  {
>      UartState *s = CADENCE_UART(dev);
>
> -    memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000);
> -    sysbus_init_mmio(dev, &s->iomem);
> -    sysbus_init_irq(dev, &s->irq);
> -
> -    s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> -            (QEMUTimerCB *)fifo_trigger_update, s);
> -
> -    s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
> -
>      s->chr = qemu_char_get_next_serial();
>
>      if (s->chr) {
>          qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive,
>                                uart_event, s);
>      }
> +}
>
> -    return 0;
> +static void cadence_uart_init(Object *obj)
> +{
> +    UartState *s = CADENCE_UART(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +    sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> +
> +    s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> +            (QEMUTimerCB *)fifo_trigger_update, s);
> +
> +    s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
>  }
>
>  static int cadence_uart_post_load(void *opaque, int version_id)
> @@ -520,9 +523,8 @@ static const VMStateDescription vmstate_cadence_uart = {
>  static void cadence_uart_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>
> -    sdc->init = cadence_uart_init;
> +    dc->realize = candence_uart_realize;
>      dc->vmsd = &vmstate_cadence_uart;
>      dc->reset = cadence_uart_reset;
>  }
> @@ -531,6 +533,7 @@ static const TypeInfo cadence_uart_info = {
>      .name          = TYPE_CADENCE_UART,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(UartState),
> +    .instance_init = cadence_uart_init,
>      .class_init    = cadence_uart_class_init,
>  };
>
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Peter Maydell]

On 27 June 2014 01:11, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Tue, Jun 24, 2014 at 4:06 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> SysBusDevice::init is deprecated. Convert to Object::init and
>> Device::realize as prescribed by QOM conventions.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> CC Peter for target-arm.

I think at this point given we're quite close to hardfreeze
I'd prefer not to take this, since it's just cleanup.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Peter Maydell]

On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
> SysBusDevice::init is deprecated. Convert to Object::init and
> Device::realize as prescribed by QOM conventions.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>  1 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index bf0c853..5a22a72 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>      uart_update_status(s);
>  }
>
> -static int cadence_uart_init(SysBusDevice *dev)
> +static void candence_uart_realize(DeviceState *dev, Error **errp)

Typo in your new function name :-)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Andreas Färber]

Am 03.07.2014 18:46, schrieb Peter Maydell:
> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> SysBusDevice::init is deprecated. Convert to Object::init and

Note that there is no Object::init, only TypeInfo::instance_init.

>> Device::realize as prescribed by QOM conventions.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>>  1 files changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> index bf0c853..5a22a72 100644
>> --- a/hw/char/cadence_uart.c
>> +++ b/hw/char/cadence_uart.c
>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>>      uart_update_status(s);
>>  }
>>
>> -static int cadence_uart_init(SysBusDevice *dev)
>> +static void candence_uart_realize(DeviceState *dev, Error **errp)
> 
> Typo in your new function name :-)
> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

LGTM, but I wonder whether the work Alex is doing on SysBusDevice
requires us to introduce SysBusDevice::realize, called from
Device::realize in SysBusDevice code?

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Peter Crosthwaite]

On Fri, Jul 4, 2014 at 2:59 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 03.07.2014 18:46, schrieb Peter Maydell:
>> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> SysBusDevice::init is deprecated. Convert to Object::init and
>
> Note that there is no Object::init, only TypeInfo::instance_init.
>

So I've been using these C++ish ::s in commentry liberally without
worrying about their literal correctness. TypeInfo requires a bit of
QOM internal understanding to get the point, whearas a much wider
audience has awarness of the Object type and the fact that is has an
init. If anything I would just soften to plain english - "object
init".

>>> Device::realize as prescribed by QOM conventions.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>>  hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>>>  1 files changed, 16 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>>> index bf0c853..5a22a72 100644
>>> --- a/hw/char/cadence_uart.c
>>> +++ b/hw/char/cadence_uart.c
>>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>>>      uart_update_status(s);
>>>  }
>>>
>>> -static int cadence_uart_init(SysBusDevice *dev)
>>> +static void candence_uart_realize(DeviceState *dev, Error **errp)
>>
>> Typo in your new function name :-)
>>

Will fix v2 (i'll respin this one)

>> Otherwise
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>

Thanks,

Regards,
Peter

> LGTM, but I wonder whether the work Alex is doing on SysBusDevice
> requires us to introduce SysBusDevice::realize, called from
> Device::realize in SysBusDevice code?
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Alexander Graf]

On 03.07.14 18:59, Andreas Färber wrote:
> Am 03.07.2014 18:46, schrieb Peter Maydell:
>> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> SysBusDevice::init is deprecated. Convert to Object::init and
> Note that there is no Object::init, only TypeInfo::instance_init.
>
>>> Device::realize as prescribed by QOM conventions.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>>   hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>>>   1 files changed, 16 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>>> index bf0c853..5a22a72 100644
>>> --- a/hw/char/cadence_uart.c
>>> +++ b/hw/char/cadence_uart.c
>>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>>>       uart_update_status(s);
>>>   }
>>>
>>> -static int cadence_uart_init(SysBusDevice *dev)
>>> +static void candence_uart_realize(DeviceState *dev, Error **errp)
>> Typo in your new function name :-)
>>
>> Otherwise
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> LGTM, but I wonder whether the work Alex is doing on SysBusDevice
> requires us to introduce SysBusDevice::realize, called from
> Device::realize in SysBusDevice code?

I don't think we need a realize function with my patch set - all work 
happens either during the creation phase (generate hint properties) or 
in the machine, after realize has successfully passed.


Alex

Re: [Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()

[Alistair Francis]

On Fri, Jul 4, 2014 at 9:17 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Fri, Jul 4, 2014 at 2:59 AM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 03.07.2014 18:46, schrieb Peter Maydell:
>>> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>>> SysBusDevice::init is deprecated. Convert to Object::init and
>>
>> Note that there is no Object::init, only TypeInfo::instance_init.
>>
>
> So I've been using these C++ish ::s in commentry liberally without
> worrying about their literal correctness. TypeInfo requires a bit of
> QOM internal understanding to get the point, whearas a much wider
> audience has awarness of the Object type and the fact that is has an
> init. If anything I would just soften to plain english - "object
> init".
>
>>>> Device::realize as prescribed by QOM conventions.
>>>>
>>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>>> ---
>>>>
>>>>  hw/char/cadence_uart.c |   29 ++++++++++++++++-------------
>>>>  1 files changed, 16 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>>>> index bf0c853..5a22a72 100644
>>>> --- a/hw/char/cadence_uart.c
>>>> +++ b/hw/char/cadence_uart.c
>>>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
>>>>      uart_update_status(s);
>>>>  }
>>>>
>>>> -static int cadence_uart_init(SysBusDevice *dev)
>>>> +static void candence_uart_realize(DeviceState *dev, Error **errp)
>>>
>>> Typo in your new function name :-)
>>>
>
> Will fix v2 (i'll respin this one)

What ever happened to this? I can fix the typo and resend

>
>>> Otherwise
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>
> Thanks,
>
> Regards,
> Peter
>
>> LGTM, but I wonder whether the work Alex is doing on SysBusDevice
>> requires us to introduce SysBusDevice::realize, called from
>> Device::realize in SysBusDevice code?
>>
>> Regards,
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>>
>