[Qemu-devel] [PATCH 0/6] Drop the qemu_char_get_next_serial function

[Qemu-devel] [PATCH 0/6] Drop the qemu_char_get_next_serial function

[xiaoqiang zhao]

This patch series's main idea is to drop the use of
qemu_char_get_next_serial. patches 1-5 QOM'ify the related 
device model and the affected board code. Most of them
are arm related. The patch 6 get rid of the function from
the code base.

I only test the code with ARM versatilepb machine. The other
machine need test!

xiaoqiang zhao (6):
  hw/char: QOM'ify pl011 model
  hw/char: QOM'ify cadence_uart model
  hw/char: QOM'ify digic-uart model
  hw/char: QOM'ify stm32f2xx_usart model
  hw/char: QOM'ify xilinx_uartlite model
  char: get rid of qemu_char_get_next_serial

 hw/arm/bcm2835_peripherals.c             | 16 ++--------
 hw/arm/digic.c                           |  2 ++
 hw/arm/highbank.c                        |  3 +-
 hw/arm/integratorcp.c                    |  5 +--
 hw/arm/realview.c                        |  9 +++---
 hw/arm/stellaris.c                       |  6 ++--
 hw/arm/stm32f205_soc.c                   |  1 +
 hw/arm/versatilepb.c                     |  9 +++---
 hw/arm/vexpress.c                        |  9 +++---
 hw/arm/virt.c                            |  1 +
 hw/arm/xilinx_zynq.c                     |  5 +--
 hw/arm/xlnx-zynqmp.c                     |  1 +
 hw/char/cadence_uart.c                   | 13 ++++----
 hw/char/digic-uart.c                     | 10 +++---
 hw/char/pl011.c                          | 11 ++++---
 hw/char/stm32f2xx_usart.c                | 11 ++++---
 hw/char/xilinx_uartlite.c                | 10 +++---
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  5 +--
 include/hw/char/cadence_uart.h           | 17 +++++++++++
 include/hw/char/pl011.h                  | 52 ++++++++++++++++++++++++++++++++
 include/hw/char/xilinx_uartlite.h        | 35 +++++++++++++++++++++
 include/sysemu/char.h                    |  1 -
 qemu-char.c                              | 16 ----------
 23 files changed, 173 insertions(+), 75 deletions(-)
 create mode 100644 include/hw/char/pl011.h
 create mode 100644 include/hw/char/xilinx_uartlite.h

-- 
2.1.4

[Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[xiaoqiang zhao]

* drop qemu_char_get_next_serial and use chardev prop
* add pl011_create wrapper function to create pl011 uart device
* change affected board code to use the new way

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/arm/bcm2835_peripherals.c | 16 +++-----------
 hw/arm/highbank.c            |  3 ++-
 hw/arm/integratorcp.c        |  5 +++--
 hw/arm/realview.c            |  9 ++++----
 hw/arm/stellaris.c           |  6 +++--
 hw/arm/versatilepb.c         |  9 ++++----
 hw/arm/vexpress.c            |  9 ++++----
 hw/arm/virt.c                |  1 +
 hw/char/pl011.c              | 11 +++++-----
 include/hw/char/pl011.h      | 52 ++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 86 insertions(+), 35 deletions(-)
 create mode 100644 include/hw/char/pl011.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 234d518..46c320f 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -14,6 +14,7 @@
 #include "hw/misc/bcm2835_mbox_defs.h"
 #include "hw/arm/raspi_platform.h"
 #include "sysemu/char.h"
+#include "sysemu/sysemu.h"
 
 /* Peripheral base address on the VC (GPU) system bus */
 #define BCM2835_VC_PERI_BASE 0x7e000000
@@ -106,7 +107,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     MemoryRegion *ram;
     Error *err = NULL;
     uint32_t ram_size, vcram_size;
-    CharDriverState *chr;
     int n;
 
     obj = object_property_get_link(OBJECT(dev), "ram", &err);
@@ -147,6 +147,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
 
     /* UART0 */
+    qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hds[0]);
     object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
@@ -158,17 +159,8 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(s->uart0, 0,
         qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
                                INTERRUPT_UART));
-
     /* AUX / UART1 */
-    /* TODO: don't call qemu_char_get_next_serial() here, instead set
-     * chardev properties for each uart at the board level, once pl011
-     * (uart0) has been updated to avoid qemu_char_get_next_serial()
-     */
-    chr = qemu_char_get_next_serial();
-    if (chr == NULL) {
-        chr = qemu_chr_new("bcm2835.uart1", "null", NULL);
-    }
-    qdev_prop_set_chr(DEVICE(&s->aux), "chardev", chr);
+    qdev_prop_set_chr(DEVICE(&s->aux), "chardev", serial_hds[1]);
 
     object_property_set_bool(OBJECT(&s->aux), true, "realized", &err);
     if (err) {
@@ -292,8 +284,6 @@ static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = bcm2835_peripherals_realize;
-    /* Reason: realize() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo bcm2835_peripherals_type_info = {
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 41029a6..80e5fd4 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -30,6 +30,7 @@
 #include "sysemu/block-backend.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
+#include "hw/char/pl011.h"
 
 #define SMP_BOOT_ADDR           0x100
 #define SMP_BOOT_REG            0x40
@@ -326,7 +327,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     busdev = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(busdev, 0, 0xfff34000);
     sysbus_connect_irq(busdev, 0, pic[18]);
-    sysbus_create_simple("pl011", 0xfff36000, pic[20]);
+    pl011_create(0xfff36000, pic[20], serial_hds[0]);
 
     dev = qdev_create(NULL, "highbank-regs");
     qdev_init_nofail(dev);
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 24f1687..96dc150 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -20,6 +20,7 @@
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
+#include "hw/char/pl011.h"
 
 #define TYPE_INTEGRATOR_CM "integrator_core"
 #define INTEGRATOR_CM(obj) \
@@ -588,8 +589,8 @@ static void integratorcp_init(MachineState *machine)
     sysbus_create_varargs("integrator_pit", 0x13000000,
                           pic[5], pic[6], pic[7], NULL);
     sysbus_create_simple("pl031", 0x15000000, pic[8]);
-    sysbus_create_simple("pl011", 0x16000000, pic[1]);
-    sysbus_create_simple("pl011", 0x17000000, pic[2]);
+    pl011_create(0x16000000, pic[1], serial_hds[0]);
+    pl011_create(0x17000000, pic[2], serial_hds[1]);
     icp = sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000,
                                qdev_get_gpio_in(sic, 3));
     sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 3222b36..7d0aa6f 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -23,6 +23,7 @@
 #include "sysemu/block-backend.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
+#include "hw/char/pl011.h"
 
 #define SMP_BOOT_ADDR 0xe0000000
 #define SMP_BOOTREG_ADDR 0x10000030
@@ -202,10 +203,10 @@ static void realview_init(MachineState *machine,
     sysbus_create_simple("pl050_keyboard", 0x10006000, pic[20]);
     sysbus_create_simple("pl050_mouse", 0x10007000, pic[21]);
 
-    sysbus_create_simple("pl011", 0x10009000, pic[12]);
-    sysbus_create_simple("pl011", 0x1000a000, pic[13]);
-    sysbus_create_simple("pl011", 0x1000b000, pic[14]);
-    sysbus_create_simple("pl011", 0x1000c000, pic[15]);
+    pl011_create(0x10009000, pic[12], serial_hds[0]);
+    pl011_create(0x1000a000, pic[13], serial_hds[1]);
+    pl011_create(0x1000b000, pic[14], serial_hds[2]);
+    pl011_create(0x1000c000, pic[15], serial_hds[3]);
 
     /* DMA controller is optional, apparently.  */
     sysbus_create_simple("pl081", 0x10030000, pic[24]);
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 4459171..794a3ad 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -20,6 +20,7 @@
 #include "qemu/log.h"
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
+#include "hw/char/pl011.h"
 
 #define GPIO_A 0
 #define GPIO_B 1
@@ -1303,8 +1304,9 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
 
     for (i = 0; i < 4; i++) {
         if (board->dc2 & (1 << i)) {
-            sysbus_create_simple("pl011_luminary", 0x4000c000 + i * 0x1000,
-                                 qdev_get_gpio_in(nvic, uart_irq[i]));
+            pl011_luminary_create(0x4000c000 + i * 0x1000,
+                                  qdev_get_gpio_in(nvic, uart_irq[i]),
+                                  serial_hds[i]);
         }
     }
     if (board->dc2 & (1 << 4)) {
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index d079bc9..20dd356 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -23,6 +23,7 @@
 #include "exec/address-spaces.h"
 #include "hw/block/flash.h"
 #include "qemu/error-report.h"
+#include "hw/char/pl011.h"
 
 #define VERSATILE_FLASH_ADDR 0x34000000
 #define VERSATILE_FLASH_SIZE (64 * 1024 * 1024)
@@ -284,10 +285,10 @@ static void versatile_init(MachineState *machine, int board_id)
         n--;
     }
 
-    sysbus_create_simple("pl011", 0x101f1000, pic[12]);
-    sysbus_create_simple("pl011", 0x101f2000, pic[13]);
-    sysbus_create_simple("pl011", 0x101f3000, pic[14]);
-    sysbus_create_simple("pl011", 0x10009000, sic[6]);
+    pl011_create(0x101f1000, pic[12], serial_hds[0]);
+    pl011_create(0x101f2000, pic[13], serial_hds[1]);
+    pl011_create(0x101f3000, pic[14], serial_hds[2]);
+    pl011_create(0x10009000, sic[6], serial_hds[3]);
 
     sysbus_create_simple("pl080", 0x10130000, pic[17]);
     sysbus_create_simple("sp804", 0x101e2000, pic[4]);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 70b3e70..58760f4 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -39,6 +39,7 @@
 #include "sysemu/device_tree.h"
 #include "qemu/error-report.h"
 #include <libfdt.h>
+#include "hw/char/pl011.h"
 
 #define VEXPRESS_BOARD_ID 0x8e0
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
@@ -631,10 +632,10 @@ static void vexpress_common_init(MachineState *machine)
     sysbus_create_simple("pl050_keyboard", map[VE_KMI0], pic[12]);
     sysbus_create_simple("pl050_mouse", map[VE_KMI1], pic[13]);
 
-    sysbus_create_simple("pl011", map[VE_UART0], pic[5]);
-    sysbus_create_simple("pl011", map[VE_UART1], pic[6]);
-    sysbus_create_simple("pl011", map[VE_UART2], pic[7]);
-    sysbus_create_simple("pl011", map[VE_UART3], pic[8]);
+    pl011_create(map[VE_UART0], pic[5], serial_hds[0]);
+    pl011_create(map[VE_UART1], pic[6], serial_hds[1]);
+    pl011_create(map[VE_UART2], pic[7], serial_hds[2]);
+    pl011_create(map[VE_UART3], pic[8], serial_hds[3]);
 
     sysbus_create_simple("sp804", map[VE_TIMER01], pic[2]);
     sysbus_create_simple("sp804", map[VE_TIMER23], pic[3]);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e77ed88..883d89c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -536,6 +536,7 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic, int uart,
     DeviceState *dev = qdev_create(NULL, "pl011");
     SysBusDevice *s = SYS_BUS_DEVICE(dev);
 
+    qdev_prop_set_chr(dev, "chardev", serial_hds[0]);
     qdev_init_nofail(dev);
     memory_region_add_subregion(mem, base,
                                 sysbus_mmio_get_region(s, 0));
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 6876ea6..c0fbf8a 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -274,6 +274,11 @@ static const VMStateDescription vmstate_pl011 = {
     }
 };
 
+static Property pl011_properties[] = {
+    DEFINE_PROP_CHR("chardev", PL011State, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pl011_init(Object *obj)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
@@ -295,9 +300,6 @@ static void pl011_realize(DeviceState *dev, Error **errp)
 {
     PL011State *s = PL011(dev);
 
-    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
-    s->chr = qemu_char_get_next_serial();
-
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, pl011_can_receive, pl011_receive,
                               pl011_event, s);
@@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc, void *data)
 
     dc->realize = pl011_realize;
     dc->vmsd = &vmstate_pl011;
-    /* Reason: realize() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->props = pl011_properties;
 }
 
 static const TypeInfo pl011_arm_info = {
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
new file mode 100644
index 0000000..93bd7ee
--- /dev/null
+++ b/include/hw/char/pl011.h
@@ -0,0 +1,52 @@
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef PL011_UART_H
+#define PL011_UART_H
+
+static inline DeviceState *pl011_create(hwaddr addr,
+                                        qemu_irq irq,
+                                        CharDriverState *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    dev = qdev_create(NULL, "pl011");
+    s = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(s, 0, addr);
+    sysbus_connect_irq(s, 0, irq);
+
+    return dev;
+}
+
+static inline DeviceState *pl011_luminary_create(hwaddr addr,
+                                                 qemu_irq irq,
+                                                 CharDriverState *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    dev = qdev_create(NULL, "pl011_luminary");
+    s = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(s, 0, addr);
+    sysbus_connect_irq(s, 0, irq);
+
+    return dev;
+}
+
+#endif
-- 
2.1.4

[Qemu-devel] [PATCH 2/6] hw/char: QOM'ify cadence_uart model

[xiaoqiang zhao]

* drop qemu_char_get_next_serial and use chardev prop
* create cadence_uart_create wrapper function to create
  cadence_uart_device
* change affected board code to use the new way

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/arm/xilinx_zynq.c           |  5 +++--
 hw/arm/xlnx-zynqmp.c           |  1 +
 hw/char/cadence_uart.c         | 13 +++++++------
 include/hw/char/cadence_uart.h | 17 +++++++++++++++++
 4 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 98b17c9..aefebcf 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -32,6 +32,7 @@
 #include "hw/ssi/ssi.h"
 #include "qemu/error-report.h"
 #include "hw/sd/sd.h"
+#include "hw/char/cadence_uart.h"
 
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
@@ -235,8 +236,8 @@ static void zynq_init(MachineState *machine)
     sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
     sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
 
-    sysbus_create_simple("cadence_uart", 0xE0000000, pic[59-IRQ_OFFSET]);
-    sysbus_create_simple("cadence_uart", 0xE0001000, pic[82-IRQ_OFFSET]);
+    cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hds[0]);
+    cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hds[1]);
 
     sysbus_create_varargs("cadence_ttc", 0xF8001000,
             pic[42-IRQ_OFFSET], pic[43-IRQ_OFFSET], pic[44-IRQ_OFFSET], NULL);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4d504da..9b893d7 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -308,6 +308,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
+        qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", serial_hds[i]);
         object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 442dac5..c856fc3 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -468,9 +468,6 @@ static void cadence_uart_realize(DeviceState *dev, Error **errp)
     s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                           fifo_trigger_update, s);
 
-    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
-    s->chr = qemu_char_get_next_serial();
-
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive,
                               uart_event, s);
@@ -517,6 +514,11 @@ static const VMStateDescription vmstate_cadence_uart = {
     }
 };
 
+static Property cadence_uart_properties[] = {
+    DEFINE_PROP_CHR("chardev", CadenceUARTState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void cadence_uart_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -524,9 +526,8 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data)
     dc->realize = cadence_uart_realize;
     dc->vmsd = &vmstate_cadence_uart;
     dc->reset = cadence_uart_reset;
-    /* Reason: realize() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
-}
+    dc->props = cadence_uart_properties;
+  }
 
 static const TypeInfo cadence_uart_info = {
     .name          = TYPE_CADENCE_UART,
diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
index 6310f52..a12773c 100644
--- a/include/hw/char/cadence_uart.h
+++ b/include/hw/char/cadence_uart.h
@@ -49,5 +49,22 @@ typedef struct {
     QEMUTimer *fifo_trigger_handle;
 } CadenceUARTState;
 
+static inline DeviceState *cadence_uart_create(hwaddr addr,
+                                        qemu_irq irq,
+                                        CharDriverState *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    dev = qdev_create(NULL, TYPE_CADENCE_UART);
+    s = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(s, 0, addr);
+    sysbus_connect_irq(s, 0, irq);
+
+    return dev;
+}
+
 #define CADENCE_UART_H
 #endif
-- 
2.1.4

[Qemu-devel] [PATCH 3/6] hw/char: QOM'ify digic-uart model

[xiaoqiang zhao]

* drop qemu_char_get_next_serial and use chardev prop
* change affected board code to use the new way

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/arm/digic.c       |  2 ++
 hw/char/digic-uart.c | 10 ++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index e0f9730..d60ea39 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -23,6 +23,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/arm/digic.h"
+#include "sysemu/sysemu.h"
 
 #define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
 
@@ -84,6 +85,7 @@ static void digic_realize(DeviceState *dev, Error **errp)
         sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
     }
 
+    qdev_prop_set_chr(DEVICE(&s->uart), "chardev", serial_hds[0]);
     object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
index 0e44878..c7604e6 100644
--- a/hw/char/digic-uart.c
+++ b/hw/char/digic-uart.c
@@ -145,8 +145,6 @@ static void digic_uart_realize(DeviceState *dev, Error **errp)
 {
     DigicUartState *s = DIGIC_UART(dev);
 
-    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
-    s->chr = qemu_char_get_next_serial();
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
     }
@@ -172,6 +170,11 @@ static const VMStateDescription vmstate_digic_uart = {
     }
 };
 
+static Property digic_uart_properties[] = {
+    DEFINE_PROP_CHR("chardev", DigicUartState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void digic_uart_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -179,8 +182,7 @@ static void digic_uart_class_init(ObjectClass *klass, void *data)
     dc->realize = digic_uart_realize;
     dc->reset = digic_uart_reset;
     dc->vmsd = &vmstate_digic_uart;
-    /* Reason: realize() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->props = digic_uart_properties;
 }
 
 static const TypeInfo digic_uart_info = {
-- 
2.1.4

[Qemu-devel] [PATCH 4/6] hw/char: QOM'ify stm32f2xx_usart model

[xiaoqiang zhao]

* drop qemu_char_get_next_serial and use chardev prop
* change affected board code to use the new way

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/arm/stm32f205_soc.c    |  1 +
 hw/char/stm32f2xx_usart.c | 11 ++++++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 9c1dafb..9d5bcd4 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -107,6 +107,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
     /* Attach UART (uses USART registers) and USART controllers */
     for (i = 0; i < STM_NUM_USARTS; i++) {
         usartdev = DEVICE(&(s->usart[i]));
+        qdev_prop_set_chr(usartdev, "chardev", serial_hds[i]);
         object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
         if (err != NULL) {
             error_propagate(errp, err);
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c
index 72305ec..9424abf 100644
--- a/hw/char/stm32f2xx_usart.c
+++ b/hw/char/stm32f2xx_usart.c
@@ -190,6 +190,11 @@ static const MemoryRegionOps stm32f2xx_usart_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static Property stm32f2xx_usart_properties[] = {
+    DEFINE_PROP_CHR("chardev", STM32F2XXUsartState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void stm32f2xx_usart_init(Object *obj)
 {
     STM32F2XXUsartState *s = STM32F2XX_USART(obj);
@@ -200,9 +205,6 @@ static void stm32f2xx_usart_init(Object *obj)
                           TYPE_STM32F2XX_USART, 0x2000);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
 
-    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
-    s->chr = qemu_char_get_next_serial();
-
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, stm32f2xx_usart_can_receive,
                               stm32f2xx_usart_receive, NULL, s);
@@ -214,8 +216,7 @@ static void stm32f2xx_usart_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->reset = stm32f2xx_usart_reset;
-    /* Reason: instance_init() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->props = stm32f2xx_usart_properties;
 }
 
 static const TypeInfo stm32f2xx_usart_info = {
-- 
2.1.4

[Qemu-devel] [PATCH 5/6] hw/char: QOM'ify xilinx_uartlite model

[xiaoqiang zhao]

* drop qemu_char_get_next_serial and use chardev prop
* create xilinx_uartlite_create wrapper function to create
  xilinx_uartlite device
* change affected board code to use the new way

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/char/xilinx_uartlite.c                | 10 +++++----
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  5 +++--
 include/hw/char/xilinx_uartlite.h        | 35 ++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 include/hw/char/xilinx_uartlite.h

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index 911af4a..4847efb 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -172,6 +172,11 @@ static const MemoryRegionOps uart_ops = {
     }
 };
 
+static Property xilinx_uartlite_properties[] = {
+    DEFINE_PROP_CHR("chardev", XilinxUARTLite, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void uart_rx(void *opaque, const uint8_t *buf, int size)
 {
     XilinxUARTLite *s = opaque;
@@ -206,8 +211,6 @@ static void xilinx_uartlite_realize(DeviceState *dev, Error **errp)
 {
     XilinxUARTLite *s = XILINX_UARTLITE(dev);
 
-    /* FIXME use a qdev chardev prop instead of qemu_char_get_next_serial() */
-    s->chr = qemu_char_get_next_serial();
     if (s->chr)
         qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
 }
@@ -229,8 +232,7 @@ static void xilinx_uartlite_class_init(ObjectClass *klass, void *data)
 
     dc->reset = xilinx_uartlite_reset;
     dc->realize = xilinx_uartlite_realize;
-    /* Reason: realize() method uses qemu_char_get_next_serial() */
-    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->props = xilinx_uartlite_properties;
 }
 
 static const TypeInfo xilinx_uartlite_info = {
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index f821e1c..423bcd7 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -36,6 +36,7 @@
 #include "hw/boards.h"
 #include "sysemu/block-backend.h"
 #include "exec/address-spaces.h"
+#include "hw/char/xilinx_uartlite.h"
 
 #include "boot.h"
 
@@ -103,8 +104,8 @@ petalogix_s3adsp1800_init(MachineState *machine)
         irq[i] = qdev_get_gpio_in(dev, i);
     }
 
-    sysbus_create_simple("xlnx.xps-uartlite", UARTLITE_BASEADDR,
-                         irq[UARTLITE_IRQ]);
+    xilinx_uartlite_create(UARTLITE_BASEADDR, irq[UARTLITE_IRQ],
+                           serial_hds[0]);
 
     /* 2 timers at irq 2 @ 62 Mhz.  */
     dev = qdev_create(NULL, "xlnx.xps-timer");
diff --git a/include/hw/char/xilinx_uartlite.h b/include/hw/char/xilinx_uartlite.h
new file mode 100644
index 0000000..8b4fc54
--- /dev/null
+++ b/include/hw/char/xilinx_uartlite.h
@@ -0,0 +1,35 @@
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef XILINX_UARTLITE_H
+#define XILINX_UARTLITE_H
+
+static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
+                                        qemu_irq irq,
+                                        CharDriverState *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    dev = qdev_create(NULL, "xlnx.xps-uartlite");
+    s = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(s, 0, addr);
+    sysbus_connect_irq(s, 0, irq);
+
+    return dev;
+}
+
+#endif
-- 
2.1.4

[Qemu-devel] [PATCH 6/6] char: get rid of qemu_char_get_next_serial

[xiaoqiang zhao]

since there is no user of qemu_char_get_next_serial any more,
it's time to let it go away.

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 include/sysemu/char.h |  1 -
 qemu-char.c           | 16 ----------------
 2 files changed, 17 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 307fd8f..372a6fd 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -403,7 +403,6 @@ void register_char_driver(const char *name, ChardevBackendKind kind,
 
 extern int term_escape_char;
 
-CharDriverState *qemu_char_get_next_serial(void);
 
 /* console.c */
 typedef CharDriverState *(VcHandler)(ChardevVC *vc, Error **errp);
diff --git a/qemu-char.c b/qemu-char.c
index b597ee1..b13ecbb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4088,22 +4088,6 @@ CharDriverState *qemu_chr_find(const char *name)
     return NULL;
 }
 
-/* Get a character (serial) device interface.  */
-CharDriverState *qemu_char_get_next_serial(void)
-{
-    static int next_serial;
-    CharDriverState *chr;
-
-    /* FIXME: This function needs to go away: use chardev properties!  */
-
-    while (next_serial < MAX_SERIAL_PORTS && serial_hds[next_serial]) {
-        chr = serial_hds[next_serial++];
-        qemu_chr_fe_claim_no_fail(chr);
-        return chr;
-    }
-    return NULL;
-}
-
 QemuOptsList qemu_chardev_opts = {
     .name = "chardev",
     .implied_opt_name = "backend",
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Paolo Bonzini]

On 25/05/2016 12:58, xiaoqiang zhao wrote:
>  #define BCM2835_VC_PERI_BASE 0x7e000000
> @@ -106,7 +107,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      MemoryRegion *ram;
>      Error *err = NULL;
>      uint32_t ram_size, vcram_size;
> -    CharDriverState *chr;
>      int n;
>  
>      obj = object_property_get_link(OBJECT(dev), "ram", &err);
> @@ -147,6 +147,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
>  
>      /* UART0 */
> +    qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hds[0]);
>      object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
> @@ -158,17 +159,8 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(s->uart0, 0,
>          qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
>                                 INTERRUPT_UART));
> -
>      /* AUX / UART1 */
> -    /* TODO: don't call qemu_char_get_next_serial() here, instead set
> -     * chardev properties for each uart at the board level, once pl011
> -     * (uart0) has been updated to avoid qemu_char_get_next_serial()
> -     */
> -    chr = qemu_char_get_next_serial();
> -    if (chr == NULL) {
> -        chr = qemu_chr_new("bcm2835.uart1", "null", NULL);
> -    }
> -    qdev_prop_set_chr(DEVICE(&s->aux), "chardev", chr);
> +    qdev_prop_set_chr(DEVICE(&s->aux), "chardev", serial_hds[1]);
>  
>      object_property_set_bool(OBJECT(&s->aux), true, "realized", &err);
>      if (err) {
> @@ -292,8 +284,6 @@ static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      dc->realize = bcm2835_peripherals_realize;
> -    /* Reason: realize() method uses qemu_char_get_next_serial() */
> -    dc->cannot_instantiate_with_device_add_yet = true;

This must remain, though the reason is now "realize() method uses
serial_hds[]".

Otherwise, looks good.

Thanks,

Paolo

>  }

Re: [Qemu-devel] [PATCH 0/6] Drop the qemu_char_get_next_serial function

[Paolo Bonzini]

On 25/05/2016 12:58, xiaoqiang zhao wrote:
> This patch series's main idea is to drop the use of
> qemu_char_get_next_serial. patches 1-5 QOM'ify the related 
> device model and the affected board code. Most of them
> are arm related. The patch 6 get rid of the function from
> the code base.
> 
> I only test the code with ARM versatilepb machine. The other
> machine need test!
> 
> xiaoqiang zhao (6):
>   hw/char: QOM'ify pl011 model
>   hw/char: QOM'ify cadence_uart model
>   hw/char: QOM'ify digic-uart model
>   hw/char: QOM'ify stm32f2xx_usart model
>   hw/char: QOM'ify xilinx_uartlite model
>   char: get rid of qemu_char_get_next_serial

These are all ARM boards, so I guess they will be merged through Peter's
ARM tree.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 6/6] char: get rid of qemu_char_get_next_serial

[Paolo Bonzini]

On 25/05/2016 12:58, xiaoqiang zhao wrote:
> since there is no user of qemu_char_get_next_serial any more,
> it's time to let it go away.
> 
> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
> ---
>  include/sysemu/char.h |  1 -
>  qemu-char.c           | 16 ----------------
>  2 files changed, 17 deletions(-)

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Peter Maydell]

On 25 May 2016 at 11:58, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
> * drop qemu_char_get_next_serial and use chardev prop
> * add pl011_create wrapper function to create pl011 uart device
> * change affected board code to use the new way
>
> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
> ---
>  hw/arm/bcm2835_peripherals.c | 16 +++-----------
>  hw/arm/highbank.c            |  3 ++-
>  hw/arm/integratorcp.c        |  5 +++--
>  hw/arm/realview.c            |  9 ++++----
>  hw/arm/stellaris.c           |  6 +++--
>  hw/arm/versatilepb.c         |  9 ++++----
>  hw/arm/vexpress.c            |  9 ++++----
>  hw/arm/virt.c                |  1 +
>  hw/char/pl011.c              | 11 +++++-----
>  include/hw/char/pl011.h      | 52 ++++++++++++++++++++++++++++++++++++++++++++
>  10 files changed, 86 insertions(+), 35 deletions(-)
>  create mode 100644 include/hw/char/pl011.h
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 234d518..46c320f 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -14,6 +14,7 @@
>  #include "hw/misc/bcm2835_mbox_defs.h"
>  #include "hw/arm/raspi_platform.h"
>  #include "sysemu/char.h"
> +#include "sysemu/sysemu.h"
>
>  /* Peripheral base address on the VC (GPU) system bus */
>  #define BCM2835_VC_PERI_BASE 0x7e000000
> @@ -106,7 +107,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      MemoryRegion *ram;
>      Error *err = NULL;
>      uint32_t ram_size, vcram_size;
> -    CharDriverState *chr;
>      int n;
>
>      obj = object_property_get_link(OBJECT(dev), "ram", &err);
> @@ -147,6 +147,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
>
>      /* UART0 */
> +    qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hds[0]);
>      object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
> @@ -158,17 +159,8 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(s->uart0, 0,
>          qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
>                                 INTERRUPT_UART));
> -
>      /* AUX / UART1 */
> -    /* TODO: don't call qemu_char_get_next_serial() here, instead set
> -     * chardev properties for each uart at the board level, once pl011
> -     * (uart0) has been updated to avoid qemu_char_get_next_serial()
> -     */

This comment says this should be fixed by having board-level
properties; you've removed it but this patch isn't adding
the properties to this (SoC-level) device. I think the board
level should be looking at serial_hds[], not this code.

> @@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc, void *data)
>
>      dc->realize = pl011_realize;
>      dc->vmsd = &vmstate_pl011;
> -    /* Reason: realize() method uses qemu_char_get_next_serial() */
> -    dc->cannot_instantiate_with_device_add_yet = true;

Why does instantiating with device_add work now? There's
still no way to wire up interrupt lines or map mmio regions.
(This has never made much sense to me -- Markus?)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Paolo Bonzini]

On 25/05/2016 15:08, Peter Maydell wrote:
>> >      /* AUX / UART1 */
>> > -    /* TODO: don't call qemu_char_get_next_serial() here, instead set
>> > -     * chardev properties for each uart at the board level, once pl011
>> > -     * (uart0) has been updated to avoid qemu_char_get_next_serial()
>> > -     */
> This comment says this should be fixed by having board-level
> properties; you've removed it but this patch isn't adding
> the properties to this (SoC-level) device. I think the board
> level should be looking at serial_hds[], not this code.
> 
>> > @@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc, void *data)
>> >
>> >      dc->realize = pl011_realize;
>> >      dc->vmsd = &vmstate_pl011;
>> > -    /* Reason: realize() method uses qemu_char_get_next_serial() */
>> > -    dc->cannot_instantiate_with_device_add_yet = true;
> Why does instantiating with device_add work now? There's
> still no way to wire up interrupt lines or map mmio regions.
> (This has never made much sense to me -- Markus?)

You can create it with -device if your board provides a platform bus and
there's an FDT creator function.  If you don't, you get an error.

Regarding device_add, it's disabled anyway because sysbus is not
hotpluggable.

Eiter way it's safe to remove this line, which is for outliers that
otherwise would not give any error message.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[xiaoqiang zhao]

在 2016年05月25日 21:08, Peter Maydell 写道:
>> -
>> >      /* AUX / UART1 */
>> >-    /* TODO: don't call qemu_char_get_next_serial() here, instead set
>> >-     * chardev properties for each uart at the board level, once pl011
>> >-     * (uart0) has been updated to avoid qemu_char_get_next_serial()
>> >-     */
> This comment says this should be fixed by having board-level
> properties; you've removed it but this patch isn't adding
> the properties to this (SoC-level) device. I think the board
> level should be looking at serial_hds[], not this code.
>
Hi, Peter:

I found bcm2835-peripherals is only used by bcm2836 code, you mean 
this(bcm2836) is the 'board level' and I should set chardev properties 
here ?

Thanks.

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[xiaoqiang zhao]

在 2016年05月25日 21:15, Paolo Bonzini 写道:
>>>> @@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc, void *data)
>>>> >> >
>>>> >> >      dc->realize = pl011_realize;
>>>> >> >      dc->vmsd = &vmstate_pl011;
>>>> >> >-    /* Reason: realize() method uses qemu_char_get_next_serial() */
>>>> >> >-    dc->cannot_instantiate_with_device_add_yet = true;
>> >Why does instantiating with device_add work now? There's
>> >still no way to wire up interrupt lines or map mmio regions.
>> >(This has never made much sense to me -- Markus?)
> You can create it with -device if your board provides a platform bus and
> there's an FDT creator function.  If you don't, you get an error.
>
> Regarding device_add, it's disabled anyway because sysbus is not
> hotpluggable.
>
> Eiter way it's safe to remove this line, which is for outliers that
> otherwise would not give any error message.
Paolo: Do I need to send a new version to keep the

"dc->cannot_instantiate_with_device_add_yet = true"
untouched ?

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Peter Maydell]

On 27 May 2016 at 03:06, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
> 在 2016年05月25日 21:08, Peter Maydell 写道:
>>>
>>> -
>>> >      /* AUX / UART1 */
>>> >-    /* TODO: don't call qemu_char_get_next_serial() here, instead set
>>> >-     * chardev properties for each uart at the board level, once pl011
>>> >-     * (uart0) has been updated to avoid qemu_char_get_next_serial()
>>> >-     */
>>
>> This comment says this should be fixed by having board-level
>> properties; you've removed it but this patch isn't adding
>> the properties to this (SoC-level) device. I think the board
>> level should be looking at serial_hds[], not this code.
>>
> Hi, Peter:
>
> I found bcm2835-peripherals is only used by bcm2836 code, you mean
> this(bcm2836) is the 'board level' and I should set chardev properties here?

Device models subclass DeviceClass (possibly indirectly).
Boards subclass MachineClass. Only boards should be looking
at serial_hds[]. Sometimes you have a device which is just
a "container" which encapsulates several other devices
(typically this is a model of an SoC). For that kind of
container device, it should expose chardev properties for
its UARTs, which will usually be alias properties for
the properties on the actual UART devices which the
container has.

bcm2835-peripherals is a device, not a board, so it shouldn't
be using serial_hds[].

If it's clearer, you can use a separate patch to push the
serial_hds[] usage out another layer, rather than folding
it into this one.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Paolo Bonzini]

On 27/05/2016 04:13, xiaoqiang zhao wrote:
> 
> 
> 在 2016年05月25日 21:15, Paolo Bonzini 写道:
>>>>> @@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc,
>>>>> void *data)
>>>>> >> >
>>>>> >> >      dc->realize = pl011_realize;
>>>>> >> >      dc->vmsd = &vmstate_pl011;
>>>>> >> >-    /* Reason: realize() method uses
>>>>> qemu_char_get_next_serial() */
>>>>> >> >-    dc->cannot_instantiate_with_device_add_yet = true;
>>> >Why does instantiating with device_add work now? There's
>>> >still no way to wire up interrupt lines or map mmio regions.
>>> >(This has never made much sense to me -- Markus?)
>> You can create it with -device if your board provides a platform bus and
>> there's an FDT creator function.  If you don't, you get an error.
>>
>> Regarding device_add, it's disabled anyway because sysbus is not
>> hotpluggable.
>>
>> Eiter way it's safe to remove this line, which is for outliers that
>> otherwise would not give any error message.
> Paolo: Do I need to send a new version to keep the
> 
> "dc->cannot_instantiate_with_device_add_yet = true"
> untouched ?

No, you should remove it.  cannot_instantiate_with_device_add_yet is set
if you cannot_instantiate_with_device_add_yet *and* there is no code
elsewhere to detect that.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Paolo Bonzini]

On 27/05/2016 11:01, Peter Maydell wrote:
>> > I found bcm2835-peripherals is only used by bcm2836 code, you mean
>> > this(bcm2836) is the 'board level' and I should set chardev properties here?
> Device models subclass DeviceClass (possibly indirectly).
> Boards subclass MachineClass. Only boards should be looking
> at serial_hds[]. Sometimes you have a device which is just
> a "container" which encapsulates several other devices
> (typically this is a model of an SoC). For that kind of
> container device, it should expose chardev properties for
> its UARTs, which will usually be alias properties for
> the properties on the actual UART devices which the
> container has.
> 
> bcm2835-peripherals is a device, not a board, so it shouldn't
> be using serial_hds[].
> 
> If it's clearer, you can use a separate patch to push the
> serial_hds[] usage out another layer, rather than folding
> it into this one.

I agree.  This patch is fine (perhaps with the FIXME comment left
there); then chardev properties can be added to bcm2835-peripherals and
set in bcm2836.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[xiaoqiang zhao]

> 在 2016年5月27日，17:42，Paolo Bonzini <pbonzini@redhat.com> 写道：
> 
> 
> 
> On 27/05/2016 11:01, Peter Maydell wrote:
>>>> I found bcm2835-peripherals is only used by bcm2836 code, you mean
>>>> this(bcm2836) is the 'board level' and I should set chardev properties here?
>> Device models subclass DeviceClass (possibly indirectly).
>> Boards subclass MachineClass. Only boards should be looking
>> at serial_hds[]. Sometimes you have a device which is just
>> a "container" which encapsulates several other devices
>> (typically this is a model of an SoC). For that kind of
>> container device, it should expose chardev properties for
>> its UARTs, which will usually be alias properties for
>> the properties on the actual UART devices which the
>> container has.
>> 
>> bcm2835-peripherals is a device, not a board, so it shouldn't
>> be using serial_hds[].
>> 
>> If it's clearer, you can use a separate patch to push the
>> serial_hds[] usage out another layer, rather than folding
>> it into this one.
> 
> I agree.  This patch is fine (perhaps with the FIXME comment left
> there); then chardev properties can be added to bcm2835-peripherals and
> set in bcm2836.
> 
> Thanks,
> 
> Paolo
> 
Much clear.
Thank you guys！

Re: [Qemu-devel] [PATCH 1/6] hw/char: QOM'ify pl011 model

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 May 2016 at 11:58, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
>> * drop qemu_char_get_next_serial and use chardev prop
>> * add pl011_create wrapper function to create pl011 uart device
>> * change affected board code to use the new way
>>
>> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
>> ---
>>  hw/arm/bcm2835_peripherals.c | 16 +++-----------
>>  hw/arm/highbank.c            |  3 ++-
>>  hw/arm/integratorcp.c        |  5 +++--
>>  hw/arm/realview.c            |  9 ++++----
>>  hw/arm/stellaris.c           |  6 +++--
>>  hw/arm/versatilepb.c         |  9 ++++----
>>  hw/arm/vexpress.c            |  9 ++++----
>>  hw/arm/virt.c                |  1 +
>>  hw/char/pl011.c              | 11 +++++-----
>>  include/hw/char/pl011.h      | 52 ++++++++++++++++++++++++++++++++++++++++++++
>>  10 files changed, 86 insertions(+), 35 deletions(-)
>>  create mode 100644 include/hw/char/pl011.h
>>
>> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
>> index 234d518..46c320f 100644
>> --- a/hw/arm/bcm2835_peripherals.c
>> +++ b/hw/arm/bcm2835_peripherals.c
>> @@ -14,6 +14,7 @@
>>  #include "hw/misc/bcm2835_mbox_defs.h"
>>  #include "hw/arm/raspi_platform.h"
>>  #include "sysemu/char.h"
>> +#include "sysemu/sysemu.h"
>>
>>  /* Peripheral base address on the VC (GPU) system bus */
>>  #define BCM2835_VC_PERI_BASE 0x7e000000
>> @@ -106,7 +107,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>>      MemoryRegion *ram;
>>      Error *err = NULL;
>>      uint32_t ram_size, vcram_size;
>> -    CharDriverState *chr;
>>      int n;
>>
>>      obj = object_property_get_link(OBJECT(dev), "ram", &err);
>> @@ -147,6 +147,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>>      sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
>>
>>      /* UART0 */
>> +    qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hds[0]);
>>      object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
>>      if (err) {
>>          error_propagate(errp, err);
>> @@ -158,17 +159,8 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>>      sysbus_connect_irq(s->uart0, 0,
>>          qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
>>                                 INTERRUPT_UART));
>> -
>>      /* AUX / UART1 */
>> -    /* TODO: don't call qemu_char_get_next_serial() here, instead set
>> -     * chardev properties for each uart at the board level, once pl011
>> -     * (uart0) has been updated to avoid qemu_char_get_next_serial()
>> -     */
>
> This comment says this should be fixed by having board-level
> properties; you've removed it but this patch isn't adding
> the properties to this (SoC-level) device. I think the board
> level should be looking at serial_hds[], not this code.

Device models should not fish backends out of global state.  Whether
they fish directly or via some helper like qemu_char_get_next_serial()
doesn't matter.  The ones that still do need to set
cannot_instantiate_with_device_add_yet with a suitable comment.

>> @@ -310,8 +312,7 @@ static void pl011_class_init(ObjectClass *oc, void *data)
>>
>>      dc->realize = pl011_realize;
>>      dc->vmsd = &vmstate_pl011;
>> -    /* Reason: realize() method uses qemu_char_get_next_serial() */
>> -    dc->cannot_instantiate_with_device_add_yet = true;
>
> Why does instantiating with device_add work now? There's
> still no way to wire up interrupt lines or map mmio regions.
> (This has never made much sense to me -- Markus?)

Uh, which part does "this" refer to?

I systematically reviewed devices for my "Clean up and fix no_user"
series (commit f976b09..7ea5e78), and wrote down my findings in
"Reason:" comments next to cannot_instantiate_with_device_add_yet
assignments.  Any such assignment must have such a comment.

Testing can catch cases where we missed *all* reasons.  Example: my "Fix
device introspection regressions" series (commit b37686f..33fe968).  It
can't catch cases where we missed *some* reasons.

Note that cannot_instantiate_with_device_add_yet can get set by
(possibly abstract) parent devices as well.  If a parent device sets it,
its children should nevertheless set it *again* if they have additional
reasons.  I believe this is such a case.  I'm not 100% sure, because I
haven't been 100% sure about anything related to sysbus devices ever
since Alex's commit 33cd52b "sysbus: Make devices spawnable via -device"
dropped cannot_instantiate_with_device_add_yet from
sysbus_device_class_init(), quoted below.  As you see, that assignment
covered "no way to wire up interrupt lines or map mmio regions."


diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 19437e6..7bfe381 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -283,13 +283,6 @@ static void sysbus_device_class_init(ObjectClass *klass, vo
id *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
-    /*
-     * device_add plugs devices into suitable bus.  For "real" buses,
-     * that actually connects the device.  For sysbus, the connections
-     * need to be made separately, and device_add can't do that.  The
-     * device would be left unconnected, and could not possibly work.
-     */
-    k->cannot_instantiate_with_device_add_yet = true;
 }
 

Re: [Qemu-devel] [PATCH 0/6] Drop the qemu_char_get_next_serial function

[Peter Maydell]

On 25 May 2016 at 11:58, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
> This patch series's main idea is to drop the use of
> qemu_char_get_next_serial. patches 1-5 QOM'ify the related
> device model and the affected board code. Most of them
> are arm related. The patch 6 get rid of the function from
> the code base.
>
> I only test the code with ARM versatilepb machine. The other
> machine need test!
>
> xiaoqiang zhao (6):
>   hw/char: QOM'ify pl011 model
>   hw/char: QOM'ify cadence_uart model
>   hw/char: QOM'ify digic-uart model
>   hw/char: QOM'ify stm32f2xx_usart model
>   hw/char: QOM'ify xilinx_uartlite model
>   char: get rid of qemu_char_get_next_serial

Hi; I'm afraid this patch series doesn't pass "make check"
(it looks like we segfault trying to instantiate the raspi2
board).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/6] Drop the qemu_char_get_next_serial function

[xiaoqiang zhao]

> 在 2016年6月4日，02:24，Peter Maydell <peter.maydell@linaro.org> 写道：
> 
>> On 25 May 2016 at 11:58, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
>> This patch series's main idea is to drop the use of
>> qemu_char_get_next_serial. patches 1-5 QOM'ify the related
>> device model and the affected board code. Most of them
>> are arm related. The patch 6 get rid of the function from
>> the code base.
>> 
>> I only test the code with ARM versatilepb machine. The other
>> machine need test!
>> 
>> xiaoqiang zhao (6):
>>  hw/char: QOM'ify pl011 model
>>  hw/char: QOM'ify cadence_uart model
>>  hw/char: QOM'ify digic-uart model
>>  hw/char: QOM'ify stm32f2xx_usart model
>>  hw/char: QOM'ify xilinx_uartlite model
>>  char: get rid of qemu_char_get_next_serial
> 
> Hi; I'm afraid this patch series doesn't pass "make check"
> (it looks like we segfault trying to instantiate the raspi2
> board).
> 
> thanks
> -- PMM
> 

Will fix in the next version.