[PATCH v3] net: sfp: add SMBus I2C block support

Jonas Jelonek posted 1 patch 1 month ago
There is a newer version of this series
drivers/net/phy/sfp.c | 77 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 75 insertions(+), 2 deletions(-)
[PATCH v3] net: sfp: add SMBus I2C block support
Posted by Jonas Jelonek 1 month ago
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>

---
v3: fix previous attempt of v2 to fix return value
v2: return number of written bytes in sfp_smbus_block_write
v2: https://lore.kernel.org/netdev/20260105154653.575397-1-jelonek.jonas@gmail.com/
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: c303e8b86d9dbd6868f5216272973292f7f3b7f1
prerequisite-patch-id: ae039dad1e17867fce9182b6b36ac3b1926b254a
-- 
2.48.1
Re: [PATCH v3] net: sfp: add SMBus I2C block support
Posted by Russell King (Oracle) 1 month ago
On Mon, Jan 05, 2026 at 04:12:42PM +0000, Jonas Jelonek wrote:
> base-commit: c303e8b86d9dbd6868f5216272973292f7f3b7f1
> prerequisite-patch-id: ae039dad1e17867fce9182b6b36ac3b1926b254a

This seems to be almost useless information. While base-commit exists
in the net-next tree, commit ae039dad1e17867fce9182b6b36ac3b1926b254a
doesn't exist in either net-next nor net trees.

My guess is you applied Maxime's patch locally, and that is the
commit ID of that patch.

Given that Maxime's patch is targetting the net tree (because it's
a fix), and your patch is new development, so is for net-next, you
either need to:
- wait until Maxime's patch has been merged, and then the net tree
  has been merged into net-next.
or:
- resend without Maxime's patch.

In either case, please read https://docs.kernel.org/process/maintainer-netdev.html
In particular, the salient points are:
- no resends in under 24 hours (see point 1.6.7)
- specify the tree that your patch is targetting in the subject line
  (see point 1.6.1) E.g. [PATCH net-next v...] net: blah blah

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v3] net: sfp: add SMBus I2C block support
Posted by Jonas Jelonek 1 month ago
Hi Russell,

On 05.01.26 17:28, Russell King (Oracle) wrote:
> On Mon, Jan 05, 2026 at 04:12:42PM +0000, Jonas Jelonek wrote:
>> base-commit: c303e8b86d9dbd6868f5216272973292f7f3b7f1
>> prerequisite-patch-id: ae039dad1e17867fce9182b6b36ac3b1926b254a
> This seems to be almost useless information. While base-commit exists
> in the net-next tree, commit ae039dad1e17867fce9182b6b36ac3b1926b254a
> doesn't exist in either net-next nor net trees.
>
> My guess is you applied Maxime's patch locally, and that is the
> commit ID of that patch.

This was supposed to be the stable patch-id obtained with 
'git patch-id --stable'.

> Given that Maxime's patch is targetting the net tree (because it's
> a fix), and your patch is new development, so is for net-next, you
> either need to:
> - wait until Maxime's patch has been merged, and then the net tree
>   has been merged into net-next.
> or:
> - resend without Maxime's patch.
>
> In either case, please read https://docs.kernel.org/process/maintainer-netdev.html
> In particular, the salient points are:
> - no resends in under 24 hours (see point 1.6.7)
> - specify the tree that your patch is targetting in the subject line
>   (see point 1.6.1) E.g. [PATCH net-next v...] net: blah blah
>
> Thanks.
>

Ack. I'll fix the points you mentioned.
Thanks for your patience.


Best,
Jonas
Re: [PATCH v3] net: sfp: add SMBus I2C block support
Posted by Russell King (Oracle) 1 month ago
On Mon, Jan 05, 2026 at 05:53:42PM +0100, Jonas Jelonek wrote:
> Hi Russell,
> 
> On 05.01.26 17:28, Russell King (Oracle) wrote:
> > On Mon, Jan 05, 2026 at 04:12:42PM +0000, Jonas Jelonek wrote:
> >> base-commit: c303e8b86d9dbd6868f5216272973292f7f3b7f1
> >> prerequisite-patch-id: ae039dad1e17867fce9182b6b36ac3b1926b254a
> > This seems to be almost useless information. While base-commit exists
> > in the net-next tree, commit ae039dad1e17867fce9182b6b36ac3b1926b254a
> > doesn't exist in either net-next nor net trees.
> >
> > My guess is you applied Maxime's patch locally, and that is the
> > commit ID of that patch.
> 
> This was supposed to be the stable patch-id obtained with 
> 'git patch-id --stable'.

Hmm, didn't know about that... but in this context, I wonder how
useful it is. As a maintainer, given that patches submitted don't
specify their patch-id, tracking down which patch is the
pre-requisit would be a mammoth task, whereas a lore.kernel.org
link would indicate the pre-requisit immediately.

lore.kernel.org links are easy:

https://lore.kernel.org/r/<message id of the email containing the patch>

I can see patch-ids are useful for automation, but lore.kernel.org
URLs are useful for humans.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v3] net: sfp: add SMBus I2C block support
Posted by Jakub Kicinski 1 month ago
On Mon, 5 Jan 2026 17:13:45 +0000 Russell King (Oracle) wrote:
> > > This seems to be almost useless information. While base-commit exists
> > > in the net-next tree, commit ae039dad1e17867fce9182b6b36ac3b1926b254a
> > > doesn't exist in either net-next nor net trees.
> > >
> > > My guess is you applied Maxime's patch locally, and that is the
> > > commit ID of that patch.  
> > 
> > This was supposed to be the stable patch-id obtained with 
> > 'git patch-id --stable'.  
> 
> Hmm, didn't know about that... but in this context, I wonder how
> useful it is. As a maintainer, given that patches submitted don't
> specify their patch-id, tracking down which patch is the
> pre-requisit would be a mammoth task [...]

+1, please wait for any pre-requisites to be in the tree before posting.
If you'd like to get early reviews send as RFC. Note that net gets
merged into net-next every Thu (in case the dependency is cross-tree
this is an extra wait).