Re: [RFC PATCH 1/1]PCI: defer enablement of SRIOV BARS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux