Skip to contentSkip to conversation

milkymist-uart fixes

PATCH Superseded Antony Pavlov 2 participants 4 messages Aug 31, 2013

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/2 milkymist-uart: use qemu_chr_fe_write_all() instead of qemu_chr_fe_write()

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 <[email protected]>
CC Peter Crosthwaite <[email protected]>
CC Michael Walle <[email protected]>
---
 hw/char/milkymist-uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
hw/char/milkymist-uart.c +1 -1
index 2e4b5c5..6e4bc20 100644
@@ -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;
Signature · 1 line
1.8.4.rc3
author
#2 lore raw

PATCH 2/2 milkymist-uart: use Device::realize instead of SysBusDevice::init

In reply to 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 <[email protected]>
CC Peter Crosthwaite <[email protected]>
CC Michael Walle <[email protected]>
---
 hw/char/milkymist-uart.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
hw/char/milkymist-uart.c +4 -7
index 6e4bc20..66fba82 100644
@@ -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;
 }
Signature · 1 line
1.8.4.rc3
#3 lore raw

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

Am 31.08.2013 07:33, schrieb Antony Pavlov:

Quoted 7 lines

<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>

Quoted 6 lines
Signed-off-by Antony Pavlov <[email protected]>
CC Peter Crosthwaite <[email protected]>
CC Michael Walle <[email protected]>
---
 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 <[email protected]>

before your Sob to attribute that to him. (Subject is perfect, thanks.)

Quoted 10 lines
hw/char/milkymist-uart.c +1 -1
index 6e4bc20..66fba82 100644
@@ -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)

hw/char/milkymist-uart.c static void milkymist_uart_reset(DeviceState *d) · line 206

Quoted 10 lines
hw/char/milkymist-uart.c +2 -2
 {
     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

Quoted 21 lines
hw/char/milkymist-uart.c +1 -4
     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;
 }
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg