-----Original Message-----
From: Vladimir Oltean <olteanv@gmail.com>
Sent: Monday, January 27, 2020 5:31 PM
To: Madalin Bucur (OSS) <redacted>
Cc: David S. Miller <davem@davemloft.net>; Andrew Lunn <andrew@lunn.ch>;
Florian Fainelli [off-list ref]; Heiner Kallweit
[off-list ref]; netdev [off-list ref]; ykaukab@suse.de
Subject: Re: [PATCH v2 2/2] dpaa_eth: support all modes with rate adapting
PHYs

Hi Madalin,

On Mon, 27 Jan 2020 at 17:08, Madalin Bucur [off-list ref]
wrote:

quoted

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")

quoted

Before this commit the values advertised by the PHY were not
influenced by the dpaa_eth driver removal of system-side unsupported
modes as the aqr_config_aneg() was basically a no-op. After this
commit, the modes removed by the dpaa_eth driver were no longer
advertised thus autonegotiation with 1G link partners failed.

Reported-by: Mian Yousaf Kaukab <redacted>
Signed-off-by: Madalin Bucur <redacted>
---
 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

quoted

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)

quoted

                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

*/

quoted

+               ethtool_convert_legacy_u32_to_link_mode(mask,
+                                                       mac_dev-
if_support);
+               linkmode_and(phy_dev->supported, phy_dev->supported,

mask);

quoted

+       }

Is this sufficient?
I suppose this works because you have flow control enabled by default?
What would happen if the user would disable flow control with ethtool?

User misconfiguration is not something I'd try to prevent from driver code...

quoted

        phy_support_asym_pause(phy_dev);

--
2.1.0

Regards,
-Vladimir