Re: [PATCH V3 net-next 5/7] net: hibmcge: Add pauseparam supported in this module

on 2024/11/12 1:58, Andrew Lunn wrote:

On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote:

quoted

The MAC can automatically send or respond to pause frames.
This patch supports the function of enabling pause frames
by using ethtool.

Pause auto-negotiation is not supported currently.

What is actually missing to support auto-neg pause? You are using
phylib, so it will do most of the work. You just need your adjust_link
callback to configure the hardware to the result of the negotiation.
And call phy_support_asym_pause() to let phylib know what the MAC
supports.

	Andrew

Thanks for your guidance,

I haven't really figured out the difference between phy_support_sym_pause()
and phy_support_asym_paus(). However, according to your guidance
and referring to the phylib interface and other drivers code,
I implemented the auto-neg pause function:


+static void hbg_ethtool_get_pauseparam(struct net_device *net_dev,
+				       struct ethtool_pauseparam *param)
+{
+	struct hbg_priv *priv = netdev_priv(net_dev);
+
+	param->autoneg = priv->mac.pause_autoneg;
+	hbg_hw_get_pause_enable(priv, &param->tx_pause, &param->rx_pause);
+}
+
+static int hbg_ethtool_set_pauseparam(struct net_device *net_dev,
+				      struct ethtool_pauseparam *param)
+{
+	struct hbg_priv *priv = netdev_priv(net_dev);
+	struct phy_device *phydev = priv->mac.phydev;
+
+	phy_set_asym_pause(phydev, param->rx_pause, param->tx_pause);
+
+	priv->mac.pause_autoneg = param->autoneg;
+	if (!param->autoneg)
+		hbg_hw_set_pause_enable(priv, param->tx_pause, param->rx_pause);
+
+	return 0;
+}

......

+static void hbg_flowctrl_cfg(struct hbg_priv *priv)
+{
+	struct phy_device *phydev = priv->mac.phydev;
+	bool rx_pause;
+	bool tx_pause;
+
+	if (!priv->mac.pause_autoneg)
+		return;
+
+	phy_get_pause(phydev, &tx_pause, &rx_pause);
+	hbg_hw_set_pause_enable(priv, tx_pause, rx_pause);
+}
+
  static void hbg_phy_adjust_link(struct net_device *netdev)
  {
  	struct hbg_priv *priv = netdev_priv(netdev);

@@ -140,6 +153,7 @@ static void hbg_phy_adjust_link(struct net_device *netdev)
  			priv->mac.duplex = phydev->duplex;
  			priv->mac.autoneg = phydev->autoneg;
  			hbg_hw_adjust_link(priv, speed, phydev->duplex);
+			hbg_flowctrl_cfg(priv);
  		}
  
  		priv->mac.link_status = phydev->link;

@@ -168,6 +182,7 @@ static int hbg_phy_connect(struct hbg_priv *priv)
  		return ret;
  
  	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+	phy_support_asym_pause(phydev);
  	phy_attached_info(phydev);
  
  	return 0;

......

Can the auto-neg pause function be fully supported?
If the code is ok, I'll add it in the next version.

Thanks,
Jijie Shao