linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Daniel Golle <daniel@makrotopia.org>
Cc: "Andrew Lunn" <andrew@lunn.ch>,
	"Vladimir Oltean" <vladimir.oltean@nxp.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Alexander Couzens" <lynxis@fe80.eu>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	"Mark Lee" <Mark-MC.Lee@mediatek.com>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	linux-mediatek@lists.infradead.org,
	"John Crispin" <john@phrozen.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Jianhui Zhao" <zhaojh329@gmail.com>,
	"Vladimir Oltean" <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Bjørn Mork" <bjorn@mork.no>, "Felix Fietkau" <nbd@nbd.name>
Subject: Re: [PATCH net-next v12 08/18] net: ethernet: mtk_eth_soc: fix 1000Base-X and 2500Base-X modes
Date: Wed, 8 Mar 2023 15:01:07 +0000	[thread overview]
Message-ID: <ZAijM91F18lWC80+@shell.armlinux.org.uk> (raw)
In-Reply-To: <ZAiciK5fElvLXYQ9@makrotopia.org>

On Wed, Mar 08, 2023 at 02:32:40PM +0000, Daniel Golle wrote:
> In general it sound reasonable. We may need more SFP qurik bits to
> indicate presence of a PHY on SFP modules which do not expose that
> PHY via i2c-mdio or otherwise let the host know about it's presence.

That's a whole load of fun - some modules where the PHY is inaccessible
will be using 1000base-X, others will be using SGMII. So yes, its
likely that we may need quirks for these. We don't have quirks yet
because you're the first to suggest there's a problem.

> For my TP-LINK TL-SM410U 2500Base-T SFP this unfortunately seems to
> be the case, and I assume it's actually like that for most
> 2500Base-T as well as xPON SFPs... (xPON SFPs are usually managed
> via high-level protocols, even Web-UI is common there. They don't
> tell you much about them via I2C, I suppose to get them to work in
> as many SFP host devices as possible without any software changes).

xPON SFPs are a whole different ball game. For some, they auto-detect
while booting and try 2500base-X or 1000base-X to see which will sync
and if not they try the other. Other xPON SFPs run their host interface
at a speed determined by the configuration set by the remote end. Other
xPON SFPs may do something entirely different.

In many cases, their EEPROM is a full of errors - such as advertising
that they're 1.2 or 1.3 Gbd while operating in 2500base-X mode.

They do weird stuff with their status pins as well, for example, some
use the RX_LOS pin as a uart - which is a problem if it's e.g. tied
from the cage to a switch that uses the pin to gate the link-up
indication without any software control of that!

With xPON SFPs, it's just a total minefield, which lots of SFF MSA
violations all over the place. They're essentially a law to themselves
(this is exactly why we have the quirks infrastructure.)

> FYI:
> TP-LINK TL-SM410U 2500Base-T module:
> 
> sfp EE: 00000000: 03 04 07 00 00 00 00 00 00 40 00 01 1f 00 00 00  .........@......
> sfp EE: 00000010: 00 00 00 00 54 50 2d 4c 49 4e 4b 20 20 20 20 20  ....TP-LINK     
> sfp EE: 00000020: 20 20 20 20 00 30 b5 c2 54 4c 2d 53 4d 34 31 30      .0..TL-SM410
> sfp EE: 00000030: 55 20 20 20 20 20 20 20 32 2e 30 20 00 00 00 1b  U       2.0 ....
> sfp EE: 00000040: 00 08 01 00 80 ff ff ff 40 3d f0 0d c0 ff ff ff  ........@=......
> sfp EE: 00000050: c8 39 7a 08 c0 ff ff ff 50 3d f0 0d c0 ff ff ff  .9z.....P=......
> sfp sfp2: module TP-LINK          TL-SM410U        rev 2.0  sn 12260M4001782    dc 220622  

I'm guessing this is a module with a checksum problem...

> And this is the ATS SFP-GE-T 10/100/1000M copper module doing
> rate-adaptation to 1000Base-X:
> 
> sfp sfp1: EEPROM extended structure checksum failure: 0xb0 != 0xaf

Given how close that is, it looks like they used the wrong algorithm.

> sfp EE: 00000000: 03 04 07 00 00 00 02 12 00 01 01 01 0c 00 03 00  ................
> sfp EE: 00000010: 00 00 00 00 4f 45 4d 20 20 20 20 20 20 20 20 20  ....OEM         
> sfp EE: 00000020: 20 20 20 20 00 00 90 65 53 46 50 2d 47 45 2d 54      ...eSFP-GE-T
> sfp EE: 00000030: 20 20 20 20 00 00 00 00 43 20 20 20 00 00 00 f0      ....C   ....
> sfp EE: 00000040: 00 12 00 00 32 31 30 37 31 30 41 30 30 31 32 37  ....210710A00127
> sfp EE: 00000050: 33 39 00 00 32 31 30 37 31 30 20 20 60 00 01 af  39..210710  `...
> sfp sfp1: module OEM              SFP-GE-T     rev C    sn  dc 

Welcome to the wonderful world of horribly broken SFPs.

Do we know what form of rate adaption this module needs on the
transmit path? Does it require the host to pace itself to the media
speed (which I suspect will be unreadable if the PHY isn't accessible)
or will it send pause frames?

It would be nice to add these to my database - please send me the
output of ethtool -m $iface raw on > foo.bin for each module.

> That one already needs quirks to even work at all as TX-FAULT is not
> reported properly by the module, see
> 
> https://github.com/dangowrt/linux/commit/2c694bd494583f08858fabca97cfdc79de8ba089

I'm guessing that's not on a kernel version that has:

73472c830eae net: sfp: add support for HALNy GPON SFP
5029be761161 net: sfp: move Huawei MA5671A fixup
275416754e9a net: sfp: move Alcatel Lucent 3FE46541AA fixup
23571c7b9643 net: sfp: move quirk handling into sfp.c
8475c4b70b04 net: sfp: re-implement soft state polling setup

which reworks how we deal with the soft/hard state signals.

I think the problem space is growing, and I fear that if we try to
address all these issues in one go, we're going to end up with way
too much to deal with in one go (which means poor reviews etc.)

Can we try to concentrate on fixing one problem at a time, rather
than throwing a whole load of problems into the mix?

Thanks.

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


  reply	other threads:[~2023-03-08 15:01 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-07 15:52 [PATCH net-next v12 00/18] net: ethernet: mtk_eth_soc: various enhancements Daniel Golle
2023-03-07 15:52 ` [PATCH net-next v12 01/18] net: ethernet: mtk_eth_soc: add support for MT7981 SoC Daniel Golle
2023-03-07 15:52 ` [PATCH net-next v12 02/18] dt-bindings: net: mediatek,net: add mt7981-eth binding Daniel Golle
2023-03-07 15:52 ` [PATCH net-next v12 03/18] dt-bindings: arm: mediatek: sgmiisys: Convert to DT schema Daniel Golle
2023-03-07 15:53 ` [PATCH net-next v12 04/18] dt-bindings: arm: mediatek: sgmiisys: add MT7981 SoC Daniel Golle
2023-03-07 15:53 ` [PATCH net-next v12 05/18] net: ethernet: mtk_eth_soc: set MDIO bus clock frequency Daniel Golle
2023-03-07 15:53 ` [PATCH net-next v12 06/18] net: ethernet: mtk_eth_soc: reset PCS state Daniel Golle
2023-03-07 18:49   ` Aw: " Frank Wunderlich
2023-03-07 15:53 ` [PATCH net-next v12 07/18] net: ethernet: mtk_eth_soc: only write values if needed Daniel Golle
2023-03-07 18:53   ` Aw: " Frank Wunderlich
2023-03-07 15:53 ` [PATCH net-next v12 08/18] net: ethernet: mtk_eth_soc: fix 1000Base-X and 2500Base-X modes Daniel Golle
2023-03-07 18:52   ` Aw: " Frank Wunderlich
2023-03-08 11:35   ` Russell King (Oracle)
2023-03-08 12:11     ` Daniel Golle
2023-03-08 12:41       ` Russell King (Oracle)
2023-03-08 12:53         ` Daniel Golle
2023-03-08 13:12           ` Russell King (Oracle)
2023-03-08 13:46             ` Vladimir Oltean
2023-03-08 14:12               ` Russell King (Oracle)
2023-03-08 14:32                 ` Daniel Golle
2023-03-08 15:01                   ` Russell King (Oracle) [this message]
2023-03-08 15:08                     ` Daniel Golle
2023-03-08 15:24                       ` Russell King (Oracle)
2023-03-11 12:05                         ` Aw: " Frank Wunderlich
2023-03-11 13:26                           ` Frank Wunderlich
2023-03-11 14:51                             ` Daniel Golle
2023-03-11 20:02                               ` Russell King (Oracle)
2023-03-11 20:34                                 ` Daniel Golle
2023-03-11 20:00                           ` Russell King (Oracle)
2023-03-11 20:21                             ` Frank Wunderlich
2023-03-11 20:30                               ` Russell King (Oracle)
2023-03-12 12:40                                 ` Aw: " Frank Wunderlich
2023-03-12 14:26                                   ` Frank Wunderlich
2023-03-12 16:50                                     ` Frank Wunderlich
2023-03-12 20:10                                       ` Russell King (Oracle)
2023-03-12 20:05                                   ` Russell King (Oracle)
2023-03-13 10:59                                     ` Russell King (Oracle)
2023-03-13 18:39                                       ` Aw: " Frank Wunderlich
2023-03-13 22:57                                         ` Russell King (Oracle)
     [not found]                                           ` <trinity-e2c457f1-c897-45f1-907a-8ea3664b7512-1678783872771@3c-app-gmx-bap66>
     [not found]                                             ` <ZBA6gszARdJY26Mz@shell.armlinux.org.uk>
     [not found]                                               ` <trinity-bc4bbf4e-812a-4682-ac8c-5178320467f5-1678788102813@3c-app-gmx-bap66>
     [not found]                                                 ` <ZBBIDqZaqdSfwu9g@shell.armlinux.org.uk>
2023-03-14 13:59                                                   ` Aw: " Frank Wunderlich
2023-03-14 14:11                                                     ` Russell King (Oracle)
2023-03-07 15:54 ` [PATCH net-next v12 09/18] net: ethernet: mtk_eth_soc: Fix link status for none-SGMII modes Daniel Golle
2023-03-07 18:50   ` Aw: " Frank Wunderlich
2023-03-08 11:38   ` Russell King (Oracle)
2023-03-08 11:44     ` Frank Wunderlich
2023-03-08 12:05       ` Daniel Golle
2023-03-08 16:24         ` Russell King (Oracle)
2023-03-08 12:35       ` Russell King (Oracle)
2023-03-07 15:54 ` [PATCH net-next v12 10/18] net: ethernet: mtk_eth_soc: ppe: add support for flow accounting Daniel Golle
2023-03-07 15:54 ` [PATCH net-next v12 11/18] net: pcs: add driver for MediaTek SGMII PCS Daniel Golle
2023-03-07 18:52   ` Aw: " Frank Wunderlich
2023-03-07 15:54 ` [PATCH net-next v12 12/18] net: ethernet: mtk_eth_soc: switch to external PCS driver Daniel Golle
2023-03-07 15:55 ` [PATCH net-next v12 13/18] net: dsa: mt7530: use " Daniel Golle
2023-03-07 15:55 ` [PATCH net-next v12 14/18] net: ethernet: mtk_eth_soc: add MTK_NETSYS_V1 capability bit Daniel Golle
2023-03-07 15:55 ` [PATCH net-next v12 15/18] net: ethernet: mtk_eth_soc: move MAX_DEVS in mtk_soc_data Daniel Golle
2023-03-07 15:55 ` [PATCH v12 16/18] net: ethernet: mtk_eth_soc: rely on num_devs and remove MTK_MAC_COUNT Daniel Golle
2023-03-07 15:55 ` [PATCH v12 17/18] net: ethernet: mtk_eth_soc: add MTK_NETSYS_V3 capability bit Daniel Golle
2023-03-07 15:56 ` [PATCH v12 18/18] net: ethernet: mtk_eth_soc: convert caps in mtk_soc_data struct to u64 Daniel Golle

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=ZAijM91F18lWC80+@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Landen.Chao@mediatek.com \
    --cc=Mark-MC.Lee@mediatek.com \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=bjorn@mork.no \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=john@phrozen.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=lorenzo@kernel.org \
    --cc=lynxis@fe80.eu \
    --cc=matthias.bgg@gmail.com \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=sean.wang@mediatek.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=zhaojh329@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).