[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
From: |
Mark Cave-Ayland |
Subject: |
Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function |
Date: |
Tue, 17 Mar 2020 14:41:06 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 17/03/2020 14:07, BALATON Zoltan 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.
Hmmm actually yes I think isn't quite right. Here you have PCI_DEVFN(10, 0) for
the
PIIX4_PCI_DEVICE, and PCI_DEVFN(10, 0) + 1 for "piix4-ide". But since you've
reassigned pci then "piix4-usb-uhci" now ends up as (PCI_DEVFN(10, 0) + 1) + 2 =
PCI_DEVFN(10, 0) + 3 rather than PCI_DEVFN(10, 0) + 2 as it was before.
ATB,
Mark.
- [PATCH v2 0/7] Misc hw/ide legacy clean up, BALATON Zoltan, 2020/03/17
- [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, BALATON Zoltan, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, Philippe Mathieu-Daudé, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, Philippe Mathieu-Daudé, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, John Snow, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, Philippe Mathieu-Daudé, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, BALATON Zoltan, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, Philippe Mathieu-Daudé, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function,
Mark Cave-Ayland <=
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, BALATON Zoltan, 2020/03/17
- Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function, John Snow, 2020/03/17
[PATCH v2 5/7] hw/ide: Do ide_drive_get() within pci_ide_create_devs(), BALATON Zoltan, 2020/03/17
[PATCH v2 3/7] hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h, BALATON Zoltan, 2020/03/17
[PATCH v2 1/7] hw/ide: Get rid of piix3_init functions, BALATON Zoltan, 2020/03/17
[PATCH v2 4/7] hw/ide/pci.c: Coding style update to fix checkpatch errors, BALATON Zoltan, 2020/03/17
[PATCH v2 7/7] hw/ide: Remove unneeded inclusion of hw/ide.h, BALATON Zoltan, 2020/03/17
[PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h, BALATON Zoltan, 2020/03/17