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

[Choong Yong Liang <yong.liang.choong@linux.intel.com>]

On 26/9/2023 6:55 pm, Serge Semin wrote:
> On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
>> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>>
>> 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@linux.intel.com>
>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
>> ---
>>   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
>>
>>
Hi Russell and Serge,

Thank you for pointing that out.
The ifdef was removed and the config serdes will be implemented in 
mac_link_up in the new patch series.