Re: [PATCH net-next v11 2/2] net: sfp: extend SMBus support

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

Hi Jonas,

On 6/14/26 15:34, Jonas Jelonek wrote:
> Commit 7662abf4db94 ("net: phy: sfp: Add support for SMBus module access")
> added SMBus access for SFP modules, but limited it to single-byte
> transfers. As a side effect, hwmon is disabled (16-bit reads cannot be
> guaranteed atomic) and a warning is printed.
> 
> Many SMBus-only I2C controllers in the wild support more than just
> byte access, and SFP cages are often wired to such controllers
> rather than to a full-featured I2C controller -- e.g. the SMBus
> controllers in the Realtek longan and mango SoCs, which advertise
> word access and I2C block reads. Today, they cannot drive an SFP at
> all without falling back to the byte-only path.
> 
> Extend sfp_smbus_read()/sfp_smbus_write() so that, in addition to
> the existing byte access, they also use SMBus word access and SMBus
> I2C block access whenever the adapter advertises them. Both
> directions are handled in a single read and a single write helper
> that pick the largest supported transfer per chunk and fall back as
> needed.
> 
> I2C-block is preferred unconditionally when available: the protocol
> carries any length 1..32, so it can serve every chunk -- including
> the 1- and 2-byte tails -- without help from word or byte access.
> Note that this requires I2C_FUNC_SMBUS_I2C_BLOCK, which reads a
> caller-specified number of bytes. This deviates from the official
> SMBus Block Read (length is supplied by the slave) but is widely
> supported by Linux I2C controllers/drivers.
> 
> Capability matrix this implementation supports:
> 
>   - BYTE only:                  works (unchanged behaviour); 1-byte
>                                 xfers, hwmon disabled.
>   - BYTE + WORD:                word for >=2-byte chunks, byte for
>                                 trailing odd byte.
>   - I2C_BLOCK present (with or
>     without BYTE/WORD):         block as the universal transport for
>                                 every chunk.
>   - WORD only (no BYTE/BLOCK):  accepted with WARN_ONCE. Even-length
>                                 transfers work; odd-length transfers
>                                 (e.g. the 3-byte cotsworks fixup
>                                 write) hit the BYTE branch which the
>                                 adapter does not implement, so the
>                                 xfer returns an error and the
>                                 operation is aborted. No mainline
>                                 I2C driver was found to advertise
>                                 WORD without BYTE; the warning lets
>                                 us learn about it if it ever shows
>                                 up.
> 
> Adapters with asymmetric R/W capabilities (e.g. only READ_I2C_BLOCK
> but not WRITE_I2C_BLOCK) remain functionally correct -- the
> per-iteration fallback uses the direction-specific bits -- but the
> shared i2c_max_block_size is sized by the all-bits-set check, so a
> transfer in the better-supported direction is not upgraded. None of
> the mainline I2C bus drivers surveyed during review advertise such
> asymmetry; promoting i2c_max_block_size to per-direction sizes can
> be revisited if needed.
> 
> Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>

Thanks for this work,

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime