Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function

[John Snow <jsnow@redhat.com>]

On 3/17/20 10:05 AM, BALATON Zoltan wrote:
> On Tue, 17 Mar 2020, John Snow wrote:
>> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>>> other ide devices.
>>>>>
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>>> was not assigned.
>>>
>>
>> Is there an issue you've noticed, or you are just no longer certain
>> enough to give an RB?
> 
> My previous replies to this question:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04356.html
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04381.html
> 
> End result after all patches in the series looks like this:
> 
> DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus
> **smbus)
> {
>     PCIDevice *pci;
>     DeviceState *dev;
> 
>     pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
>                                           true, TYPE_PIIX4_PCI_DEVICE);
>     dev = DEVICE(pci);
>     if (isa_bus) {
>         *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
>     }
> 
>     pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
>     pci_ide_create_devs(pci);
> 
>     pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
>     if (smbus) {
>         *smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
>                                isa_get_irq(NULL, 9), NULL, 0, NULL);
>    }
> 
>     return dev;
> }
> 
> I think it's clear enough what the pci variable is used for here and
> there was no further reply from Philippe and Michael and others were OK
> with this so there was no definitive decision here. Mayby Philippe still
> does not like reusal of this variable enough to give Reviewed tag but
> there should be no other problem with it otherwise.
> 
> Regards,
> BALATON Zoltan

Thanks for the pointers -- my attention was focused elsewhere, and the
discussion was so vigorous I felt comfortable letting you handle it :)

I'm seeing a compilation error -- I cleared my cache and am looking for
the commit that introduced it. Stay tuned, please.

--js