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

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Nov 12, 2024 at 10:37:27PM +0800, Jijie Shao wrote:
> 
> on 2024/11/12 1:58, Andrew Lunn wrote:
> > On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote:
> > > 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().

sym_pause means that when the MAC pauses, it does it in both
directions, receive and transmit. Asymmetric pause means it can pause
just receive, or just transmit.

Since you have both tx_pause and rx_pause, you can do both.

> +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);

Not needed. This just tells phylib what the MAC is capable of. The
capabilities does not change, so telling it once in hbg_phy_connect()
is sufficient.

	Andrew