Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

From: Serge Semin
Date: Tue Sep 26 2023 - 06:55:35 EST


On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
> From: "Tan, Tee Min" <tee.min.tan@xxxxxxxxxxxxxxx>
>
> Initially, Intel mGbE was only able to configure the overclocking of 2.5
> times clock rate to enable 2.5Gbps in the BIOS during boot time. Kernel
> driver had no access to modify the clock rate for 1G/2.5G mode at runtime.
>
> Now, this patch enables the runtime 1G/2.5G auto-negotiation support to
> gets rid of the dependency on BIOS to change the 1G/2.5G clock rate.
>
> This patch adds several new functions below:-
> - intel_tsn_interface_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.
> - stmmac_mac_prepare(): To obtain the latest PHY interface from phylink
> during initialization and call intel_config_serdes() to proceed with
> SERDES configuration.
> - intel_config_serdes(): To configure the SERDES based on the assigned
> lane and latest PHY interface, 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: Tan, Tee Min <tee.min.tan@xxxxxxxxxxxxxxx>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
> .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 134 +++++++++++++++++-
> .../net/ethernet/stmicro/stmmac/dwmac-intel.h | 79 +++++++++++
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 +++
> include/linux/stmmac.h | 1 +
> 5 files changed, 231 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index a2b9e289aa36..4340efd9bd50 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -273,6 +273,7 @@ config DWMAC_INTEL
> default X86
> depends on X86 && STMMAC_ETH && PCI
> depends on COMMON_CLK
> + 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 a3a249c63598..a211f42914a2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -5,6 +5,7 @@
> #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"
> @@ -14,6 +15,9 @@ struct intel_priv_data {
> int mdio_adhoc_addr; /* mdio address for serdes & etc */
> unsigned long crossts_adj;
> bool is_pse;
> + const int *tsn_lane_registers;
> + int max_tsn_lane_registers;
> + int pid_modphy;
> };
>
> /* This struct is used to associate PCI Function of MAC controller on a board,
> @@ -93,7 +97,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
> @@ -414,6 +418,106 @@ static void intel_mgbe_pse_crossts_adj(struct intel_priv_data *intel_priv,
> }
> }
>
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +static bool intel_tsn_interface_is_available(struct net_device *ndev,
> + struct intel_priv_data *intel_priv)
> +{
> + struct stmmac_priv *priv = netdev_priv(ndev);
> + struct pmc_ipc_cmd tmp = {0};
> + u32 rbuf[4] = {0};
> + int ret, i, j;
> +
> + if (priv->plat->serdes_powerup) {
> + tmp.cmd = IPC_SOC_REGISTER_ACCESS;
> + tmp.sub_cmd = IPC_SOC_SUB_CMD_READ;
> +
> + for (i = 0; i < 5; i++) {
> + 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 false;
> + }
> +
> + for (j = 0; j <= intel_priv->max_tsn_lane_registers; j++)
> + if ((rbuf[0] >>
> + (4 * (intel_priv->tsn_lane_registers[j] % 8)) &
> + B_PCH_FIA_PCR_L0O) == 0xB)
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +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 = {0};
> + u32 buf[4] = {0};
> +
> + 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_config_serdes(struct net_device *ndev, void *intel_data)
> +{
> + struct intel_priv_data *intel_priv = intel_data;
> + struct stmmac_priv *priv = netdev_priv(ndev);
> + int ret = 0;
> +
> + if (!intel_tsn_interface_is_available(ndev, intel_priv)) {
> + netdev_info(priv->dev,
> + "No TSN interface available to set the registers.\n");
> + goto pmc_read_error;
> + }
> +
> + if (intel_priv->pid_modphy == PID_MODPHY1) {
> + if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
> + ret = intel_set_reg_access(pid_modphy1_2p5g_regs,
> + ARRAY_SIZE(pid_modphy1_2p5g_regs));
> + } else {
> + ret = intel_set_reg_access(pid_modphy1_1g_regs,
> + ARRAY_SIZE(pid_modphy1_1g_regs));
> + }
> + } else {
> + if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
> + ret = intel_set_reg_access(pid_modphy3_2p5g_regs,
> + ARRAY_SIZE(pid_modphy3_2p5g_regs));
> + } else {
> + ret = intel_set_reg_access(pid_modphy3_1g_regs,
> + ARRAY_SIZE(pid_modphy3_1g_regs));
> + }
> + }
> +
> + if (ret < 0)
> + goto pmc_read_error;
> +
> +pmc_read_error:
> + intel_serdes_powerdown(ndev, intel_priv);
> + intel_serdes_powerup(ndev, intel_priv);
> +
> + return ret;
> +}
> +#else
> +static int intel_config_serdes(struct net_device *ndev, void *intel_data)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_INTEL_PMC_IPC */
> +
> static void common_default_data(struct plat_stmmacenet_data *plat)
> {
> plat->clk_csr = 2; /* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
> @@ -624,6 +728,8 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
> static int ehl_common_data(struct pci_dev *pdev,
> struct plat_stmmacenet_data *plat)
> {
> + struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
> plat->rx_queues_to_use = 8;
> plat->tx_queues_to_use = 8;
> plat->flags |= STMMAC_FLAG_USE_PHY_WOL;
> @@ -639,20 +745,28 @@ static int ehl_common_data(struct pci_dev *pdev,
> plat->safety_feat_cfg->prtyen = 0;
> plat->safety_feat_cfg->tmouten = 0;
>
> + intel_priv->tsn_lane_registers = ehl_tsn_lane_registers;
> + intel_priv->max_tsn_lane_registers = ARRAY_SIZE(ehl_tsn_lane_registers);
> +
> return intel_mgbe_common_data(pdev, plat);
> }
>
> static int ehl_sgmii_data(struct pci_dev *pdev,
> struct plat_stmmacenet_data *plat)
> {
> + struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
> plat->bus_id = 1;
> plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
> - plat->speed_mode_2500 = intel_speed_mode_2500;
> + plat->max_speed = SPEED_2500;
> plat->serdes_powerup = intel_serdes_powerup;
> plat->serdes_powerdown = intel_serdes_powerdown;
> + plat->config_serdes = intel_config_serdes;
>
> plat->clk_ptp_rate = 204800000;
>
> + intel_priv->pid_modphy = PID_MODPHY3;
> +
> return ehl_common_data(pdev, plat);
> }
>
> @@ -705,10 +819,16 @@ static struct stmmac_pci_info ehl_pse0_rgmii1g_info = {
> static int ehl_pse0_sgmii1g_data(struct pci_dev *pdev,
> struct plat_stmmacenet_data *plat)
> {
> + struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
> plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
> - plat->speed_mode_2500 = intel_speed_mode_2500;
> + plat->max_speed = SPEED_2500;
> plat->serdes_powerup = intel_serdes_powerup;
> plat->serdes_powerdown = intel_serdes_powerdown;
> + plat->config_serdes = intel_config_serdes;
> +
> + intel_priv->pid_modphy = PID_MODPHY1;
> +
> return ehl_pse0_common_data(pdev, plat);
> }
>
> @@ -746,10 +866,16 @@ static struct stmmac_pci_info ehl_pse1_rgmii1g_info = {
> static int ehl_pse1_sgmii1g_data(struct pci_dev *pdev,
> struct plat_stmmacenet_data *plat)
> {
> + struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
> plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
> - plat->speed_mode_2500 = intel_speed_mode_2500;
> + plat->max_speed = SPEED_2500;
> plat->serdes_powerup = intel_serdes_powerup;
> plat->serdes_powerdown = intel_serdes_powerdown;
> + plat->config_serdes = intel_config_serdes;
> +
> + intel_priv->pid_modphy = PID_MODPHY1;
> +
> return ehl_pse1_common_data(pdev, plat);
> }
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> index 0a37987478c1..093eed977ab0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> @@ -50,4 +50,83 @@
> #define PCH_PTP_CLK_FREQ_19_2MHZ (GMAC_GPO0)
> #define PCH_PTP_CLK_FREQ_200MHZ (0)
>
> +#define PID_MODPHY1 0xAA
> +#define PID_MODPHY3 0xA8
> +
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +struct pmc_serdes_regs {
> + u8 index;
> + u32 val;
> +};
> +
> +/* Modphy Register index */
> +#define R_PCH_FIA_15_PCR_LOS1_REG_BASE 8
> +#define R_PCH_FIA_15_PCR_LOS2_REG_BASE 9
> +#define R_PCH_FIA_15_PCR_LOS3_REG_BASE 10
> +#define R_PCH_FIA_15_PCR_LOS4_REG_BASE 11
> +#define R_PCH_FIA_15_PCR_LOS5_REG_BASE 12
> +#define B_PCH_FIA_PCR_L0O GENMASK(3, 0)
> +#define PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0 13
> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2 14
> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7 15
> +#define PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10 16
> +#define PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30 17
> +#define PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0 18
> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2 19
> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7 20
> +#define PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10 21
> +#define PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30 22
> +
> +#define B_MODPHY_PCR_LCPLL_DWORD0_1G 0x46AAAA41
> +#define N_MODPHY_PCR_LCPLL_DWORD2_1G 0x00000139
> +#define N_MODPHY_PCR_LCPLL_DWORD7_1G 0x002A0003
> +#define N_MODPHY_PCR_LPPLL_DWORD10_1G 0x00170008
> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_1G 0x0000D4AC
> +#define B_MODPHY_PCR_LCPLL_DWORD0_2P5G 0x58555551
> +#define N_MODPHY_PCR_LCPLL_DWORD2_2P5G 0x0000012D
> +#define N_MODPHY_PCR_LCPLL_DWORD7_2P5G 0x001F0003
> +#define N_MODPHY_PCR_LPPLL_DWORD10_2P5G 0x00170008
> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G 0x8200ACAC
> +
> +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 const int ehl_tsn_lane_registers[] = {7, 8, 9, 10, 11};
> +#else
> +static const int ehl_tsn_lane_registers[] = {};
> +#endif /* CONFIG_INTEL_PMC_IPC */
> +
> #endif /* __DWMAC_INTEL_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9201ed778ebc..75765cf52cd1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1103,11 +1103,31 @@ static void stmmac_mac_link_up(struct phylink_config *config,
> stmmac_hwtstamp_correct_latency(priv, priv);
> }
>

> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
> + phy_interface_t interface)
> +{
> + struct net_device *ndev = to_net_dev(config->dev);
> + struct stmmac_priv *priv = netdev_priv(ndev);
> + int ret = 0;
> +
> + priv->plat->phy_interface = interface;
> +
> + if (priv->plat->config_serdes)
> + ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);
> +
> + return ret;
> +}
> +#endif
> +
> static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
> .mac_select_pcs = stmmac_mac_select_pcs,
> .mac_config = stmmac_mac_config,
> .mac_link_down = stmmac_mac_link_down,
> .mac_link_up = stmmac_mac_link_up,
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> + .mac_prepare = stmmac_mac_prepare,
> +#endif

Please no for the platform-specific ifdef's in the generic code.
STMMAC driver is already overwhelmed with clumsy solutions. Let's not
add another one.

-Serge(y)

> };
>
> /**
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index c0079a7574ae..aa7d4d96391c 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
> int (*serdes_powerup)(struct net_device *ndev, void *priv);
> void (*serdes_powerdown)(struct net_device *ndev, void *priv);
> void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> + int (*config_serdes)(struct net_device *ndev, void *priv);
> void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
> int (*init)(struct platform_device *pdev, void *priv);
> void (*exit)(struct platform_device *pdev, void *priv);
> --
> 2.25.1
>
>