On Mon, Jun 12, 2023 at 10:35:21AM -0400, Sean Anderson wrote: > > And if SERDES protocol switching was not physically possible, would this > > patch set still have value? > > Yes. To e.g. set up SGMII25 or to fix the clocking situation. Let me analyze the reasons one by one. The clocking situation only exists due to a hope that protocol changing would be possible. If you were sure it wasn't possible, your board would have had refclks set up for protocol 3333 via the supported PLL mapping 2222. In essence, I consider this almost a non-argument, as it fixes a self-inflicted problem. Have you actually tested changing individual lanes from SGMII to SGMII 2.5? I know it works on LS1028A, but I don't know if that's also true on LS1046A. Your comments do say "LYNX_PROTO_SGMII25, /* Not tested */". Assuming a start from SERDES protocol 3333 with PLL mapping 2222, this protocol change implies powering on PLL 1 (reset state machine wants it to be disabled) and moving those lanes from PLL 2 to PLL 1. At first sight you might appear to have a point related to the fact that PLL register writes are necessary, and thus this whole shebang is necessary. But this can all be done using PBI commands, with the added benefit that U-Boot can also use those SERDES networking ports, and not just Linux. You can use the RCW+PBL specific to your board to inform the SoC that your platform's refclk 1 is 156 MHz (something which the reset state machine seems unable to learn, with protocol 0x3333). You don't have to put that in the device tree. You don't have to push code to any open source project to expose your platform specific details. Then, just like in the case of the Lynx 28G driver on LX2160, the SERDES driver could just treat the PLL configuration as read-only, which would greatly simplify things and eliminate the need for a clk driver. Here is an illustrative example (sorry, I don't have a board with the right refclk on that PLL, to verify all the way): Add this to ./serdes_10g.rcw in the root of the qoriq-rcw repository: /* * Registers for the Lynx 10G SERDES block. * * Must be included by an SoC-specific header that defines the * SRDS_BASE value. */ #define PLLnRSTCTL(n) (SRDS_BASE + (0x20 * (n))) #define PLLnCR0(n) (SRDS_BASE + (0x20 * (n)) + 0x0004) #define POFF(x) (((x) << 31) & 0x80000000) #define REFCLK_SEL(x) (((x) << 28) & 0x70000000) #define REFCLK_EN(x) (((x) << 27) & 0x08000000) #define FRATE_SEL(x) (((x) << 16) & 0x000f0000) #define DLYDIV_SEL(x) ((x) & 0x00000003) #define PCCR8 (SRDS_BASE + 0x0220) #define SGMIIA_KX(x) (((x) << 31) & 0x80000000) #define SGMIIA_CFG(x) (((x) << 28) & 0x70000000) #define SGMIIB_KX(x) (((x) << 27) & 0x08000000) #define SGMIIB_CFG(x) (((x) << 24) & 0x07000000) #define SGMIIC_KX(x) (((x) << 23) & 0x00800000) #define SGMIIC_CFG(x) (((x) << 20) & 0x00700000) #define SGMIID_KX(x) (((x) << 19) & 0x00080000) #define SGMIID_CFG(x) (((x) << 16) & 0x00070000) #define SGMIIE_KX(x) (((x) << 15) & 0x00008000) #define SGMIIE_CFG(x) (((x) << 12) & 0x00007000) #define SGMIIF_KX(x) (((x) << 11) & 0x00000800) #define SGMIIF_CFG(x) (((x) << 8) & 0x00000700) #define SGMIIG_KX(x) (((x) << 7) & 0x00000080) #define SGMIIG_CFG(x) (((x) << 4) & 0x00000070) #define SGMIIH_KX(x) (((x) << 3) & 0x00000008) #define SGMIIH_CFG(x) ((x) & 0x00000007) #define PCCRB (SRDS_BASE + 0x022c) #define XFIA_CFG(x) (((x) << 28) & 0x70000000) #define XFIB_CFG(x) (((x) << 24) & 0x07000000) #define XFIC_CFG(x) (((x) << 20) & 0x00700000) #define XFID_CFG(x) (((x) << 16) & 0x00070000) #define XFIE_CFG(x) (((x) << 12) & 0x00007000) #define XFIF_CFG(x) (((x) << 8) & 0x00000700) #define XFIG_CFG(x) (((x) << 4) & 0x00000070) #define XFIH_CFG(x) ((x) & 0x00000007) #define LNmGCR0(m) (SRDS_BASE + (0x40 * (m)) + 0x0800) #define RPLL_LES(x) (((x) << 31) & 0x80000000) #define RRAT_SEL(x) (((x) << 28) & 0x30000000) #define TPLL_LES(x) (((x) << 27) & 0x08000000) #define TRAT_SEL(x) (((x) << 24) & 0x03000000) #define RRST_B(x) (((x) << 22) & 0x00400000) #define TRST_B(x) (((x) << 21) & 0x00200000) #define RX_PD(x) (((x) << 20) & 0x00100000) #define TX_PD(x) (((x) << 19) & 0x00080000) #define IF20BIT_EN(x) (((x) << 18) & 0x00040000) #define FIRST_LANE(x) (((x) << 16) & 0x00010000) #define GCR0_RSV 0x1000 #define PROTS(x) (((x) << 7) & 0x00000f80) #define LNmGCR1(m) (SRDS_BASE + (0x40 * (m)) + 0x0804) #define RDAT_INV(x) (((x) << 31) & 0x80000000) #define TDAT_INV(x) (((x) << 30) & 0x40000000) #define OPAD_CTL(x) (((x) << 26) & 0x04000000) #define REIDL_TH(x) (((x) << 20) & 0x00700000) #define REIDL_EX_SEL(x) (((x) << 18) & 0x000C0000) #define REIDL_ET_SEL(x) (((x) << 16) & 0x00030000) #define REIDL_EX_MSB(x) (((x) << 15) & 0x00008000) #define REIDL_ET_MSB(x) (((x) << 14) & 0x00004000) #define REQ_CTL_SNP(x) (((x) << 13) & 0x00002000) #define REQ_CDR_SNP(x) (((x) << 12) & 0x00001000) #define TRSTDIR(x) (((x) << 7) & 0x00000080) #define REQ_BIN_SNP(x) (((x) << 6) & 0x00000040) #define ISLEW_RCTL(x) (((x) << 4) & 0x00000030) #define GCR1_RSV 0x8 #define OSLEW_RCTL(x) ((x) & 0x3) #define LNmRECR0(m) (SRDS_BASE + (0x40 * (m)) + 0x0810) #define RXEQ_BST(x) (((x) << 28) & 0x10000000) #define GK2OVD(x) (((x) << 24) & 0x0f000000) #define GK3OVD(x) (((x) << 16) & 0x000f0000) #define GK2OVD_EN(x) (((x) << 15) & 0x00008000) #define GK3OVD_EN(x) (((x) << 14) & 0x00004000) #define OSETOVD_EN(x) (((x) << 13) & 0x00002000) #define BASE_WAND(x) (((x) << 10) & 0x00000c00) #define OSETOVD(x) ((x) & 0x0000007F) #define LNmTECR0(m) (SRDS_BASE + (0x40 * (m)) + 0x0818) #define TEQ_TYPE(x) (((x) << 28) & 0x30000000) #define SGN_PREQ(x) (((x) << 26) & 0x04000000) #define RATIO_PREQ(x) (((x) << 22) & 0x03C00000) #define SGN_POST1Q(x) (((x) << 21) & 0x00200000) #define RATIO_PST1Q(x) (((x) << 16) & 0x001F0000) #define ADPT_EQ(x) (((x) << 8) & 0x00003F00) #define AMP_RED(x) ((x) & 0x0000003f) #define LNmTTLCR0(m) (SRDS_BASE + (0x40 * (m)) + 0x0820) #define LNmTCSR0(m) (SRDS_BASE + (0x40 * (m)) + 0x0830) #define LNmTCSR1(m) (SRDS_BASE + (0x40 * (m)) + 0x0834) #define LNmTCSR2(m) (SRDS_BASE + (0x40 * (m)) + 0x0838) #define LNmTCSR3(m) (SRDS_BASE + (0x40 * (m)) + 0x083c) #define SGMIIaCR1(a) (SRDS_BASE + (0x10 * (a)) + 0x1804) #define SGMII_MDEV_PORT(x) (((x) << 27) & 0xf8000000) #define SGPCS_EN(x) (((x) << 11) & 0x00000800) #define XFIaCR1(a) (SRDS_BASE + (0x10 * (a)) + 0x1984) #define XFI_MDEV_PORT(x) (((x) << 27) & 0xf8000000) Add this to ./ls1046ardb/serdes_pll1_power_on.rcw in the same repo (and finish the writes): /* * Assuming that the SoC starts from SERDES1 protocol 0x3333, power on PLL 1, * which is required by the reset state machine to be disabled. */ #define SRDS_BASE 0xea0000 /* SERDES 1 */ #include <../serdes_10g.rcw> .pbi write PLLnCR0(0), POFF(0) | REFCLK_SEL(2) | REFCLK_EN(0) wait 2500 write PLLnRSTCTL(0), ... .end Add this at the end of your board RCW: #include <../ls1046ardb/serdes_pll1_power_on.rcw> In short, I believe the reasons you have cited do not justify the complexity of the solution that you propose.
