[PATCH] pci: pcie-brcmstb: do not rely on CLKREQ# signal

[PATCH] pci: pcie-brcmstb: do not rely on CLKREQ# signal

[Sam Edwards]

When the Broadcom STB PCIe controller is initialized, it must be set
into one of three CLKREQ# modes: "none"/"aspm"/"l1ss". The Linux driver,
through today, hard-codes "aspm" since the vast majority of boards using
this driver have a fixed PCIe bus with the CLKREQ# signal wired up.

The Raspberry Pi CM4, however, can be connected to a plethora of PCIe
devices, some of which do not connect the CLKREQ# line (they just leave
it floating). So "aspm" mode is no longer appropriate in all cases. In
Linux, there is a proposed patchset [1] to determine the proper mode.
This doesn't really make sense in U-Boot's case, so we just change the
assumption from "aspm" to "none" (which is always safe).

This patch DOES resolve a real-world crash that occurs when U-Boot is
running on a Raspberry Pi CM4 installed in slot 3 of a Turing Pi 2
cluster board.

[1]: https://lore.kernel.org/all/20230428223500.23337-1-jim2101024@gmail.com/

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 drivers/pci/pcie_brcmstb.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
index 1de2802113..b8105654c5 100644
--- a/drivers/pci/pcie_brcmstb.c
+++ b/drivers/pci/pcie_brcmstb.c
@@ -33,6 +33,9 @@
 #define PCIE_RC_CFG_PRIV1_ID_VAL3			0x043c
 #define  CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK		0xffffff
 
+#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY			0x04dc
+#define  PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK	0xc00
+
 #define PCIE_RC_DL_MDIO_ADDR				0x1100
 #define PCIE_RC_DL_MDIO_WR_DATA				0x1104
 #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
@@ -88,7 +91,6 @@
 	 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
 
 #define PCIE_MISC_HARD_PCIE_HARD_DEBUG			0x4204
-#define  PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK	0x2
 #define  PCIE_HARD_DEBUG_SERDES_IDDQ_MASK		0x08000000
 
 #define PCIE_MSI_INTR2_CLR				0x4508
@@ -562,12 +564,18 @@ static int brcm_pcie_probe(struct udevice *dev)
 	clrsetbits_le32(base + PCIE_RC_CFG_VENDOR_SPECIFIC_REG1,
 			VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK,
 			VENDOR_SPECIFIC_REG1_LITTLE_ENDIAN);
+
 	/*
-	 * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
-	 * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
+	 * We used to enable the CLKREQ# input here, but a few PCIe cards don't
+	 * attach anything to the CLKREQ# line, so we shouldn't assume that
+	 * it's connected and working. The controller does allow detecting
+	 * whether the port on the other side of our link is/was driving this
+	 * signal, so we could check before we assume. But because this signal
+	 * is for power management, which doesn't make sense in a bootloader,
+	 * let's instead just unadvertise ASPM support.
 	 */
-	setbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG,
-		     PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK);
+	clrbits_le32(base + PCIE_RC_CFG_PRIV1_LINK_CAPABILITY,
+		     PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK);
 
 	return 0;
 }
-- 
2.41.0

Re: [PATCH] pci: pcie-brcmstb: do not rely on CLKREQ# signal

[Tom Rini]

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

On Wed, Aug 16, 2023 at 03:27:53PM -0700, Sam Edwards wrote:

> When the Broadcom STB PCIe controller is initialized, it must be set
> into one of three CLKREQ# modes: "none"/"aspm"/"l1ss". The Linux driver,
> through today, hard-codes "aspm" since the vast majority of boards using
> this driver have a fixed PCIe bus with the CLKREQ# signal wired up.
> 
> The Raspberry Pi CM4, however, can be connected to a plethora of PCIe
> devices, some of which do not connect the CLKREQ# line (they just leave
> it floating). So "aspm" mode is no longer appropriate in all cases. In
> Linux, there is a proposed patchset [1] to determine the proper mode.
> This doesn't really make sense in U-Boot's case, so we just change the
> assumption from "aspm" to "none" (which is always safe).
> 
> This patch DOES resolve a real-world crash that occurs when U-Boot is
> running on a Raspberry Pi CM4 installed in slot 3 of a Turing Pi 2
> cluster board.
> 
> [1]: https://lore.kernel.org/all/20230428223500.23337-1-jim2101024@gmail.com/
> 
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>

Applied to u-boot/next, thanks!

-- 
Tom

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