1. Patchew
  2. QEMU
  3. [PATCH 0/6] QOM minor fixes
  4. View patch

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

Mark Cave-Ayland posted 6 patches 5 years, 1 month ago
Maintainers: Artyom Tarasenko <atar4qemu@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
[PATCH 5/6] macio: don't reference serial_hd() directly within the device
Posted by Mark Cave-Ayland 5 years, 1 month ago 
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;
 }
 
 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),
-- 
2.20.1
Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device
Posted by BALATON Zoltan via 5 years, 1 month ago 
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),
>
Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device
Posted by Mark Cave-Ayland 5 years, 1 month ago 
On 20/09/2020 11:52, BALATON Zoltan via wrote:

> 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

Ah yes indeed, thanks for the reference. I'll see if anyone else has any further
comments on the series before posting an updated v2 with this line removed.


ATB,

Mark.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.