Re: [PATCH net-next v4] net: sfp: add SMBus I2C block support

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Jan 09, 2026 at 12:13:06PM +0000, Russell King (Oracle) wrote:
> On Fri, Jan 09, 2026 at 10:13:21AM +0000, Jonas Jelonek wrote:
> > Commit 7662abf4db94 ("net: phy: sfp: Add support for SMBus module access")
> > added support for SMBus-only controllers for module access. However,
> > this is restricted to single-byte accesses and has the implication that
> > hwmon is disabled (due to missing atomicity of 16-bit accesses) and
> > warnings are printed.
> > 
> > There are probably a lot of SMBus-only I2C controllers out in the wild
> > which support block reads. Right now, they don't work with SFP modules.
> > This applies - amongst others - to I2C/SMBus-only controllers in Realtek
> > longan and mango SoCs.
> > 
> > Downstream in OpenWrt, a patch similar to the abovementioned patch is
> > used for current LTS kernel 6.12. However, this uses byte-access for all
> > kinds of access and thus disregards the atomicity for wider access.
> > 
> > Introduce read/write SMBus I2C block operations to support SMBus-only
> > controllers with appropriate support for block read/write. Those
> > operations are used for all accesses if supported, otherwise the
> > single-byte operations will be used. With block reads, atomicity for
> > 16-bit reads as required by hwmon is preserved and thus, hwmon can be
> > used.
> > 
> > The implementation requires the I2C_FUNC_SMBUS_I2C_BLOCK to be
> > supported as it relies on reading a pre-defined amount of bytes.
> > This isn't intended by the official SMBus Block Read but supported by
> > several I2C controllers/drivers.
> > 
> > Support for word access is not implemented due to issues regarding
> > endianness.
> 
> I'm wondering whether we should go further with this - we implement
> byte mode SMBus support, but there is also word mode, too, which
> would solve the HWMON issues. It looks like more SMBus devices support
> word mode than I2C block mode.
> 
> So, if we're seeing more SMBus adapters being used with SFPs, maybe
> we should be thinking about a more adaptive approach to SMBus, where
> we try to do the best with the features that the SMBus adapter
> provides us.
> 
> Maybe something like:
> 
> static int sfp_smbus_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
>                            size_t len)
> {
> 	size_t this_len, transferred, total;
> 	union i2c_smbus_data smbus_data;
> 	u8 bus_addr = a2 ? 0x51 : 0x50;
> 	u32 functionality;
> 	int ret;
> 
> 	functioality = i2c_get_functionality(sfp->i2c);
> 	total = len;
> 
> 	while (len) {
> 		if (len > sfp->i2c_max_block_size)
> 			this_len = sfp->i2c_max_block_size;
> 		else
> 			this_len = len;
> 
> 		if (this_len > 2 &&
> 		    functionality & I2C_FUNC_SMBUS_READ_I2C_BLOCK) {
> 			.. use smbus i2c block mode ..
> 			transferred = this_len;
> 		} else if (this_len >= 2 &&
> 		           functionality & I2C_FUNC_SMBUS_READ_WORD_DATA) {
> 			.. use smbus word mode ..
> 			transferred = 2;
> 		} else {
> 			.. use smbus byte mode ..
> 			transferred = 1;
> 		}
> 
> 		buf += transferred;
> 		len -= transferred;
> 	}
> 
> 	return ret < 0 : ret : total - len;
> }
> 
> sfp_hwmon_probe() will do the right thing based upon i2c_block_size, so
> where only byte mode is supported, we don't get hwmon support.

I should also note that, when checking for the appropriate
functionality in sfp_i2c_configure(), we need to be careful that
we can read single bytes.

In other words, if an adapter reports that it supports smbus word data
access, we need it to also support smbus byte data access or smbus i2c
block access.

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