Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off
       [not found] <CGME20201015084513eucas1p234e2fa7a42b973ee7feafbdac6267a84@eucas1p2.samsung.com>
@ 2020-10-15  8:44 ` Łukasz Stelmach
  2020-10-16 18:09   ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Łukasz Stelmach @ 2020-10-15  8:44 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Jakub Kicinski, netdev, linux-kernel
  Cc: Bartłomiej Żolnierkiewicz, Marek Szyprowski,
	Ł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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off
  2020-10-15  8:44 ` [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off Łukasz Stelmach
@ 2020-10-16 18:09   ` Andrew Lunn
       [not found]     ` <CGME20201016193736eucas1p1eb94190e16b194f473ade8c49ca34275@eucas1p1.samsung.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Lunn @ 2020-10-16 18:09 UTC (permalink / raw)
  To: Łukasz Stelmach
  Cc: Heiner Kallweit, Russell King, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel, Bartłomiej Żolnierkiewicz,
	Marek Szyprowski

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off
       [not found]     ` <CGME20201016193736eucas1p1eb94190e16b194f473ade8c49ca34275@eucas1p1.samsung.com>
@ 2020-10-16 19:37       ` Lukasz Stelmach
  2020-10-16 21:00         ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Lukasz Stelmach @ 2020-10-16 19:37 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel, Bartłomiej Żolnierkiewicz,
	Marek Szyprowski


[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off
  2020-10-16 19:37       ` Lukasz Stelmach
@ 2020-10-16 21:00         ` Andrew Lunn
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2020-10-16 21:00 UTC (permalink / raw)
  To: Lukasz Stelmach
  Cc: Heiner Kallweit, Russell King, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel, Bartłomiej Żolnierkiewicz,
	Marek Szyprowski

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20201015084513eucas1p234e2fa7a42b973ee7feafbdac6267a84@eucas1p2.samsung.com>
2020-10-15  8:44 ` [PATCH v2] net: phy: Prevent reporting advertised modes when autoneg is off Łukasz Stelmach
2020-10-16 18:09   ` Andrew Lunn
     [not found]     ` <CGME20201016193736eucas1p1eb94190e16b194f473ade8c49ca34275@eucas1p1.samsung.com>
2020-10-16 19:37       ` Lukasz Stelmach
2020-10-16 21:00         ` Andrew Lunn

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git