[Skiboot] [PATCH] pci: Fix presence detect on PHB3

Benjamin Herrenschmidt benh at kernel.crashing.org
Thu Apr 16 11:49:54 AEST 2015


The presence detect bit in the standard root complex config space is
not properly implemented on some IBM PHBs. Using it during probe is
incorrect.

We already have a workaround using the hotplug override "AB" detect
bits in the PHB3 code but it somewhat relies on the standard presence
detect bit returning false positives, which happened on Venice/Murano
but no longer happens in Naples.

Similarly, all the slot control stuff in the generic pci_enable_bridge()
isn't going to work properly on the PHB root complex and is unnecessary
as this code is only called after the upper layers have verified the
presence of a valid link on the PHB (the slot power control for the PHB
is handled separately).

This fixes it all by removing the AB detect flag, and unconditionally
using those bits in PHB3 presence detect along with making sure the
code in pci_enable_bridge() that manipulates the slot controls is
only executed on downstream ports.

Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
---

diff --git a/core/pci.c b/core/pci.c
index 5d97d4b..be61e84 100644
--- a/core/pci.c
+++ b/core/pci.c
@@ -301,8 +301,11 @@ static bool pci_enable_bridge(struct phb *phb, struct pci_device *pd)
 	bctl &= ~PCI_CFG_BRCTL_MABORT_REPORT;
 	pci_cfg_write16(phb, pd->bdfn, PCI_CFG_BRCTL, bctl);
 
-	/* PCI-E bridge, check the slot state */
-	if (pd->dev_type == PCIE_TYPE_ROOT_PORT ||
+	/* PCI-E bridge, check the slot state. We don't do that on the
+	 * root complex as this is handled separately and not all our
+	 * RCs implement the standard register set.
+	 */
+	if ((pd->dev_type == PCIE_TYPE_ROOT_PORT && pd->primary_bus > 0) ||
 	    pd->dev_type == PCIE_TYPE_SWITCH_DNPORT) {
 		uint16_t slctl, slcap, slsta, lctl;
 
diff --git a/hw/phb3.c b/hw/phb3.c
index 51e5679..c349323 100644
--- a/hw/phb3.c
+++ b/hw/phb3.c
@@ -470,9 +470,7 @@ static int64_t phb3_pci_reinit(struct phb *phb, uint64_t scope, uint64_t data)
 static int64_t phb3_presence_detect(struct phb *phb)
 {
 	struct phb3 *p = phb_to_phb3(phb);
-	uint16_t slot_stat;
 	uint64_t hp_override;
-	int64_t rc;
 
 	/* Test for PHB in error state ? */
 	if (p->state == PHB3_STATE_BROKEN)
@@ -480,30 +478,16 @@ static int64_t phb3_presence_detect(struct phb *phb)
 
 	/* XXX Check bifurcation stuff ? */
 
-	/* Read slot status register */
-	rc = phb3_pcicfg_read16(&p->phb, 0, p->ecap + PCICAP_EXP_SLOTSTAT,
-					&slot_stat);
-	if (rc != OPAL_SUCCESS)
-		return OPAL_HARDWARE;
-
 	/* Read hotplug override */
 	hp_override = in_be64(p->regs + PHB_HOTPLUG_OVERRIDE);
 
-	PHBDBG(p, "slot_stat: 0x%04x, hp_override: 0x%016llx\n",
-	       slot_stat, hp_override);
-
-	/* So if the slot status says nothing connected, we bail out */
-	if (!(slot_stat & PCICAP_EXP_SLOTSTAT_PDETECTST))
-		return OPAL_SHPC_DEV_NOT_PRESENT;
+	PHBDBG(p, "hp_override: 0x%016llx\n", hp_override);
 
 	/*
-	 * At this point, we can have one of those funky IBM
-	 * systems that has the presence bit set in the slot
-	 * status and nothing actually connected. If so, we
-	 * check the hotplug override A/B bits
+	 * On P8, the slot status isn't wired up properly, we have to
+	 * use the hotplug override A/B bits.
 	 */
-	if (p->use_ab_detect &&
-	    (hp_override & PHB_HPOVR_PRESENCE_A) &&
+	if ((hp_override & PHB_HPOVR_PRESENCE_A) &&
 	    (hp_override & PHB_HPOVR_PRESENCE_B))
 		return OPAL_SHPC_DEV_NOT_PRESENT;
 
@@ -4259,9 +4243,6 @@ static void phb3_create(struct dt_node *np)
 	       p->mm0_base, p->mm0_base + p->mm0_size - 1);
 	free(path);
 
-	/* Check if we can use the A/B detect pins */
-	p->use_ab_detect = dt_has_node_property(np, "ibm,use-ab-detect", NULL);
-
 	/* Find base location code from root node */
 	p->phb.base_loc_code = dt_prop_get_def(dt_root,
 					       "ibm,io-base-loc-code", NULL);
diff --git a/include/phb3.h b/include/phb3.h
index 3db57ad..f785916 100644
--- a/include/phb3.h
+++ b/include/phb3.h
@@ -292,7 +292,6 @@ struct phb3 {
 
 	bool			skip_perst; /* Skip first perst */
 	bool			has_link;
-	bool			use_ab_detect;
 	enum phb3_state		state;
 	uint64_t		delay_tgt_tb;
 	uint64_t		retries;




More information about the Skiboot mailing list