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

[Jijie Shao]

on 2024/11/13 0:32, Andrew Lunn wrote:
> 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


Maybe there is an error in this code.
If I want to disable auto-neg pause, do I need to set phy_set_asym_pause(phydev, 0, 0)?

Thanks
Jijie Shao