* [git pull] PCI fixes
@ 2011-11-23 22:44 Jesse Barnes
2011-11-23 23:02 ` Linus Torvalds
0 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-23 22:44 UTC (permalink / raw)
To: Linus Torvalds, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]
The following changes since commit
839d8810747bbf39e0a5a7f223b67bffa7945f8d:
Merge branch 'i2c-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jdelvare/staging (2011-10-30 15:54:59 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci for-linus
Nothing too exciting this time, mostly some minor fixes for things
normal people don't really hit. Happy Thanksgiving.
Bjorn Helgaas (1):
PCI hotplug: shpchp: don't blindly claim non-AMD 0x7450 device IDs
Joerg Roedel (2):
PCI: Fix compile errors with PCI_ATS and !PCI_IOV
PCI: Let PCI_PRI depend on PCI
Kenji Kaneshige (2):
PCI: pciehp: wait 1000 ms before Link Training check
PCI: pciehp: wait 100 ms after Link Training check
Rafael J. Wysocki (1):
PCI / ACPI: Make acpiphp ignore root bridges using PCIe native hotplug
Yinghai Lu (1):
PCI: pciehp: Retrieve link speed after link is trained
drivers/pci/Kconfig | 1 +
drivers/pci/hotplug/acpiphp_glue.c | 29 ++++++++++++++++++++++++-----
drivers/pci/hotplug/pciehp_ctrl.c | 3 ---
drivers/pci/hotplug/pciehp_hpc.c | 27 ++++++++++++++++++---------
drivers/pci/hotplug/shpchp_core.c | 4 ++--
drivers/pci/hotplug/shpchp_hpc.c | 4 ++--
include/linux/pci-ats.h | 6 +++---
include/linux/pci.h | 2 +-
8 files changed, 51 insertions(+), 25 deletions(-)
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-11-23 22:44 Jesse Barnes
@ 2011-11-23 23:02 ` Linus Torvalds
2011-12-05 19:22 ` Jesse Barnes
0 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2011-11-23 23:02 UTC (permalink / raw)
To: Jesse Barnes; +Cc: linux-pci, linux-kernel
On Wed, Nov 23, 2011 at 2:44 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> Nothing too exciting this time, mostly some minor fixes for things
> normal people don't really hit. Happy Thanksgiving.
Ugh. This looks bogus:
> Kenji Kaneshige (2):
> PCI: pciehp: wait 1000 ms before Link Training check
Look at that patch more closely. After the patch, the code looks like this:
if (ctrl->link_active_reporting)
pcie_wait_link_active(ctrl);
else
msleep(1000);
+ /*
+ * Need to wait for 1000 ms after Data Link Layer Link Active
+ * (DLLLA) bit reads 1b before sending configuration request.
+ * We need it before checking Link Training (LT) bit becuase
+ * LT is still set even after DLLLA bit is set on some platform.
+ */
+ msleep(1000);
and I'm pretty sure you should remove the "else msleep(1000)" there.
Doing the 1s wait *twice* seems entirely bogus, even if you are
missing link_active_reporting. No?
I pulled it since I can't test it, but it really smells fishy to me.
Linus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-11-23 23:02 ` Linus Torvalds
@ 2011-12-05 19:22 ` Jesse Barnes
2011-12-06 8:08 ` Kenji Kaneshige
0 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-12-05 19:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-pci, linux-kernel, Kenji Kaneshige
[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]
On Wed, 23 Nov 2011 15:02:01 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, Nov 23, 2011 at 2:44 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >
> > Nothing too exciting this time, mostly some minor fixes for things
> > normal people don't really hit. Happy Thanksgiving.
>
> Ugh. This looks bogus:
>
> > Kenji Kaneshige (2):
> > PCI: pciehp: wait 1000 ms before Link Training check
>
> Look at that patch more closely. After the patch, the code looks like this:
>
>
> if (ctrl->link_active_reporting)
> pcie_wait_link_active(ctrl);
> else
> msleep(1000);
>
> + /*
> + * Need to wait for 1000 ms after Data Link Layer Link Active
> + * (DLLLA) bit reads 1b before sending configuration request.
> + * We need it before checking Link Training (LT) bit becuase
> + * LT is still set even after DLLLA bit is set on some platform.
> + */
> + msleep(1000);
>
> and I'm pretty sure you should remove the "else msleep(1000)" there.
> Doing the 1s wait *twice* seems entirely bogus, even if you are
> missing link_active_reporting. No?
>
> I pulled it since I can't test it, but it really smells fishy to me.
Sure looks like it... Kenji-san, you went back and forth on this one a
little, can you confirm (and preferably test)?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-05 19:22 ` Jesse Barnes
@ 2011-12-06 8:08 ` Kenji Kaneshige
2011-12-06 16:14 ` Linus Torvalds
0 siblings, 1 reply; 17+ messages in thread
From: Kenji Kaneshige @ 2011-12-06 8:08 UTC (permalink / raw)
To: Jesse Barnes, Linus Torvalds; +Cc: linux-pci, linux-kernel
(2011/12/06 4:22), Jesse Barnes wrote:
> On Wed, 23 Nov 2011 15:02:01 -0800
> Linus Torvalds<torvalds@linux-foundation.org> wrote:
>
>> On Wed, Nov 23, 2011 at 2:44 PM, Jesse Barnes<jbarnes@virtuousgeek.org> wrote:
>>>
>>> Nothing too exciting this time, mostly some minor fixes for things
>>> normal people don't really hit. Happy Thanksgiving.
>>
>> Ugh. This looks bogus:
>>
>>> Kenji Kaneshige (2):
>>> PCI: pciehp: wait 1000 ms before Link Training check
>>
>> Look at that patch more closely. After the patch, the code looks like this:
>>
>>
>> if (ctrl->link_active_reporting)
>> pcie_wait_link_active(ctrl);
>> else
>> msleep(1000);
>>
>> + /*
>> + * Need to wait for 1000 ms after Data Link Layer Link Active
>> + * (DLLLA) bit reads 1b before sending configuration request.
>> + * We need it before checking Link Training (LT) bit becuase
>> + * LT is still set even after DLLLA bit is set on some platform.
>> + */
>> + msleep(1000);
>>
>> and I'm pretty sure you should remove the "else msleep(1000)" there.
>> Doing the 1s wait *twice* seems entirely bogus, even if you are
>> missing link_active_reporting. No?
>>
>> I pulled it since I can't test it, but it really smells fishy to me.
>
> Sure looks like it... Kenji-san, you went back and forth on this one a
> little, can you confirm (and preferably test)?
>
> Thanks,
I'm very sorry for the long delay.
Each 1000 ms wait has defferent meaning. But as Linus pointed out, the
first one ("else msleep(1000)") can be removed. I'll work on this.
Though the patch is not clean, I tested it from the regression point of
view. And Yinghai also confirmed it fix his problem.
Here are explanation for each 1000 ms with history.
>> if (ctrl->link_active_reporting)
>> pcie_wait_link_active(ctrl);
>> else
>> msleep(1000);
This chunk is to wait for Link Active. After PCIe specification 1.1,
downstream port with hotplug slots needs to support link active reporting.
But older specification PCIe 1.0a doesn't have link active reporting. The
"else msleep(1000)" is for PCIe 1.0a. It wait for 1 sec instead of polling
data link layer link active bit. In PCIe 1.0a spec, the bit corresponding
to the data link layer link active bit in PCIe 1.1 or later is defined
reserved zero. So as Linus pointed out, we can remove "else msleep(1000)".
The reason why I added "if (crtl->link_active_reporting)" here was that I
didn't want to change the behavior for old hot-plug controller (PCIe 1.0a),
but I think it was too much.
>> + /*
>> + * Need to wait for 1000 ms after Data Link Layer Link Active
>> + * (DLLLA) bit reads 1b before sending configuration request.
>> + * We need it before checking Link Training (LT) bit becuase
>> + * LT is still set even after DLLLA bit is set on some platform.
>> + */
>> + msleep(1000);
In the past, pciehp waits 100 ms instead of 1 sec after checking the link
state. This 100 ms was based on PCIe spec. But we encountered the problem
that configuration access to some HBA card doesn't return the proper value.
To fix this problem, we replaced this 100 ms wait with 1 sec wait. This is
based on the PCIe description "software must allow 1 second after the Data
Link Layer Link Active bit reads 1b before it is permitted to determine
that a hot plugged device which fails to return a Successful Completion for
a Valid Configuration Request is a broken device". Then, another problem
was reported by Yinghai Lu. The problem is that LT bit is still set even
after DLLLA bit is set on his platform. To fix the problem 1 sec wait was
moved to before LT check, which was placed after LT check.
Regards,
Kenji Kaneshige
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-06 8:08 ` Kenji Kaneshige
@ 2011-12-06 16:14 ` Linus Torvalds
2011-12-06 22:36 ` Yinghai Lu
2011-12-07 7:58 ` Kenji Kaneshige
0 siblings, 2 replies; 17+ messages in thread
From: Linus Torvalds @ 2011-12-06 16:14 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: Jesse Barnes, linux-pci, linux-kernel
On Tue, Dec 6, 2011 at 12:08 AM, Kenji Kaneshige
<kaneshige.kenji@jp.fujitsu.com> wrote:
>
> In the past, pciehp waits 100 ms instead of 1 sec after checking the link
> state. This 100 ms was based on PCIe spec.
I would like to point out that these kinds of delays are *really*
annoying to users. And they add up.
One second per se is not a huge problem, but imagine that you're
hotplugging some regular user device (think thunderbolt or something
that we'd expect normal users to see). First one second for the kernel
to even see it, then some random udev rules, then some disk spinup
times or whatever, and soon a few delays here and a few delays there,
and it takes say three seconds for the folder to show up on the
desktop (or whatever acknowledgement of "yes, I see your device now").
That's a *long* time, and it's irritating to the user. It makes the
user think "the machine is slow".
We used to have this exact problem with USB hotplugging with slow
devices, so I know. It's still not necessarily immediate, but it's
better than it has been.
One second *total* is what people will consider pretty much immediate.
Any more than that is "thumb twiddling time".
And quite frankly, an unconditional one-second delay here seems bad.
Two seconds was unacceptable, one second is just bad.
> This is
> based on the PCIe description "software must allow 1 second after the Data
> Link Layer Link Active bit reads 1b before it is permitted to determine
> that a hot plugged device which fails to return a Successful Completion for
> a Valid Configuration Request is a broken device".
Quite frankly, the way that reads to me says "you must wait at most 1s
before you consider a device broken".
But a *successful* read of the LT bit should abort the wait early. So
that good devices that aren't broken can complete setup much faster.
Please try to make something like that work. Instead of always waiting
for one second, wait for up to one second only for failure cases. Any
possibility of that?
Clearly most devices are perfectly fine almost immediately. It's sad
to wait for good devices for no good reason.
Linus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-06 16:14 ` Linus Torvalds
@ 2011-12-06 22:36 ` Yinghai Lu
2011-12-07 8:18 ` Kenji Kaneshige
2011-12-07 7:58 ` Kenji Kaneshige
1 sibling, 1 reply; 17+ messages in thread
From: Yinghai Lu @ 2011-12-06 22:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Kenji Kaneshige, Jesse Barnes, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On Tue, Dec 6, 2011 at 8:14 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Please try to make something like that work. Instead of always waiting
> for one second, wait for up to one second only for failure cases. Any
> possibility of that?
i had another version that was not send out. that worked on my test setups too.
it will try to read pci conf several times in 1s.
please check refreshed version against your tree.
Thanks
Yinghai
[-- Attachment #2: pciehp_debug_x_3_1_access_pci_conf.patch --]
[-- Type: text/x-patch, Size: 2158 bytes --]
---
drivers/pci/hotplug/pciehp_hpc.c | 47 ++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 15 deletions(-)
Index: linux-2.6/drivers/pci/hotplug/pciehp_hpc.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_hpc.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_hpc.c
@@ -265,6 +265,35 @@ static void pcie_wait_link_active(struct
ctrl_dbg(ctrl, "Data Link Layer Link Active not set in 1000 msec\n");
}
+static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
+{
+ u32 l;
+ int delay = 1000;
+
+again:
+ if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, &l))
+ goto wait;
+
+ /* some broken boards return 0 or ~0 if a slot is empty: */
+ if (l == 0xffffffff || l == 0x00000000 ||
+ l == 0x0000ffff || l == 0xffff0000)
+ goto wait;
+
+ /* Configuration request Retry Status */
+ if (l == 0xffff0001)
+ goto wait;
+
+ return true;
+
+wait:
+ mdelay(100);
+ delay -= 100;
+ if (delay > 0)
+ goto again;
+
+ return false;
+}
+
int pciehp_check_link_status(struct controller *ctrl)
{
u16 lnk_status;
@@ -280,13 +309,9 @@ int pciehp_check_link_status(struct cont
else
msleep(1000);
- /*
- * Need to wait for 1000 ms after Data Link Layer Link Active
- * (DLLLA) bit reads 1b before sending configuration request.
- * We need it before checking Link Training (LT) bit becuase
- * LT is still set even after DLLLA bit is set on some platform.
- */
- msleep(1000);
+ /* wait 100ms before read pci conf, and try in 1s */
+ msleep(100);
+ pci_bus_check_dev(ctrl->pcie->port->subordinate, PCI_DEVFN(0, 0));
retval = pciehp_readw(ctrl, PCI_EXP_LNKSTA, &lnk_status);
if (retval) {
@@ -302,14 +327,6 @@ int pciehp_check_link_status(struct cont
return retval;
}
- /*
- * If the port supports Link speeds greater than 5.0 GT/s, we
- * must wait for 100 ms after Link training completes before
- * sending configuration request.
- */
- if (ctrl->pcie->port->subordinate->max_bus_speed > PCIE_SPEED_5_0GT)
- msleep(100);
-
pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status);
return retval;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-06 16:14 ` Linus Torvalds
2011-12-06 22:36 ` Yinghai Lu
@ 2011-12-07 7:58 ` Kenji Kaneshige
1 sibling, 0 replies; 17+ messages in thread
From: Kenji Kaneshige @ 2011-12-07 7:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jesse Barnes, linux-pci, linux-kernel
(2011/12/07 1:14), Linus Torvalds wrote:
> On Tue, Dec 6, 2011 at 12:08 AM, Kenji Kaneshige
> <kaneshige.kenji@jp.fujitsu.com> wrote:
>>
>> In the past, pciehp waits 100 ms instead of 1 sec after checking the link
>> state. This 100 ms was based on PCIe spec.
>
> I would like to point out that these kinds of delays are *really*
> annoying to users. And they add up.
>
> One second per se is not a huge problem, but imagine that you're
> hotplugging some regular user device (think thunderbolt or something
> that we'd expect normal users to see). First one second for the kernel
> to even see it, then some random udev rules, then some disk spinup
> times or whatever, and soon a few delays here and a few delays there,
> and it takes say three seconds for the folder to show up on the
> desktop (or whatever acknowledgement of "yes, I see your device now").
>
> That's a *long* time, and it's irritating to the user. It makes the
> user think "the machine is slow".
>
> We used to have this exact problem with USB hotplugging with slow
> devices, so I know. It's still not necessarily immediate, but it's
> better than it has been.
>
> One second *total* is what people will consider pretty much immediate.
> Any more than that is "thumb twiddling time".
>
> And quite frankly, an unconditional one-second delay here seems bad.
> Two seconds was unacceptable, one second is just bad.
>
>> This is
>> based on the PCIe description "software must allow 1 second after the Data
>> Link Layer Link Active bit reads 1b before it is permitted to determine
>> that a hot plugged device which fails to return a Successful Completion for
>> a Valid Configuration Request is a broken device".
>
> Quite frankly, the way that reads to me says "you must wait at most 1s
> before you consider a device broken".
>
> But a *successful* read of the LT bit should abort the wait early. So
> that good devices that aren't broken can complete setup much faster.
>
> Please try to make something like that work. Instead of always waiting
> for one second, wait for up to one second only for failure cases. Any
> possibility of that?
>
> Clearly most devices are perfectly fine almost immediately. It's sad
> to wait for good devices for no good reason.
>
Thank you for comments. Yes, I think we can do more efforts on this.
To improve this, I think what we need is something to know if configuration
request to the hot-added device starts working. One possible idea so far is
to enable CRS Software Visibility and check vendor ID. The pci_scan_device()
function already has vendor ID check, but the code to enable CRS was removed
for some reason.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ad7edfe0490877864dc0312e5f3315ea37fc4b3a
Regards,
Kenji Kaneshige
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-06 22:36 ` Yinghai Lu
@ 2011-12-07 8:18 ` Kenji Kaneshige
2011-12-07 19:20 ` Yinghai Lu
0 siblings, 1 reply; 17+ messages in thread
From: Kenji Kaneshige @ 2011-12-07 8:18 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Linus Torvalds, Jesse Barnes, linux-pci, linux-kernel
+ /* Configuration request Retry Status */
+ if (l == 0xffff0001)
+ goto wait;
I think this works only when CRS Software Visibility is enabled,
isn't it? Currently there is no code to enable CRS Software
Visibility. And there can be root ports that doesn't support CRS
Software Visibility. We need consideration about this.
+ mdelay(100);
+ delay -= 100;
+ if (delay > 0)
+ goto again;
We must use msleep() instead of mdelay().
Regards,
Kenji Kaneshige
(2011/12/07 7:36), Yinghai Lu wrote:
> On Tue, Dec 6, 2011 at 8:14 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> Please try to make something like that work. Instead of always waiting
>> for one second, wait for up to one second only for failure cases. Any
>> possibility of that?
>
> i had another version that was not send out. that worked on my test setups too.
> it will try to read pci conf several times in 1s.
>
> please check refreshed version against your tree.
>
> Thanks
>
> Yinghai
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-07 8:18 ` Kenji Kaneshige
@ 2011-12-07 19:20 ` Yinghai Lu
0 siblings, 0 replies; 17+ messages in thread
From: Yinghai Lu @ 2011-12-07 19:20 UTC (permalink / raw)
To: Kenji Kaneshige; +Cc: Linus Torvalds, Jesse Barnes, linux-pci, linux-kernel
On Wed, Dec 7, 2011 at 12:18 AM, Kenji Kaneshige
<kaneshige.kenji@jp.fujitsu.com> wrote:
> + /* Configuration request Retry Status */
> + if (l == 0xffff0001)
> + goto wait;
>
> I think this works only when CRS Software Visibility is enabled,
> isn't it? Currently there is no code to enable CRS Software
> Visibility. And there can be root ports that doesn't support CRS
> Software Visibility. We need consideration about this.
I copied and change from pci_scan_device...
/*
* Read the config data for a PCI device, sanity-check it
* and fill in the dev structure...
*/
static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
{
struct pci_dev *dev;
u32 l;
int delay = 1;
if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, &l))
return NULL;
/* some broken boards return 0 or ~0 if a slot is empty: */
if (l == 0xffffffff || l == 0x00000000 ||
l == 0x0000ffff || l == 0xffff0000)
return NULL;
/* Configuration request Retry Status */
while (l == 0xffff0001) {
msleep(delay);
delay *= 2;
if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, &l))
return NULL;
/* Card hasn't responded in 60 seconds? Must be stuck. */
if (delay > 60 * 1000) {
printk(KERN_WARNING "pci %04x:%02x:%02x.%d: not "
"responding\n", pci_domain_nr(bus),
bus->number, PCI_SLOT(devfn),
PCI_FUNC(devfn));
return NULL;
}
}
so CRS code is not removed cleanly?
>
> + mdelay(100);
> + delay -= 100;
> + if (delay > 0)
> + goto again;
>
> We must use msleep() instead of mdelay().
yes.
Thanks
Yinghai Lu
^ permalink raw reply [flat|nested] 17+ messages in thread
* [git pull] PCI fixes
@ 2011-12-17 17:29 Jesse Barnes
2011-12-18 2:33 ` Yinghai Lu
0 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-12-17 17:29 UTC (permalink / raw)
To: Linus Torvalds, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]
The following changes since commit
8e8da023f5af71662867729db5547dc54786093c:
x86: Fix boot failures on older AMD CPU's (2011-12-04 11:57:09 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci for-linus
Just a few regression fixes. We have a fix for the extra msleep you
pointed out last time as well, but I was saving it for -next since it
took awhile for Kenji-san and Yinghai to settle on something, and it
changes a core behavior.
Ajaykumar Hotchandani (1):
PCI: Set device power state to PCI_D0 for device without native PM support
James Bottomley (1):
PCI: fix ats compile failure
Rafael J. Wysocki (1):
PCI hotplug: Always allow acpiphp to handle non-PCIe bridges
Ram Pai (1):
PCI: defer enablement of SRIOV BARS
drivers/pci/ats.c | 1 +
drivers/pci/hotplug/acpiphp_glue.c | 30 ++++++++++++------------------
drivers/pci/iov.c | 7 +++++++
drivers/pci/pci.c | 5 ++++-
4 files changed, 24 insertions(+), 19 deletions(-)
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-17 17:29 Jesse Barnes
@ 2011-12-18 2:33 ` Yinghai Lu
2011-12-18 5:52 ` Jesse Barnes
0 siblings, 1 reply; 17+ messages in thread
From: Yinghai Lu @ 2011-12-18 2:33 UTC (permalink / raw)
To: Jesse Barnes, Linus Torvalds; +Cc: linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 285 bytes --]
On Sat, Dec 17, 2011 at 9:29 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Ram Pai (1):
> PCI: defer enablement of SRIOV BARS
oh, no!
It seems you let the old version slip into linus tree.
Now we need attached patch get into linus tree.
Thanks
Yinghai Lu
[-- Attachment #2: fix_sriov_late.patch --]
[-- Type: text/x-patch, Size: 1660 bytes --]
[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@us.ibm.com>
| 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.
Discussion can be found at:
http://www.spinics.net/lists/linux-pci/msg12874.html
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
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);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-18 2:33 ` Yinghai Lu
@ 2011-12-18 5:52 ` Jesse Barnes
2011-12-18 22:14 ` Linus Torvalds
0 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-12-18 5:52 UTC (permalink / raw)
To: Yinghai Lu, Linus Torvalds; +Cc: linux-pci, linux-kernel
Yinghai Lu <yinghai@kernel.org> wrote:
>On Sat, Dec 17, 2011 at 9:29 AM, Jesse Barnes
><jbarnes@virtuousgeek.org> wrote:
>> Ram Pai (1):
>> PCI: defer enablement of SRIOV BARS
>
>oh, no!
>
>It seems you let the old version slip into linus tree.
>
>Now we need attached patch get into linus tree.
>
>Thanks
>
>Yinghai Lu
Crap, and after you guys spent so long coming up with a clean version.
Linus, can you just apply this one on top? I think it's the one Yinghai and Ram agreed to.
Thanks,
Jesse
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-18 5:52 ` Jesse Barnes
@ 2011-12-18 22:14 ` Linus Torvalds
2012-01-06 20:46 ` Jesse Barnes
0 siblings, 1 reply; 17+ messages in thread
From: Linus Torvalds @ 2011-12-18 22:14 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Yinghai Lu, linux-pci, linux-kernel
On Sat, Dec 17, 2011 at 9:52 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> Crap, and after you guys spent so long coming up with a clean version.
>
> Linus, can you just apply this one on top? I think it's the one Yinghai and Ram agreed to.
Ok, applied, but I have to say that I hate it.
Why don't we just move the stupid iov indexes to the end, and then
ignore them by not counting through them in
__pci_enable_device_flags()? So the regular PCI code would always walk
through the resources 0 .. PCI_BRIDGE_RESOURCES_END or something. And
then the magic IOV code could look at its own ones that the generic
code apparently doesn't even want to know about.
Hmm?
But as mentioned, in the meantime I applied this.
Linus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2011-12-18 22:14 ` Linus Torvalds
@ 2012-01-06 20:46 ` Jesse Barnes
2012-01-07 1:14 ` Yinghai Lu
0 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2012-01-06 20:46 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Yinghai Lu, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]
On Sun, 18 Dec 2011 14:14:13 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sat, Dec 17, 2011 at 9:52 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >
> > Crap, and after you guys spent so long coming up with a clean version.
> >
> > Linus, can you just apply this one on top? I think it's the one Yinghai and Ram agreed to.
>
> Ok, applied, but I have to say that I hate it.
>
> Why don't we just move the stupid iov indexes to the end, and then
> ignore them by not counting through them in
> __pci_enable_device_flags()? So the regular PCI code would always walk
> through the resources 0 .. PCI_BRIDGE_RESOURCES_END or something. And
> then the magic IOV code could look at its own ones that the generic
> code apparently doesn't even want to know about.
>
> Hmm?
There was some talk awhile back of pulling the SR-IOV resources out of
the pci_dev resource array (adding a separate member to track them only
if CONFIG_PCI_IOV is enabled), that would probably be the way to go.
Then fix up whatever resource walking code that cares about IOV BARs
(there isn't much I think).
Any thoughts Yinghai?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [git pull] PCI fixes
2012-01-06 20:46 ` Jesse Barnes
@ 2012-01-07 1:14 ` Yinghai Lu
0 siblings, 0 replies; 17+ messages in thread
From: Yinghai Lu @ 2012-01-07 1:14 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Linus Torvalds, linux-pci, linux-kernel
On Fri, Jan 6, 2012 at 12:46 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> There was some talk awhile back of pulling the SR-IOV resources out of
> the pci_dev resource array (adding a separate member to track them only
> if CONFIG_PCI_IOV is enabled), that would probably be the way to go.
> Then fix up whatever resource walking code that cares about IOV BARs
> (there isn't much I think).
>
> Any thoughts Yinghai?
We can add helpers like
for_each_resource_for_dev()
for_each_resource_for_dev_bridge()
Thanks
Yinghai
^ permalink raw reply [flat|nested] 17+ messages in thread
* [git pull] PCI fixes
@ 2012-02-17 17:24 Jesse Barnes
0 siblings, 0 replies; 17+ messages in thread
From: Jesse Barnes @ 2012-02-17 17:24 UTC (permalink / raw)
To: Linus Torvalds, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
The following changes since commit
acb42a3b611d7ad4cb173c3b37674b549df2ffeb:
Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux (2012-01-27 07:56:25 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci for-linus
One regression fix for SR-IOV on PPC and a couple of misc fixes from
Yinghai.
Vaidyanathan Srinivasan (1):
PCI: set pci sriov page size before reading SRIOV BAR
Yinghai Lu (2):
PCI: workaround hard-wired bus number V2
PCI: Fix pci cardbus removal
drivers/pci/iov.c | 3 +--
drivers/pci/probe.c | 5 +++++
drivers/pci/remove.c | 28 ++++++++++++++++++++++------
3 files changed, 28 insertions(+), 8 deletions(-)
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [git pull] PCI fixes
@ 2012-03-05 21:49 Jesse Barnes
0 siblings, 0 replies; 17+ messages in thread
From: Jesse Barnes @ 2012-03-05 21:49 UTC (permalink / raw)
To: Linus Torvalds, linux-pci, linux-kernel
The following changes since commit
45196cee28a5bcfb6ddbe2bffa4270cbed66ae4b:
Merge tag 'usb-3.3-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb (2012-02-22 13:00:53 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci for-linus
A couple of fixes for booting specific machines, and one for a minor
memory leak on pre-_CRS platforms.
Jonathan Nieder (2):
x86/PCI: use host bridge _CRS info on MSI MS-7253
x86/PCI: do not tie MSI MS-7253 use_crs quirk to BIOS version
Yinghai Lu (1):
PCI: fix memleak when ACPI _CRS is not used.
arch/x86/pci/acpi.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
Thanks,
Jesse
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-03-05 21:49 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-17 17:24 [git pull] PCI fixes Jesse Barnes
-- strict thread matches above, loose matches on Subject: below --
2012-03-05 21:49 Jesse Barnes
2011-12-17 17:29 Jesse Barnes
2011-12-18 2:33 ` Yinghai Lu
2011-12-18 5:52 ` Jesse Barnes
2011-12-18 22:14 ` Linus Torvalds
2012-01-06 20:46 ` Jesse Barnes
2012-01-07 1:14 ` Yinghai Lu
2011-11-23 22:44 Jesse Barnes
2011-11-23 23:02 ` Linus Torvalds
2011-12-05 19:22 ` Jesse Barnes
2011-12-06 8:08 ` Kenji Kaneshige
2011-12-06 16:14 ` Linus Torvalds
2011-12-06 22:36 ` Yinghai Lu
2011-12-07 8:18 ` Kenji Kaneshige
2011-12-07 19:20 ` Yinghai Lu
2011-12-07 7:58 ` Kenji Kaneshige
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).