| Message ID | 20250206131859.2960543-1-yong.liang.choong@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Enable SGMII and 2500BASEX interface mode switching for Intel platforms | expand |
On Thu, 6 Feb 2025, Choong Yong Liang wrote: > Intel platform will configure the SerDes through PMC API based on the > provided interface mode. > > This patch adds several new functions below:- > - intel_tsn_lane_is_available(): This new function reads FIA lane > ownership registers and common lane registers through IPC commands > to know which lane the mGbE port is assigned to. > - intel_mac_finish(): To configure the SerDes based on the assigned > lane and latest interface mode, it sends IPC command to the PMC through > PMC driver/API. The PMC acts as a proxy for R/W on behalf of the driver. > - intel_set_reg_access(): Set the register access to the available TSN > interface. > > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > --- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 + > .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 146 +++++++++++++++++- > .../net/ethernet/stmicro/stmmac/dwmac-intel.h | 29 ++++ > include/linux/stmmac.h | 4 + > 4 files changed, 179 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > index 4cc85a36a1ab..25154b915b02 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -307,6 +307,8 @@ config DWMAC_INTEL > default X86 > depends on X86 && STMMAC_ETH && PCI > depends on COMMON_CLK > + depends on ACPI > + select INTEL_PMC_IPC > help > This selects the Intel platform specific bus support for the > stmmac driver. This driver is used for Intel Quark/EHL/TGL. > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > index 48acba5eb178..837fd3fbaedb 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > @@ -5,15 +5,30 @@ > #include <linux/clk-provider.h> > #include <linux/pci.h> > #include <linux/dmi.h> > +#include <linux/platform_data/x86/intel_pmc_ipc.h> > #include "dwmac-intel.h" > #include "dwmac4.h" > #include "stmmac.h" > #include "stmmac_ptp.h" > > +struct pmc_serdes_regs { > + u8 index; > + u32 val; > +}; > + > +struct pmc_serdes_reg_info { > + const struct pmc_serdes_regs *regs; > + u8 num_regs; > +}; > + > struct intel_priv_data { > int mdio_adhoc_addr; /* mdio address for serdes & etc */ > unsigned long crossts_adj; > bool is_pse; > + const int *tsn_lane_regs; > + int max_tsn_lane_regs; > + struct pmc_serdes_reg_info pid_1g; > + struct pmc_serdes_reg_info pid_2p5g; > }; > > /* This struct is used to associate PCI Function of MAC controller on a board, > @@ -35,6 +50,42 @@ struct stmmac_pci_info { > int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); > }; > > +static const struct pmc_serdes_regs pid_modphy3_1g_regs[] = { > + { PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0, B_MODPHY_PCR_LCPLL_DWORD0_1G }, > + { PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2, N_MODPHY_PCR_LCPLL_DWORD2_1G }, > + { PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7, N_MODPHY_PCR_LCPLL_DWORD7_1G }, > + { PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10, N_MODPHY_PCR_LPPLL_DWORD10_1G }, > + { PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30, N_MODPHY_PCR_CMN_ANA_DWORD30_1G }, > + {} > +}; > + > +static const struct pmc_serdes_regs pid_modphy3_2p5g_regs[] = { > + { PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0, B_MODPHY_PCR_LCPLL_DWORD0_2P5G }, > + { PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2, N_MODPHY_PCR_LCPLL_DWORD2_2P5G }, > + { PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7, N_MODPHY_PCR_LCPLL_DWORD7_2P5G }, > + { PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10, N_MODPHY_PCR_LPPLL_DWORD10_2P5G }, > + { PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30, N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G }, > + {} > +}; > + > +static const struct pmc_serdes_regs pid_modphy1_1g_regs[] = { > + { PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0, B_MODPHY_PCR_LCPLL_DWORD0_1G }, > + { PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2, N_MODPHY_PCR_LCPLL_DWORD2_1G }, > + { PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7, N_MODPHY_PCR_LCPLL_DWORD7_1G }, > + { PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10, N_MODPHY_PCR_LPPLL_DWORD10_1G }, > + { PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30, N_MODPHY_PCR_CMN_ANA_DWORD30_1G }, > + {} > +}; > + > +static const struct pmc_serdes_regs pid_modphy1_2p5g_regs[] = { > + { PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0, B_MODPHY_PCR_LCPLL_DWORD0_2P5G }, > + { PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2, N_MODPHY_PCR_LCPLL_DWORD2_2P5G }, > + { PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7, N_MODPHY_PCR_LCPLL_DWORD7_2P5G }, > + { PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10, N_MODPHY_PCR_LPPLL_DWORD10_2P5G }, > + { PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30, N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G }, > + {} > +}; > + > static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, > const struct dmi_system_id *dmi_list) > { > @@ -93,7 +144,7 @@ static int intel_serdes_powerup(struct net_device *ndev, void *priv_data) > data &= ~SERDES_RATE_MASK; > data &= ~SERDES_PCLK_MASK; > > - if (priv->plat->max_speed == 2500) > + if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) > data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT | > SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT; > else > @@ -415,6 +466,95 @@ static void intel_mgbe_pse_crossts_adj(struct intel_priv_data *intel_priv, > } > } > > +static int intel_tsn_lane_is_available(struct net_device *ndev, > + struct intel_priv_data *intel_priv) > +{ > + struct stmmac_priv *priv = netdev_priv(ndev); > + struct pmc_ipc_cmd tmp = {}; > + u32 rbuf[4] = {}; > + int ret = 0, i, j; > + const int max_fia_regs = 5; > + > + tmp.cmd = IPC_SOC_REGISTER_ACCESS; > + tmp.sub_cmd = IPC_SOC_SUB_CMD_READ; > + > + for (i = 0; i < max_fia_regs; i++) { Usually, defines are used for true consts. > + tmp.wbuf[0] = R_PCH_FIA_15_PCR_LOS1_REG_BASE + i; > + > + ret = intel_pmc_ipc(&tmp, rbuf); > + if (ret < 0) { > + netdev_info(priv->dev, "Failed to read from PMC.\n"); > + return ret; > + } > + > + for (j = 0; j <= intel_priv->max_tsn_lane_regs; j++) > + if ((rbuf[0] >> > + (4 * (intel_priv->tsn_lane_regs[j] % 8)) & > + B_PCH_FIA_PCR_L0O) == 0xB) > + return ret; > + } > + > + return ret; > +} > + > +static int intel_set_reg_access(const struct pmc_serdes_regs *regs, int max_regs) > +{ > + int ret = 0, i; > + > + for (i = 0; i < max_regs; i++) { > + struct pmc_ipc_cmd tmp = {}; > + u32 buf[4] = {}; > + > + tmp.cmd = IPC_SOC_REGISTER_ACCESS; > + tmp.sub_cmd = IPC_SOC_SUB_CMD_WRITE; > + tmp.wbuf[0] = (u32)regs[i].index; > + tmp.wbuf[1] = regs[i].val; > + > + ret = intel_pmc_ipc(&tmp, buf); > + if (ret < 0) > + return ret; > + } > + > + return ret; > +} > + > +static int intel_mac_finish(struct net_device *ndev, > + void *intel_data, > + unsigned int mode, > + phy_interface_t interface) > +{ > + struct intel_priv_data *intel_priv = intel_data; > + struct stmmac_priv *priv = netdev_priv(ndev); > + const struct pmc_serdes_regs *regs; > + int max_regs = 0; > + int ret = 0; > + > + ret = intel_tsn_lane_is_available(ndev, intel_priv); > + if (ret < 0) { > + netdev_info(priv->dev, "No TSN lane available to set the registers.\n"); > + return ret; > + } > + > + if (interface == PHY_INTERFACE_MODE_2500BASEX) { > + regs = intel_priv->pid_2p5g.regs; > + max_regs = intel_priv->pid_2p5g.num_regs; > + } else { > + regs = intel_priv->pid_1g.regs; > + max_regs = intel_priv->pid_1g.num_regs; > + } > + > + ret = intel_set_reg_access(regs, max_regs); > + if (ret < 0) > + return ret; This looks much cleaner now, thanks the update. However, the intel_priv fields you introduced are not setup until patch 6/7? Will this cause NULL ptr deref issues in between the two changes? By introducing the reg arrays in this patch but only use them after patch 6, you'll also get unused variable warnings out of them in between the changes which is unacceptable. > + > + priv->plat->phy_interface = interface; > + > + intel_serdes_powerdown(ndev, intel_priv); > + intel_serdes_powerup(ndev, intel_priv); > + > + return ret; > +}
On 6/2/2025 9:31 pm, Ilpo Järvinen wrote: >> +static int intel_tsn_lane_is_available(struct net_device *ndev, >> + struct intel_priv_data *intel_priv) >> +{ >> + struct stmmac_priv *priv = netdev_priv(ndev); >> + struct pmc_ipc_cmd tmp = {}; >> + u32 rbuf[4] = {}; >> + int ret = 0, i, j; >> + const int max_fia_regs = 5; >> + >> + tmp.cmd = IPC_SOC_REGISTER_ACCESS; >> + tmp.sub_cmd = IPC_SOC_SUB_CMD_READ; >> + >> + for (i = 0; i < max_fia_regs; i++) { > > Usually, defines are used for true consts. > Hi Ilpo, Thank you for your feedback. I used const int max_fia_regs = 5; to leverage type safety and scope control in modern C. However, I understand that #define is a common practice. Please let me know if you prefer I switch to #define for consistency. >> +static int intel_mac_finish(struct net_device *ndev, >> + void *intel_data, >> + unsigned int mode, >> + phy_interface_t interface) >> +{ >> + struct intel_priv_data *intel_priv = intel_data; >> + struct stmmac_priv *priv = netdev_priv(ndev); >> + const struct pmc_serdes_regs *regs; >> + int max_regs = 0; >> + int ret = 0; >> + >> + ret = intel_tsn_lane_is_available(ndev, intel_priv); >> + if (ret < 0) { >> + netdev_info(priv->dev, "No TSN lane available to set the registers.\n"); >> + return ret; >> + } >> + >> + if (interface == PHY_INTERFACE_MODE_2500BASEX) { >> + regs = intel_priv->pid_2p5g.regs; >> + max_regs = intel_priv->pid_2p5g.num_regs; >> + } else { >> + regs = intel_priv->pid_1g.regs; >> + max_regs = intel_priv->pid_1g.num_regs; >> + } >> + >> + ret = intel_set_reg_access(regs, max_regs); >> + if (ret < 0) >> + return ret; > > This looks much cleaner now, thanks the update. > > However, the intel_priv fields you introduced are not setup until patch > 6/7? Will this cause NULL ptr deref issues in between the two changes? By > introducing the reg arrays in this patch but only use them after patch 6, > you'll also get unused variable warnings out of them in between the > changes which is unacceptable. > Thank you for pointing out the potential issues with the intel_priv fields. I will move the changes from patch 6 into this patch to avoid NULL pointer de-reference issues and unused variable warnings. This will ensure everything is properly set up and used within the same patch.
During the interface mode change, the 'phylink_major_config' function will be triggered in phylink. The modification of the following functions will support the switching between SGMII and 2500BASE-X interface modes for the Intel platform: - xpcs_switch_interface_mode: Re-initiates clause 37 auto-negotiation for the SGMII interface mode to perform auto-negotiation. - mac_finish: Configures the SerDes according to the interface mode. With the above changes, the code will work as follows during the interface mode change. The PCS will reconfigure according to the pcs_neg_mode and the selected interface mode. Then, the MAC driver will perform SerDes configuration in 'mac_finish' based on the selected interface mode. During the SerDes configuration, the selected interface mode will identify TSN lane registers from FIA and then send IPC commands to the Power Management Controller (PMC) through the PMC driver/API. The PMC will act as a proxy to program the PLL registers. Change log: v1 -> v2: - Add static to pmc_lpm_modes declaration - Add cur_link_an_mode to the kernel doc - Combine 2 commits i.e. "stmmac: intel: Separate driver_data of ADL-N from TGL" and "net: stmmac: Add 1G/2.5G auto-negotiation support for ADL-N" into 1 commit. v2 -> v3: - Create `pmc_ipc.c` file for `intel_pmc_ipc()` function and allocate the file in `arch/x86/platform/intel/` directory. - Update phylink's AN mode during phy interface change and not exposing phylink's AN mode into phylib. v3 -> v4: - Introduce `allow_switch_interface` flag to have all ethtool link modes that are supported and advertised will be published. - Introduce `mac_get_pcs_neg_mode` function that selects the PCS negotiation mode according to the interface mode. - Remove pcs-xpcs.c changes and handle pcs during `mac_select_pcs` function - Configure SerDes base on the interface on `mac_finish` function. v4 -> v5: - remove 'allow_switch_interface' related patches. - remove 'mac_select_pcs' related patches. - add a soft reset according to XPCS datasheet for re-initiate Clause 37 auto-negotiation when switching to SGMII interface mode. v5 -> v6: - Remove 'mac_get_pcs_neg_mode' related patches. The pcs_neg_mode is properly handled by the 'net: add negotiation of in-band capabilities' patch series: https://patchwork.kernel.org/project/netdevbpf/cover/Z08kCwxdkU4n2V6x@shell.armlinux.org.uk/ - Using act_link_an_mode to determine PHY, as cfg_link_an_mode was not updated for the 2500BASE-X interface mode, caused a failure to link up. - Clean up and standardize the interface mode switch for xpcs. v6 -> v7: - Remove the "net: phylink: use act_link_an_mode to determine PHY" patch. - Use pl->link_interface in phylink_expects_phy(). - Remove priv->plat->serdes_powerup in intel_tsn_lane_is_available() as it is always true. - Refactor the code in intel_config_serdes(). - Rename intel_config_serdes() to intel_mac_finish() with an AN mode parameter. - Define the magic number as "max_fia_regs". - Store the pointer and the number of elements in the platform info structure. - Move the arrays to the C file. v1: https://patchwork.kernel.org/project/netdevbpf/cover/20230622041905.629430-1-yong.liang.choong@linux.intel.com/ v2: https://patchwork.kernel.org/project/netdevbpf/cover/20230804084527.2082302-1-yong.liang.choong@linux.intel.com/ v3: https://patchwork.kernel.org/project/netdevbpf/cover/20230921121946.3025771-1-yong.liang.choong@linux.intel.com/ v4: https://patchwork.kernel.org/project/netdevbpf/cover/20240129130253.1400707-1-yong.liang.choong@linux.intel.com/ v5: https://patchwork.kernel.org/project/netdevbpf/cover/20240215030500.3067426-1-yong.liang.choong@linux.intel.com/ v6: https://patchwork.kernel.org/project/netdevbpf/cover/20250204061020.1199124-1-yong.liang.choong@linux.intel.com/ Choong Yong Liang (6): net: phylink: use pl->link_interface in phylink_expects_phy() net: pcs: xpcs: re-initiate clause 37 Auto-negotiation stmmac: intel: configure SerDes according to the interface mode net: stmmac: configure SerDes on mac_finish stmmac: intel: interface switching support for EHL platform stmmac: intel: interface switching support for ADL-N platform David E. Box (1): arch: x86: add IPC mailbox accessor function and add SoC register access MAINTAINERS | 2 + arch/x86/Kconfig | 9 + arch/x86/platform/intel/Makefile | 1 + arch/x86/platform/intel/pmc_ipc.c | 75 ++++++ drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 + .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 229 +++++++++++++++++- .../net/ethernet/stmicro/stmmac/dwmac-intel.h | 29 +++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 13 + drivers/net/pcs/pcs-xpcs-wx.c | 4 +- drivers/net/pcs/pcs-xpcs.c | 60 ++++- drivers/net/phy/phylink.c | 2 +- .../linux/platform_data/x86/intel_pmc_ipc.h | 34 +++ include/linux/stmmac.h | 4 + 13 files changed, 452 insertions(+), 12 deletions(-) create mode 100644 arch/x86/platform/intel/pmc_ipc.c create mode 100644 include/linux/platform_data/x86/intel_pmc_ipc.h