[PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

Stanimir Varbanov posted 11 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
Posted by Stanimir Varbanov 1 month, 1 week ago
Use canned MDIO writes from Broadcom that switch the ref_clk output
pair to run from the internal fractional PLL, and set the internal
PLL to expect a 54MHz input reference clock.

Without this RPi5 PCIe cannot enumerate endpoint devices on
extension connector.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v2 -> v3:
 - New patch.

 drivers/pci/controller/pcie-brcmstb.c | 35 +++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 407343a30439..12591e292c0c 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -55,6 +55,10 @@
 #define PCIE_RC_DL_MDIO_WR_DATA				0x1104
 #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
 
+#define PCIE_RC_PL_PHY_CTL_15				0x184c
+#define  PCIE_RC_PL_PHY_CTL_15_DIS_PLL_PD_MASK		0x400000
+#define  PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK	0xff
+
 #define PCIE_MISC_MISC_CTRL				0x4008
 #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK	0x80
 #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK	0x400
@@ -251,6 +255,7 @@ struct pcie_cfg_data {
 	u8 num_inbound_wins;
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
+	int (*post_setup)(struct brcm_pcie *pcie);
 };
 
 struct subdev_regulators {
@@ -826,6 +831,32 @@ static int brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val)
 	return 0;
 }
 
+static int brcm_pcie_post_setup_bcm2712(struct brcm_pcie *pcie)
+{
+	const u16 data[] = { 0x50b9, 0xbda1, 0x0094, 0x97b4, 0x5030, 0x5030, 0x0007 };
+	const u8 regs[] = { 0x16, 0x17, 0x18, 0x19, 0x1b, 0x1c, 0x1e };
+	u32 tmp;
+	int i;
+
+	/* Allow a 54MHz (xosc) refclk source */
+
+	brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, SET_ADDR_OFFSET, 0x1600);
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++)
+		brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, regs[i], data[i]);
+
+	usleep_range(100, 200);
+
+	/* Fix for L1SS errata */
+	tmp = readl(pcie->base + PCIE_RC_PL_PHY_CTL_15);
+	tmp &= ~PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK;
+	/* PM clock period is 18.52ns (round down) */
+	tmp |= 0x12;
+	writel(tmp, pcie->base + PCIE_RC_PL_PHY_CTL_15);
+
+	return 0;
+}
+
 static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
 			    u64 cpu_addr, u64 pci_offset)
 {
@@ -1189,6 +1220,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 		PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
 	writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
 
+	if (pcie->cfg->post_setup)
+		pcie->cfg->post_setup(pcie);
+
 	return 0;
 }
 
@@ -1761,6 +1795,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
 	.soc_base	= BCM7712,
 	.perst_set	= brcm_pcie_perst_set_7278,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
+	.post_setup	= brcm_pcie_post_setup_bcm2712,
 	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN,
 	.num_inbound_wins = 10,
 };
-- 
2.43.0
Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
Posted by Florian Fainelli 1 month, 1 week ago
On 10/14/24 06:07, Stanimir Varbanov wrote:
> Use canned MDIO writes from Broadcom that switch the ref_clk output
> pair to run from the internal fractional PLL, and set the internal
> PLL to expect a 54MHz input reference clock.
> 
> Without this RPi5 PCIe cannot enumerate endpoint devices on
> extension connector.

You could say that the default reference clock for the PLL is 100MHz, 
except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1 
might have been 100MHz as well, so whether we need to support that 
revision of the chip or not might be TBD.

> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>   - New patch.
> 
>   drivers/pci/controller/pcie-brcmstb.c | 35 +++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 407343a30439..12591e292c0c 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -55,6 +55,10 @@
>   #define PCIE_RC_DL_MDIO_WR_DATA				0x1104
>   #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
>   
> +#define PCIE_RC_PL_PHY_CTL_15				0x184c
> +#define  PCIE_RC_PL_PHY_CTL_15_DIS_PLL_PD_MASK		0x400000
> +#define  PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK	0xff
> +
>   #define PCIE_MISC_MISC_CTRL				0x4008
>   #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK	0x80
>   #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK	0x400
> @@ -251,6 +255,7 @@ struct pcie_cfg_data {
>   	u8 num_inbound_wins;
>   	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
>   	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> +	int (*post_setup)(struct brcm_pcie *pcie);
>   };
>   
>   struct subdev_regulators {
> @@ -826,6 +831,32 @@ static int brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val)
>   	return 0;
>   }
>   
> +static int brcm_pcie_post_setup_bcm2712(struct brcm_pcie *pcie)
> +{
> +	const u16 data[] = { 0x50b9, 0xbda1, 0x0094, 0x97b4, 0x5030, 0x5030, 0x0007 };
> +	const u8 regs[] = { 0x16, 0x17, 0x18, 0x19, 0x1b, 0x1c, 0x1e };
> +	u32 tmp;
> +	int i;
> +
> +	/* Allow a 54MHz (xosc) refclk source */
> +

This newline is not necessary. Other than that:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian
Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
Posted by Stanimir Varbanov 1 month, 1 week ago
Hi Florian,

On 10/14/24 20:07, Florian Fainelli wrote:
> On 10/14/24 06:07, Stanimir Varbanov wrote:
>> Use canned MDIO writes from Broadcom that switch the ref_clk output
>> pair to run from the internal fractional PLL, and set the internal
>> PLL to expect a 54MHz input reference clock.
>>
>> Without this RPi5 PCIe cannot enumerate endpoint devices on
>> extension connector.
> 
> You could say that the default reference clock for the PLL is 100MHz,
> except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
> might have been 100MHz as well, so whether we need to support that
> revision of the chip or not might be TBD.

I'm confused now, according to [1] :

BCM2712C1 - 4GB and 8GB RPi5 models
BCM2712D0 - 2GB RPi5 models

My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
according to your comment the PLL PHY adjustment is not needed. But I
see that the PCIex1 RC cannot enumerate devices on ext PCI connector
because of link training failure. Implementing PLL adjustment fixes the
failure.


~Stan

[1]
https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712
Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
Posted by Jonathan Bell 1 month ago
On Thu, 17 Oct 2024 at 15:42, Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> Hi Florian,
>
> On 10/14/24 20:07, Florian Fainelli wrote:
> > On 10/14/24 06:07, Stanimir Varbanov wrote:
> >> Use canned MDIO writes from Broadcom that switch the ref_clk output
> >> pair to run from the internal fractional PLL, and set the internal
> >> PLL to expect a 54MHz input reference clock.
> >>
> >> Without this RPi5 PCIe cannot enumerate endpoint devices on
> >> extension connector.
> >
> > You could say that the default reference clock for the PLL is 100MHz,
> > except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
> > might have been 100MHz as well, so whether we need to support that
> > revision of the chip or not might be TBD.
>
> I'm confused now, according to [1] :
>
> BCM2712C1 - 4GB and 8GB RPi5 models
> BCM2712D0 - 2GB RPi5 models
>
> My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
> according to your comment the PLL PHY adjustment is not needed. But I
> see that the PCIex1 RC cannot enumerate devices on ext PCI connector
> because of link training failure. Implementing PLL adjustment fixes the
> failure.
>
>
> ~Stan
>
> [1]
> https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712

The MDIO writes for 2712C1 are required because platform firmware
arranges for the reference input clock to be 54MHz.
2712D0 can't generate a 100MHz reference input, it's 54MHz only. The
MDIO register defaults are also changed to suit, but there's no harm
in applying the writes anyway.
Both steppings need to behave identically for compliance and interop reasons.
RP1 is very tolerant of out-of-spec reference clocks, which is why
only the expansion connector appears to be affected.

Regards
Jonathan
Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
Posted by Stanimir Varbanov 1 month ago
Hi,

On 10/21/24 15:56, Jonathan Bell wrote:
> On Thu, 17 Oct 2024 at 15:42, Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>> Hi Florian,
>>
>> On 10/14/24 20:07, Florian Fainelli wrote:
>>> On 10/14/24 06:07, Stanimir Varbanov wrote:
>>>> Use canned MDIO writes from Broadcom that switch the ref_clk output
>>>> pair to run from the internal fractional PLL, and set the internal
>>>> PLL to expect a 54MHz input reference clock.
>>>>
>>>> Without this RPi5 PCIe cannot enumerate endpoint devices on
>>>> extension connector.
>>>
>>> You could say that the default reference clock for the PLL is 100MHz,
>>> except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
>>> might have been 100MHz as well, so whether we need to support that
>>> revision of the chip or not might be TBD.
>>
>> I'm confused now, according to [1] :
>>
>> BCM2712C1 - 4GB and 8GB RPi5 models
>> BCM2712D0 - 2GB RPi5 models
>>
>> My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
>> according to your comment the PLL PHY adjustment is not needed. But I
>> see that the PCIex1 RC cannot enumerate devices on ext PCI connector
>> because of link training failure. Implementing PLL adjustment fixes the
>> failure.
>>
>>
>> ~Stan
>>
>> [1]
>> https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712
> 

Thanks for jumping in, Jon.

> The MDIO writes for 2712C1 are required because platform firmware
> arranges for the reference input clock to be 54MHz.
> 2712D0 can't generate a 100MHz reference input, it's 54MHz only. The
> MDIO register defaults are also changed to suit, but there's no harm

I see that MDIO register defaults for pcie2 (where RP1 is connected) are
changed to suit to 54Mhz but this is not true for pcie1 (expansion
connector). And that could explain why the link training is failing on
pcie1.

> in applying the writes anyway.
> Both steppings need to behave identically for compliance and interop reasons.

Yes, for sure.

> RP1 is very tolerant of out-of-spec reference clocks, which is why
> only the expansion connector appears to be affected.

Thank you for clarifications.

~Stan

[1] Firmware version: RPi: BOOTSYS release VERSION:790da7ef DATE:
2024/07/30 TIME: 15:25:46