Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device

[BALATON Zoltan]

On Sun, 20 Sep 2020, Mark Cave-Ayland wrote:
> Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
> Mac Old World and New World machine level.
>
> Also remove the now obsolete comment referring to the use of serial_hd() and
> change user_createable to true accordingly.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/misc/macio/macio.c | 5 +----
> hw/ppc/mac_newworld.c | 6 ++++++
> hw/ppc/mac_oldworld.c | 6 ++++++
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 679722628e..ce641d41fd 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
>     qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
>     qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
>     qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4);
> -    qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hd(0));
> -    qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1));
>     qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial);
>     qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial);
>     if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) {
> @@ -458,8 +456,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
>     k->class_id = PCI_CLASS_OTHERS << 8;
>     device_class_set_props(dc, macio_properties);
>     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> -    /* Reason: Uses serial_hds in macio_instance_init */
> -    dc->user_creatable = false;
> +    dc->user_creatable = true;

According to a comment in

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/core/qdev.c;h=96772a15bd5b76d3ebe27d45ed1f2c1beb7f5386;hb=HEAD#l1135

user_creatable = true is the default and most devices don't set it 
explicitely so I think you can just remove the line setting it here.

Regards,
BALATON Zoltan

> }
>
> static const TypeInfo macio_bus_info = {
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index e42bd7a626..e59b30e0a7 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -123,6 +123,7 @@ static void ppc_core99_init(MachineState *machine)
>     UNINHostState *uninorth_pci;
>     PCIBus *pci_bus;
>     PCIDevice *macio;
> +    ESCCState *escc;
>     bool has_pmu, has_adb;
>     MACIOIDEState *macio_ide;
>     BusState *adb_bus;
> @@ -382,6 +383,11 @@ static void ppc_core99_init(MachineState *machine)
>     qdev_prop_set_bit(dev, "has-adb", has_adb);
>     object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
>                              &error_abort);
> +
> +    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> +    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> +    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
>     pci_realize_and_unref(macio, pci_bus, &error_fatal);
>
>     /* We only emulate 2 out of 3 IDE controllers for now */
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 7aba040f1b..25ade63ba3 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -96,6 +96,7 @@ static void ppc_heathrow_init(MachineState *machine)
>     PCIBus *pci_bus;
>     PCIDevice *macio;
>     MACIOIDEState *macio_ide;
> +    ESCCState *escc;
>     SysBusDevice *s;
>     DeviceState *dev, *pic_dev;
>     BusState *adb_bus;
> @@ -283,6 +284,11 @@ static void ppc_heathrow_init(MachineState *machine)
>     qdev_prop_set_uint64(dev, "frequency", tbfreq);
>     object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
>                              &error_abort);
> +
> +    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> +    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> +    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
>     pci_realize_and_unref(macio, pci_bus, &error_fatal);
>
>     macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),