[PATCH] PCI: workaround hard-wired bus number

[PATCH] PCI: workaround hard-wired bus number

[Thomas Renninger]

From: Yinghai Lu <yinghai.lu@oracle.com>

Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
when using ACPI resources (_CRS).
This is default, a manual workaround (without this patch)
would be pci=nocrs boot param.

Tested-by: garyhade@us.ibm.com
CC: trenn@suse.de
CC: bhelgaas@google.com
Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 drivers/pci/probe.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7cc9e2f..1e255c8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -651,6 +651,10 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
 		secondary, subordinate, pass);
 
+	/* some bridge primary bus is hard wired to 0 */
+	if (!primary && (primary != bus->number) && secondary && subordinate)
+		primary = bus->number;
+
 	/* Check if setup is sensible at all */
 	if (!pass &&
 	    (primary != bus->number || secondary <= bus->number)) {
-- 
1.7.6.1

Re: [PATCH] PCI: workaround hard-wired bus number

[Bjorn Helgaas]

On Fri, Jan 27, 2012 at 2:11 AM, Thomas Renninger <trenn@suse.de> wrote:
> From: Yinghai Lu <yinghai.lu@oracle.com>
>
> Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
> when using ACPI resources (_CRS).
> This is default, a manual workaround (without this patch)
> would be pci=nocrs boot param.
>
> Tested-by: garyhade@us.ibm.com
> CC: trenn@suse.de
> CC: bhelgaas@google.com
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> ---
>  drivers/pci/probe.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 7cc9e2f..1e255c8 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -651,6 +651,10 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>        dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
>                secondary, subordinate, pass);
>
> +       /* some bridge primary bus is hard wired to 0 */
> +       if (!primary && (primary != bus->number) && secondary && subordinate)
> +               primary = bus->number;
> +
>        /* Check if setup is sensible at all */
>        if (!pass &&
>            (primary != bus->number || secondary <= bus->number)) {
> --
> 1.7.6.1
>

I'm not thrilled with adding a machine-specific quirk in this generic
code path because future rework of this path will always have to
consider and preserve it.

I wonder whether it would be possible to set this up so we read
PCI_PRIMARY_BUS earlier, so we could update that primary bus number in
a quirk before getting into the rest of pci_scan_bridge().  That might
take non-trivial rework, since the cached copy of the primary bus
number is currently in struct pci_bus, not in struct pci_dev.  But
that in itself is non-intuitive to me: the primary, secondary, and
subordinate bus numbers are all in struct pci_bus, when they seem more
like a property of the bridge device than of the bus.

If we put something like this in mainline, I think it needs a bugzilla
reference, preferably in bugzilla.kernel.org, where the details (DMI
info, vendor/device ID of the bridge, etc.) will be public (there is a
Novell bugzilla, but I don't think it's public).

Bjorn

Re: [PATCH] PCI: workaround hard-wired bus number

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

On Fri, 27 Jan 2012 09:26:16 -0700
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Fri, Jan 27, 2012 at 2:11 AM, Thomas Renninger <trenn@suse.de> wrote:
> > From: Yinghai Lu <yinghai.lu@oracle.com>
> >
> > Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
> > when using ACPI resources (_CRS).
> > This is default, a manual workaround (without this patch)
> > would be pci=nocrs boot param.
> >
> > Tested-by: garyhade@us.ibm.com
> > CC: trenn@suse.de
> > CC: bhelgaas@google.com
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > ---
> >  drivers/pci/probe.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 7cc9e2f..1e255c8 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -651,6 +651,10 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
> >        dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
> >                secondary, subordinate, pass);
> >
> > +       /* some bridge primary bus is hard wired to 0 */
> > +       if (!primary && (primary != bus->number) && secondary && subordinate)
> > +               primary = bus->number;
> > +
> >        /* Check if setup is sensible at all */
> >        if (!pass &&
> >            (primary != bus->number || secondary <= bus->number)) {
> > --
> > 1.7.6.1
> >
> 
> I'm not thrilled with adding a machine-specific quirk in this generic
> code path because future rework of this path will always have to
> consider and preserve it.
> 
> I wonder whether it would be possible to set this up so we read
> PCI_PRIMARY_BUS earlier, so we could update that primary bus number in
> a quirk before getting into the rest of pci_scan_bridge().  That might
> take non-trivial rework, since the cached copy of the primary bus
> number is currently in struct pci_bus, not in struct pci_dev.  But
> that in itself is non-intuitive to me: the primary, secondary, and
> subordinate bus numbers are all in struct pci_bus, when they seem more
> like a property of the bridge device than of the bus.
> 
> If we put something like this in mainline, I think it needs a bugzilla
> reference, preferably in bugzilla.kernel.org, where the details (DMI
> info, vendor/device ID of the bridge, etc.) will be public (there is a
> Novell bugzilla, but I don't think it's public).

Thomas or Yinghai, any references for this one just in case someone
wants to clean it up later or get testing for a related change?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] PCI: workaround hard-wired bus number

[Thomas Renninger]

On Friday, January 27, 2012 06:38:35 PM Jesse Barnes wrote:
...
> Thomas or Yinghai, any references for this one just in case someone
> wants to clean it up later or get testing for a related change?

https://bugzilla.kernel.org/show_bug.cgi?id=42692

  Thomas

Re: [PATCH] PCI: workaround hard-wired bus number V2

[Thomas Renninger]

From: Yinghai Lu <yinghai.lu@oracle.com>

Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
when using ACPI resources (_CRS).
This is default, a manual workaround (without this patch)
would be pci=nocrs boot param.

V2: Add dev_warn if the workaround is hit. This should reveal
how common such setups are (via google) and point to possible
problems if things are still not working as expected.
-> Suggested by Jan Beulich.

Tested-by: garyhade@us.ibm.com
---
 drivers/pci/probe.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7cc9e2f..71eac9c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -651,6 +651,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
 		secondary, subordinate, pass);
 
+	if (!primary && (primary != bus->number) && secondary && subordinate) {
+		dev_warn(&dev->dev, "Primary bus is hard wired to 0\n");
+		primary = bus->number;
+	}
+
 	/* Check if setup is sensible at all */
 	if (!pass &&
 	    (primary != bus->number || secondary <= bus->number)) {

Re: [PATCH] PCI: workaround hard-wired bus number V2

[Thomas Renninger]

From: Yinghai Lu <yinghai.lu@oracle.com>

Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
when using ACPI resources (_CRS).
This is default, a manual workaround (without this patch)
would be pci=nocrs boot param.

V2: Add dev_warn if the workaround is hit. This should reveal
how common such setups are (via google) and point to possible
problems if things are still not working as expected.
-> Suggested by Jan Beulich.

Tested-by: garyhade@us.ibm.com
CC: stable@vger.kernel.org
CC: trenn@suse.de

---
 drivers/pci/probe.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7cc9e2f..71eac9c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -651,6 +651,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
 		secondary, subordinate, pass);
 
+	if (!primary && (primary != bus->number) && secondary && subordinate) {
+		dev_warn(&dev->dev, "Primary bus is hard wired to 0\n");
+		primary = bus->number;
+	}
+
 	/* Check if setup is sensible at all */
 	if (!pass &&
 	    (primary != bus->number || secondary <= bus->number)) {

Re: [PATCH] PCI: workaround hard-wired bus number V2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

On Mon, 30 Jan 2012 12:25:24 +0100
Thomas Renninger <trenn@suse.de> wrote:

> From: Yinghai Lu <yinghai.lu@oracle.com>
> 
> Fixes PCI device detection on IBM xSeries IBM 3850 M2 / x3950 M2
> when using ACPI resources (_CRS).
> This is default, a manual workaround (without this patch)
> would be pci=nocrs boot param.
> 
> V2: Add dev_warn if the workaround is hit. This should reveal
> how common such setups are (via google) and point to possible
> problems if things are still not working as expected.
> -> Suggested by Jan Beulich.
> 
> Tested-by: garyhade@us.ibm.com
> CC: stable@vger.kernel.org
> CC: trenn@suse.de
> 
> ---
>  drivers/pci/probe.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 7cc9e2f..71eac9c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -651,6 +651,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>  	dev_dbg(&dev->dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
>  		secondary, subordinate, pass);
>  
> +	if (!primary && (primary != bus->number) && secondary && subordinate) {
> +		dev_warn(&dev->dev, "Primary bus is hard wired to 0\n");
> +		primary = bus->number;
> +	}
> +
>  	/* Check if setup is sensible at all */
>  	if (!pass &&
>  	    (primary != bus->number || secondary <= bus->number)) {
> 

Applied, thanks Thomas.  I'll send this over to Linus next week
assuming it doesn't blow up before then.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]