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

[Peter Maydell]

On 25 May 2016 at 11:58, xiaoqiang zhao <address@hidden> 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 <address@hidden>
> ---
>  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