On 12/07/2011 03:35 PM, Nishanth Aravamudan wrote:
On 07.12.2011 [15:23:38 -0500], Don Dutile wrote:
On 12/07/2011 04:25 AM, Ram Pai wrote:
On Wed, Dec 07, 2011 at 12:22:47AM -0800, Yinghai Lu wrote:
On Mon, Dec 5, 2011 at 10:32 AM, Jesse Barnes<jbarnes@xxxxxxxxxxxxxxxx> wrote:
On Sun, 6 Nov 2011 10:33:10 +0800
Ram Pai<linuxram@xxxxxxxxxx> wrote:
NOTE: Note, there is subtle change in the pci_enable_device() API.
Any driver that depends on SRIOV BARS to be enabled in pci_enable_device()
can fail.
---
Applied to my for-linus branch, thanks.
please don't push to linus now.
this one causes regression.
please check attached patch.
Thanks
Yinghai
[PATCH] pci: Fix hotplug of Express Module with pci bridges
Found hotplug of one setup does not work with recent change in pci tree.
After checking the bridge conf setup, found bridges get assigned, but not get enabled.
Finally found following commit, simplely ignore bridge resource when enabling pci device.
| commit bbef98ab0f019f1b0c25c1acdf1683c68933d41b
| Author: Ram Pai<linuxram@xxxxxxxxxx>
| Date: Sun Nov 6 10:33:10 2011 +0800
|
| PCI: defer enablement of SRIOV BARS
|...
| NOTE: Note, there is subtle change in the pci_enable_device() API. Any
| driver that depends on SRIOV BARS to be enabled in pci_enable_device()
| can fail.
Put back bridge resource and ROM resource checking to fix the problem.
That should fix regression like BIOS does not assign correct resource to bridge.
Signed-off-by: Yinghai Lu<yinghai@xxxxxxxxxx>
---
drivers/pci/pci.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1139,7 +1139,11 @@ static int __pci_enable_device_flags(str
if (atomic_add_return(1,&dev->enable_cnt)> 1)
return 0; /* already enabled */
- for (i = 0; i< PCI_ROM_RESOURCE; i++)
+ /* only skip sriov related */
+ for (i = 0; i<= PCI_ROM_RESOURCE; i++)
+ if (dev->resource[i].flags& flags)
+ bars |= (1<< i);
+ for (i = PCI_BRIDGE_RESOURCES; i< DEVICE_COUNT_RESOURCE; i++)
if (dev->resource[i].flags& flags)
bars |= (1<< i);
Oops. My patch inadvertently dropped ROM and BRIDGE resources.
This patch is right. However would it help if we did something like this
to avoid some code duplication?
for (i = 0; i< DEVICE_COUNT_RESOURCE;
i == PCI_ROM_RESOURCE ? PCI_BRIDGE_RESOURCES: i++)
if (dev->resource[i].flags& flags)
bars |= (1<< i);
why not something more explicit like:
for (i = 0; i< DEVICE_COUNT_RESOURCE; i++) {
if ((i>= PCI_IOV_RESOURCES)&& (i<= PCI_IOV_RESOURCE_END))
continue; /* skip sriov related resources */
if (dev->resource[i].flags& flags)
bars |= (1<< i);
}
I agree with the more explicit approach, self-commenting and easier to
read. Although, you may want to add a comment as to *why* we're skipping
IOV BARs here.
+1 for those not living it, yes, good for historical (& noobie) understanding
-Nish
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
