Re: [PATCH net-next 0/2] net: phy: sfp: Add single-byte SMBus SFP access

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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