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

[Mark Cave-Ayland]

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.