[PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off

[PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off

[Łukasz Stelmach]

Do not report advertised link modes (local and remote) when
autonegotiation is turned off. mii_ethtool_get_link_ksettings() exhibits
the same behaviour and this patch aims at unifying the behavior of both
functions.

Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
Changes in v2:
  - clear lp_advertising
  - set ETHTOOL_LINK_MODE_TP_BIT and ETHTOOL_LINK_MODE_MII_BIT in advertising

 drivers/net/phy/phy.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 35525a671400..6ede9c1c138c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -315,8 +315,17 @@ void phy_ethtool_ksettings_get(struct phy_device *phydev,
 			       struct ethtool_link_ksettings *cmd)
 {
 	linkmode_copy(cmd->link_modes.supported, phydev->supported);
-	linkmode_copy(cmd->link_modes.advertising, phydev->advertising);
-	linkmode_copy(cmd->link_modes.lp_advertising, phydev->lp_advertising);
+	if (phydev->autoneg) {
+		linkmode_copy(cmd->link_modes.advertising, phydev->advertising);
+		linkmode_copy(cmd->link_modes.lp_advertising, phydev->lp_advertising);
+	} else {
+		linkmode_zero(cmd->link_modes.lp_advertising);
+		linkmode_zero(cmd->link_modes.advertising);
+		linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT,
+				 cmd->link_modes.advertising);
+		linkmode_set_bit(ETHTOOL_LINK_MODE_MII_BIT,
+				 cmd->link_modes.advertising);
+	}
 
 	cmd->base.speed = phydev->speed;
 	cmd->base.duplex = phydev->duplex;
-- 
2.26.2

Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off

[Andrew Lunn]

On Thu, Oct 15, 2020 at 10:44:35AM +0200, Łukasz Stelmach wrote:
> Do not report advertised link modes (local and remote) when
> autonegotiation is turned off. mii_ethtool_get_link_ksettings() exhibits
> the same behaviour and this patch aims at unifying the behavior of both
> functions.

Does ethtool allow you to configure advertised modes with autoneg off?
If it can, it would be useful to see what is being configured, before
it is actually turned on.

ethtool -s eth42 autoneg off advertise 0xf

does not give an error on an interface i have.

     Andrew

Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off

[Lukasz Stelmach]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

It was <2020-10-16 pią 20:09>, when Andrew Lunn wrote:
> On Thu, Oct 15, 2020 at 10:44:35AM +0200, Łukasz Stelmach wrote:
>> Do not report advertised link modes (local and remote) when
>> autonegotiation is turned off. mii_ethtool_get_link_ksettings() exhibits
>> the same behaviour and this patch aims at unifying the behavior of both
>> functions.
>
> Does ethtool allow you to configure advertised modes with autoneg off?
> If it can, it would be useful to see what is being configured, before
> it is actually turned on.
>
> ethtool -s eth42 autoneg off advertise 0xf
>
> does not give an error on an interface i have.

Yes, this is a good point. Do you think I should change the if()[1] in 
mii_ethtool_get_link_ksettings() instead? I really think these two
function should report the same.

[1] https://elixir.bootlin.com/linux/v5.9/source/drivers/net/mii.c#L174
[2] https://elixir.bootlin.com/linux/v5.9/source/drivers/net/mii.c#L145

Kind regards,
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off

[Andrew Lunn]

On Fri, Oct 16, 2020 at 09:37:22PM +0200, Lukasz Stelmach wrote:
> It was <2020-10-16 pią 20:09>, when Andrew Lunn wrote:
> > On Thu, Oct 15, 2020 at 10:44:35AM +0200, Łukasz Stelmach wrote:
> >> Do not report advertised link modes (local and remote) when
> >> autonegotiation is turned off. mii_ethtool_get_link_ksettings() exhibits
> >> the same behaviour and this patch aims at unifying the behavior of both
> >> functions.
> >
> > Does ethtool allow you to configure advertised modes with autoneg off?
> > If it can, it would be useful to see what is being configured, before
> > it is actually turned on.
> >
> > ethtool -s eth42 autoneg off advertise 0xf
> >
> > does not give an error on an interface i have.
> 
> Yes, this is a good point. Do you think I should change the if()[1] in 
> mii_ethtool_get_link_ksettings() instead? I really think these two
> function should report the same.

Yes, i would change mii. Ideally we want all drivers to use
phylib/phylink, not mii. So i would modify mii to match
phylib/phylink, not the other way around.

And then there will be drivers which do their own PHY handling, hidden
away in firmware, and not using either of mii or phylib/phylink. You
can expect them to be inconsistent.

    Andrew