Re: [PATCH v4 12/12] hw/isa/vt82c686: Create rtc-time alias in boards instead

[Bernhard Beschow]

Am 31. August 2022 16:30:10 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>> According to good QOM practice, an object should only deal with objects
>> of its own sub tree. Having devices create an alias on the machine
>> object doesn't respect this good practice. To resolve this, create the
>> alias in the machine's code.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c   | 2 --
>> hw/mips/fuloong2e.c | 4 ++++
>> hw/ppc/pegasos2.c   | 4 ++++
>> 3 files changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 48cd4d0036..3f9bd0c04d 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -632,8 +632,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
>>         return;
>>     }
>> -    object_property_add_alias(qdev_get_machine(), "rtc-time", 
>> OBJECT(&s->rtc),
>> -                              "date");
>>     isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
>> 
>>     for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 2d8723ab74..0f4cfe1188 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -203,6 +203,10 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, 
>> int slot, qemu_irq intc,
>> 
>>     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
>>                                           TYPE_VT82C686B_ISA);
>> +    object_property_add_alias(qdev_get_machine(), "rtc-time",
>> +                              object_resolve_path_component(OBJECT(via),
>> +                                                            "rtc"),
>> +                              "date");
>>     qdev_connect_gpio_out(DEVICE(via), 0, intc);
>> 
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index 09fdb7557f..f50e1d8b3f 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -161,6 +161,10 @@ static void pegasos2_init(MachineState *machine)
>>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>>     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>>                                           TYPE_VT8231_ISA);
>> +    object_property_add_alias(qdev_get_machine(), "rtc-time",
>
>I did not check it in previous version but Phillppe noted this 
>qdev_get_machine() should be machine (the parameter to pegasos2_init) instead 
>and I agree with that.

Sounds good! I'm all about removing access to globals.

>Also if you get rid of the now very much cut down vt82c686b_southbridge_init 
>func in fuloong2e and just inline what's left of it at the only call site then 
>the same machine pointer could be used there too and would be simpler then 
>going through the function now that it's moved to via-isa mostly.

Sure, I'll add another patch on top.

>Sorry that this needs another respin but that's the last, I won't look at it 
>again :-)

No worries. It's very convenient with git-publish.

>You can also add to the whole series:
>
>Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Will do. Thanks for your quick replies!

Regards,
Bernhard

>Regards,
>BALATON Zoltan
>
>> +                              object_resolve_path_component(OBJECT(via),
>> +                                                            "rtc"),
>> +                              "date");
>>     qdev_connect_gpio_out(DEVICE(via), 0,
>>                           qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>> 
>>