Re: [PATCH net-next v12 08/18] net: ethernet: mtk_eth_soc: fix 1000Base-X and 2500Base-X modes
From: Russell King (Oracle)
Date: Tue Mar 14 2023 - 06:10:50 EST
On Tue, Mar 14, 2023 at 11:01:42AM +0100, Frank Wunderlich wrote:
> Hi
>
> > Gesendet: Dienstag, 14. März 2023 um 10:12 Uhr
> > Von: "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx>
> > On Tue, Mar 14, 2023 at 09:51:12AM +0100, Frank Wunderlich wrote:
> > > Hi,
> > >
> > > at least the error-message is gone, and interface gets up when i call ethtoo to switch off autoneg.
> ...
> > > [ 34.400860] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/Unknown/Unknown/none adv=00,00000000,00000000,0000e400 pause=04 link=0 an=1
> >
> > Looking good - apart from that pesky an=1 (which isn't used by the PCS
> > driver, and I've been thinking of killing it off anyway.) Until such
> > time that happens, we really ought to set that correctly, which means
> > an extra bit is needed in phylink_sfp_set_config(). However, this
> > should not affect anything.
> >
> > > root@bpi-r3:~#
> > > root@bpi-r3:~# ethtool -s eth1 autoneg off
> > > root@bpi-r3:~# [ 131.031902] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 2.5Gbps/Full - flow control off
> > > [ 131.040366] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
> > >
> > > full log here:
> > > https://pastebin.com/yDC7PuM2
> > >
> > > i see that an is still 1..maybe because of the fixed value here?
> > >
> > > https://elixir.bootlin.com/linux/v6.3-rc1/source/drivers/net/phy/phylink.c#L3038
> >
> > Not sure what that line has to do with it - this is what the above
> > points to:
> >
> > phylink_sfp_set_config(pl, MLO_AN_INBAND, pl->sfp_support, &config);
>
> MLO_AN_INBAND => may cause the an=1 and mode=inband if previously (?) disabled :)
For 802.3z modes, MLO_AN_INBAND with Autoneg clear in the advertising mode
disables in-band negotiation. This is exactly how "ethtool -s ethX
autoneg off" works.
> > The patch below should result in ethtool reporting 2500baseT rather than
> > 2500baseX, and that an=1 should now be an=0. Please try it, and dump the
> > ethtool eth1 before asking for autoneg to be manually disabled, and also
> > report the kernel messages.
>
> i see no Patch below ;)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 1a2f074685fa..b8844dfcbf51 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2898,6 +2898,10 @@ static void phylink_sfp_set_config(struct phylink *pl, u8 mode,
changed = true;
}
+ if (pl->link_config.an_enabled != state->an_enabled)
+ changed = true;
+ pl->link_config.an_enabled = state->an_enabled;
+
if (pl->cur_link_an_mode != mode ||
pl->link_config.interface != state->interface) {
pl->cur_link_an_mode = mode;
@@ -3001,7 +3005,8 @@ static int phylink_sfp_config_optical(struct phylink *pl)
config.speed = SPEED_UNKNOWN;
config.duplex = DUPLEX_UNKNOWN;
config.pause = MLO_PAUSE_AN;
- config.an_enabled = true;
+ config.an_enabled = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ support);
/* For all the interfaces that are supported, reduce the sfp_support
* mask to only those link modes that can be supported.
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);
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),
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!