On Wed, Dec 7, 2011 at 1:25 AM, Ram Pai <linuxram@xxxxxxxxxx> 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); just don't want to make for () too big. Thanks Yinghai -- 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