[PATCH] powerpc/eeh: Fix slot locations on NPU and legacy platforms

[PATCH] powerpc/eeh: Fix slot locations on NPU and legacy platforms

[Russell Currey]

The slot location code as part of EEH has never functioned perfectly on
every powerpc system.  The device node properties "ibm,slot-loc",
"ibm,slot-location-code" and "ibm,io-base-loc-code" have all been
presented in different cases, and in some situations, there are legacy
platforms not conforming to the conventions of populating root buses with
"ibm,io-base-loc-code" and child nodes with "ibm,slot-location-code".

Specifically, some legacy platforms use "ibm,loc-code" instead, which
stopped working with 7e56f627768.  In addition, EEH PEs for NPU devices
have slot locations specified on the devices instead of buses due to their
architecture, and these were not printed.  This has been fixed by looking
at the top device of a PE for a slot location before checking its bus.

Fixes: 7e56f627768 "powerpc/eeh: Fix PE location code"
Cc: <stable@vger.kernel.org> #4.4+
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/kernel/eeh_pe.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index f0520da..034538c 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -881,17 +881,34 @@ void eeh_pe_restore_bars(struct eeh_pe *pe)
  * eeh_pe_loc_get - Retrieve location code binding to the given PE
  * @pe: EEH PE
  *
- * Retrieve the location code of the given PE. If the primary PE bus
- * is root bus, we will grab location code from PHB device tree node
- * or root port. Otherwise, the upstream bridge's device tree node
- * of the primary PE bus will be checked for the location code.
+ * Retrieve the location code of the given PE. The first device associated
+ * with the PE is checked for a slot location.  If missing, the bus of the
+ * device is checked instead.  If this is a root bus, the location code is
+ * taken from the PHB device tree node or root port.  If not, the upstream
+ * bridge's device tree node of the primary PE bus will be checked instead.
+ * If a slot location isn't found on the bus, walk through parent buses
+ * until a location is found.
  */
 const char *eeh_pe_loc_get(struct eeh_pe *pe)
 {
-	struct pci_bus *bus = eeh_pe_bus_get(pe);
+	struct pci_bus *bus;
 	struct device_node *dn;
 	const char *loc = NULL;
 
+	/* Check the slot location of the first (top) PCI device */
+	struct eeh_dev *edev =
+		list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
+
+	if (edev) {
+		loc = of_get_property(edev->pdn->node,
+				      "ibm,slot-location-code", NULL);
+		if (loc)
+			return loc;
+	}
+
+	/* If there's nothing on the device, look at the bus */
+	bus = eeh_pe_bus_get(pe);
+
 	while (bus) {
 		dn = pci_bus_to_OF_node(bus);
 		if (!dn) {
@@ -905,6 +922,10 @@ const char *eeh_pe_loc_get(struct eeh_pe *pe)
 			loc = of_get_property(dn, "ibm,slot-location-code",
 					      NULL);
 
+		/* Fall back to ibm,loc-code if nothing else is found */
+		if (!loc)
+			loc = of_get_property(dn, "ibm,loc-code", NULL);
+
 		if (loc)
 			return loc;
 
-- 
2.9.2

Re: [PATCH] powerpc/eeh: Fix slot locations on NPU and legacy platforms

[Gavin Shan]

On Thu, Aug 04, 2016 at 03:01:15PM +1000, Russell Currey wrote:
>The slot location code as part of EEH has never functioned perfectly on
>every powerpc system.  The device node properties "ibm,slot-loc",
>"ibm,slot-location-code" and "ibm,io-base-loc-code" have all been
>presented in different cases, and in some situations, there are legacy
>platforms not conforming to the conventions of populating root buses with
>"ibm,io-base-loc-code" and child nodes with "ibm,slot-location-code".
>
>Specifically, some legacy platforms use "ibm,loc-code" instead, which
>stopped working with 7e56f627768.  In addition, EEH PEs for NPU devices
>have slot locations specified on the devices instead of buses due to their
>architecture, and these were not printed.  This has been fixed by looking
>at the top device of a PE for a slot location before checking its bus.
>
>Fixes: 7e56f627768 "powerpc/eeh: Fix PE location code"

Please describe what the patch is going to do if possible. I didn't completely
git it from the changelog.

Fixes: 7e56f627768 ("powerpc/eeh: Fix PE location code")

>Cc: <stable@vger.kernel.org> #4.4+
>Signed-off-by: Russell Currey <ruscur@russell.cc>
>---
> arch/powerpc/kernel/eeh_pe.c | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
>index f0520da..034538c 100644
>--- a/arch/powerpc/kernel/eeh_pe.c
>+++ b/arch/powerpc/kernel/eeh_pe.c
>@@ -881,17 +881,34 @@ void eeh_pe_restore_bars(struct eeh_pe *pe)
>  * eeh_pe_loc_get - Retrieve location code binding to the given PE
>  * @pe: EEH PE
>  *
>- * Retrieve the location code of the given PE. If the primary PE bus
>- * is root bus, we will grab location code from PHB device tree node
>- * or root port. Otherwise, the upstream bridge's device tree node
>- * of the primary PE bus will be checked for the location code.
>+ * Retrieve the location code of the given PE. The first device associated
>+ * with the PE is checked for a slot location.  If missing, the bus of the
>+ * device is checked instead.  If this is a root bus, the location code is
>+ * taken from the PHB device tree node or root port.  If not, the upstream
>+ * bridge's device tree node of the primary PE bus will be checked instead.
>+ * If a slot location isn't found on the bus, walk through parent buses
>+ * until a location is found.
>  */
> const char *eeh_pe_loc_get(struct eeh_pe *pe)
> {
>-	struct pci_bus *bus = eeh_pe_bus_get(pe);
>+	struct pci_bus *bus;
> 	struct device_node *dn;
> 	const char *loc = NULL;
>
>+	/* Check the slot location of the first (top) PCI device */
>+	struct eeh_dev *edev =
>+		list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
>+
>+	if (edev) {
>+		loc = of_get_property(edev->pdn->node,
>+				      "ibm,slot-location-code", NULL);
>+		if (loc)
>+			return loc;
>+	}

I don't think it's correct as it's to return the child PE's location code.
The @edev's "ibm,loc-code" should be used in this case. On PowerNV, we
needn't check "ibm,loc-code" as it's created based on "ibm,slot-location-code"
of its bus's node, which is covered by original code. However, "ibm,loc-code"
should be checked on pSeries as we don't have "ibm,slot-location-code" and
"ibm,slot-label" there.

Please use eeh_dev_to_pdn() when getting pdn from a EEH device.

>+
>+	/* If there's nothing on the device, look at the bus */
>+	bus = eeh_pe_bus_get(pe);
>+
> 	while (bus) {
> 		dn = pci_bus_to_OF_node(bus);
> 		if (!dn) {
>@@ -905,6 +922,10 @@ const char *eeh_pe_loc_get(struct eeh_pe *pe)
> 			loc = of_get_property(dn, "ibm,slot-location-code",
> 					      NULL);
>
>+		/* Fall back to ibm,loc-code if nothing else is found */
>+		if (!loc)
>+			loc = of_get_property(dn, "ibm,loc-code", NULL);
>+

We actually need consistent order as above: "ibm,loc-code" has high
priority than "ibm,slot-location-code" and "ibm,io-base-loc-code".

Thanks,
Gavin

> 		if (loc)
> 			return loc;
>
>-- 
>2.9.2
>