| Message ID | 20241014130710.413-10-svarbanov@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | Add PCIe support for bcm2712 | expand |
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>
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
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
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
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, };
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(+)