Yinghai, Jesse,
I tested pciehp with your set of patches. I have some comments below.
(1) I got a following warning message on compiling the patch [5/7].
drivers/pci/hotplug/pciehp_hpc.c:281: warning: 'pcie_wait_link_not_active' defined but not used
(2) I got following warning messages on compiling the patch [6/7]
drivers/pci/hotplug/pciehp_hpc.c:381: warning: 'pciehp_link_enable' defined but not used
drivers/pci/hotplug/pciehp_hpc.c:386: warning: 'pciehp_link_disable' defined but not used
(3) I've asked Naoki Yanagimoto, who reported that configuration read
on some hot-added PCIe device returns invalid value, to test the
patch. Unfortunately, the problem happens with your patch. But
after some discussion and testing, it turned out that problem doesn't
happen when the same card with updated bios is used. So it seems the
problem is in PCIe card side.
As a result, problems I found are (1) and (2). Please fix those.
Other than that, pciehp seems to work well.
Tested-by: Kenji Kaneshige <[email protected]>
Regards,
Kenji Kaneshige
(2012/01/28 3:55), Yinghai Lu wrote:
> 75e4615: pciehp: Disable/enable link during slot power off/on
> 92bdfaf: pciehp: Add Disable/enable link functions
> 47442c1: pciehp: Add pcie_wait_link_not_active()
> 40b6c9b: pciehp: print out link status when dlla get active.
> dcc66b6: pciehp: Checking pci conf reading to new added device instead of sleep 1s
> aadd74c: PCI: Separate pci_bus_read_dev_vendor_id from pci_scan_device
> e7457be: PCI: Make sriov work with hotplug remove
>
> drivers/pci/hotplug/pciehp_hpc.c | 133 +++++++++++++++++++++++++++++++-------
> drivers/pci/pci.h | 2 +
> drivers/pci/probe.c | 48 +++++++++-----
> drivers/pci/remove.c | 10 +++-
> 4 files changed, 151 insertions(+), 42 deletions(-)
>
> First one is for hotplug with sriov under bridge.
> following two are for hotplug probing with pci conf reading.
> Last four are for pcie link disable when power off slots.
>
> -v2: update first one according to reviewing from linus.
> other like pci_dev_read_config return checking is not changed.
>
> Resending whole set according to Jesse.
>
> Thanks
>
> Yinghai Lu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Thu, Feb 2, 2012 at 2:00 AM, Kenji Kaneshige
<[email protected]> wrote:
> Yinghai, Jesse,
>
> I tested pciehp with your set of patches. I have some comments below.
>
> (1) I got a following warning message on compiling the patch [5/7].
>
> ? ?drivers/pci/hotplug/pciehp_hpc.c:281: warning:
> 'pcie_wait_link_not_active' defined but not used
>
> (2) I got following warning messages on compiling the patch [6/7]
>
> ? ?drivers/pci/hotplug/pciehp_hpc.c:381: warning: 'pciehp_link_enable'
> defined but not used
> ? ?drivers/pci/hotplug/pciehp_hpc.c:386: warning: 'pciehp_link_disable'
> defined but not used
>
> (3) I've asked Naoki Yanagimoto, who reported that configuration read
> ? ?on some hot-added PCIe device returns invalid value, to test the
> ? ?patch. Unfortunately, the problem happens with your patch. But
> ? ?after some discussion and testing, it turned out that problem doesn't
> ? ?happen when the same card with updated bios is used. So it seems the
> ? ?problem is in PCIe card side.
>
> As a result, problems I found are (1) and (2). Please fix those.
> Other than that, pciehp seems to work well.
>
> Tested-by: Kenji Kaneshige <[email protected]>
>
Great. thanks for confirmation.
for (1) and (2), patch 5, and 6 will add some helper functions and
they will be used by patch 7.
so when patch 7 is applied, there will be no compiling warning anymore.
Thanks
Yinghai
(2012/02/03 5:39), Yinghai Lu wrote:
> On Thu, Feb 2, 2012 at 2:00 AM, Kenji Kaneshige
> <[email protected]> wrote:
>> Yinghai, Jesse,
>>
>> I tested pciehp with your set of patches. I have some comments below.
>>
>> (1) I got a following warning message on compiling the patch [5/7].
>>
>> drivers/pci/hotplug/pciehp_hpc.c:281: warning:
>> 'pcie_wait_link_not_active' defined but not used
>>
>> (2) I got following warning messages on compiling the patch [6/7]
>>
>> drivers/pci/hotplug/pciehp_hpc.c:381: warning: 'pciehp_link_enable'
>> defined but not used
>> drivers/pci/hotplug/pciehp_hpc.c:386: warning: 'pciehp_link_disable'
>> defined but not used
>>
>> (3) I've asked Naoki Yanagimoto, who reported that configuration read
>> on some hot-added PCIe device returns invalid value, to test the
>> patch. Unfortunately, the problem happens with your patch. But
>> after some discussion and testing, it turned out that problem doesn't
>> happen when the same card with updated bios is used. So it seems the
>> problem is in PCIe card side.
>>
>> As a result, problems I found are (1) and (2). Please fix those.
>> Other than that, pciehp seems to work well.
>>
>> Tested-by: Kenji Kaneshige<[email protected]>
>>
>
> Great. thanks for confirmation.
>
> for (1) and (2), patch 5, and 6 will add some helper functions and
> they will be used by patch 7.
>
> so when patch 7 is applied, there will be no compiling warning anymore.
I know that.
But I think each patch should be compiled without warnings.
Patch 5/7 and 6/7 are useless without 7/7. How about merging them?
Regards,
Kenji Kaneshige
On Thu, Feb 2, 2012 at 7:36 PM, Kenji Kaneshige
<[email protected]> wrote:
> (2012/02/03 5:39), Yinghai Lu wrote:
>>
>> On Thu, Feb 2, 2012 at 2:00 AM, Kenji Kaneshige
>> <[email protected]> ?wrote:
>>>
>>> Yinghai, Jesse,
>>>
>>> I tested pciehp with your set of patches. I have some comments below.
>>>
>>> (1) I got a following warning message on compiling the patch [5/7].
>>>
>>> ? ?drivers/pci/hotplug/pciehp_hpc.c:281: warning:
>>> 'pcie_wait_link_not_active' defined but not used
>>>
>>> (2) I got following warning messages on compiling the patch [6/7]
>>>
>>> ? ?drivers/pci/hotplug/pciehp_hpc.c:381: warning: 'pciehp_link_enable'
>>> defined but not used
>>> ? ?drivers/pci/hotplug/pciehp_hpc.c:386: warning: 'pciehp_link_disable'
>>> defined but not used
>>>
>>> (3) I've asked Naoki Yanagimoto, who reported that configuration read
>>> ? ?on some hot-added PCIe device returns invalid value, to test the
>>> ? ?patch. Unfortunately, the problem happens with your patch. But
>>> ? ?after some discussion and testing, it turned out that problem doesn't
>>> ? ?happen when the same card with updated bios is used. So it seems the
>>> ? ?problem is in PCIe card side.
>>>
>>> As a result, problems I found are (1) and (2). Please fix those.
>>> Other than that, pciehp seems to work well.
>>>
>>> Tested-by: Kenji Kaneshige<[email protected]>
>>>
>>
>> Great. thanks for confirmation.
>>
>> for (1) and (2), patch 5, and 6 will add some helper functions and
>> they will be used by patch 7.
>>
>> so when patch 7 is applied, there will be no compiling warning anymore.
>
>
> I know that.
> But I think each patch should be compiled without warnings.
> Patch 5/7 and 6/7 are useless without 7/7. How about merging them?
just want to keep patch small and easy to be reviewed.
and I split it to three intentionally.
Thanks
Yinghai