Re: [PATCH 1/4] net: phy: aquantia: Enable Tx/Rx pause frame support in aquantia PHY

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Wed, Jun 28, 2023 at 06:13:23PM +0530, Revanth Kumar Uppala wrote:
> From: Narayan Reddy <narayanr@nvidia.com>
> 
> Enable flow control support using pause frames in aquantia phy driver.
> 
> Signed-off-by: Narayan Reddy <narayanr@nvidia.com>
> Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com>

I think this is over-complex.

>  #define MDIO_PHYXS_VEND_IF_STATUS		0xe812
>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK	GENMASK(7, 3)
>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR	0
> @@ -583,6 +585,17 @@ static int aqr107_config_init(struct phy_device *phydev)
>  	if (!ret)
>  		aqr107_chip_info(phydev);
>  
> +	/* Advertize flow control */
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->supported);
> +	linkmode_copy(phydev->advertising, phydev->supported);

This is the wrong place to be doing this, since pause support depends
not only on the PHY but also on the MAC. There are phylib interfaces
that MACs should call so that phylib knows that the MAC supports pause
frames.

Secondly, the PHY driver needs to tell phylib that the PHY supports
pause frames, and that's done through either setting the .features
member in the PHY driver, or by providing a .get_features
implementation.

Configuration of the pause advertisement should already be happening
through the core phylib code.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!