All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module
@ 2023-03-21 16:58 Russell King (Oracle)
  2023-03-21 16:58 ` [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits Russell King (Oracle)
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-21 16:58 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit
  Cc: David S. Miller, Eric Dumazet, Frank Wunderlich, Jakub Kicinski,
	netdev, Paolo Abeni

Hi,

Frank Wunderlich reports that this copper module requires a quirk in
order to function - in that the module needs to use 2500base-X.
Moreover, negotiation must be disabled.

An example of this device would be:

  https://www.optcore.net/product/sfp-2g-t-gen

 drivers/net/phy/sfp-bus.c |  8 ++++----
 drivers/net/phy/sfp.c     | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

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

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

* [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits
  2023-03-21 16:58 [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module Russell King (Oracle)
@ 2023-03-21 16:58 ` Russell King (Oracle)
  2023-03-22 15:52   ` Simon Horman
  2023-03-21 16:58 ` [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP Russell King (Oracle)
  2023-03-23  5:50 ` [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module patchwork-bot+netdevbpf
  2 siblings, 1 reply; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-21 16:58 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit
  Cc: David S. Miller, Eric Dumazet, Frank Wunderlich, Jakub Kicinski,
	netdev, Paolo Abeni

Allow SFP quirks to override the Autoneg, Pause and Asym_Pause bits in
the support mask.

Some modules have an inaccessible PHY on which is only accessible via
2500base-X without Autonegotiation. We therefore want to be able to
clear the Autoneg bit. Rearrange sfp_parse_support() to allow a SFP
modes quirk to override this bit.

Tested-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp-bus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index daac293e8ede..1dd50f2ca05d 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -151,6 +151,10 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 	unsigned int br_min, br_nom, br_max;
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(modes) = { 0, };
 
+	phylink_set(modes, Autoneg);
+	phylink_set(modes, Pause);
+	phylink_set(modes, Asym_Pause);
+
 	/* Decode the bitrate information to MBd */
 	br_min = br_nom = br_max = 0;
 	if (id->base.br_nominal) {
@@ -329,10 +333,6 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		bus->sfp_quirk->modes(id, modes, interfaces);
 
 	linkmode_or(support, support, modes);
-
-	phylink_set(support, Autoneg);
-	phylink_set(support, Pause);
-	phylink_set(support, Asym_Pause);
 }
 EXPORT_SYMBOL_GPL(sfp_parse_support);
 
-- 
2.30.2


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

* [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-21 16:58 [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module Russell King (Oracle)
  2023-03-21 16:58 ` [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits Russell King (Oracle)
@ 2023-03-21 16:58 ` Russell King (Oracle)
  2023-03-22 15:52   ` Simon Horman
  2023-03-25  2:12   ` Daniel Golle
  2023-03-23  5:50 ` [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module patchwork-bot+netdevbpf
  2 siblings, 2 replies; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-21 16:58 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit
  Cc: David S. Miller, Eric Dumazet, Frank Wunderlich, Jakub Kicinski,
	netdev, Paolo Abeni

Add a quirk for a copper SFP that identifies itself as "OEM"
"SFP-2.5G-T". This module's PHY is inaccessible, and can only run
at 2500base-X with the host without negotiation. Add a quirk to
enable the 2500base-X interface mode with 2500base-T support, and
disable autonegotiation.

Reported-by: Frank Wunderlich <frank-w@public-files.de>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 39e3095796d0..9c1fa0b1737f 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -360,6 +360,23 @@ static void sfp_quirk_2500basex(const struct sfp_eeprom_id *id,
 	__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
 }
 
+static void sfp_quirk_disable_autoneg(const struct sfp_eeprom_id *id,
+				      unsigned long *modes,
+				      unsigned long *interfaces)
+{
+	linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, modes);
+}
+
+static void sfp_quirk_oem_2_5g(const struct sfp_eeprom_id *id,
+			       unsigned long *modes,
+			       unsigned long *interfaces)
+{
+	/* Copper 2.5G SFP */
+	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, modes);
+	__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
+	sfp_quirk_disable_autoneg(id, modes, interfaces);
+}
+
 static void sfp_quirk_ubnt_uf_instant(const struct sfp_eeprom_id *id,
 				      unsigned long *modes,
 				      unsigned long *interfaces)
@@ -401,6 +418,7 @@ static const struct sfp_quirk sfp_quirks[] = {
 	SFP_QUIRK_M("UBNT", "UF-INSTANT", sfp_quirk_ubnt_uf_instant),
 
 	SFP_QUIRK_F("OEM", "SFP-10G-T", sfp_fixup_rollball_cc),
+	SFP_QUIRK_M("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
 	SFP_QUIRK_F("OEM", "RTSFP-10", sfp_fixup_rollball_cc),
 	SFP_QUIRK_F("OEM", "RTSFP-10G", sfp_fixup_rollball_cc),
 	SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
-- 
2.30.2


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

* Re: [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits
  2023-03-21 16:58 ` [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits Russell King (Oracle)
@ 2023-03-22 15:52   ` Simon Horman
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Horman @ 2023-03-22 15:52 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Tue, Mar 21, 2023 at 04:58:46PM +0000, Russell King (Oracle) wrote:
> Allow SFP quirks to override the Autoneg, Pause and Asym_Pause bits in
> the support mask.
> 
> Some modules have an inaccessible PHY on which is only accessible via
> 2500base-X without Autonegotiation. We therefore want to be able to
> clear the Autoneg bit. Rearrange sfp_parse_support() to allow a SFP
> modes quirk to override this bit.
> 
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-21 16:58 ` [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP Russell King (Oracle)
@ 2023-03-22 15:52   ` Simon Horman
  2023-03-25  2:12   ` Daniel Golle
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Horman @ 2023-03-22 15:52 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> Add a quirk for a copper SFP that identifies itself as "OEM"
> "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> at 2500base-X with the host without negotiation. Add a quirk to
> enable the 2500base-X interface mode with 2500base-T support, and
> disable autonegotiation.
> 
> Reported-by: Frank Wunderlich <frank-w@public-files.de>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


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

* Re: [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module
  2023-03-21 16:58 [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module Russell King (Oracle)
  2023-03-21 16:58 ` [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits Russell King (Oracle)
  2023-03-21 16:58 ` [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP Russell King (Oracle)
@ 2023-03-23  5:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-23  5:50 UTC (permalink / raw)
  To: Russell King
  Cc: andrew, hkallweit1, davem, edumazet, frank-w, kuba, netdev, pabeni

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 21 Mar 2023 16:58:26 +0000 you wrote:
> Hi,
> 
> Frank Wunderlich reports that this copper module requires a quirk in
> order to function - in that the module needs to use 2500base-X.
> Moreover, negotiation must be disabled.
> 
> An example of this device would be:
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits
    https://git.kernel.org/netdev/net-next/c/8110633db49d
  - [net-next,2/2] net: sfp: add quirk for 2.5G copper SFP
    https://git.kernel.org/netdev/net-next/c/50e96acbe116

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-21 16:58 ` [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP Russell King (Oracle)
  2023-03-22 15:52   ` Simon Horman
@ 2023-03-25  2:12   ` Daniel Golle
  2023-03-25 14:05     ` Russell King (Oracle)
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Golle @ 2023-03-25  2:12 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

Hi Russell,

On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> Add a quirk for a copper SFP that identifies itself as "OEM"
> "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> at 2500base-X with the host without negotiation. Add a quirk to
> enable the 2500base-X interface mode with 2500base-T support, and
> disable autonegotiation.
> 
> Reported-by: Frank Wunderlich <frank-w@public-files.de>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

I've tried the same fix also with my 2500Base-T SFP module:
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 4223c9fa6902..c7a18a72d2c5 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
        SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
        SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
        SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
+       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
 };

 static size_t sfp_strlen(const char *str, size_t maxlen)

However, the results are a bit of a mixed bag. The link now does come up
without having to manually disable autonegotiation. However, I see this
new warning in the bootlog:
[   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
...
[   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440

Also link speed and status appears unknown, though we do know at least
that the speed is 2500M, and also full duplex will always be true for
2500Base-T:
[   56.004937] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off

root@OpenWrt:/# ethtool sfp2
Settings for sfp2:
        Supported ports: [ FIBRE ]
        Supported link modes:   2500baseT/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: No
        Supported FEC modes: Not reported
        Advertised link modes:  2500baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: No
        Advertised FEC modes: Not reported
        Speed: Unknown!
        Duplex: Unknown! (255)
        Auto-negotiation: off
        Port: FIBRE
        PHYAD: 0
        Transceiver: internal
        Supports Wake-on: d
        Wake-on: d
        Link detected: yes

So it seems like this new quirk has brought also some new problems or at
least doesn't address them all.

> ---
>  drivers/net/phy/sfp.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 39e3095796d0..9c1fa0b1737f 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -360,6 +360,23 @@ static void sfp_quirk_2500basex(const struct sfp_eeprom_id *id,
>  	__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
>  }
>  
> +static void sfp_quirk_disable_autoneg(const struct sfp_eeprom_id *id,
> +				      unsigned long *modes,
> +				      unsigned long *interfaces)
> +{
> +	linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, modes);
> +}
> +
> +static void sfp_quirk_oem_2_5g(const struct sfp_eeprom_id *id,
> +			       unsigned long *modes,
> +			       unsigned long *interfaces)
> +{
> +	/* Copper 2.5G SFP */
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, modes);
> +	__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
> +	sfp_quirk_disable_autoneg(id, modes, interfaces);
> +}
> +
>  static void sfp_quirk_ubnt_uf_instant(const struct sfp_eeprom_id *id,
>  				      unsigned long *modes,
>  				      unsigned long *interfaces)
> @@ -401,6 +418,7 @@ static const struct sfp_quirk sfp_quirks[] = {
>  	SFP_QUIRK_M("UBNT", "UF-INSTANT", sfp_quirk_ubnt_uf_instant),
>  
>  	SFP_QUIRK_F("OEM", "SFP-10G-T", sfp_fixup_rollball_cc),
> +	SFP_QUIRK_M("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
>  	SFP_QUIRK_F("OEM", "RTSFP-10", sfp_fixup_rollball_cc),
>  	SFP_QUIRK_F("OEM", "RTSFP-10G", sfp_fixup_rollball_cc),
>  	SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> -- 
> 2.30.2
> 

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-25  2:12   ` Daniel Golle
@ 2023-03-25 14:05     ` Russell King (Oracle)
  2023-03-25 15:35       ` Daniel Golle
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-25 14:05 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> Hi Russell,
> 
> On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > Add a quirk for a copper SFP that identifies itself as "OEM"
> > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > at 2500base-X with the host without negotiation. Add a quirk to
> > enable the 2500base-X interface mode with 2500base-T support, and
> > disable autonegotiation.
> > 
> > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> 
> I've tried the same fix also with my 2500Base-T SFP module:
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 4223c9fa6902..c7a18a72d2c5 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
>         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
>         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
>         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
>  };
> 
>  static size_t sfp_strlen(const char *str, size_t maxlen)

Thanks for testing.

> However, the results are a bit of a mixed bag. The link now does come up
> without having to manually disable autonegotiation. However, I see this
> new warning in the bootlog:
> [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> ...
> [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440

This will be the result of issuing an ethtool command, and phylink
doesn't know what to do with the advertising mask - which is saying:

   Autoneg, Fibre, Pause, AsymPause

In other words, there are no capabilities to be advertised, which is
invalid, and suggests user error. What ethtool command was being
issued?

> Also link speed and status appears unknown, though we do know at least
> that the speed is 2500M, and also full duplex will always be true for
> 2500Base-T:
> [   56.004937] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off

I would guess this is because we set the advertising mask to be 2.5bT
FD, and the PCS resolution (being all that we have) reports that we
got 2.5bX FD - and when we try to convert those to a speed/duplex we
fail because there appears to be no mutual ethtool capabilities that
can be agreed.

However, given that the media may be doing 2.5G, 1G or 100M with this
module, and we have no idea what the media may be doing because we
can't access the PHY, it seems to me that reporting "Unknown" speed
and "Unknown" duplex is entirely appropriate and correct, if a little
odd.

The solution... obviously is to have access to the PHY so we know
what the media is doing.

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

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-25 14:05     ` Russell King (Oracle)
@ 2023-03-25 15:35       ` Daniel Golle
  2023-03-25 19:36         ` Russell King (Oracle)
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Golle @ 2023-03-25 15:35 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Sat, Mar 25, 2023 at 02:05:51PM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> > Hi Russell,
> > 
> > On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > > Add a quirk for a copper SFP that identifies itself as "OEM"
> > > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > > at 2500base-X with the host without negotiation. Add a quirk to
> > > enable the 2500base-X interface mode with 2500base-T support, and
> > > disable autonegotiation.
> > > 
> > > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > 
> > I've tried the same fix also with my 2500Base-T SFP module:
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 4223c9fa6902..c7a18a72d2c5 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
> >         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> >         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
> >         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> > +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
> >  };
> > 
> >  static size_t sfp_strlen(const char *str, size_t maxlen)
> 
> Thanks for testing.
> 
> > However, the results are a bit of a mixed bag. The link now does come up
> > without having to manually disable autonegotiation. However, I see this
> > new warning in the bootlog:
> > [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> > ...
> > [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> 
> This will be the result of issuing an ethtool command, and phylink
> doesn't know what to do with the advertising mask - which is saying:
> 
>    Autoneg, Fibre, Pause, AsymPause
> 
> In other words, there are no capabilities to be advertised, which is
> invalid, and suggests user error. What ethtool command was being
> issued?

This was simply adding the interface to a bridge and bringing it up.
No ethtool involved afaik.

> 
> > Also link speed and status appears unknown, though we do know at least
> > that the speed is 2500M, and also full duplex will always be true for
> > 2500Base-T:
> > [   56.004937] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off
> 
> I would guess this is because we set the advertising mask to be 2.5bT
> FD, and the PCS resolution (being all that we have) reports that we
> got 2.5bX FD - and when we try to convert those to a speed/duplex we
> fail because there appears to be no mutual ethtool capabilities that
> can be agreed.
> 
> However, given that the media may be doing 2.5G, 1G or 100M with this
> module, and we have no idea what the media may be doing because we
> can't access the PHY, it seems to me that reporting "Unknown" speed
> and "Unknown" duplex is entirely appropriate and correct, if a little
> odd.
> 
> The solution... obviously is to have access to the PHY so we know
> what the media is doing.

In the case of this SFP the internal PHY *only* supports 2500Base-T.
Slower links (1000M/100M/10M) simply don't come up.

I don't know the situation with the 2.5G module Frank was testing, ie.
which modes it supports on the RJ-45 interface, it could be that in his
case you are right.

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-25 15:35       ` Daniel Golle
@ 2023-03-25 19:36         ` Russell King (Oracle)
  2023-03-26 11:36           ` Aw: " Frank Wunderlich
  2023-03-28 15:10           ` Russell King (Oracle)
  0 siblings, 2 replies; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-25 19:36 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Sat, Mar 25, 2023 at 03:35:01PM +0000, Daniel Golle wrote:
> On Sat, Mar 25, 2023 at 02:05:51PM +0000, Russell King (Oracle) wrote:
> > On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> > > Hi Russell,
> > > 
> > > On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > > > Add a quirk for a copper SFP that identifies itself as "OEM"
> > > > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > > > at 2500base-X with the host without negotiation. Add a quirk to
> > > > enable the 2500base-X interface mode with 2500base-T support, and
> > > > disable autonegotiation.
> > > > 
> > > > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > > > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > 
> > > I've tried the same fix also with my 2500Base-T SFP module:
> > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > > index 4223c9fa6902..c7a18a72d2c5 100644
> > > --- a/drivers/net/phy/sfp.c
> > > +++ b/drivers/net/phy/sfp.c
> > > @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
> > >         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> > >         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
> > >         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> > > +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
> > >  };
> > > 
> > >  static size_t sfp_strlen(const char *str, size_t maxlen)
> > 
> > Thanks for testing.
> > 
> > > However, the results are a bit of a mixed bag. The link now does come up
> > > without having to manually disable autonegotiation. However, I see this
> > > new warning in the bootlog:
> > > [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> > > ...
> > > [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> > 
> > This will be the result of issuing an ethtool command, and phylink
> > doesn't know what to do with the advertising mask - which is saying:
> > 
> >    Autoneg, Fibre, Pause, AsymPause
> > 
> > In other words, there are no capabilities to be advertised, which is
> > invalid, and suggests user error. What ethtool command was being
> > issued?
> 
> This was simply adding the interface to a bridge and bringing it up.
> No ethtool involved afaik.

If its not ethtool, then there is only one other possibility which I
thought had already been ruled out - and that is the PHY is actually
accessible, but either we don't have a driver for it, or when reading
the PHY's "features" we don't know what it is.

Therefore, as the PHY is accessible, we need to identify what it is
and have a driver for it.

Please apply the following patch to print some useful information
about the PHY:

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index aec8e48bdd4f..6b67262d5706 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2978,9 +2978,37 @@ static int phylink_sfp_config_phy(struct phylink *pl, u8 mode,
 
 	iface = sfp_select_interface(pl->sfp_bus, config.advertising);
 	if (iface == PHY_INTERFACE_MODE_NA) {
+		const int num_ids = ARRAY_SIZE(phy->c45_ids.device_ids);
+		u32 id;
+		int i;
+
+		if (phy->is_c45) {
+			for (i = 0; i < num_ids; i++) {
+				id = phy->c45_ids.device_ids[i];
+				if (id != 0xffffffff)
+					break;
+			}
+		} else {
+			id = phy->phy_id;
+		}
+		phylink_err(pl,
+			    "Clause %s PHY [0x%04x:0x%04x] driver %s found but\n",
+			    phy->is_c45 ? "45" : "22",
+			    id >> 16, id & 0xffff,
+			    phy->drv ? phy->drv->name : "[unbound]");
 		phylink_err(pl,
 			    "selection of interface failed, advertisement %*pb\n",
 			    __ETHTOOL_LINK_MODE_MASK_NBITS, config.advertising);
+
+		if (phy->is_c45) {
+			phylink_err(pl, "Further PHY IDs:\n");
+			for (i = 0; i < num_ids; i++) {
+				id = phy->c45_ids.device_ids[i];
+				if (id != 0xffffffff)
+					phylink_err(pl, "  MMD %d [0x%04x:0x%04x]\n",
+						    i, id >> 16, id & 0xffff);
+			}
+		}
 		return -EINVAL;
 	}
 

Thanks.

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

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

* Aw: Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-25 19:36         ` Russell King (Oracle)
@ 2023-03-26 11:36           ` Frank Wunderlich
  2023-03-26 14:44             ` Russell King (Oracle)
  2023-03-28 15:10           ` Russell King (Oracle)
  1 sibling, 1 reply; 16+ messages in thread
From: Frank Wunderlich @ 2023-03-26 11:36 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Daniel Golle, Andrew Lunn, Heiner Kallweit, David S. Miller,
	Eric Dumazet, Jakub Kicinski, netdev, Paolo Abeni

> Gesendet: Samstag, 25. März 2023 um 21:36 Uhr
> Von: "Russell King (Oracle)" <linux@armlinux.org.uk>
> An: "Daniel Golle" <daniel@makrotopia.org>
> Cc: "Andrew Lunn" <andrew@lunn.ch>, "Heiner Kallweit" <hkallweit1@gmail.com>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Frank Wunderlich" <frank-w@public-files.de>, "Jakub Kicinski" <kuba@kernel.org>, netdev@vger.kernel.org, "Paolo Abeni" <pabeni@redhat.com>
> Betreff: Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
>
> On Sat, Mar 25, 2023 at 03:35:01PM +0000, Daniel Golle wrote:
> > On Sat, Mar 25, 2023 at 02:05:51PM +0000, Russell King (Oracle) wrote:
> > > On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> > > > Hi Russell,
> > > > 
> > > > On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > > > > Add a quirk for a copper SFP that identifies itself as "OEM"
> > > > > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > > > > at 2500base-X with the host without negotiation. Add a quirk to
> > > > > enable the 2500base-X interface mode with 2500base-T support, and
> > > > > disable autonegotiation.
> > > > > 
> > > > > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > 
> > > > I've tried the same fix also with my 2500Base-T SFP module:
> > > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > > > index 4223c9fa6902..c7a18a72d2c5 100644
> > > > --- a/drivers/net/phy/sfp.c
> > > > +++ b/drivers/net/phy/sfp.c
> > > > @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
> > > >         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> > > >         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
> > > >         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> > > > +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
> > > >  };
> > > > 
> > > >  static size_t sfp_strlen(const char *str, size_t maxlen)
> > > 
> > > Thanks for testing.
> > > 
> > > > However, the results are a bit of a mixed bag. The link now does come up
> > > > without having to manually disable autonegotiation. However, I see this
> > > > new warning in the bootlog:
> > > > [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> > > > ...
> > > > [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> > > 
> > > This will be the result of issuing an ethtool command, and phylink
> > > doesn't know what to do with the advertising mask - which is saying:
> > > 
> > >    Autoneg, Fibre, Pause, AsymPause
> > > 
> > > In other words, there are no capabilities to be advertised, which is
> > > invalid, and suggests user error. What ethtool command was being
> > > issued?
> > 
> > This was simply adding the interface to a bridge and bringing it up.
> > No ethtool involved afaik.
> 
> If its not ethtool, then there is only one other possibility which I
> thought had already been ruled out - and that is the PHY is actually
> accessible, but either we don't have a driver for it, or when reading
> the PHY's "features" we don't know what it is.
> 
> Therefore, as the PHY is accessible, we need to identify what it is
> and have a driver for it.
> 
> Please apply the following patch to print some useful information
> about the PHY:


i tried this patch too to get more information about the phy of my sfp (i use gmac1 instead of the mt7531 port5), but see nothing new

root@bpi-r3:~# dmesg | grep 'sfp\|phy'
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
[    0.000000] arch_timer: cp15 timer(s) running at 13.00MHz (phys).
[    1.654975] sfp sfp-1: Host maximum power 1.0W
[    1.659976] sfp sfp-2: Host maximum power 1.0W
[    2.001284] sfp sfp-1: module OEM              SFP-2.5G-T       rev 1.0  sn SK2301110008     dc 230110  
[    2.010789] mtk_soc_eth 15100000.ethernet eth1: optical SFP: interfaces=[mac=2-4,21-22, sfp=22]
[    3.261039] mt7530 mdio-bus:1f: phylink_mac_config: mode=fixed/2500base-x/2.5Gbps/Full/none adv=00,00000000,00008000,00006200 pause=03 link=0 an=1
[    3.293176] mt7530 mdio-bus:1f wan (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
[    3.326808] mt7530 mdio-bus:1f lan0 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
[    3.360144] mt7530 mdio-bus:1f lan1 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
[    3.393490] mt7530 mdio-bus:1f lan2 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
[    3.426819] mt7530 mdio-bus:1f lan3 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
[   15.156727] mtk_soc_eth 15100000.ethernet eth0: phylink_mac_config: mode=fixed/2500base-x/2.5Gbps/Full/none adv=00,00000000,00008000,00006240 pause=03 link=0 an=1
[   15.178021] mt7530 mdio-bus:1f lan3: configuring for phy/gmii link mode
[   15.192190] mt7530 mdio-bus:1f lan3: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
[   15.208137] mt7530 mdio-bus:1f lan3: phy link down gmii/Unknown/Unknown/none/off
[   15.216371] mt7530 mdio-bus:1f lan2: configuring for phy/gmii link mode
[   15.228163] mt7530 mdio-bus:1f lan2: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
[   15.242579] mt7530 mdio-bus:1f lan1: configuring for phy/gmii link mode
[   15.245731] mt7530 mdio-bus:1f lan2: phy link down gmii/Unknown/Unknown/none/off
[   15.261771] mt7530 mdio-bus:1f lan1: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
[   15.277381] mt7530 mdio-bus:1f lan0: configuring for phy/gmii link mode
[   15.278665] mt7530 mdio-bus:1f lan1: phy link down gmii/Unknown/Unknown/none/off
[   15.296641] mt7530 mdio-bus:1f lan0: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
[   15.312570] mt7530 mdio-bus:1f lan0: phy link down gmii/Unknown/Unknown/none/off
[   15.392799] mt7530 mdio-bus:1f wan: configuring for phy/gmii link mode
[   15.404425] mt7530 mdio-bus:1f wan: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
[   15.420491] mt7530 mdio-bus:1f wan: phy link up gmii/1Gbps/Full/none/rx/tx
[  262.106630] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/Unknown/Unknown/none adv=00,00000000,00008000,00006400 pause=00 link=0 an=0

full log: https://pastebin.com/9DbCayjv

root@bpi-r3:~# ethtool eth1
Settings for eth1:
        Supported ports: [ FIBRE ]
        Supported link modes:   2500baseT/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: No
        Supported FEC modes: Not reported
        Advertised link modes:  2500baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: No
        Advertised FEC modes: Not reported
        Speed: Unknown!
        Duplex: Unknown! (255)
        Auto-negotiation: off
        Port: FIBRE
        PHYAD: 0
        Transceiver: internal
        Current message level: 0x000000ff (255)
                               drv probe link timer ifdown ifup rx_err tx_err
        Link detected: yes
root@bpi-r3:~# ethtool -m eth1
        Identifier                                : 0x03 (SFP)
        Extended identifier                       : 0x04 (GBIC/SFP defined by 2-wire interface ID)
        Connector                                 : 0x07 (LC)
        Transceiver codes                         : 0x00 0x01 0x00 0x00 0x00 0x00 0x02 0x00 0x00
        Transceiver type                          : SONET: OC-48, short reach
        Encoding                                  : 0x05 (SONET Scrambled)
        BR, Nominal                               : 2500MBd
...

i guess because sfp interface is not PHY_INTERFACE_MODE_NA but 2500BaseX...should we move this out of the condition?

regards Frank

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

* Re: Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-26 11:36           ` Aw: " Frank Wunderlich
@ 2023-03-26 14:44             ` Russell King (Oracle)
  0 siblings, 0 replies; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-26 14:44 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Daniel Golle, Andrew Lunn, Heiner Kallweit, David S. Miller,
	Eric Dumazet, Jakub Kicinski, netdev, Paolo Abeni

On Sun, Mar 26, 2023 at 01:36:11PM +0200, Frank Wunderlich wrote:
> i tried this patch too to get more information about the phy of my sfp (i use gmac1 instead of the mt7531 port5), but see nothing new
> 
> root@bpi-r3:~# dmesg | grep 'sfp\|phy'
> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
> [    0.000000] arch_timer: cp15 timer(s) running at 13.00MHz (phys).
> [    1.654975] sfp sfp-1: Host maximum power 1.0W
> [    1.659976] sfp sfp-2: Host maximum power 1.0W
> [    2.001284] sfp sfp-1: module OEM              SFP-2.5G-T       rev 1.0  sn SK2301110008     dc 230110  
> [    2.010789] mtk_soc_eth 15100000.ethernet eth1: optical SFP: interfaces=[mac=2-4,21-22, sfp=22]
> [    3.261039] mt7530 mdio-bus:1f: phylink_mac_config: mode=fixed/2500base-x/2.5Gbps/Full/none adv=00,00000000,00008000,00006200 pause=03 link=0 an=1
> [    3.293176] mt7530 mdio-bus:1f wan (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
> [    3.326808] mt7530 mdio-bus:1f lan0 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
> [    3.360144] mt7530 mdio-bus:1f lan1 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
> [    3.393490] mt7530 mdio-bus:1f lan2 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
> [    3.426819] mt7530 mdio-bus:1f lan3 (uninitialized): phy: gmii setting supported 00,00000000,00000000,000062ef advertising 00,00000000,00000000,000062ef
> [   15.156727] mtk_soc_eth 15100000.ethernet eth0: phylink_mac_config: mode=fixed/2500base-x/2.5Gbps/Full/none adv=00,00000000,00008000,00006240 pause=03 link=0 an=1
> [   15.178021] mt7530 mdio-bus:1f lan3: configuring for phy/gmii link mode
> [   15.192190] mt7530 mdio-bus:1f lan3: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
> [   15.208137] mt7530 mdio-bus:1f lan3: phy link down gmii/Unknown/Unknown/none/off
> [   15.216371] mt7530 mdio-bus:1f lan2: configuring for phy/gmii link mode
> [   15.228163] mt7530 mdio-bus:1f lan2: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
> [   15.242579] mt7530 mdio-bus:1f lan1: configuring for phy/gmii link mode
> [   15.245731] mt7530 mdio-bus:1f lan2: phy link down gmii/Unknown/Unknown/none/off
> [   15.261771] mt7530 mdio-bus:1f lan1: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
> [   15.277381] mt7530 mdio-bus:1f lan0: configuring for phy/gmii link mode
> [   15.278665] mt7530 mdio-bus:1f lan1: phy link down gmii/Unknown/Unknown/none/off
> [   15.296641] mt7530 mdio-bus:1f lan0: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
> [   15.312570] mt7530 mdio-bus:1f lan0: phy link down gmii/Unknown/Unknown/none/off
> [   15.392799] mt7530 mdio-bus:1f wan: configuring for phy/gmii link mode
> [   15.404425] mt7530 mdio-bus:1f wan: phylink_mac_config: mode=phy/gmii/Unknown/Unknown/none adv=00,00000000,00000000,00000000 pause=00 link=0 an=0
> [   15.420491] mt7530 mdio-bus:1f wan: phy link up gmii/1Gbps/Full/none/rx/tx
> [  262.106630] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/Unknown/Unknown/none adv=00,00000000,00008000,00006400 pause=00 link=0 an=0

Yours isn't detecting a PHY, and this this patch will have no effect
as the patch only changes things in a path that is used when a PHY
is detected.

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

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-25 19:36         ` Russell King (Oracle)
  2023-03-26 11:36           ` Aw: " Frank Wunderlich
@ 2023-03-28 15:10           ` Russell King (Oracle)
  2023-03-28 18:28             ` Daniel Golle
  1 sibling, 1 reply; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-28 15:10 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

Hi Daniel,

Any feedback with this patch applied? Can't move forward without that.

Thanks.

On Sat, Mar 25, 2023 at 07:36:10PM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 25, 2023 at 03:35:01PM +0000, Daniel Golle wrote:
> > On Sat, Mar 25, 2023 at 02:05:51PM +0000, Russell King (Oracle) wrote:
> > > On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> > > > Hi Russell,
> > > > 
> > > > On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > > > > Add a quirk for a copper SFP that identifies itself as "OEM"
> > > > > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > > > > at 2500base-X with the host without negotiation. Add a quirk to
> > > > > enable the 2500base-X interface mode with 2500base-T support, and
> > > > > disable autonegotiation.
> > > > > 
> > > > > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > 
> > > > I've tried the same fix also with my 2500Base-T SFP module:
> > > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > > > index 4223c9fa6902..c7a18a72d2c5 100644
> > > > --- a/drivers/net/phy/sfp.c
> > > > +++ b/drivers/net/phy/sfp.c
> > > > @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
> > > >         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> > > >         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
> > > >         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> > > > +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
> > > >  };
> > > > 
> > > >  static size_t sfp_strlen(const char *str, size_t maxlen)
> > > 
> > > Thanks for testing.
> > > 
> > > > However, the results are a bit of a mixed bag. The link now does come up
> > > > without having to manually disable autonegotiation. However, I see this
> > > > new warning in the bootlog:
> > > > [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> > > > ...
> > > > [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> > > 
> > > This will be the result of issuing an ethtool command, and phylink
> > > doesn't know what to do with the advertising mask - which is saying:
> > > 
> > >    Autoneg, Fibre, Pause, AsymPause
> > > 
> > > In other words, there are no capabilities to be advertised, which is
> > > invalid, and suggests user error. What ethtool command was being
> > > issued?
> > 
> > This was simply adding the interface to a bridge and bringing it up.
> > No ethtool involved afaik.
> 
> If its not ethtool, then there is only one other possibility which I
> thought had already been ruled out - and that is the PHY is actually
> accessible, but either we don't have a driver for it, or when reading
> the PHY's "features" we don't know what it is.
> 
> Therefore, as the PHY is accessible, we need to identify what it is
> and have a driver for it.
> 
> Please apply the following patch to print some useful information
> about the PHY:
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index aec8e48bdd4f..6b67262d5706 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -2978,9 +2978,37 @@ static int phylink_sfp_config_phy(struct phylink *pl, u8 mode,
>  
>  	iface = sfp_select_interface(pl->sfp_bus, config.advertising);
>  	if (iface == PHY_INTERFACE_MODE_NA) {
> +		const int num_ids = ARRAY_SIZE(phy->c45_ids.device_ids);
> +		u32 id;
> +		int i;
> +
> +		if (phy->is_c45) {
> +			for (i = 0; i < num_ids; i++) {
> +				id = phy->c45_ids.device_ids[i];
> +				if (id != 0xffffffff)
> +					break;
> +			}
> +		} else {
> +			id = phy->phy_id;
> +		}
> +		phylink_err(pl,
> +			    "Clause %s PHY [0x%04x:0x%04x] driver %s found but\n",
> +			    phy->is_c45 ? "45" : "22",
> +			    id >> 16, id & 0xffff,
> +			    phy->drv ? phy->drv->name : "[unbound]");
>  		phylink_err(pl,
>  			    "selection of interface failed, advertisement %*pb\n",
>  			    __ETHTOOL_LINK_MODE_MASK_NBITS, config.advertising);
> +
> +		if (phy->is_c45) {
> +			phylink_err(pl, "Further PHY IDs:\n");
> +			for (i = 0; i < num_ids; i++) {
> +				id = phy->c45_ids.device_ids[i];
> +				if (id != 0xffffffff)
> +					phylink_err(pl, "  MMD %d [0x%04x:0x%04x]\n",
> +						    i, id >> 16, id & 0xffff);
> +			}
> +		}
>  		return -EINVAL;
>  	}
>  
> 
> Thanks.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-28 15:10           ` Russell King (Oracle)
@ 2023-03-28 18:28             ` Daniel Golle
  2023-03-28 19:16               ` Russell King (Oracle)
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Golle @ 2023-03-28 18:28 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

Hi Russell,

On Tue, Mar 28, 2023 at 04:10:58PM +0100, Russell King (Oracle) wrote:
> Hi Daniel,
> 
> Any feedback with this patch applied? Can't move forward without that.

Sorry for the delay, I only got back to it today.
I've tried your patch and do not see any additional output on the
kernel log, just like it is the case for Frank's 2.5G SFP module as
well. I conclude that the PHY is inaccessible.

I've tried with and without the sfp_quirk_oem_2_5g.

With the quirk:
[   55.111856] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off

Without the quirk:
[   44.603495] mt7530 mdio-bus:1f sfp2: unsupported SFP module: no common interface modes

Note that as there are probably also other similar 2500Base-T SFP modules around
I suspect that the introduction of the quirk might have broken them, in
the sense that previously they were working if one manually disabled AN
using ethtool, now they won't work at all :(


> 
> Thanks.
> 
> On Sat, Mar 25, 2023 at 07:36:10PM +0000, Russell King (Oracle) wrote:
> > On Sat, Mar 25, 2023 at 03:35:01PM +0000, Daniel Golle wrote:
> > > On Sat, Mar 25, 2023 at 02:05:51PM +0000, Russell King (Oracle) wrote:
> > > > On Sat, Mar 25, 2023 at 02:12:16AM +0000, Daniel Golle wrote:
> > > > > Hi Russell,
> > > > > 
> > > > > On Tue, Mar 21, 2023 at 04:58:51PM +0000, Russell King (Oracle) wrote:
> > > > > > Add a quirk for a copper SFP that identifies itself as "OEM"
> > > > > > "SFP-2.5G-T". This module's PHY is inaccessible, and can only run
> > > > > > at 2500base-X with the host without negotiation. Add a quirk to
> > > > > > enable the 2500base-X interface mode with 2500base-T support, and
> > > > > > disable autonegotiation.
> > > > > > 
> > > > > > Reported-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > > 
> > > > > I've tried the same fix also with my 2500Base-T SFP module:
> > > > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > > > > index 4223c9fa6902..c7a18a72d2c5 100644
> > > > > --- a/drivers/net/phy/sfp.c
> > > > > +++ b/drivers/net/phy/sfp.c
> > > > > @@ -424,6 +424,7 @@ static const struct sfp_quirk sfp_quirks[] = {
> > > > >         SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
> > > > >         SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
> > > > >         SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
> > > > > +       SFP_QUIRK_M("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
> > > > >  };
> > > > > 
> > > > >  static size_t sfp_strlen(const char *str, size_t maxlen)
> > > > 
> > > > Thanks for testing.
> > > > 
> > > > > However, the results are a bit of a mixed bag. The link now does come up
> > > > > without having to manually disable autonegotiation. However, I see this
> > > > > new warning in the bootlog:
> > > > > [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn 12154J6000864    dc 210606  
> > > > > ...
> > > > > [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> > > > 
> > > > This will be the result of issuing an ethtool command, and phylink
> > > > doesn't know what to do with the advertising mask - which is saying:
> > > > 
> > > >    Autoneg, Fibre, Pause, AsymPause
> > > > 
> > > > In other words, there are no capabilities to be advertised, which is
> > > > invalid, and suggests user error. What ethtool command was being
> > > > issued?
> > > 
> > > This was simply adding the interface to a bridge and bringing it up.
> > > No ethtool involved afaik.
> > 
> > If its not ethtool, then there is only one other possibility which I
> > thought had already been ruled out - and that is the PHY is actually
> > accessible, but either we don't have a driver for it, or when reading
> > the PHY's "features" we don't know what it is.
> > 
> > Therefore, as the PHY is accessible, we need to identify what it is
> > and have a driver for it.
> > 
> > Please apply the following patch to print some useful information
> > about the PHY:
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index aec8e48bdd4f..6b67262d5706 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -2978,9 +2978,37 @@ static int phylink_sfp_config_phy(struct phylink *pl, u8 mode,
> >  
> >  	iface = sfp_select_interface(pl->sfp_bus, config.advertising);
> >  	if (iface == PHY_INTERFACE_MODE_NA) {
> > +		const int num_ids = ARRAY_SIZE(phy->c45_ids.device_ids);
> > +		u32 id;
> > +		int i;
> > +
> > +		if (phy->is_c45) {
> > +			for (i = 0; i < num_ids; i++) {
> > +				id = phy->c45_ids.device_ids[i];
> > +				if (id != 0xffffffff)
> > +					break;
> > +			}
> > +		} else {
> > +			id = phy->phy_id;
> > +		}
> > +		phylink_err(pl,
> > +			    "Clause %s PHY [0x%04x:0x%04x] driver %s found but\n",
> > +			    phy->is_c45 ? "45" : "22",
> > +			    id >> 16, id & 0xffff,
> > +			    phy->drv ? phy->drv->name : "[unbound]");
> >  		phylink_err(pl,
> >  			    "selection of interface failed, advertisement %*pb\n",
> >  			    __ETHTOOL_LINK_MODE_MASK_NBITS, config.advertising);
> > +
> > +		if (phy->is_c45) {
> > +			phylink_err(pl, "Further PHY IDs:\n");
> > +			for (i = 0; i < num_ids; i++) {
> > +				id = phy->c45_ids.device_ids[i];
> > +				if (id != 0xffffffff)
> > +					phylink_err(pl, "  MMD %d [0x%04x:0x%04x]\n",
> > +						    i, id >> 16, id & 0xffff);
> > +			}
> > +		}
> >  		return -EINVAL;
> >  	}
> >  
> > 
> > Thanks.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-28 18:28             ` Daniel Golle
@ 2023-03-28 19:16               ` Russell King (Oracle)
  2023-03-28 20:56                 ` Daniel Golle
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King (Oracle) @ 2023-03-28 19:16 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Tue, Mar 28, 2023 at 07:28:53PM +0100, Daniel Golle wrote:
> Hi Russell,
> 
> On Tue, Mar 28, 2023 at 04:10:58PM +0100, Russell King (Oracle) wrote:
> > Hi Daniel,
> > 
> > Any feedback with this patch applied? Can't move forward without that.
> 
> Sorry for the delay, I only got back to it today.
> I've tried your patch and do not see any additional output on the
> kernel log, just like it is the case for Frank's 2.5G SFP module as
> well. I conclude that the PHY is inaccessible.
> 
> I've tried with and without the sfp_quirk_oem_2_5g.
> 
> With the quirk:
> [   55.111856] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off
> 
> Without the quirk:
> [   44.603495] mt7530 mdio-bus:1f sfp2: unsupported SFP module: no common interface modes

This is all getting really very messy, and I have no idea what's going
on and which modules you're testing from report to report.

The patch was to be used with the module which you previously reported
earlier in this thread:

[   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn    12154J6000864    dc 210606
...
[   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440

That second message - "selection of interface failed" only appears in
two places:

1) in phylink_ethtool_ksettings_set() which will be called in response
to ethtool being used, but you've said it isn't, so this can't be it.
2) in phylink_sfp_config_phy(), which will be called when we have
detected a PHY on the SFP module and we're trying to set it up.
This means we must have discovered a PHY on the TL-SM410U module.

This new message you report:

	"unsupported SFP module: no common interface modes"

is produced by phylink_sfp_config_optical(), which is called when we
think we have an optical module (in other words when sfp_may_have_phy()
returns false) or it returns true but we start the module without
having discovered a PHY.

So we can only get to this message if we think the module does not
have a PHY detected.

If it's the exact same module, that would suggest that the module does
have an accessible PHY, but there could be a hardware race between the
PHY becoming accessible and our probing for it. However, we do retry
probing for the PHY up to 12 times at 50ms intervals.

Maybe you could shed some light on what's going on? Is it the exact
same module? Maybe enable debugging in both sfp.c

At the moment I'm rather confused.

Thanks.

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

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

* Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
  2023-03-28 19:16               ` Russell King (Oracle)
@ 2023-03-28 20:56                 ` Daniel Golle
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Golle @ 2023-03-28 20:56 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Frank Wunderlich, Jakub Kicinski, netdev, Paolo Abeni

On Tue, Mar 28, 2023 at 08:16:09PM +0100, Russell King (Oracle) wrote:
> On Tue, Mar 28, 2023 at 07:28:53PM +0100, Daniel Golle wrote:
> > Hi Russell,
> > 
> > On Tue, Mar 28, 2023 at 04:10:58PM +0100, Russell King (Oracle) wrote:
> > > Hi Daniel,
> > > 
> > > Any feedback with this patch applied? Can't move forward without that.
> > 
> > Sorry for the delay, I only got back to it today.
> > I've tried your patch and do not see any additional output on the
> > kernel log, just like it is the case for Frank's 2.5G SFP module as
> > well. I conclude that the PHY is inaccessible.
> > 
> > I've tried with and without the sfp_quirk_oem_2_5g.
> > 
> > With the quirk:
> > [   55.111856] mt7530 mdio-bus:1f sfp2: Link is Up - Unknown/Unknown - flow control off
> > 
> > Without the quirk:
> > [   44.603495] mt7530 mdio-bus:1f sfp2: unsupported SFP module: no common interface modes
> 
> This is all getting really very messy, and I have no idea what's going
> on and which modules you're testing from report to report.
> 
> The patch was to be used with the module which you previously reported
> earlier in this thread:
> 
> [   17.344155] sfp sfp2: module TP-LINK          TL-SM410U        rev 1.0  sn    12154J6000864    dc 210606
> ...
> [   21.653812] mt7530 mdio-bus:1f sfp2: selection of interface failed, advertisement 00,00000000,00000000,00006440
> 
> That second message - "selection of interface failed" only appears in
> two places:
> 
> 1) in phylink_ethtool_ksettings_set() which will be called in response
> to ethtool being used, but you've said it isn't, so this can't be it.
> 2) in phylink_sfp_config_phy(), which will be called when we have
> detected a PHY on the SFP module and we're trying to set it up.
> This means we must have discovered a PHY on the TL-SM410U module.
> 
> This new message you report:
> 
> 	"unsupported SFP module: no common interface modes"
> 
> is produced by phylink_sfp_config_optical(), which is called when we
> think we have an optical module (in other words when sfp_may_have_phy()
> returns false) or it returns true but we start the module without
> having discovered a PHY.
> 
> So we can only get to this message if we think the module does not
> have a PHY detected.
> 
> If it's the exact same module, that would suggest that the module does
> have an accessible PHY, but there could be a hardware race between the
> PHY becoming accessible and our probing for it. However, we do retry
> probing for the PHY up to 12 times at 50ms intervals.
> 
> Maybe you could shed some light on what's going on? Is it the exact
> same module? Maybe enable debugging in both sfp.c

Yes, this is all TL-SM410U. Just one time with the qurik added and one
time without. It can be that OpenWrt's netifd issues some ethtool
ioctls...

> 
> At the moment I'm rather confused.
> 
> Thanks.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

end of thread, other threads:[~2023-03-28 20:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-21 16:58 [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module Russell King (Oracle)
2023-03-21 16:58 ` [PATCH net-next 1/2] net: sfp-bus: allow SFP quirks to override Autoneg and pause bits Russell King (Oracle)
2023-03-22 15:52   ` Simon Horman
2023-03-21 16:58 ` [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP Russell King (Oracle)
2023-03-22 15:52   ` Simon Horman
2023-03-25  2:12   ` Daniel Golle
2023-03-25 14:05     ` Russell King (Oracle)
2023-03-25 15:35       ` Daniel Golle
2023-03-25 19:36         ` Russell King (Oracle)
2023-03-26 11:36           ` Aw: " Frank Wunderlich
2023-03-26 14:44             ` Russell King (Oracle)
2023-03-28 15:10           ` Russell King (Oracle)
2023-03-28 18:28             ` Daniel Golle
2023-03-28 19:16               ` Russell King (Oracle)
2023-03-28 20:56                 ` Daniel Golle
2023-03-23  5:50 ` [PATCH net-next 0/2] Quirk for OEM SFP-2.5G-T copper module patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.