[Qemu-devel] [PATCH 0/2] milkymist-uart fixes

[Qemu-devel] [PATCH 0/2] milkymist-uart fixes

[Antony Pavlov]

[PATCH 1/2] milkymist-uart: use qemu_chr_fe_write_all() instead of
[PATCH 2/2] milkymist-uart: use Device::realize instead of

This patch series based on DIGIC support adding comments:
 http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg04748.html

[Qemu-devel] [PATCH 1/2] milkymist-uart: use qemu_chr_fe_write_all() instead of qemu_chr_fe_write()

[Antony Pavlov]

Here is a quote from the qemu-devel maillist:

<quote author="Peter Crosthwaite"
 url="http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg04748.html">
  qemu_chr_fe_write() is capable of returning 0
  to indicate EAGAIN (and friends) and you don't
  handle this.
</quote>

Just change it to qemu_chr_fe_write_all() to fix.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
CC: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
CC: Michael Walle <michael@walle.cc>
---
 hw/char/milkymist-uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/char/milkymist-uart.c b/hw/char/milkymist-uart.c
index 2e4b5c5..6e4bc20 100644
--- a/hw/char/milkymist-uart.c
+++ b/hw/char/milkymist-uart.c
@@ -124,7 +124,7 @@ static void uart_write(void *opaque, hwaddr addr, uint64_t value,
     switch (addr) {
     case R_RXTX:
         if (s->chr) {
-            qemu_chr_fe_write(s->chr, &ch, 1);
+            qemu_chr_fe_write_all(s->chr, &ch, 1);
         }
         s->regs[R_STAT] |= STAT_TX_EVT;
         break;
-- 
1.8.4.rc3

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

[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 <antonynpavlov@gmail.com>
CC: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
CC: Michael Walle <michael@walle.cc>
---
 hw/char/milkymist-uart.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

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);
 
     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;
 }
-- 
1.8.4.rc3

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 <antonynpavlov@gmail.com>
> CC: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> CC: Michael Walle <michael@walle.cc>
> ---
>  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 <peter.crosthwaite@xilinx.com>
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