[PATCHv2 0/3] PCI Speed Cap Fixes for ppc64

[PATCHv2 0/3] PCI Speed Cap Fixes for ppc64

[Lucas Kannebley Tavares]

This patch series first implements a function called pcie_get_speed_cap_mask 
in the PCI subsystem based off from drm_pcie_get_speed_cap_mask in drm. Then 
it removes the latter and fixes all references to it. And ultimately, it
implements an architecture-specific version of the same function for ppc64.

The refactor is done because the function that was used by drm is more 
architecture goo than module-specific. Whilst the function also needed a 
platform-specific implementation to get PCIE Gen2 speeds on ppc64.

Lucas Kannebley Tavares (3):
  pci: added pcie_get_speed_cap_mask function
  drm: removed drm_pcie_get_speed_cap_mask function
  ppc64: implemented pcibios_get_speed_cap_mask

 arch/powerpc/platforms/pseries/pci.c |   35 +++++++++++++++++++++++++++
 drivers/gpu/drm/drm_pci.c            |   38 -----------------------------
 drivers/gpu/drm/radeon/evergreen.c   |    5 ++-
 drivers/gpu/drm/radeon/r600.c        |    5 ++-
 drivers/gpu/drm/radeon/rv770.c       |    5 ++-
 drivers/pci/pci.c                    |   44 ++++++++++++++++++++++++++++++++++
 include/drm/drmP.h                   |    6 ----
 include/linux/pci.h                  |    6 ++++
 8 files changed, 94 insertions(+), 50 deletions(-)

-- 
1.7.4.4

[PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function

[Lucas Kannebley Tavares]

Added function to gather the speed cap for a device and return a mask to
supported speeds. The function is divided into an interface and a weak
implementation so that architecture-specific functions can be called.

This is the first step in moving function drm_pcie_get_speed_cap_mask
from the drm subsystem to the pci one.

Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
 drivers/pci/pci.c   |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    6 ++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b099e00..d94ab79 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3931,6 +3931,50 @@ static int __init pci_setup(char *str)
 }
 early_param("pci", pci_setup);
 
+int __weak pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+	struct pci_dev *root;
+	u32 lnkcap, lnkcap2;
+
+	*mask = 0;
+	if (!dev)
+		return -EINVAL;
+
+	root = dev->bus->self;
+
+	/* we've been informed via and serverworks don't make the cut */
+	if (root->vendor == PCI_VENDOR_ID_VIA ||
+	    root->vendor == PCI_VENDOR_ID_SERVERWORKS)
+		return -EINVAL;
+
+	pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap);
+	pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
+
+	if (lnkcap2) {	/* PCIe r3.0-compliant */
+		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
+			*mask |= PCIE_SPEED_25;
+		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
+			*mask |= PCIE_SPEED_50;
+		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
+			*mask |= PCIE_SPEED_80;
+	} else {	/* pre-r3.0 */
+		if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
+			*mask |= PCIE_SPEED_25;
+		if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
+			*mask |= (PCIE_SPEED_25 | PCIE_SPEED_50);
+	}
+
+	dev_info(&dev->dev, "probing gen 2 caps for device %x:%x = %x/%x\n",
+		root->vendor, root->device, lnkcap, lnkcap2);
+	return 0;
+}
+
+int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+	return pcibios_get_speed_cap_mask(dev, mask);
+}
+EXPORT_SYMBOL(pcie_get_speed_cap_mask);
+
 EXPORT_SYMBOL(pci_reenable_device);
 EXPORT_SYMBOL(pci_enable_device_io);
 EXPORT_SYMBOL(pci_enable_device_mem);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2461033a..24a2f63 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1861,4 +1861,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
  */
 struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
 
+#define PCIE_SPEED_25 1
+#define PCIE_SPEED_50 2
+#define PCIE_SPEED_80 4
+
+extern int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *speed_mask);
+
 #endif /* LINUX_PCI_H */
-- 
1.7.4.4

Re: [PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function

[Bjorn Helgaas]

On Tue, Mar 19, 2013 at 11:24 PM, Lucas Kannebley Tavares
<lucaskt@linux.vnet.ibm.com> wrote:
> Added function to gather the speed cap for a device and return a mask to
> supported speeds. The function is divided into an interface and a weak
> implementation so that architecture-specific functions can be called.
>
> This is the first step in moving function drm_pcie_get_speed_cap_mask
> from the drm subsystem to the pci one.

This still doesn't feel right to me.

I'm definitely not a hardware guy, but my understanding based on the
PCIe spec r3.0, sec 6.11, is that the hardware will automatically
maintain the link at the highest speed supported by both ends of the
link, unless software sets a lower Target Link Speed.  The only users
of this function are some Radeon drivers, and it looks like they only
use it because the hardware doesn't conform to this aspect of the spec
and requires device-specific tweaking.

We already have bus->max_bus_speed, which should tell you the max
speed supported by the upstream component.  Is that enough
information?  Maybe the radeon drivers could simply do something like
this:

    speed = rdev->ddev->pdev->bus->max_bus_speed;
    if (speed == PCI_SPEED_UNKNOWN || speed < PCIE_SPEED_5_0GT)
        return;

In your original email [1], you mentioned a null pointer dereference,
presumably when reading the PCIe capability for dev->pdev->bus->self,
with "self" being NULL.  This only happens for a bus with no upstream
P2P bridge, i.e., self will be NULL only if pdev is on a root bus.
But we also know pdev is a PCIe device, and I think a PCIe device on a
root bus must be a "Root Complex Integrated Endpoint" (PCIe spec sec
1.3.2.3).  Such a device does not have a link at all, so there's no
point in fiddling with its link speed.

I don't see how a radeon device could be an integrated endpoint, but
in your hypervisor environment, maybe it isn't quite spec-compliant in
terms of its attachment.  Can you collect the output of "lspci -vv"?
Maybe that will make things clearer.

In any case, if a radeon device is on a root bus, I think the root bus
max_bus_speed will be PCI_SPEED_UNKNOWN, and if it's not on a root
bus, max_bus_speed should be set correctly based on the upstream PCIe
port.

Bjorn

[1] http://lkml.kernel.org/r/50819140.8030806@linux.vnet.ibm.com

> Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
> ---
>  drivers/pci/pci.c   |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    6 ++++++
>  2 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b099e00..d94ab79 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3931,6 +3931,50 @@ static int __init pci_setup(char *str)
>  }
>  early_param("pci", pci_setup);
>
> +int __weak pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
> +{
> +       struct pci_dev *root;
> +       u32 lnkcap, lnkcap2;
> +
> +       *mask = 0;
> +       if (!dev)
> +               return -EINVAL;
> +
> +       root = dev->bus->self;
> +
> +       /* we've been informed via and serverworks don't make the cut */
> +       if (root->vendor == PCI_VENDOR_ID_VIA ||
> +           root->vendor == PCI_VENDOR_ID_SERVERWORKS)
> +               return -EINVAL;
> +
> +       pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap);
> +       pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
> +
> +       if (lnkcap2) {  /* PCIe r3.0-compliant */
> +               if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
> +                       *mask |= PCIE_SPEED_25;
> +               if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
> +                       *mask |= PCIE_SPEED_50;
> +               if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
> +                       *mask |= PCIE_SPEED_80;
> +       } else {        /* pre-r3.0 */
> +               if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> +                       *mask |= PCIE_SPEED_25;
> +               if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> +                       *mask |= (PCIE_SPEED_25 | PCIE_SPEED_50);
> +       }
> +
> +       dev_info(&dev->dev, "probing gen 2 caps for device %x:%x = %x/%x\n",
> +               root->vendor, root->device, lnkcap, lnkcap2);
> +       return 0;
> +}
> +
> +int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
> +{
> +       return pcibios_get_speed_cap_mask(dev, mask);
> +}
> +EXPORT_SYMBOL(pcie_get_speed_cap_mask);
> +
>  EXPORT_SYMBOL(pci_reenable_device);
>  EXPORT_SYMBOL(pci_enable_device_io);
>  EXPORT_SYMBOL(pci_enable_device_mem);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2461033a..24a2f63 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1861,4 +1861,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>   */
>  struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
>
> +#define PCIE_SPEED_25 1
> +#define PCIE_SPEED_50 2
> +#define PCIE_SPEED_80 4
> +
> +extern int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *speed_mask);
> +
>  #endif /* LINUX_PCI_H */
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function

[Benjamin Herrenschmidt]

On Tue, 2013-03-26 at 12:39 -0600, Bjorn Helgaas wrote:
> But we also know pdev is a PCIe device, and I think a PCIe device on a
> root bus must be a "Root Complex Integrated Endpoint" (PCIe spec sec
> 1.3.2.3).  Such a device does not have a link at all, so there's no
> point in fiddling with its link speed.

This is where our IBM hypervisor makes things murky. It doesn't expose
the PCIe parents (basically somewhat makes PCIe look like PCI except we
still have the PCIe caps on the child devices, just no access to the
parent device).

It's garbage but can't be fixed (would break AIX :-)

However we might be able to populate the bus->max_bus_speed from some
architecture specific quirk and have radeon use that.

Cheers,
Ben.

Re: [PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function

[Bjorn Helgaas]

On Wed, Mar 27, 2013 at 9:25 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2013-03-26 at 12:39 -0600, Bjorn Helgaas wrote:
>> But we also know pdev is a PCIe device, and I think a PCIe device on a
>> root bus must be a "Root Complex Integrated Endpoint" (PCIe spec sec
>> 1.3.2.3).  Such a device does not have a link at all, so there's no
>> point in fiddling with its link speed.
>
> This is where our IBM hypervisor makes things murky. It doesn't expose
> the PCIe parents (basically somewhat makes PCIe look like PCI except we
> still have the PCIe caps on the child devices, just no access to the
> parent device).

Interesting.  I wonder if we'll trip over this anywhere else, e.g.,
ASPM or other link-related things.  I guess we'll just have to see if
anything else breaks.

> It's garbage but can't be fixed (would break AIX :-)
>
> However we might be able to populate the bus->max_bus_speed from some
> architecture specific quirk and have radeon use that.

That sounds like a good solution to me.  It seems like it's really an
arch-specific deviation from the spec, so it'd be nice to have an
arch-specific solution for it.

Bjorn

[PATCHv2 2/3] drm: removed drm_pcie_get_speed_cap_mask function

[Lucas Kannebley Tavares]

This function was moved to the pci subsystem where it fits better, as
this is much more of a generic pci task, than a drm specific one. All
references to the function (all in the radeon driver) are updated.

This is the second step in moving function drm_pcie_get_speed_cap_mask
from the drm subsystem to the pci one.

Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
 drivers/gpu/drm/drm_pci.c          |   38 ------------------------------------
 drivers/gpu/drm/radeon/evergreen.c |    5 ++-
 drivers/gpu/drm/radeon/r600.c      |    5 ++-
 drivers/gpu/drm/radeon/rv770.c     |    5 ++-
 include/drm/drmP.h                 |    6 -----
 5 files changed, 9 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index bd719e9..ba70844 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -439,44 +439,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 	return 0;
 }
 
-int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask)
-{
-	struct pci_dev *root;
-	u32 lnkcap, lnkcap2;
-
-	*mask = 0;
-	if (!dev->pdev)
-		return -EINVAL;
-
-	root = dev->pdev->bus->self;
-
-	/* we've been informed via and serverworks don't make the cut */
-	if (root->vendor == PCI_VENDOR_ID_VIA ||
-	    root->vendor == PCI_VENDOR_ID_SERVERWORKS)
-		return -EINVAL;
-
-	pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap);
-	pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
-
-	if (lnkcap2) {	/* PCIe r3.0-compliant */
-		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
-			*mask |= DRM_PCIE_SPEED_25;
-		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
-			*mask |= DRM_PCIE_SPEED_50;
-		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
-			*mask |= DRM_PCIE_SPEED_80;
-	} else {	/* pre-r3.0 */
-		if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
-			*mask |= DRM_PCIE_SPEED_25;
-		if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
-			*mask |= (DRM_PCIE_SPEED_25 | DRM_PCIE_SPEED_50);
-	}
-
-	DRM_INFO("probing gen 2 caps for device %x:%x = %x/%x\n", root->vendor, root->device, lnkcap, lnkcap2);
-	return 0;
-}
-EXPORT_SYMBOL(drm_pcie_get_speed_cap_mask);
-
 #else
 
 int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 305a657..6ba204d 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -24,6 +24,7 @@
 #include <linux/firmware.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 #include <drm/drmP.h>
 #include "radeon.h"
 #include "radeon_asic.h"
@@ -3871,11 +3872,11 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
 	if (ASIC_IS_X2(rdev))
 		return;
 
-	ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+	ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
 	if (ret != 0)
 		return;
 
-	if (!(mask & DRM_PCIE_SPEED_50))
+	if (!(mask & PCIE_SPEED_50))
 		return;
 
 	speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 0740db3..89a7387 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -30,6 +30,7 @@
 #include <linux/firmware.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <drm/drmP.h>
 #include <drm/radeon_drm.h>
 #include "radeon.h"
@@ -4371,11 +4372,11 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev)
 	if (rdev->family <= CHIP_R600)
 		return;
 
-	ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+	ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
 	if (ret != 0)
 		return;
 
-	if (!(mask & DRM_PCIE_SPEED_50))
+	if (!(mask & PCIE_SPEED_50))
 		return;
 
 	speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index d63fe1d..81c7f1c 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -28,6 +28,7 @@
 #include <linux/firmware.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 #include <drm/drmP.h>
 #include "radeon.h"
 #include "radeon_asic.h"
@@ -1254,11 +1255,11 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev)
 	if (ASIC_IS_X2(rdev))
 		return;
 
-	ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+	ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
 	if (ret != 0)
 		return;
 
-	if (!(mask & DRM_PCIE_SPEED_50))
+	if (!(mask & PCIE_SPEED_50))
 		return;
 
 	DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n");
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..39b2872 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1788,12 +1788,6 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
 			   const struct pci_device_id *ent,
 			   struct drm_driver *driver);
 
-#define DRM_PCIE_SPEED_25 1
-#define DRM_PCIE_SPEED_50 2
-#define DRM_PCIE_SPEED_80 4
-
-extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
-
 /* platform section */
 extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
 extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
-- 
1.7.4.4

[PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask

[Lucas Kannebley Tavares]

Implementation of a architecture-specific pcibios_get_speed_cap_mask.
This implementation detects bus capabilities based on OF
ibm,pcie-link-speed-stats property.

Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/pci.c |   35 ++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 0b580f4..4da8deb 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/string.h>
+#include <linux/device.h>
 
 #include <asm/eeh.h>
 #include <asm/pci-bridge.h>
@@ -108,3 +109,37 @@ static void fixup_winbond_82c105(struct pci_dev* dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105,
 			 fixup_winbond_82c105);
+
+int pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+	struct device_node *dn, *pdn;
+	const uint32_t *pcie_link_speed_stats = NULL;
+
+	*mask = 0;
+	dn = pci_bus_to_OF_node(dev->bus);
+
+	/* Find nearest ibm,pcie-link-speed-stats, walking up the device tree */
+	for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
+		pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn,
+			"ibm,pcie-link-speed-stats", NULL);
+		if (pcie_link_speed_stats != NULL)
+			break;
+	}
+
+	if (pcie_link_speed_stats == NULL) {
+		dev_info(&dev->dev, "no ibm,pcie-link-speed-stats property\n");
+		return -EINVAL;
+	}
+
+	switch (pcie_link_speed_stats[0]) {
+	case 0x02:
+		*mask |= PCIE_SPEED_50;
+	case 0x01:
+		*mask |= PCIE_SPEED_25;
+	case 0x00:
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
-- 
1.7.4.4

Re: [PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask

[Benjamin Herrenschmidt]

On Wed, 2013-03-20 at 02:24 -0300, Lucas Kannebley Tavares wrote:
> Implementation of a architecture-specific pcibios_get_speed_cap_mask.
> This implementation detects bus capabilities based on OF
> ibm,pcie-link-speed-stats property.

The problem with your approach is that it's not a runtime detection...

If the pseries machine is compiled into the kernel binary, it will
override pcibios_get_speed_cap_mask() using the device-tree, regardless
of whether the machine is currently booted on a pseries machine or not.

This wouldn't be a big problem if the pseries
pcibios_get_speed_cap_mask() was capable of doing a fallback to the
generic one if the device-tree property is absent but that isn't the
case.

I think what you need to do is:

  - Make it so the generic one can be called by the override. This can
look a bit tricky but it's better that way. One way to do it is to have
the actual implementation be in a __pci_* variant called by the weak
pcibios_* variant

  - Move the powerpc on to arch/powerpc/kernel/pci-common.c and make
it call a ppc_md.pcibios_get_speed_cap_mask(). If the hook is absent
(NULL), make it call the generic one

  - pseries can then populate the hook in ppc_md. with its custom
variant.

Cheers,
Ben.

Re: [PATCHv2 0/3] PCI Speed Cap Fixes for ppc64

[Jerome Glisse]

On Wed, Mar 20, 2013 at 1:24 AM, Lucas Kannebley Tavares
<lucaskt@linux.vnet.ibm.com> wrote:
> This patch series first implements a function called pcie_get_speed_cap_mask
> in the PCI subsystem based off from drm_pcie_get_speed_cap_mask in drm. Then
> it removes the latter and fixes all references to it. And ultimately, it
> implements an architecture-specific version of the same function for ppc64.
>
> The refactor is done because the function that was used by drm is more
> architecture goo than module-specific. Whilst the function also needed a
> platform-specific implementation to get PCIE Gen2 speeds on ppc64.

Looks good to me but we probably want some one from the pci side to take a look

Reviewed-by: Jerome Glisse <jglisse@redhat.com>

>
> Lucas Kannebley Tavares (3):
>   pci: added pcie_get_speed_cap_mask function
>   drm: removed drm_pcie_get_speed_cap_mask function
>   ppc64: implemented pcibios_get_speed_cap_mask
>
>  arch/powerpc/platforms/pseries/pci.c |   35 +++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_pci.c            |   38 -----------------------------
>  drivers/gpu/drm/radeon/evergreen.c   |    5 ++-
>  drivers/gpu/drm/radeon/r600.c        |    5 ++-
>  drivers/gpu/drm/radeon/rv770.c       |    5 ++-
>  drivers/pci/pci.c                    |   44 ++++++++++++++++++++++++++++++++++
>  include/drm/drmP.h                   |    6 ----
>  include/linux/pci.h                  |    6 ++++
>  8 files changed, 94 insertions(+), 50 deletions(-)
>
> --
> 1.7.4.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel