drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++- drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++--- 2 files changed, 138 insertions(+), 6 deletions(-)
Hi everyone, Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to access SFP modules downstream. These controllers are actually SMBus controllers that can only perform single-byte accesses for read and write. This series adds support for accessing SFP modules through single-byte SMBus, which could be relevant for other setups. The first patch deals with the SFP module access by itself, for addresses 0x50 and 0x51. The second patch allows accessing embedded PHYs within the module with single-byte SMBus, adding this in the mdio-i2c driver. As raw i2c transfers are always more efficient, we make sure that the smbus accesses are only used if we really have no other choices. This has been tested with the following modules (as reported upon module insertion) Fiber modules : UBNT UF-MM-1G rev sn FT20051201212 dc 200512 PROLABS SFP-1GSXLC-T-C rev A1 sn PR2109CA1080 dc 220607 CISCOSOLIDOPTICS CWDM-SFP-1490 rev 1.0 sn SOSC49U0891 dc 181008 CISCOSOLIDOPTICS CWDM-SFP-1470 rev 1.0 sn SOSC47U1175 dc 190620 OEM SFP-10G-SR rev 02 sn CSSSRIC3174 dc 181201 FINISAR CORP. FTLF1217P2BTL-HA rev A sn PA3A0L6 dc 230716 OEM ES8512-3LCD05 rev 10 sn ESC22SX296055 dc 220722 SOURCEPHOTONICS SPP10ESRCDFF rev 10 sn E8G2017450 dc 140715 CXR SFP-STM1-MM-850 rev 0000 sn K719017031 dc 200720 Copper modules OEM SFT-7000-RJ45-AL rev 11.0 sn EB1902240862 dc 190313 FINISAR CORP. FCLF8521P2BTL rev A sn P1KBAPD dc 190508 CHAMPION ONE 1000SFPT rev - sn GBC59750 dc 19110401 DAC : OEM SFP-H10GB-CU1M rev R sn CSC200803140115 dc 200827 In all cases, read/write operations happened without errors, and the internal PHY (if any) was always properly detected and accessible I haven't tested with any RollBall SFPs though, as I don't have any, and I don't have Copper modules with anything else than a Marvell 88e1111 inside. The support for the VSC8552 SMBus may follow at some point. Thanks, Maxime Maxime Chevallier (2): net: phy: sfp: Add support for SMBus module access net: mdio: mdio-i2c: Add support for single-byte SMBus operations drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++- drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++--- 2 files changed, 138 insertions(+), 6 deletions(-) -- 2.48.1
On 2/23/25 12:28, Maxime Chevallier wrote: > Hi everyone, > > Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to > access SFP modules downstream. These controllers are actually SMBus controllers > that can only perform single-byte accesses for read and write. > > This series adds support for accessing SFP modules through single-byte SMBus, > which could be relevant for other setups. > > The first patch deals with the SFP module access by itself, for addresses 0x50 > and 0x51. > > The second patch allows accessing embedded PHYs within the module with single-byte > SMBus, adding this in the mdio-i2c driver. > > As raw i2c transfers are always more efficient, we make sure that the smbus accesses > are only used if we really have no other choices. > > This has been tested with the following modules (as reported upon module insertion) > > Fiber modules : > > UBNT UF-MM-1G rev sn FT20051201212 dc 200512 > PROLABS SFP-1GSXLC-T-C rev A1 sn PR2109CA1080 dc 220607 > CISCOSOLIDOPTICS CWDM-SFP-1490 rev 1.0 sn SOSC49U0891 dc 181008 > CISCOSOLIDOPTICS CWDM-SFP-1470 rev 1.0 sn SOSC47U1175 dc 190620 > OEM SFP-10G-SR rev 02 sn CSSSRIC3174 dc 181201 > FINISAR CORP. FTLF1217P2BTL-HA rev A sn PA3A0L6 dc 230716 > OEM ES8512-3LCD05 rev 10 sn ESC22SX296055 dc 220722 > SOURCEPHOTONICS SPP10ESRCDFF rev 10 sn E8G2017450 dc 140715 > CXR SFP-STM1-MM-850 rev 0000 sn K719017031 dc 200720 > > Copper modules > > OEM SFT-7000-RJ45-AL rev 11.0 sn EB1902240862 dc 190313 > FINISAR CORP. FCLF8521P2BTL rev A sn P1KBAPD dc 190508 > CHAMPION ONE 1000SFPT rev - sn GBC59750 dc 19110401 > > DAC : > > OEM SFP-H10GB-CU1M rev R sn CSC200803140115 dc 200827 > > In all cases, read/write operations happened without errors, and the internal > PHY (if any) was always properly detected and accessible > > I haven't tested with any RollBall SFPs though, as I don't have any, and I don't > have Copper modules with anything else than a Marvell 88e1111 inside. The support > for the VSC8552 SMBus may follow at some point. > > Thanks, > > Maxime > > Maxime Chevallier (2): > net: phy: sfp: Add support for SMBus module access > net: mdio: mdio-i2c: Add support for single-byte SMBus operations > > drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++- > drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++--- > 2 files changed, 138 insertions(+), 6 deletions(-) > For this series: Tested-by: Sean Anderson <sean.anderson@linux.dev> With a FS SFP-GB-GE-T rev F sn F2030222359 dc 200729 See [1] for the original "bug report." Note that as discussed later in the thread, this is a Fiber Store (fs.com) module and not a Finisar one. --Sean [1] https://lore.kernel.org/netdev/55f6cec4-2497-45a4-cb1a-3edafa7d80d3@seco.com/
Maxime Chevallier <maxime.chevallier@bootlin.com> writes:
> Hi everyone,
>
> Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to
> access SFP modules downstream. These controllers are actually SMBus controllers
> that can only perform single-byte accesses for read and write.
>
> This series adds support for accessing SFP modules through single-byte SMBus,
> which could be relevant for other setups.
>
> The first patch deals with the SFP module access by itself, for addresses 0x50
> and 0x51.
>
> The second patch allows accessing embedded PHYs within the module with single-byte
> SMBus, adding this in the mdio-i2c driver.
>
> As raw i2c transfers are always more efficient, we make sure that the smbus accesses
> are only used if we really have no other choices.
>
> This has been tested with the following modules (as reported upon module insertion)
>
> Fiber modules :
>
> UBNT UF-MM-1G rev sn FT20051201212 dc 200512
> PROLABS SFP-1GSXLC-T-C rev A1 sn PR2109CA1080 dc 220607
> CISCOSOLIDOPTICS CWDM-SFP-1490 rev 1.0 sn SOSC49U0891 dc 181008
> CISCOSOLIDOPTICS CWDM-SFP-1470 rev 1.0 sn SOSC47U1175 dc 190620
> OEM SFP-10G-SR rev 02 sn CSSSRIC3174 dc 181201
> FINISAR CORP. FTLF1217P2BTL-HA rev A sn PA3A0L6 dc 230716
> OEM ES8512-3LCD05 rev 10 sn ESC22SX296055 dc 220722
> SOURCEPHOTONICS SPP10ESRCDFF rev 10 sn E8G2017450 dc 140715
> CXR SFP-STM1-MM-850 rev 0000 sn K719017031 dc 200720
>
> Copper modules
>
> OEM SFT-7000-RJ45-AL rev 11.0 sn EB1902240862 dc 190313
> FINISAR CORP. FCLF8521P2BTL rev A sn P1KBAPD dc 190508
> CHAMPION ONE 1000SFPT rev - sn GBC59750 dc 19110401
>
> DAC :
>
> OEM SFP-H10GB-CU1M rev R sn CSC200803140115 dc 200827
>
> In all cases, read/write operations happened without errors, and the internal
> PHY (if any) was always properly detected and accessible
>
> I haven't tested with any RollBall SFPs though, as I don't have any, and I don't
> have Copper modules with anything else than a Marvell 88e1111 inside. The support
> for the VSC8552 SMBus may follow at some point.
>
> Thanks,
>
> Maxime
>
> Maxime Chevallier (2):
> net: phy: sfp: Add support for SMBus module access
> net: mdio: mdio-i2c: Add support for single-byte SMBus operations
>
> drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++-
> drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++---
> 2 files changed, 138 insertions(+), 6 deletions(-)
Nice! Don't know if you're aware, but OpenWrt have had patches for
SMBus access to SFPs for some time:
https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/714-net-phy-sfp-add-support-for-SMBus.patch
https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/712-net-phy-add-an-MDIO-SMBus-library.patch
The reason they carry these is that they support Realtek rtl930x based
switches. The rtl930x SoCs include an 8 channel SMBus host which is
typically connected to any SFP+ slots on the switch.
There has been work going on for a while to bring the support for these
SoCs to mainline, and the SMBus host driver is already here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-rtl9300.c?id=c366be720235301fdadf67e6f1ea6ff32669c074
I assume DSA and ethernet eventually will follow, making SMBus SFP
support necessary for this platform too.
So thanks for doing this!
FWIW, I don't think the OpenWrt mdio patch works at all. I've recently
been playing with an 8 SFP+ port switch based on rtl9303, and have tried
to fixup both the clause 22 support and add RollBall and clause 45.
This is still a somewhat untested hack, and I was not planning on
presenting it here as such, but since this discussion is open:
https://github.com/openwrt/openwrt/pull/17950/commits/c40387104af62a065797bc3e23dfb9f36e03851b
Sorry for the format. This is a patch for the patch already present in
OpenWrt. Let me know if you want me to post the complete patched
mdio-smbus.c for easier reading.
The main point I wanted to make is that we also need RollBall and clause
45 over SMBus. Maybe not today, but at some point. Ideally, the code
should be shared with the i2c implementation, but I found that very hard
to do as it is.
As for Russel's comments regarding atomic reads, I'm hoping for the
pragmatic approach and allow all possible features over SMBus. It's not
like we have the option of using i2c on a host which only supports
SMBus. My experience is that both hwmon and phy access works pretty
well with SMBus byte accesses.
Some examples:
root@s508cl:~# ethtool -m lan1
Identifier : 0x03 (SFP)
Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
Connector : 0x07 (LC)
Transceiver codes : 0x10 0x00 0x00 0x00 0x00 0x00 0x06 0x00 0x00
Transceiver type : 10G Ethernet: 10G Base-SR
Transceiver type : FC: Multimode, 50um (M5)
Encoding : 0x03 (NRZ)
BR, Nominal : 10300MBd
Rate identifier : 0x00 (unspecified)
Length (SMF,km) : 0km
Length (SMF) : 0m
Length (50um) : 300m
Length (62.5um) : 300m
Length (Copper) : 0m
Length (OM3) : 300m
Laser wavelength : 850nm
Vendor name : OEM
Vendor OUI : 00:00:00
Vendor PN : SFP-10G-SR
Vendor rev : B
Option values : 0x00 0x1a
Option : RX_LOS implemented
Option : TX_FAULT implemented
Option : TX_DISABLE implemented
BR margin, max : 0%
BR margin, min : 0%
Vendor SN : 202412240025
Date code : 241224
Optical diagnostics support : Yes
Laser bias current : 7.146 mA
Laser output power : 0.4005 mW / -3.97 dBm
Receiver signal average optical power : 0.5088 mW / -2.93 dBm
Module temperature : 52.13 degrees C / 125.83 degrees F
Module voltage : 3.2644 V
Alarm/warning flags implemented : Yes
Laser bias current high alarm : Off
Laser bias current low alarm : Off
Laser bias current high warning : Off
Laser bias current low warning : Off
Laser output power high alarm : Off
Laser output power low alarm : Off
Laser output power high warning : Off
Laser output power low warning : Off
Module temperature high alarm : Off
Module temperature low alarm : Off
Module temperature high warning : Off
Module temperature low warning : Off
Module voltage high alarm : Off
Module voltage low alarm : Off
Module voltage high warning : Off
Module voltage low warning : Off
Laser rx power high alarm : Off
Laser rx power low alarm : Off
Laser rx power high warning : Off
Laser rx power low warning : Off
Laser bias current high alarm threshold : 12.000 mA
Laser bias current low alarm threshold : 1.000 mA
Laser bias current high warning threshold : 10.000 mA
Laser bias current low warning threshold : 2.000 mA
Laser output power high alarm threshold : 1.5849 mW / 2.00 dBm
Laser output power low alarm threshold : 0.1000 mW / -10.00 dBm
Laser output power high warning threshold : 1.2589 mW / 1.00 dBm
Laser output power low warning threshold : 0.1259 mW / -9.00 dBm
Module temperature high alarm threshold : 85.00 degrees C / 185.00 degrees F
Module temperature low alarm threshold : -10.00 degrees C / 14.00 degrees F
Module temperature high warning threshold : 80.00 degrees C / 176.00 degrees F
Module temperature low warning threshold : -5.00 degrees C / 23.00 degrees F
Module voltage high alarm threshold : 3.7000 V
Module voltage low alarm threshold : 2.9000 V
Module voltage high warning threshold : 3.6000 V
Module voltage low warning threshold : 3.0000 V
Laser rx power high alarm threshold : 1.9953 mW / 3.00 dBm
Laser rx power low alarm threshold : 0.0398 mW / -14.00 dBm
Laser rx power high warning threshold : 1.5849 mW / 2.00 dBm
Laser rx power low warning threshold : 0.0501 mW / -13.00 dBm
root@s508cl:~# ethtool -m lan3
Identifier : 0x03 (SFP)
Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
Connector : 0x07 (LC)
Transceiver codes : 0x10 0x00 0x00 0x00 0x40 0x00 0x0c 0x00 0x00
Transceiver type : 10G Ethernet: 10G Base-SR
Transceiver type : FC: short distance (S)
Transceiver type : FC: Multimode, 62.5um (M6)
Transceiver type : FC: Multimode, 50um (M5)
Encoding : 0x06 (64B/66B)
BR, Nominal : 10300MBd
Rate identifier : 0x00 (unspecified)
Length (SMF,km) : 0km
Length (SMF) : 0m
Length (50um) : 30m
Length (62.5um) : 10m
Length (Copper) : 0m
Length (OM3) : 0m
Laser wavelength : 850nm
Vendor name : FS
Vendor OUI : 00:00:00
Vendor PN : SFP-10G-T
Vendor rev :
Option values : 0x00 0x1a
Option : RX_LOS implemented
Option : TX_FAULT implemented
Option : TX_DISABLE implemented
BR margin, max : 10%
BR margin, min : 88%
Vendor SN : F2220644072
Date code : 220824
Optical diagnostics support : Yes
Laser bias current : 6.000 mA
Laser output power : 0.5000 mW / -3.01 dBm
Receiver signal average optical power : 0.0000 mW / -inf dBm
Module temperature : 54.22 degrees C / 129.59 degrees F
Module voltage : 3.3368 V
Alarm/warning flags implemented : Yes
Laser bias current high alarm : Off
Laser bias current low alarm : Off
Laser bias current high warning : Off
Laser bias current low warning : Off
Laser output power high alarm : Off
Laser output power low alarm : Off
Laser output power high warning : Off
Laser output power low warning : Off
Module temperature high alarm : Off
Module temperature low alarm : Off
Module temperature high warning : Off
Module temperature low warning : Off
Module voltage high alarm : Off
Module voltage low alarm : Off
Module voltage high warning : Off
Module voltage low warning : Off
Laser rx power high alarm : Off
Laser rx power low alarm : On
Laser rx power high warning : Off
Laser rx power low warning : On
Laser bias current high alarm threshold : 15.000 mA
Laser bias current low alarm threshold : 1.000 mA
Laser bias current high warning threshold : 13.000 mA
Laser bias current low warning threshold : 2.000 mA
Laser output power high alarm threshold : 1.9952 mW / 3.00 dBm
Laser output power low alarm threshold : 0.1584 mW / -8.00 dBm
Laser output power high warning threshold : 1.5848 mW / 2.00 dBm
Laser output power low warning threshold : 0.1778 mW / -7.50 dBm
Module temperature high alarm threshold : 80.00 degrees C / 176.00 degrees F
Module temperature low alarm threshold : -10.00 degrees C / 14.00 degrees F
Module temperature high warning threshold : 75.00 degrees C / 167.00 degrees F
Module temperature low warning threshold : -5.00 degrees C / 23.00 degrees F
Module voltage high alarm threshold : 3.6000 V
Module voltage low alarm threshold : 3.0000 V
Module voltage high warning threshold : 3.5000 V
Module voltage low warning threshold : 3.1000 V
Laser rx power high alarm threshold : 1.1220 mW / 0.50 dBm
Laser rx power low alarm threshold : 0.0199 mW / -17.01 dBm
Laser rx power high warning threshold : 1.0000 mW / 0.00 dBm
Laser rx power low warning threshold : 0.0223 mW / -16.52 dBm
root@s508cl:~# ethtool -m lan8
Identifier : 0x03 (SFP)
Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
Connector : 0x07 (LC)
Transceiver codes : 0x10 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
Transceiver type : 10G Ethernet: 10G Base-SR
Encoding : 0x06 (64B/66B)
BR, Nominal : 10300MBd
Rate identifier : 0x00 (unspecified)
Length (SMF,km) : 0km
Length (SMF) : 0m
Length (50um) : 80m
Length (62.5um) : 20m
Length (Copper) : 0m
Length (OM3) : 300m
Laser wavelength : 0nm
Vendor name : OEM
Vendor OUI : 00:00:00
Vendor PN : SFP-10G-T8
Vendor rev : A
Option values : 0x00 0x1a
Option : RX_LOS implemented
Option : TX_FAULT implemented
Option : TX_DISABLE implemented
BR margin, max : 0%
BR margin, min : 0%
Vendor SN : F250114T0010
Date code : 250115
Optical diagnostics support : Yes
Laser bias current : 6.000 mA
Laser output power : 0.5010 mW / -3.00 dBm
Receiver signal average optical power : 0.5010 mW / -3.00 dBm
Module temperature : 67.64 degrees C / 153.75 degrees F
Module voltage : 3.2538 V
Alarm/warning flags implemented : Yes
Laser bias current high alarm : Off
Laser bias current low alarm : Off
Laser bias current high warning : Off
Laser bias current low warning : Off
Laser output power high alarm : Off
Laser output power low alarm : Off
Laser output power high warning : Off
Laser output power low warning : Off
Module temperature high alarm : Off
Module temperature low alarm : Off
Module temperature high warning : Off
Module temperature low warning : Off
Module voltage high alarm : Off
Module voltage low alarm : Off
Module voltage high warning : Off
Module voltage low warning : Off
Laser rx power high alarm : Off
Laser rx power low alarm : Off
Laser rx power high warning : Off
Laser rx power low warning : Off
Laser bias current high alarm threshold : 15.000 mA
Laser bias current low alarm threshold : 1.000 mA
Laser bias current high warning threshold : 13.000 mA
Laser bias current low warning threshold : 2.000 mA
Laser output power high alarm threshold : 1.1220 mW / 0.50 dBm
Laser output power low alarm threshold : 0.1862 mW / -7.30 dBm
Laser output power high warning threshold : 1.0000 mW / 0.00 dBm
Laser output power low warning threshold : 0.2344 mW / -6.30 dBm
Module temperature high alarm threshold : 80.00 degrees C / 176.00 degrees F
Module temperature low alarm threshold : -10.00 degrees C / 14.00 degrees F
Module temperature high warning threshold : 75.00 degrees C / 167.00 degrees F
Module temperature low warning threshold : -5.00 degrees C / 23.00 degrees F
Module voltage high alarm threshold : 3.5999 V
Module voltage low alarm threshold : 2.9000 V
Module voltage high warning threshold : 3.5000 V
Module voltage low warning threshold : 3.0000 V
Laser rx power high alarm threshold : 1.1220 mW / 0.50 dBm
Laser rx power low alarm threshold : 0.0645 mW / -11.90 dBm
Laser rx power high warning threshold : 1.0000 mW / 0.00 dBm
Laser rx power low warning threshold : 0.0812 mW / -10.90 dBm
Phy access works too:
root@s508cl:~# mdio smbus:sfp-p5 phy 22
BMCR(0x00): 0x1140
flags: -reset -loopback +aneg-enable -power-down -isolate -aneg-restart
-collision-test
speed: 1000-full
BMSR(0x01): 0x7949
capabilities: -100-t4 +100-tx-f +100-tx-h +10-t-f +10-t-h -100-t2-f -100-t2-h
flags: +ext-status -aneg-complete -remote-fault +aneg-capable -link
-jabber +ext-register
ID(0x02/0x03): 0x01410cc2
ESTATUS(0x0F): 0xf000
capabilities: +1000-x-f +1000-x-h +1000-t-f +1000-t-h
Even this, which is using RollBall over SMBus:
root@s508cl:~# mdio smbus:sfp-p3 mmd 17:1
CTRL1(0x00): 0x0002
flags: -reset -low-power +remote-loopback -local-loopback
speed: 10
STAT1(0x01): 0x0002
capabilities: -pias -peas +low-power
flags: -fault -link
DEVID(0x02/0x03): 0x31c31c12
SPEED(0x04): 0x6031
capabilities: -400g +5g +2.5g -200g -25g -10g-xr -100g -40g -10g/1g -10 +100
+1000 -10-ts -2-tl +10g
DEVS(0x06/0x05): 0xe000009a
devices: +vendor2 +vendor1 +c22-ext -power-unit -ofdm -pma4 -pma3 -pma2 -pma1
+aneg -tc -dte-xs +phy-xs +pcs -wis +pma/pmd -c22
CTRL2(0x07): 0x0009
flags: -pias -peas
type: 10g-t
STAT2(0x08): 0xb301
capabilities: +tx-fault +rx-fault +ext-register +tx-disable +local-loopback
-10g-sr -10g-lr -10g-er -10g-lx4 -10g-sw -10g-lw -10g-ew
flags: +present -tx-fault -rx-fault
EXTABLE(0x0B): 0x40fc
capabilities: -10g-cx4 -10g-lrm +10g-t +10g-kx4 +10g-kr +1000-t +1000-kx
+100-tx -10-t -p2mp -40g/100g -1000/100-t1 -25g -200g/400g
+2.5g/5g -1000-h
PKGID(0x0E/0x0F): 0x31c31c12
Bjørn
Hi Bjørn, On Sun, 23 Feb 2025 19:37:05 +0100 Bjørn Mork <bjorn@mork.no> wrote: > Maxime Chevallier <maxime.chevallier@bootlin.com> writes: > > > Hi everyone, > > > > Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to > > access SFP modules downstream. These controllers are actually SMBus controllers > > that can only perform single-byte accesses for read and write. > > > > This series adds support for accessing SFP modules through single-byte SMBus, > > which could be relevant for other setups. > > > > The first patch deals with the SFP module access by itself, for addresses 0x50 > > and 0x51. > > > > The second patch allows accessing embedded PHYs within the module with single-byte > > SMBus, adding this in the mdio-i2c driver. > > > > As raw i2c transfers are always more efficient, we make sure that the smbus accesses > > are only used if we really have no other choices. > > > > This has been tested with the following modules (as reported upon module insertion) > > > > Fiber modules : > > > > UBNT UF-MM-1G rev sn FT20051201212 dc 200512 > > PROLABS SFP-1GSXLC-T-C rev A1 sn PR2109CA1080 dc 220607 > > CISCOSOLIDOPTICS CWDM-SFP-1490 rev 1.0 sn SOSC49U0891 dc 181008 > > CISCOSOLIDOPTICS CWDM-SFP-1470 rev 1.0 sn SOSC47U1175 dc 190620 > > OEM SFP-10G-SR rev 02 sn CSSSRIC3174 dc 181201 > > FINISAR CORP. FTLF1217P2BTL-HA rev A sn PA3A0L6 dc 230716 > > OEM ES8512-3LCD05 rev 10 sn ESC22SX296055 dc 220722 > > SOURCEPHOTONICS SPP10ESRCDFF rev 10 sn E8G2017450 dc 140715 > > CXR SFP-STM1-MM-850 rev 0000 sn K719017031 dc 200720 > > > > Copper modules > > > > OEM SFT-7000-RJ45-AL rev 11.0 sn EB1902240862 dc 190313 > > FINISAR CORP. FCLF8521P2BTL rev A sn P1KBAPD dc 190508 > > CHAMPION ONE 1000SFPT rev - sn GBC59750 dc 19110401 > > > > DAC : > > > > OEM SFP-H10GB-CU1M rev R sn CSC200803140115 dc 200827 > > > > In all cases, read/write operations happened without errors, and the internal > > PHY (if any) was always properly detected and accessible > > > > I haven't tested with any RollBall SFPs though, as I don't have any, and I don't > > have Copper modules with anything else than a Marvell 88e1111 inside. The support > > for the VSC8552 SMBus may follow at some point. > > > > Thanks, > > > > Maxime > > > > Maxime Chevallier (2): > > net: phy: sfp: Add support for SMBus module access > > net: mdio: mdio-i2c: Add support for single-byte SMBus operations > > > > drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++- > > drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++--- > > 2 files changed, 138 insertions(+), 6 deletions(-) > > Nice! Don't know if you're aware, but OpenWrt have had patches for > SMBus access to SFPs for some time: > > https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/714-net-phy-sfp-add-support-for-SMBus.patch > https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/712-net-phy-add-an-MDIO-SMBus-library.patch > > The reason they carry these is that they support Realtek rtl930x based > switches. The rtl930x SoCs include an 8 channel SMBus host which is > typically connected to any SFP+ slots on the switch. > > There has been work going on for a while to bring the support for these > SoCs to mainline, and the SMBus host driver is already here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-rtl9300.c?id=c366be720235301fdadf67e6f1ea6ff32669c074 > > I assume DSA and ethernet eventually will follow, making SMBus SFP > support necessary for this platform too. > > So thanks for doing this! Good to know this is useful to you ! So there's at least 2 different classes of products out there with SMBus that advertise that it's "designed for SFP" ._. > FWIW, I don't think the OpenWrt mdio patch works at all. I've recently > been playing with an 8 SFP+ port switch based on rtl9303, and have tried > to fixup both the clause 22 support and add RollBall and clause 45. > This is still a somewhat untested hack, and I was not planning on > presenting it here as such, but since this discussion is open: > https://github.com/openwrt/openwrt/pull/17950/commits/c40387104af62a065797bc3e23dfb9f36e03851b > > Sorry for the format. This is a patch for the patch already present in > OpenWrt. Let me know if you want me to post the complete patched > mdio-smbus.c for easier reading. > > The main point I wanted to make is that we also need RollBall and clause > 45 over SMBus. Maybe not today, but at some point. Ideally, the code > should be shared with the i2c implementation, but I found that very hard > to do as it is. I don't have anything to test that, and yeah that can be considered as a second step, however I don't even know if this can work at all with single byte accesses :( Thanks, Maxime
On Mon, Feb 24, 2025 at 10:38:14AM +0100, Maxime Chevallier wrote: > Hi Bjørn, > > On Sun, 23 Feb 2025 19:37:05 +0100 > Bjørn Mork <bjorn@mork.no> wrote: > > > Maxime Chevallier <maxime.chevallier@bootlin.com> writes: > > > > > Hi everyone, > > > > > > Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to > > > access SFP modules downstream. These controllers are actually SMBus controllers > > > that can only perform single-byte accesses for read and write. > > > > > > This series adds support for accessing SFP modules through single-byte SMBus, > > > which could be relevant for other setups. > > > > > > The first patch deals with the SFP module access by itself, for addresses 0x50 > > > and 0x51. > > > > > > The second patch allows accessing embedded PHYs within the module with single-byte > > > SMBus, adding this in the mdio-i2c driver. > > > > > > As raw i2c transfers are always more efficient, we make sure that the smbus accesses > > > are only used if we really have no other choices. > > > > > > This has been tested with the following modules (as reported upon module insertion) > > > > > > Fiber modules : > > > > > > UBNT UF-MM-1G rev sn FT20051201212 dc 200512 > > > PROLABS SFP-1GSXLC-T-C rev A1 sn PR2109CA1080 dc 220607 > > > CISCOSOLIDOPTICS CWDM-SFP-1490 rev 1.0 sn SOSC49U0891 dc 181008 > > > CISCOSOLIDOPTICS CWDM-SFP-1470 rev 1.0 sn SOSC47U1175 dc 190620 > > > OEM SFP-10G-SR rev 02 sn CSSSRIC3174 dc 181201 > > > FINISAR CORP. FTLF1217P2BTL-HA rev A sn PA3A0L6 dc 230716 > > > OEM ES8512-3LCD05 rev 10 sn ESC22SX296055 dc 220722 > > > SOURCEPHOTONICS SPP10ESRCDFF rev 10 sn E8G2017450 dc 140715 > > > CXR SFP-STM1-MM-850 rev 0000 sn K719017031 dc 200720 > > > > > > Copper modules > > > > > > OEM SFT-7000-RJ45-AL rev 11.0 sn EB1902240862 dc 190313 > > > FINISAR CORP. FCLF8521P2BTL rev A sn P1KBAPD dc 190508 > > > CHAMPION ONE 1000SFPT rev - sn GBC59750 dc 19110401 > > > > > > DAC : > > > > > > OEM SFP-H10GB-CU1M rev R sn CSC200803140115 dc 200827 > > > > > > In all cases, read/write operations happened without errors, and the internal > > > PHY (if any) was always properly detected and accessible > > > > > > I haven't tested with any RollBall SFPs though, as I don't have any, and I don't > > > have Copper modules with anything else than a Marvell 88e1111 inside. The support > > > for the VSC8552 SMBus may follow at some point. > > > > > > Thanks, > > > > > > Maxime > > > > > > Maxime Chevallier (2): > > > net: phy: sfp: Add support for SMBus module access > > > net: mdio: mdio-i2c: Add support for single-byte SMBus operations > > > > > > drivers/net/mdio/mdio-i2c.c | 79 ++++++++++++++++++++++++++++++++++++- > > > drivers/net/phy/sfp.c | 65 +++++++++++++++++++++++++++--- > > > 2 files changed, 138 insertions(+), 6 deletions(-) > > > > Nice! Don't know if you're aware, but OpenWrt have had patches for > > SMBus access to SFPs for some time: > > > > https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/714-net-phy-sfp-add-support-for-SMBus.patch > > https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/patches-6.6/712-net-phy-add-an-MDIO-SMBus-library.patch > > > > The reason they carry these is that they support Realtek rtl930x based > > switches. The rtl930x SoCs include an 8 channel SMBus host which is > > typically connected to any SFP+ slots on the switch. > > > > There has been work going on for a while to bring the support for these > > SoCs to mainline, and the SMBus host driver is already here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-rtl9300.c?id=c366be720235301fdadf67e6f1ea6ff32669c074 > > > > I assume DSA and ethernet eventually will follow, making SMBus SFP > > support necessary for this platform too. > > > > So thanks for doing this! > > Good to know this is useful to you ! So there's at least 2 different > classes of products out there with SMBus that advertise that it's > "designed for SFP" ._. > > > FWIW, I don't think the OpenWrt mdio patch works at all. I've recently > > been playing with an 8 SFP+ port switch based on rtl9303, and have tried > > to fixup both the clause 22 support and add RollBall and clause 45. > > This is still a somewhat untested hack, and I was not planning on > > presenting it here as such, but since this discussion is open: > > https://github.com/openwrt/openwrt/pull/17950/commits/c40387104af62a065797bc3e23dfb9f36e03851b > > > > Sorry for the format. This is a patch for the patch already present in > > OpenWrt. Let me know if you want me to post the complete patched > > mdio-smbus.c for easier reading. > > > > The main point I wanted to make is that we also need RollBall and clause > > 45 over SMBus. Maybe not today, but at some point. Ideally, the code > > should be shared with the i2c implementation, but I found that very hard > > to do as it is. > > I don't have anything to test that, and yeah that can be considered as > a second step, however I don't even know if this can work at all with > single byte accesses :( I will send you some RollBall modules, Maxime. Marek
On Sun, Feb 23, 2025 at 07:37:05PM +0100, Bjørn Mork wrote: > As for Russel's comments regarding atomic reads, I'm hoping for the > pragmatic approach and allow all possible features over SMBus. It's not > like we have the option of using i2c on a host which only supports > SMBus. My experience is that both hwmon and phy access works pretty > well with SMBus byte accesses. The pragmatic approach is to avoid using things that are unsafe (and thus unreliable.) As I said, disabling hwmon makes sense because you don't want to read the 16-bit values non-atomically and end up with scrambled readings that then trigger alarms because they've exceeded the thresholds. Merely proving that "oh look it works because I can read stuff" doesn't address my point. SMBus being used is as bad as all those crappy SFP modules out there that don't conform to the SFP MSA but claim they do. It's just yet another hardware designer cocking the design up in a way that makes SFPs unreliable. Unfortunately, there's not much that we can do to influence that, but not publishing stuff that's ultimately unreliable helps to make the issue known. So, not only do I think that hwmon should be disabled if using SMBus, but I also think that the kernel should print a warning that SMBus is being used and therefore e.g. copper modules will be unreliable. We don't know how the various firmwares in various microprocessors that convert I2C to MDIO will behave when faced with SMBus transfers. All in all, I'm not happy with this, and I do wish hardware designers would get a clue. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> So, not only do I think that hwmon should be disabled if using SMBus, > but I also think that the kernel should print a warning that SMBus is > being used and therefore e.g. copper modules will be unreliable. We > don't know how the various firmwares in various microprocessors that > convert I2C to MDIO will behave when faced with SMBus transfers. I agree, hwmon should be disabled, and that the kernel should printing a warning that the hardware is broken and that networking is not guaranteed to be reliable. Andrew
Andrew Lunn <andrew@lunn.ch> writes: >> So, not only do I think that hwmon should be disabled if using SMBus, >> but I also think that the kernel should print a warning that SMBus is >> being used and therefore e.g. copper modules will be unreliable. We >> don't know how the various firmwares in various microprocessors that >> convert I2C to MDIO will behave when faced with SMBus transfers. > > I agree, hwmon should be disabled, and that the kernel should printing > a warning that the hardware is broken and that networking is not > guaranteed to be reliable. What do you think will be the effect of such a warning? Who is the target audience? You can obviously add it, and I don't really care. But I believe the result will be an endless stream of end users worrying about this scary warning and wanting to know what they can do about it. What will be your answer? No SoC/phy designer will ever see the warning. They finished designing these chips decades ago. The switch designers might see it. But probably not. They're not worried about running mainline Linux at all. They have they're vendor SDK. Linux based of course, but it's never going to have that warning no matter what you do. Firmware developers? Same as switch designers really. The hardware exists. It's not perfect. We agree so far. But I do not understand your way of dealing with that. If your intention is detecting this hardware problem in bug reports etc, then it makes more sense to me. But I believe a more subtle method will be more effeicient than a standalone and scary warning. Like embedding "smbus" or similar in existing debug/warning/error messages, e.g by making it part of the mdio bus name. Bjørn
> What do you think will be the effect of such a warning? Who is the > target audience? It will act as a disclaimer. The kernel is doing its best with broken hardware, but don't blame the kernel when it does not work correctly.... > You can obviously add it, and I don't really care. But I believe the > result will be an endless stream of end users worrying about this scary > warning and wanting to know what they can do about it. What will be > your answer? I agree that the wording needs to be though about. Maybe something like: This hardware is broken by design, and there is nothing the kernel, or the community can do about it. The kernel will try its best, but some standard SFP features are disabled, and the features which are implemented may not work correctly because of the design errors. Use with caution, and don't blame the kernel when it all goes horribly wrong. Andrew
On Mon, Feb 24, 2025 at 02:31:42PM +0100, Andrew Lunn wrote: > > What do you think will be the effect of such a warning? Who is the > > target audience? > > It will act as a disclaimer. The kernel is doing its best with broken > hardware, but don't blame the kernel when it does not work > correctly.... Indeed. > > You can obviously add it, and I don't really care. But I believe the > > result will be an endless stream of end users worrying about this scary > > warning and wanting to know what they can do about it. What will be > > your answer? > > I agree that the wording needs to be though about. Maybe something > like: > > This hardware is broken by design, and there is nothing the kernel, or > the community can do about it. The kernel will try its best, but some > standard SFP features are disabled, and the features which are > implemented may not work correctly because of the design errors. Use > with caution, and don't blame the kernel when it all goes horribly > wrong. I was hoping for something shorter, but I think it needs to be expansive so that users can fully understand. Another idea based on your suggestion above: "Please note: This hardware is broken by design. There is nothing that the kernel or community can do to fix it. The kernel will try best efforts, but some features are disabled, other features may be unreliable or sporadically fail. Use with caution. Please verify any problems on hardware that supports multi-byte I2C transactions." -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Mon, 24 Feb 2025 13:48:19 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Mon, Feb 24, 2025 at 02:31:42PM +0100, Andrew Lunn wrote: > > > What do you think will be the effect of such a warning? Who is the > > > target audience? > > > > It will act as a disclaimer. The kernel is doing its best with broken > > hardware, but don't blame the kernel when it does not work > > correctly.... > > Indeed. > > > > You can obviously add it, and I don't really care. But I believe the > > > result will be an endless stream of end users worrying about this scary > > > warning and wanting to know what they can do about it. What will be > > > your answer? > > > > I agree that the wording needs to be though about. Maybe something > > like: > > > > This hardware is broken by design, and there is nothing the kernel, or > > the community can do about it. The kernel will try its best, but some > > standard SFP features are disabled, and the features which are > > implemented may not work correctly because of the design errors. Use > > with caution, and don't blame the kernel when it all goes horribly > > wrong. > > I was hoping for something shorter, but I think it needs to be expansive > so that users can fully understand. Another idea based on your > suggestion above: > > "Please note: > This hardware is broken by design. There is nothing that the kernel or > community can do to fix it. The kernel will try best efforts, but some > features are disabled, other features may be unreliable or sporadically > fail. Use with caution. Please verify any problems on hardware that > supports multi-byte I2C transactions." > I think what's missing in this message is some indication about what is actually wrong with the hardware, so : "Please note: This SFP cage is accessed via an SMBus only capable of single byte transactions. Some features are disabled, other may be unreliable or sporadically fail. Use with caution. There is nothing that the kernel or community can do to fix it, the kernel will try best efforts. Please verify any problems on hardware that supports multi-byte I2C transactions." Maxime
On Mon, 24 Feb 2025 17:24:07 +0100 Maxime Chevallier <maxime.chevallier@bootlin.com> wrote: > On Mon, 24 Feb 2025 13:48:19 +0000 > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > > > On Mon, Feb 24, 2025 at 02:31:42PM +0100, Andrew Lunn wrote: > > > > What do you think will be the effect of such a warning? Who is the > > > > target audience? > > > > > > It will act as a disclaimer. The kernel is doing its best with broken > > > hardware, but don't blame the kernel when it does not work > > > correctly.... > > > > Indeed. > > > > > > You can obviously add it, and I don't really care. But I believe the > > > > result will be an endless stream of end users worrying about this scary > > > > warning and wanting to know what they can do about it. What will be > > > > your answer? > > > > > > I agree that the wording needs to be though about. Maybe something > > > like: > > > > > > This hardware is broken by design, and there is nothing the kernel, or > > > the community can do about it. The kernel will try its best, but some > > > standard SFP features are disabled, and the features which are > > > implemented may not work correctly because of the design errors. Use > > > with caution, and don't blame the kernel when it all goes horribly > > > wrong. > > > > I was hoping for something shorter, but I think it needs to be expansive > > so that users can fully understand. Another idea based on your > > suggestion above: > > > > "Please note: > > This hardware is broken by design. There is nothing that the kernel or > > community can do to fix it. The kernel will try best efforts, but some > > features are disabled, other features may be unreliable or sporadically > > fail. Use with caution. Please verify any problems on hardware that > > supports multi-byte I2C transactions." > > > > I think what's missing in this message is some indication about what is > actually wrong with the hardware, so : I realise that I have formulated the sentence above a bit strongly, of course this is a suggestion :) > "Please note: > This SFP cage is accessed via an SMBus only capable of single byte > transactions. Some features are disabled, other may be unreliable or > sporadically fail. Use with caution. There is nothing that the kernel > or community can do to fix it, the kernel will try best efforts. Please > verify any problems on hardware that supports multi-byte I2C transactions." Maxime
On Mon, Feb 24, 2025 at 01:48:19PM +0000, Russell King (Oracle) wrote: ... > "Please note: > This hardware is broken by design. There is nothing that the kernel or Doesn't "broken by design" mean "broken intentionally" ? Marek
On Mon, Feb 24, 2025 at 03:32:43PM +0100, Marek Behún wrote: > On Mon, Feb 24, 2025 at 01:48:19PM +0000, Russell King (Oracle) wrote: > > ... > > > "Please note: > > This hardware is broken by design. There is nothing that the kernel or > > Doesn't "broken by design" mean "broken intentionally" ? The point of this part of the discussion is to find a form of words that folk are happy with, so please feel free to suggest an alternative. To put it bluntly, it does come down to "broken by the hardware engineer not having full knowledge of the requirements of SFP modules as documented in the SNIA documentation and selecting hardware that violates some of the requirements." -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> "Please note: > This hardware is broken by design. There is nothing that the kernel or > community can do to fix it. The kernel will try best efforts, but some > features are disabled, other features may be unreliable or sporadically > fail. Use with caution. Please verify any problems on hardware that > supports multi-byte I2C transactions." I'm good with that. Andrew
On Mon, Feb 24, 2025 at 08:13:22AM +0100, Bjørn Mork wrote: > Andrew Lunn <andrew@lunn.ch> writes: > > >> So, not only do I think that hwmon should be disabled if using SMBus, > >> but I also think that the kernel should print a warning that SMBus is > >> being used and therefore e.g. copper modules will be unreliable. We > >> don't know how the various firmwares in various microprocessors that > >> convert I2C to MDIO will behave when faced with SMBus transfers. > > > > I agree, hwmon should be disabled, and that the kernel should printing > > a warning that the hardware is broken and that networking is not > > guaranteed to be reliable. > > What do you think will be the effect of such a warning? Who is the > target audience? > > You can obviously add it, and I don't really care. But I believe the > result will be an endless stream of end users worrying about this scary > warning and wanting to know what they can do about it. What will be > your answer? ... which is good, because it raises the visibility of crap hardware and will make people think twice about whether to purchase it, thus penalising (a little) the sales of badly designed hardware. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Mon, 24 Feb 2025 08:47:18 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Mon, Feb 24, 2025 at 08:13:22AM +0100, Bjørn Mork wrote: > > Andrew Lunn <andrew@lunn.ch> writes: > > > > >> So, not only do I think that hwmon should be disabled if using SMBus, > > >> but I also think that the kernel should print a warning that SMBus is > > >> being used and therefore e.g. copper modules will be unreliable. We > > >> don't know how the various firmwares in various microprocessors that > > >> convert I2C to MDIO will behave when faced with SMBus transfers. > > > > > > I agree, hwmon should be disabled, and that the kernel should printing > > > a warning that the hardware is broken and that networking is not > > > guaranteed to be reliable. > > > > What do you think will be the effect of such a warning? Who is the > > target audience? > > > > You can obviously add it, and I don't really care. But I believe the > > result will be an endless stream of end users worrying about this scary > > warning and wanting to know what they can do about it. What will be > > your answer? > > ... which is good, because it raises the visibility of crap hardware > and will make people think twice about whether to purchase it, thus > penalising (a little) the sales of badly designed hardware. It's also going to allow users to understand a bit more why all features aren't available. Plugging modules in a true i2c connected cage will report all the features, while plugging on an smbus cage will cause the use of a degraded mode. Not only for hwmon, but as Russell specifies, I doubt all SFP PHY will behave exactly the same... I'll respin without hwmon and with the warning then. Maxime
On Sun, Feb 23, 2025 at 06:28:45PM +0100, Maxime Chevallier wrote: > Hi everyone, > > Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to > access SFP modules downstream. These controllers are actually SMBus controllers > that can only perform single-byte accesses for read and write. This goes against SFF-8472, and likely breaks atomic access to 16-bit PHY registers. For the former, I quote from SFF-8472: "To guarantee coherency of the diagnostic monitoring data, the host is required to retrieve any multi-byte fields from the diagnostic monitoring data structure (e.g. Rx Power MSB - byte 104 in A2h, Rx Power LSB - byte 105 in A2h) by the use of a single two-byte read sequence across the 2-wire interface." So, if using a SMBus controller, I think we should at the very least disable exporting the hwmon parameters as these become non-atomic reads. Whether PHY access works correctly or not is probably module specific. E.g. reading the MII_BMSR register may not return latched link status because the reads of the high and low bytes may be interpreted as two seperate distinct accesses. In an ideal world, I'd prefer to say no to hardware designs like this, but unfortunately, hardware designers don't know these details of the protocol, and all they see is "two wire, oh SMBus will do". -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <linux@armlinux.org.uk> writes:
> On Sun, Feb 23, 2025 at 06:28:45PM +0100, Maxime Chevallier wrote:
>> Hi everyone,
>>
>> Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to
>> access SFP modules downstream. These controllers are actually SMBus controllers
>> that can only perform single-byte accesses for read and write.
>
> This goes against SFF-8472, and likely breaks atomic access to 16-bit
> PHY registers.
>
> For the former, I quote from SFF-8472:
>
> "To guarantee coherency of the diagnostic monitoring data, the host is
> required to retrieve any multi-byte fields from the diagnostic
> monitoring data structure (e.g. Rx Power MSB - byte 104 in A2h, Rx
> Power LSB - byte 105 in A2h) by the use of a single two-byte read
> sequence across the 2-wire interface."
>
> So, if using a SMBus controller, I think we should at the very least
> disable exporting the hwmon parameters as these become non-atomic
> reads.
Would SMBus word reads be an alternative for hwmon, if the SMBus
controller support those? Should qualify as "a single two-byte read
sequence across the 2-wire interface."
> Whether PHY access works correctly or not is probably module specific.
> E.g. reading the MII_BMSR register may not return latched link status
> because the reads of the high and low bytes may be interpreted as two
> seperate distinct accesses.
Bear with me. Trying to learn here. AFAIU, we only have a defacto
specification of the clause 22 phy interface over i2c, based on the
88E1111 implementation. As Maxime pointed out, this explicitly allows
two sequential distinct byte transactions to read or write the 16bit
registers. See figures 27 and 30 in
https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-alaska-88e1111-datasheet.pdf
Looks like the latch timing restrictions are missing, but I still do not
think that's enough reason to disallow access to phys over SMBus. If
this is all the interface specification we have?
I have been digging around for the RollBall protocol spec, but Google
isn't very helpful. This list and the mdio-i2c.c implementation is all
that comes up. It does use 4 and 6 byte transactions which will be
difficult to emulate on SMBus. But the
/* By experiment it takes up to 70 ms to access a register for these
* SFPs. Sleep 20ms between iterations and try 10 times.
*/
comment in i2c_rollball_mii_poll() indicates that it isn't very timing
sensitive at all. The RollBall SFP+ I have ("FS", "SFP-10G-T") is faster
than the comment indicates, but still leaves plenty of time for the
single byte SMBus transactions to complete.
Haven't found any formal specification of i2c clause 45 access either.
But some SFP+ vendors have been nice enough to document their protocol
in datasheets. Examples:
https://www.repotec.com/download/managed-ethernet/ds-ml/01-MOD-M10GTP-DS-verB.pdf
https://www.apacoe.com.tw/files/ASFPT-TNBT-X-NA%20V1.4.pdf
They all seem to agree that 2/4/6 byte accesses are required, and they
offer no single byte alternative even if the presence of a "smart"
bridge should allow intelligent latching. So this might be
"impossible" (aka "hard") to do over SMBus. I have no such SFP+ so I
cannot even try.
> In an ideal world, I'd prefer to say no to hardware designs like this,
> but unfortunately, hardware designers don't know these details of the
> protocol, and all they see is "two wire, oh SMBus will do".
I believe you are reading more into the spec than what's actually there.
SFF-8419 defines the interface as
"The SFP+ management interface is a two-wire interface, similar to
I2C."
There is no i2c requirement. This does not rule out SMBus. Maybe I am
reading too much into it as well, but in my view "similar to I2C" sounds
like they wanted to include SMBus.
Both the adhoc phy additions and the diagnostic parts of SFF-8472
silently ignores this. I do not think the blame for any incompatibilty
is solely on the host side here.
Bjørn
On Tue, Feb 25, 2025 at 01:38:30PM +0100, Bjørn Mork wrote:
> "Russell King (Oracle)" <linux@armlinux.org.uk> writes:
> > On Sun, Feb 23, 2025 at 06:28:45PM +0100, Maxime Chevallier wrote:
> >> Hi everyone,
> >>
> >> Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to
> >> access SFP modules downstream. These controllers are actually SMBus controllers
> >> that can only perform single-byte accesses for read and write.
> >
> > This goes against SFF-8472, and likely breaks atomic access to 16-bit
> > PHY registers.
> >
> > For the former, I quote from SFF-8472:
> >
> > "To guarantee coherency of the diagnostic monitoring data, the host is
> > required to retrieve any multi-byte fields from the diagnostic
> > monitoring data structure (e.g. Rx Power MSB - byte 104 in A2h, Rx
> > Power LSB - byte 105 in A2h) by the use of a single two-byte read
> > sequence across the 2-wire interface."
> >
> > So, if using a SMBus controller, I think we should at the very least
> > disable exporting the hwmon parameters as these become non-atomic
> > reads.
>
> Would SMBus word reads be an alternative for hwmon, if the SMBus
> controller support those? Should qualify as "a single two-byte read
> sequence across the 2-wire interface."
Looking at the SNIA documents, this should work, so if a SMBus supports
two-byte reads, then we should be able to support hwmon with such a
controller. However, if only single-byte reads are supported, then I
think we should disable hwmon as we'd be going against the statement
I provided above from the docs.
> > Whether PHY access works correctly or not is probably module specific.
> > E.g. reading the MII_BMSR register may not return latched link status
> > because the reads of the high and low bytes may be interpreted as two
> > seperate distinct accesses.
>
> Bear with me. Trying to learn here. AFAIU, we only have a defacto
> specification of the clause 22 phy interface over i2c, based on the
> 88E1111 implementation. As Maxime pointed out, this explicitly allows
> two sequential distinct byte transactions to read or write the 16bit
> registers. See figures 27 and 30 in
> https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-alaska-88e1111-datasheet.pdf
However, note that it's:
START ADDR(W) REG RE-START ADDR(R) HIGH STOP
START ADDR(W) REG RE-START ADDR(R) LOW STOP
and not:
START ADDR(W) REG STOP START ADDR(R) HIGH STOP
...
I forget whether SMBus can do re-starts.
> Looks like the latch timing restrictions are missing, but I still do not
> think that's enough reason to disallow access to phys over SMBus. If
> this is all the interface specification we have?
I'm not sure what you're referring to here with "latch timing
restrictions".
Reading more of the 88E1111, if one only transfers an odd number of
bytes for a register, it looks like that could cause problems. It
doesn't appear to specify what happens if it receives a write e.g.
for upper byte of register 0, and then receives a write for
register 4 (as a hypothetical example - caused where the lower byte of
register 0 transfer failed.)
We do need to think about these corner cases!
> I have been digging around for the RollBall protocol spec, but Google
> isn't very helpful. This list and the mdio-i2c.c implementation is all
> that comes up. It does use 4 and 6 byte transactions which will be
> difficult to emulate on SMBus. But the
>
> /* By experiment it takes up to 70 ms to access a register for these
> * SFPs. Sleep 20ms between iterations and try 10 times.
> */
>
> comment in i2c_rollball_mii_poll() indicates that it isn't very timing
> sensitive at all. The RollBall SFP+ I have ("FS", "SFP-10G-T") is faster
> than the comment indicates, but still leaves plenty of time for the
> single byte SMBus transactions to complete.
The "RollBall" protocol isn't official afaics. It got called that
because of the first module that Marek happened to have that implemented
it. Having taken several of these modules apart, it's implemented by a
microcontroller, which may be an Arm Cortex based controller, or might
be 8051 based. Whether the firmware implementation at the high-level
code is the same or not is anyone's guess. We just don't know. It
could be entirely different implementations - and that's the problem.
We already know that the timings for this protocol vary depending on
the module (possibly because of differing implementations) which brings
questions up about the behaviour in obscure cases like single-byte
reads.
> Haven't found any formal specification of i2c clause 45 access either.
> But some SFP+ vendors have been nice enough to document their protocol
> in datasheets. Examples:
>
> https://www.repotec.com/download/managed-ethernet/ds-ml/01-MOD-M10GTP-DS-verB.pdf
> https://www.apacoe.com.tw/files/ASFPT-TNBT-X-NA%20V1.4.pdf
>
> They all seem to agree that 2/4/6 byte accesses are required, and they
> offer no single byte alternative even if the presence of a "smart"
> bridge should allow intelligent latching. So this might be
> "impossible" (aka "hard") to do over SMBus. I have no such SFP+ so I
> cannot even try.
Indeed. So maybe we need a stronger kernel message to basically blame
the hardware designer and refuse to operate with modules that _might_
use the Rollball protocol. (Note, it's "might" because if we can't
access the PHY, we don't know for certain that this module is a
copper module.)
> > In an ideal world, I'd prefer to say no to hardware designs like this,
> > but unfortunately, hardware designers don't know these details of the
> > protocol, and all they see is "two wire, oh SMBus will do".
>
> I believe you are reading more into the spec than what's actually there.
So I'm making up the quote above from SFF-8472. Okay, if that's where
this discussion is going, I'm done here.
NAK to this patch set.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <linux@armlinux.org.uk> writes: >> I believe you are reading more into the spec than what's actually there. > > So I'm making up the quote above from SFF-8472. Okay, if that's where > this discussion is going, I'm done here. No, not at all. That was not what I meant. Please accept my apologies. This came out wrong. You are absolutely correct about reading the 16bit diagnostic registers you quoted. I would never doubt that. I have an extreme respect for you and your knowledge of these standards and the practical hardware implications. It was the conclusion that this fact prevents SMBus hosts I wanted to question. I still don't see that. Some SMBus hosts might be able do 2 byte reads. And if they can't, then I believe they can safely ignore these registers without being out of spec. Like the proposed solution. I'll shut up now, to avoid confusing the discussion of Maxime's patches further. Bjørn
On Tue, Feb 25, 2025 at 07:07:41PM +0100, Bjørn Mork wrote: > "Russell King (Oracle)" <linux@armlinux.org.uk> writes: > > >> I believe you are reading more into the spec than what's actually there. > > > > So I'm making up the quote above from SFF-8472. Okay, if that's where > > this discussion is going, I'm done here. > > No, not at all. That was not what I meant. Please accept my apologies. > This came out wrong. You are absolutely correct about reading the 16bit > diagnostic registers you quoted. I would never doubt that. I have an > extreme respect for you and your knowledge of these standards and the > practical hardware implications. > > It was the conclusion that this fact prevents SMBus hosts I wanted to > question. I still don't see that. Some SMBus hosts might be able do 2 > byte reads. And if they can't, then I believe they can safely ignore > these registers without being out of spec. Like the proposed solution. It doesn't prevent SMBus hosts, but it does prevent SMBus hosts from being able to be used *reliably* with the diagnostics "EEPROM" to read the values in a coherent manner. It also prevents being able to identify the Nokia 3FE46541AA module, because the module's I2C locks the bus when a single-byte read of offset 0x51 in the "identity" EEPROM at 0xA0/0x50. We do already have single-byte mode in the SFP driver, which is necessary to work around the broken I2C interface on RTL8672 and RTL9601C which emulate that EEPROM. There's a lot of "brokenness" out there, and what I've learnt from dealing with SFPs over the last 10+ years is to be cautious about *everything*. One thing that can be guaranteed is that a module will be broken in unexpected ways, and using a different behaviour for working modules will turn out to break. The Nokia 3FE46541AA is a brilliant example of a module that emulates all accesses and fails with single-byte reads, thus making it incompatible with a SMBus that can only read single bytes. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi,
On Tue, 25 Feb 2025 13:38:30 +0100
Bjørn Mork <bjorn@mork.no> wrote:
> "Russell King (Oracle)" <linux@armlinux.org.uk> writes:
> > On Sun, Feb 23, 2025 at 06:28:45PM +0100, Maxime Chevallier wrote:
> >> Hi everyone,
> >>
> >> Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to
> >> access SFP modules downstream. These controllers are actually SMBus controllers
> >> that can only perform single-byte accesses for read and write.
> >
> > This goes against SFF-8472, and likely breaks atomic access to 16-bit
> > PHY registers.
> >
> > For the former, I quote from SFF-8472:
> >
> > "To guarantee coherency of the diagnostic monitoring data, the host is
> > required to retrieve any multi-byte fields from the diagnostic
> > monitoring data structure (e.g. Rx Power MSB - byte 104 in A2h, Rx
> > Power LSB - byte 105 in A2h) by the use of a single two-byte read
> > sequence across the 2-wire interface."
> >
> > So, if using a SMBus controller, I think we should at the very least
> > disable exporting the hwmon parameters as these become non-atomic
> > reads.
>
> Would SMBus word reads be an alternative for hwmon, if the SMBus
> controller support those? Should qualify as "a single two-byte read
> sequence across the 2-wire interface."
There are different flavors when it comes to what an SMBus controller
can do. In the case of what this patchset supports, its really about
SMBus controllers that can only perform single-byte operations, which
will cause issues here.
What I have is a controller that only supports I2C_FUNC_SMBUS_BYTE, in
that the controller will issue a STOP after reading/writing one byte.
But if you have a controller that supports, say,
I2C_FUNC_SMBUS_WORD_DATA (i.e. 16 bits words xfers), that's already a
different story, as the diags situation Russell mentions will fit in a
word. That will also make MDIO accesses to embedded PHYs easier, at
least for C22.
> > Whether PHY access works correctly or not is probably module specific.
> > E.g. reading the MII_BMSR register may not return latched link status
> > because the reads of the high and low bytes may be interpreted as two
> > seperate distinct accesses.
>
> Bear with me. Trying to learn here. AFAIU, we only have a defacto
> specification of the clause 22 phy interface over i2c, based on the
> 88E1111 implementation. As Maxime pointed out, this explicitly allows
> two sequential distinct byte transactions to read or write the 16bit
> registers. See figures 27 and 30 in
> https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-alaska-88e1111-datasheet.pdf
>
> Looks like the latch timing restrictions are missing, but I still do not
> think that's enough reason to disallow access to phys over SMBus. If
> this is all the interface specification we have?
>
> I have been digging around for the RollBall protocol spec, but Google
> isn't very helpful. This list and the mdio-i2c.c implementation is all
> that comes up. It does use 4 and 6 byte transactions which will be
> difficult to emulate on SMBus. But the
>
> /* By experiment it takes up to 70 ms to access a register for these
> * SFPs. Sleep 20ms between iterations and try 10 times.
> */
>
> comment in i2c_rollball_mii_poll() indicates that it isn't very timing
> sensitive at all. The RollBall SFP+ I have ("FS", "SFP-10G-T") is faster
> than the comment indicates, but still leaves plenty of time for the
> single byte SMBus transactions to complete.
>
> Haven't found any formal specification of i2c clause 45 access either.
> But some SFP+ vendors have been nice enough to document their protocol
> in datasheets. Examples:
>
> https://www.repotec.com/download/managed-ethernet/ds-ml/01-MOD-M10GTP-DS-verB.pdf
> https://www.apacoe.com.tw/files/ASFPT-TNBT-X-NA%20V1.4.pdf
>
> They all seem to agree that 2/4/6 byte accesses are required, and they
> offer no single byte alternative even if the presence of a "smart"
> bridge should allow intelligent latching. So this might be
> "impossible" (aka "hard") to do over SMBus. I have no such SFP+ so I
> cannot even try.
>
> > In an ideal world, I'd prefer to say no to hardware designs like this,
> > but unfortunately, hardware designers don't know these details of the
> > protocol, and all they see is "two wire, oh SMBus will do".
>
> I believe you are reading more into the spec than what's actually there.
>
> SFF-8419 defines the interface as
>
> "The SFP+ management interface is a two-wire interface, similar to
> I2C."
>
> There is no i2c requirement. This does not rule out SMBus. Maybe I am
> reading too much into it as well, but in my view "similar to I2C" sounds
> like they wanted to include SMBus.
>
> Both the adhoc phy additions and the diagnostic parts of SFF-8472
> silently ignores this. I do not think the blame for any incompatibilty
> is solely on the host side here.
At least for this series, SMBus by itself isn't the main issue, the
main problem is single-byte SMBus. As soon as you can do 16 bits xfers,
that rules-out a whole class of potential problems (which doesn't mean
there will be no issue :) ), but I haven't really digged into how C45
and rollball will behave in that case.
Note that the series actually doesn't support I2C_FUNC_SMBUS_WORD_DATA,
but doesn't prevent it either :)
The driver for the SMBus controller in the rtl930x SoC you mention seem
to support I2C_FUNC_SMBUS_WORD_DATA and even longer xfers, in which
case you could add SFP support for smbus-only controllers in a much
more reliable way. It could be conceivable to remove the hwmon-disabled
restriction for I2C_FUNC_SMBUS_WORD_DATA.
Maxime
> > Would SMBus word reads be an alternative for hwmon, if the SMBus > > controller support those? Should qualify as "a single two-byte read > > sequence across the 2-wire interface." > > There are different flavors when it comes to what an SMBus controller > can do. In the case of what this patchset supports, its really about > SMBus controllers that can only perform single-byte operations, which > will cause issues here. > > What I have is a controller that only supports I2C_FUNC_SMBUS_BYTE, in > that the controller will issue a STOP after reading/writing one byte. > > But if you have a controller that supports, say, > I2C_FUNC_SMBUS_WORD_DATA (i.e. 16 bits words xfers), that's already a > different story, as the diags situation Russell mentions will fit in a > word. That will also make MDIO accesses to embedded PHYs easier, at > least for C22. Agreed. > > > Whether PHY access works correctly or not is probably module specific. > > > E.g. reading the MII_BMSR register may not return latched link status > > > because the reads of the high and low bytes may be interpreted as two > > > seperate distinct accesses. > > > > Bear with me. Trying to learn here. AFAIU, we only have a defacto > > specification of the clause 22 phy interface over i2c, based on the > > 88E1111 implementation. As Maxime pointed out, this explicitly allows > > two sequential distinct byte transactions to read or write the 16bit > > registers. See figures 27 and 30 in > > https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-alaska-88e1111-datasheet.pdf > > > > Looks like the latch timing restrictions are missing, but I still do not > > think that's enough reason to disallow access to phys over SMBus. If > > this is all the interface specification we have? We are not suggesting it is disallowed. We are simply saying add a warning that it is possibly broken, bad things can happen, and there is nothing we can do about it, so don't report issues when it does break. It might work enough that users are happy to take the risk, and need to unplug/plug the cable every so often to get the link back when the link peer changes etc. Andrew
Hi Russell On Sun, 23 Feb 2025 17:40:37 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Sun, Feb 23, 2025 at 06:28:45PM +0100, Maxime Chevallier wrote: > > Hi everyone, > > > > Some PHYs such as the VSC8552 have embedded "Two-wire Interfaces" designed to > > access SFP modules downstream. These controllers are actually SMBus controllers > > that can only perform single-byte accesses for read and write. > > This goes against SFF-8472, and likely breaks atomic access to 16-bit > PHY registers. > > For the former, I quote from SFF-8472: > > "To guarantee coherency of the diagnostic monitoring data, the host is > required to retrieve any multi-byte fields from the diagnostic > monitoring data structure (e.g. Rx Power MSB - byte 104 in A2h, Rx > Power LSB - byte 105 in A2h) by the use of a single two-byte read > sequence across the 2-wire interface." > > So, if using a SMBus controller, I think we should at the very least > disable exporting the hwmon parameters as these become non-atomic > reads. That makes sense to me, it's best-effort at that point :) > Whether PHY access works correctly or not is probably module specific. > E.g. reading the MII_BMSR register may not return latched link status > because the reads of the high and low bytes may be interpreted as two > seperate distinct accesses. > > In an ideal world, I'd prefer to say no to hardware designs like this, > but unfortunately, hardware designers don't know these details of the > protocol, and all they see is "two wire, oh SMBus will do". On that particular PHY I'm mentionning, the feature really is advertised to HW designers as "you connect that TWI to your SFP cage and you're good". I was very surprised as well when digging deeper and figuring that not only it's SMBus, and worse, 1-byte accesses only... However there are already some HW out there with this feature in use. If this is OK for you to accept that as a "best effort, degraded mode as there's no hwmon", that's already great :) Thanks for being so quick ! Maxime
© 2016 - 2025 Red Hat, Inc.