Re: [PATCH net-next 2/2] dpaa_eth: support all modes with rate adapting PHYs

[Heiner Kallweit <hkallweit1@gmail.com>]

On 22.01.2020 14:59, Madalin Bucur wrote:
> Stop removing modes that are not supported on the system interface
> when the connected PHY is capable of rate adaptation. This addresses
> an issue with the LS1046ARDB board 10G interface no longer working
> with an 1G link partner after autonegotiation support was added
> for the Aquantia PHY on board in
> 
> commit 09c4c57f7bc4 ("net: phy: aquantia: add support for auto-negotiation configuration")
> 
> As it only worked in other modes besides 10G because the PHY
> was not configured by its driver to remove them, this is not
> really a bug fix but more of a feature add.
> 

I understand the issue, however the description may be a little misleading.
mac_dev->if_support doesn't include 1Gbps mode, therefore this mode is
removed from phydev->supported. What happens:
- before referenced commit: aqr_config_aneg() basically is a
  no-op and doesn't touch the advertised modes in the chip.
  Therefore 1Gbps is advertised and aneg succeeds.
- after referenced commit: 1Gbps is removed from modes advertised by the
  PHY, therefore aneg doesn't succeed.

Maybe in the context of this change the interface mode should be fixed.
These Aquantia PHY's don't support XGMII, they support USXGMII.
USXGMII support was added to phylib not too long ago, therefore older
drivers use value PHY_INTERFACE_MODE_XGMII. For the same compatibility
reason the Aquantia PHY driver still accepts PHY_INTERFACE_MODE_XGMII.

Heiner

> Reported-by: Mian Yousaf Kaukab <ykaukab@suse.de>
> Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> ---
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index a301f0095223..d3eb235450e5 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2471,9 +2471,13 @@ static int dpaa_phy_init(struct net_device *net_dev)
>  		return -ENODEV;
>  	}
>  
> -	/* Remove any features not supported by the controller */
> -	ethtool_convert_legacy_u32_to_link_mode(mask, mac_dev->if_support);
> -	linkmode_and(phy_dev->supported, phy_dev->supported, mask);
> +	if (mac_dev->phy_if != PHY_INTERFACE_MODE_XGMII ||
> +	    !phy_dev->rate_adaptation) {
> +		/* Remove any features not supported by the controller */
> +		ethtool_convert_legacy_u32_to_link_mode(mask,
> +							mac_dev->if_support);
> +		linkmode_and(phy_dev->supported, phy_dev->supported, mask);
> +	}
>  
>  	phy_support_asym_pause(phy_dev);
>  
>