From: "Russell King (Oracle)" <linux@armlinux•org.uk>
To: 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>
Subject: Re: [PATCH net-next 2/2] net: sfp: add quirk for 2.5G copper SFP
Date: Tue, 28 Mar 2023 16:10:58 +0100 [thread overview]
Message-ID: <ZCMDgqBSvHigTcbb@shell.armlinux.org.uk> (raw)
In-Reply-To: <ZB9NKo3iXe7CZSId@shell.armlinux.org.uk>
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!
next prev parent reply other threads:[~2023-03-28 15:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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) [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZCMDgqBSvHigTcbb@shell.armlinux.org.uk \
--to=linux@armlinux$(echo .)org.uk \
--cc=andrew@lunn$(echo .)ch \
--cc=daniel@makrotopia$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=frank-w@public-files$(echo .)de \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox