[PATCH RFC net-next v2 1/8] net: stmmac: add infrastructure for hwifs to provide PCS

Andrew Halaney ahalaney at redhat.com
Wed Jun 5 12:57:34 PDT 2024


On Fri, May 31, 2024 at 12:26:15PM GMT, Russell King (Oracle) wrote:
> Allow hwifs to provide a phylink_select_pcs() implementation via struct
> stmmac_ops, which can be used to provide a phylink PCS.
> 
> Code analysis shows that when STMMAC_FLAG_HAS_INTEGRATED_PCS is set,
> then:
> 
> 	stmmac_common_interrupt()
> 	stmmac_ethtool_set_link_ksettings()
> 	stmmac_ethtool_get_link_ksettings()
> 
> will all ignore the presence of the PCS. The latter two will pass the
> ethtool commands to phylink. The former will avoid manipulating the
> netif carrier state behind phylink's back based on the PCS status.
> 
> This flag is only set by the ethqos driver. From what I can tell,
> amongst the current kernel DT files that use the ethqos driver, only
> sa8775p-ride.dts enables ethernet, and this defines a SGMII-mode link
> to its PHYs without the "managed" property. Thus, phylink will be
> operating in MLO_AN_PHY mode, and inband mode will not be used.
> 
> Therefore, it is safe to ignore the STMMAC_FLAG_HAS_INTEGRATED_PCS
> flag in stmmac_mac_select_pcs().
> 
> Further code analysis shows that XPCS is used by Intel for Cisco
> SGMII and 1000base-X modes. In this case, we do not want to provide
> the integrated PCS, but the XPCS. The same appears to also be true
> of the Lynx PCS.
> 
> Therefore, it seems that the integrated PCS provided by the hwif MAC
> code should only be used when an external PCS is not being used, so
> give priority to the external PCS.
> 
> Provide a phylink_pcs instance in struct mac_device_info for hwifs to
> use to provide their phylink PCS.
> 
> Omit the non-phylink PCS code paths when a hwif provides a
> phylink_select_pcs() method (in other words, when they are converted to
> use a phylink PCS.) This provides a way to transition parts of the
> driver in the subsequent patches.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel at armlinux.org.uk>
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h  |  9 ++++++++-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    | 19 +++++++++++++++++--
>  .../ethernet/stmicro/stmmac/stmmac_ethtool.c  | 12 ++++++++----
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++---
>  4 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index cd36ff4da68c..940e83fa1202 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -14,7 +14,7 @@
>  #include <linux/etherdevice.h>
>  #include <linux/netdevice.h>
>  #include <linux/stmmac.h>
> -#include <linux/phy.h>
> +#include <linux/phylink.h>
>  #include <linux/pcs/pcs-xpcs.h>
>  #include <linux/module.h>
>  #if IS_ENABLED(CONFIG_VLAN_8021Q)
> @@ -591,6 +591,7 @@ struct mac_device_info {
>  	const struct stmmac_tc_ops *tc;
>  	const struct stmmac_mmc_ops *mmc;
>  	const struct stmmac_est_ops *est;
> +	struct phylink_pcs mac_pcs;	/* The MAC's RGMII/SGMII "PCS" */
>  	struct dw_xpcs *xpcs;
>  	struct phylink_pcs *phylink_pcs;
>  	struct mii_regs mii;	/* MII register Addresses */
> @@ -611,6 +612,12 @@ struct mac_device_info {
>  	bool hw_vlan_en;
>  };
>  
> +static inline struct mac_device_info *
> +phylink_pcs_to_mac_dev_info(struct phylink_pcs *pcs)
> +{
> +	return container_of(pcs, struct mac_device_info, mac_pcs);
> +}
> +
>  struct stmmac_rx_routing {
>  	u32 reg_mask;
>  	u32 reg_shift;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 97934ccba5b1..aa5f6e40cb53 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -5,6 +5,7 @@
>  #ifndef __STMMAC_HWIF_H__
>  #define __STMMAC_HWIF_H__
>  
> +#include <linux/err.h>
>  #include <linux/netdevice.h>
>  #include <linux/stmmac.h>
>  
> @@ -17,13 +18,17 @@
>  	} \
>  	__result; \
>  })
> -#define stmmac_do_callback(__priv, __module, __cname,  __arg0, __args...) \
> +#define stmmac_do_typed_callback(__type, __fail_ret, __priv, __module, \
> +				 __cname,  __arg0, __args...) \
>  ({ \
> -	int __result = -EINVAL; \
> +	__type __result = __fail_ret; \
>  	if ((__priv)->hw->__module && (__priv)->hw->__module->__cname) \
>  		__result = (__priv)->hw->__module->__cname((__arg0), ##__args); \
>  	__result; \
>  })
> +#define stmmac_do_callback(__priv, __module, __cname,  __arg0, __args...) \
> +	stmmac_do_typed_callback(int, -EINVAL, __priv, __module, __cname, \
> +				 __arg0, ##__args)
>  
>  struct stmmac_extra_stats;
>  struct stmmac_priv;
> @@ -310,6 +315,9 @@ struct stmmac_ops {
>  	void (*core_init)(struct mac_device_info *hw, struct net_device *dev);
>  	/* Update MAC capabilities */
>  	void (*update_caps)(struct stmmac_priv *priv);
> +	/* Get phylink PCS (for MAC */

nit: unclosed parenthesis

> +	struct phylink_pcs *(*phylink_select_pcs)(struct stmmac_priv *priv,
> +						  phy_interface_t interface);
>  	/* Enable the MAC RX/TX */
>  	void (*set_mac)(void __iomem *ioaddr, bool enable);
>  	/* Enable and verify that the IPC module is supported */
> @@ -431,6 +439,10 @@ struct stmmac_ops {
>  	stmmac_do_void_callback(__priv, mac, core_init, __args)
>  #define stmmac_mac_update_caps(__priv) \
>  	stmmac_do_void_callback(__priv, mac, update_caps, __priv)
> +#define stmmac_mac_phylink_select_pcs(__priv, __interface) \
> +	stmmac_do_typed_callback(struct phylink_pcs *, ERR_PTR(-EOPNOTSUPP), \
> +				 __priv, mac, phylink_select_pcs, __priv,\
> +				 __interface)
>  #define stmmac_mac_set(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, set_mac, __args)
>  #define stmmac_rx_ipc(__priv, __args...) \
> @@ -530,6 +542,9 @@ struct stmmac_ops {
>  #define stmmac_fpe_irq_status(__priv, __args...) \
>  	stmmac_do_callback(__priv, mac, fpe_irq_status, __args)
>  
> +#define stmmac_has_mac_phylink_select_pcs(__priv) \
> +	((__priv)->hw->mac->phylink_select_pcs != NULL)
> +
>  /* PTP and HW Timer helpers */
>  struct stmmac_hwtimestamp {
>  	void (*config_hw_tstamping) (void __iomem *ioaddr, u32 data);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index 18468c0228f0..7c6530d63ade 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -323,7 +323,8 @@ static int stmmac_ethtool_get_link_ksettings(struct net_device *dev,
>  
>  	if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
>  	    (priv->hw->pcs & STMMAC_PCS_RGMII ||
> -	     priv->hw->pcs & STMMAC_PCS_SGMII)) {
> +	     priv->hw->pcs & STMMAC_PCS_SGMII) &&
> +	    !stmmac_has_mac_phylink_select_pcs(priv)) {
>  		struct rgmii_adv adv;
>  		u32 supported, advertising, lp_advertising;
>  
> @@ -410,7 +411,8 @@ stmmac_ethtool_set_link_ksettings(struct net_device *dev,
>  
>  	if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
>  	    (priv->hw->pcs & STMMAC_PCS_RGMII ||
> -	     priv->hw->pcs & STMMAC_PCS_SGMII)) {
> +	     priv->hw->pcs & STMMAC_PCS_SGMII) &&
> +	    !stmmac_has_mac_phylink_select_pcs(priv)) {
>  		/* Only support ANE */
>  		if (cmd->base.autoneg != AUTONEG_ENABLE)
>  			return -EINVAL;
> @@ -523,7 +525,8 @@ stmmac_get_pauseparam(struct net_device *netdev,
>  	struct stmmac_priv *priv = netdev_priv(netdev);
>  	struct rgmii_adv adv_lp;
>  
> -	if (priv->hw->pcs && !stmmac_pcs_get_adv_lp(priv, priv->ioaddr, &adv_lp)) {
> +	if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv) &&
> +	    !stmmac_pcs_get_adv_lp(priv, priv->ioaddr, &adv_lp)) {
>  		pause->autoneg = 1;
>  		if (!adv_lp.pause)
>  			return;
> @@ -539,7 +542,8 @@ stmmac_set_pauseparam(struct net_device *netdev,
>  	struct stmmac_priv *priv = netdev_priv(netdev);
>  	struct rgmii_adv adv_lp;
>  
> -	if (priv->hw->pcs && !stmmac_pcs_get_adv_lp(priv, priv->ioaddr, &adv_lp)) {
> +	if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv) &&
> +	   !stmmac_pcs_get_adv_lp(priv, priv->ioaddr, &adv_lp)) {
>  		pause->autoneg = 1;
>  		if (!adv_lp.pause)
>  			return -EOPNOTSUPP;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index bbedf2a8c60f..56c351e11952 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -953,7 +953,10 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
>  	if (priv->hw->xpcs)
>  		return &priv->hw->xpcs->pcs;
>  
> -	return priv->hw->phylink_pcs;
> +	if (priv->hw->phylink_pcs)
> +		return priv->hw->phylink_pcs;
> +
> +	return stmmac_mac_phylink_select_pcs(priv, interface);
>  }
>  
>  static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
> @@ -3482,7 +3485,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
>  		}
>  	}
>  
> -	if (priv->hw->pcs)
> +	if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv))
>  		stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, priv->hw->ps, 0);
>  
>  	/* set TX and RX rings length */
> @@ -6052,7 +6055,8 @@ static void stmmac_common_interrupt(struct stmmac_priv *priv)
>  
>  		/* PCS link status */
>  		if (priv->hw->pcs &&
> -		    !(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS)) {
> +		    !(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
> +		    !stmmac_has_mac_phylink_select_pcs(priv)) {
>  			if (priv->xstats.pcs_link)
>  				netif_carrier_on(priv->dev);
>  			else
> -- 
> 2.30.2
> 




More information about the linux-arm-kernel mailing list