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

[Philippe Mathieu-Daudé]

On Tue, Mar 17, 2020 at 3:07 PM BALATON Zoltan <address@hidden> wrote:
> On Tue, 17 Mar 2020, Philippe Mathieu-Daudé wrote:
> > On 3/17/20 2:50 PM, 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 <address@hidden>
> >>>>> Reviewed-by: Mark Cave-Ayland <address@hidden>
> >>>>> Reviewed-by: Markus Armbruster <address@hidden>
> >>>>
> >>>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> >>>
> >>> 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?
> >
> > I asked Zoltan there why he was reassigning 'pci' and he replied here:
> > https://www.mail-archive.com/address@hidden/msg63324.html
> >
> > I don't know enough the PCI API (and don't have time this week to dig into
> > it) to check how pci->devfn is used (is it populated by a pci_create()
> > call?).
> >
> >    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> >                                          true, TYPE_PIIX4_PCI_DEVICE);
> >    ...
> > +   pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> >    ...
> >    pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
> >
> > What annoys me is here -------^^^^^^ I don't know if reassigning the pci
> > variable can have an impact, so as I am not confident I prefer to withdraw 
> > my
> > review tag.
>
> OK, I did not know that's what you were asking about and did not notice
> this could be a problem. To avoid any doubt I'll send a new version to
> avoid this shortly.

Sorry I was a bit rushed for soft-freeze, I should have spent more
time in my comment then, this would have saved everybody a bit of
email traffic.
I'll try to remember next time.

Thanks for sending an updated version.

> Regards,
> BALATON Zoltan