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.

Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>

---
changes since v4:
- fix formal issues
v3: https://lore.kernel.org/netdev/20260105161242.578487-1-jelonek.jonas@gmail.com/

changes since v2:
- fix previous attempt of v2 to fix return value
v2: https://lore.kernel.org/netdev/20260105154653.575397-1-jelonek.jonas@gmail.com/

changes since v1:
- return number of written bytes instead of zero
v1: https://lore.kernel.org/netdev/20251228213331.472887-1-jelonek.jonas@gmail.com/
---
 drivers/net/phy/sfp.c | 77 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 84bef5099dda..a1deb80f630a 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -744,6 +744,35 @@ static int sfp_smbus_byte_read(struct sfp *sfp, bool a2, u8 dev_addr,
 	return data - (u8 *)buf;
 }
 
+static int sfp_smbus_block_read(struct sfp *sfp, bool a2, u8 dev_addr,
+				void *buf, size_t len)
+{
+	size_t block_size = sfp->i2c_block_size;
+	union i2c_smbus_data smbus_data;
+	u8 bus_addr = a2 ? 0x51 : 0x50;
+	u8 *data = buf;
+	u8 this_len;
+	int ret;
+
+	while (len) {
+		this_len = min(len, block_size);
+
+		smbus_data.block[0] = this_len;
+		ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
+				     I2C_SMBUS_READ, dev_addr,
+				     I2C_SMBUS_I2C_BLOCK_DATA, &smbus_data);
+		if (ret < 0)
+			return ret;
+
+		memcpy(data, &smbus_data.block[1], this_len);
+		len -= this_len;
+		data += this_len;
+		dev_addr += this_len;
+	}
+
+	return data - (u8 *)buf;
+}
+
 static int sfp_smbus_byte_write(struct sfp *sfp, bool a2, u8 dev_addr,
 				void *buf, size_t len)
 {
@@ -768,23 +797,67 @@ static int sfp_smbus_byte_write(struct sfp *sfp, bool a2, u8 dev_addr,
 	return data - (u8 *)buf;
 }
 
+static int sfp_smbus_block_write(struct sfp *sfp, bool a2, u8 dev_addr,
+				 void *buf, size_t len)
+{
+	size_t block_size = sfp->i2c_block_size;
+	union i2c_smbus_data smbus_data;
+	u8 bus_addr = a2 ? 0x51 : 0x50;
+	u8 *data = buf;
+	u8 this_len;
+	int ret;
+
+	while (len) {
+		this_len = min(len, block_size);
+
+		smbus_data.block[0] = this_len;
+		memcpy(&smbus_data.block[1], data, this_len);
+		ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
+				     I2C_SMBUS_WRITE, dev_addr,
+				     I2C_SMBUS_I2C_BLOCK_DATA, &smbus_data);
+		if (ret)
+			return ret;
+
+		len -= this_len;
+		data += this_len;
+		dev_addr += this_len;
+	}
+
+	return data - (u8 *)buf;
+}
+
 static int sfp_i2c_configure(struct sfp *sfp, struct i2c_adapter *i2c)
 {
+	size_t max_block_size;
+
 	sfp->i2c = i2c;
 
 	if (i2c_check_functionality(i2c, I2C_FUNC_I2C)) {
 		sfp->read = sfp_i2c_read;
 		sfp->write = sfp_i2c_write;
-		sfp->i2c_max_block_size = SFP_EEPROM_BLOCK_SIZE;
+		max_block_size = SFP_EEPROM_BLOCK_SIZE;
+	} else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_I2C_BLOCK)) {
+		sfp->read = sfp_smbus_block_read;
+		sfp->write = sfp_smbus_block_write;
+
+		max_block_size = SFP_EEPROM_BLOCK_SIZE;
+		if (i2c->quirks && i2c->quirks->max_read_len)
+			max_block_size = min(max_block_size,
+					     i2c->quirks->max_read_len);
+		if (i2c->quirks && i2c->quirks->max_write_len)
+			max_block_size = min(max_block_size,
+					     i2c->quirks->max_write_len);
+
 	} else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_BYTE_DATA)) {
 		sfp->read = sfp_smbus_byte_read;
 		sfp->write = sfp_smbus_byte_write;
-		sfp->i2c_max_block_size = 1;
+		max_block_size = 1;
 	} else {
 		sfp->i2c = NULL;
 		return -EINVAL;
 	}
 
+	sfp->i2c_max_block_size = max_block_size;
 	return 0;
 }
 

base-commit: fc65403d55c3be44d19e6290e641433201345a5e
-- 
2.48.1

Hi,

On 09/01/2026 11:13, 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.

This patch should probably be accompanied with a similar addition to the
mdio-i2c driver. for now, we only support full-featured I2C adapters, or
single-byte smbus, nothing in-between :(

Do you have something like this in the pipe ? not that this blocks this
particular patch, however I think that Russell's suggestion of making
this generic is the way to go.

Maxime

Hi Maxime,

On 09.01.26 16:51, Maxime Chevallier wrote:
> Hi,
>
> On 09/01/2026 11:13, 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.
> This patch should probably be accompanied with a similar addition to the
> mdio-i2c driver. for now, we only support full-featured I2C adapters, or
> single-byte smbus, nothing in-between :(
>
> Do you have something like this in the pipe ? not that this blocks this
> particular patch, however I think that Russell's suggestion of making
> this generic is the way to go.

I agree to Russell's suggestion and will work on that. Apart from that, I haven't
considered a similar change for mdio-i2c yet. No issue to deal with that but 
I think I have no way to test this properly.

> Maxime

Best,
Jonas

Hi Jonas,

On 09/01/2026 17:48, Jonas Jelonek wrote:
> Hi Maxime,
> 
> On 09.01.26 16:51, Maxime Chevallier wrote:
>> Hi,
>>
>> On 09/01/2026 11:13, 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.
>> This patch should probably be accompanied with a similar addition to the
>> mdio-i2c driver. for now, we only support full-featured I2C adapters, or
>> single-byte smbus, nothing in-between :(
>>
>> Do you have something like this in the pipe ? not that this blocks this
>> particular patch, however I think that Russell's suggestion of making
>> this generic is the way to go.
> 
> I agree to Russell's suggestion and will work on that. Apart from that, I haven't
> considered a similar change for mdio-i2c yet. No issue to deal with that but 
> I think I have no way to test this properly.

ACK, I'll gladly help with testing. This should actually be easily
achievable with a board that has a real i2c interface connected to the
SFP cage, as there's a i2c smbus emulation layer. The SMBus helpers will
work with a true I2C adapter, you may have to tweak the code though.

This is relevant for modules that have a built-in PHY that you can
access, if you don't have any I can run some tests here, I have more
than enough modules...

If you don't have time at all for that, I may give this a shot at some
point, but my time is a bit scarce right now :'(

Maxime



>> Maxime
> 
> Best,
> Jonas

Hi Maxime,

On 09.01.26 18:03, Maxime Chevallier wrote:
> ACK, I'll gladly help with testing. This should actually be easily
> achievable with a board that has a real i2c interface connected to the
> SFP cage, as there's a i2c smbus emulation layer. The SMBus helpers will
> work with a true I2C adapter, you may have to tweak the code though.
>
> This is relevant for modules that have a built-in PHY that you can
> access, if you don't have any I can run some tests here, I have more
> than enough modules...
>
> If you don't have time at all for that, I may give this a shot at some
> point, but my time is a bit scarce right now :'(
>

I'd postpone this part if that's ok. Quite busy at the moment :(

When I come to trying to work on that, should that all be kept in
mdio-i2c.c? I'm asking because we have a downstream implementation
moving that SMbus stuff to mdio-smbus.c. This covers quite a lot right
now, C22/C45 and Rollball, but just with byte access [1]. Because that
isn't my work, I'll need to check with the original authors and adapt this
for an upstream patch, trying to add word + block access.

Kind regards,
Jonas

[1] https://github.com/openwrt/openwrt/blob/66b6791abe6f08dec2924b5d9e9e7dac93f37bc4/target/linux/realtek/patches-6.12/712-net-phy-add-an-MDIO-SMBus-library.patch

Hi Jonas

On 16/01/2026 12:16, Jonas Jelonek wrote:
> Hi Maxime,
> 
> On 09.01.26 18:03, Maxime Chevallier wrote:
>> ACK, I'll gladly help with testing. This should actually be easily
>> achievable with a board that has a real i2c interface connected to the
>> SFP cage, as there's a i2c smbus emulation layer. The SMBus helpers will
>> work with a true I2C adapter, you may have to tweak the code though.
>>
>> This is relevant for modules that have a built-in PHY that you can
>> access, if you don't have any I can run some tests here, I have more
>> than enough modules...
>>
>> If you don't have time at all for that, I may give this a shot at some
>> point, but my time is a bit scarce right now :'(
>>
> 
> I'd postpone this part if that's ok. Quite busy at the moment :(

That's OK

> 
> When I come to trying to work on that, should that all be kept in
> mdio-i2c.c? I'm asking because we have a downstream implementation
> moving that SMbus stuff to mdio-smbus.c. This covers quite a lot right
> now, C22/C45 and Rollball, but just with byte access [1]. Because that
> isn't my work, I'll need to check with the original authors and adapt this
> for an upstream patch, trying to add word + block access.
> 
> Kind regards,
> Jonas
> 
> [1] https://github.com/openwrt/openwrt/blob/66b6791abe6f08dec2924b5d9e9e7dac93f37bc4/target/linux/realtek/patches-6.12/712-net-phy-add-an-MDIO-SMBus-library.patch

I don't think this patch is fit for upstream indeed. I remember Antoine
working on this back in the days, but don't know if he's even aware
that it made it into openwrt.

Nevertheless, let's keep everything in the mdio-i2c driver. I may give
this a shot at some point but I can't make any promise as to when :(

Maxime

> When I come to trying to work on that, should that all be kept in
> mdio-i2c.c? I'm asking because we have a downstream implementation
> moving that SMbus stuff to mdio-smbus.c. This covers quite a lot right
> now, C22/C45 and Rollball, but just with byte access [1].

It really should be that the I2C access mechanism, and the protocol
running on top of these accesses are orthogonal. So i could see the
code split it mdio-i2c.c, mdio-smbus.c, mdio-rollaball.c,
mdio-marvell.c. But only if there is no replicated between these
files. And i'm not sure we have reached the complexity level to make
such a split.

> Because that isn't my work, I'll need to check with the original
> authors and adapt this for an upstream patch, trying to add word +
> block access.

The code should be GPL, so you should be able to just take it and
adapt it. However, i've always had a good experience asking the
authors, they either say yes, or point out issues with the code that
need addressing. BTW: IANAL.

     Andrew

On Fri, Jan 16, 2026 at 12:16:11PM +0100, Jonas Jelonek wrote:
> Hi Maxime,
> 
> On 09.01.26 18:03, Maxime Chevallier wrote:
> > ACK, I'll gladly help with testing. This should actually be easily
> > achievable with a board that has a real i2c interface connected to the
> > SFP cage, as there's a i2c smbus emulation layer. The SMBus helpers will
> > work with a true I2C adapter, you may have to tweak the code though.
> >
> > This is relevant for modules that have a built-in PHY that you can
> > access, if you don't have any I can run some tests here, I have more
> > than enough modules...
> >
> > If you don't have time at all for that, I may give this a shot at some
> > point, but my time is a bit scarce right now :'(
> >
> 
> I'd postpone this part if that's ok. Quite busy at the moment :(
> 
> When I come to trying to work on that, should that all be kept in
> mdio-i2c.c? I'm asking because we have a downstream implementation
> moving that SMbus stuff to mdio-smbus.c. This covers quite a lot right
> now, C22/C45 and Rollball, but just with byte access [1]. Because that
> isn't my work, I'll need to check with the original authors and adapt this
> for an upstream patch, trying to add word + block access.
> 
> Kind regards,
> Jonas
> 
> [1] https://github.com/openwrt/openwrt/blob/66b6791abe6f08dec2924b5d9e9e7dac93f37bc4/target/linux/realtek/patches-6.12/712-net-phy-add-an-MDIO-SMBus-library.patch

My personal view on this is not suitable for sharing publicly. I'm sure
people can guess what my view is and why. (Look at the age of the
patch, and it's clearly "lets re-implement mdio-i2c" rather than "let's
adapt it".

-- 
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 Fri, Jan 16, 2026 at 12:16:11PM +0100, Jonas Jelonek wrote:
>
>> When I come to trying to work on that, should that all be kept in
>> mdio-i2c.c? I'm asking because we have a downstream implementation
>> moving that SMbus stuff to mdio-smbus.c. This covers quite a lot right
>> now, C22/C45 and Rollball, but just with byte access [1]. Because that
>> isn't my work, I'll need to check with the original authors and adapt this
>> for an upstream patch, trying to add word + block access.
>> 
>> Kind regards,
>> Jonas
>> 
>> [1] https://github.com/openwrt/openwrt/blob/66b6791abe6f08dec2924b5d9e9e7dac93f37bc4/target/linux/realtek/patches-6.12/712-net-phy-add-an-MDIO-SMBus-library.patch
>
> My personal view on this is not suitable for sharing publicly. I'm sure
> people can guess what my view is and why. (Look at the age of the
> patch, and it's clearly "lets re-implement mdio-i2c" rather than "let's
> adapt it".

FWIW, I agree if my guess is correct :-)

I see that my name is on that downstream patch, due to the RollBall
additions there. It was never intended for mainline in that form.  I
originally implemented the SMBus RollBall access methods as part of
mdio-i2c.c and that's where they belong IMHO.

It was briefly discussed here.  But I never had enough time and
motivation to finish this properly. So it ended up as an OpenWrt
specific hack for now.  I certainly won't object to anyone else reusing
any of the code I wrote, if it is any help at all.

Not sure the c45 SMBus stuff in openwrt ever worked BTW. I have only
tested c22 and RollBall over SMBus.


Bjørn

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.

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

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!

Hi Russell,

On 09.01.26 17:21, Russell King (Oracle) wrote:
> 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.

Makes totally sense, my initial version was just a pragmatic attempt to solve
the issues downstream for a single target. But covering byte, word and I2C block
is a better approach.

>> 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.

Thanks for providing this. I'll take that as a starting point and come up with
a better patch.

> 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.
>

Good that you mention it, haven't considered this yet.

Best,
Jonas