[PATCH v2 0/4] Increase SMBus max block size to 255

[PATCH v2 0/4] Increase SMBus max block size to 255

[Matt Johnston]

Hi,

I've taken another look at smbus v3 255-byte changes. This series combines
Daniel's previous i2c-dev patch with my i2c core patch.

Bus drivers remain opt-in to 255-byte block size, the I2C_FUNC_SMBUS3_BLOCKSIZE
flag indicates to i2c-core that the driver has been audited to handle 255
byte buffers OK. They can be added as drivers are tested with 255 byte
protocols/applications.

Drivers without that flag will be limited to 32 bytes by the I2C core. This
will take care of in-kernel compatibility issues with the increased
I2C_SMBUS_BLOCK_MAX for drivers.

The flag can also be used by userspace/drivers to warn if a driver is
non-smbus3 which would prevent a particular application from working.
Otherwise the change should be transparent to users.

The i2c-dev changes will remain compatible with these cases:

* Existing binaries, new kernel
  Will pass the old transaction type values, i2c-dev will limit
  transfers to 32 bytes

* Binaries compiled with new uapi, new kernel
  Will pass new transaction type values, i2c-dev will allow 255 bytes.
  Userspace i2c_smbus_data has sufficient space for 255 byte blocks

It won't however work with new uapi+old kernel, the new transaction types are
unknown. I wonder if it would be better to keep current constants as-is, and
let userspace opt in to 255 byte sizes with an IOCTL_SMBUS3 and
'union i2c_smbus3_data' (just for userspace, the kernel keeps using 255 byte
i2c_smbus_data everywhere). It wouldn't be necessary to look at the
I2C_FUNC_SMBUS3_BLOCKSIZE flag, just make a single first attempt with IOCTL_SMBUS3
and if that returns -ENOTTY fall back to IOCTL_SMBUS as before.
Are there any other kernel mechanisms for indicating feature support?

That would also be safer for existing userspace that may be out in the wild.
It's possible there might be libraries currently using 'union i2c_smbus_data'
in their ABI - changing its size would break library users if programs were
recompiled with new uapi headers.

I can take a look at making IOCTL_SMBUS3 changes for i2c-dev, though my main
focus here is getting the kernel side changes working (I've only been doing
hardware testing with in-kernel code).

Cheers,
Matt

--
v2:
 - i2c-dev patch based (with modifications) on
   https://lore.kernel.org/linux-i2c/20200729203658.411-1-daniel.stodden@gmail.com/
 - Rename constants to _SMBUS3_ instead of previous _V3_

Previous version part of MCTP series, i2c-core/adapter changes taken from that:
https://lore.kernel.org/all/20211115024926.205385-2-matt@codeconstruct.com.au/

v1 i2c RFC:
https://lore.kernel.org/all/20210602044113.1581347-1-matt@codeconstruct.com.au/



Daniel Stodden (1):
  i2c: dev: Support smbus3 block size of 255 bytes

Matt Johnston (3):
  i2c: core: Allow 255 byte transfers for SMBus 3.x
  i2c: aspeed: Allow 255 byte block transfers
  i2c: npcm7xx: Allow 255 byte block SMBus transfers

 drivers/i2c/busses/i2c-aspeed.c  |  5 +-
 drivers/i2c/busses/i2c-npcm7xx.c |  3 +-
 drivers/i2c/i2c-core-smbus.c     | 20 ++++---
 drivers/i2c/i2c-dev.c            | 91 +++++++++++++++++++++++++++-----
 include/uapi/linux/i2c.h         | 16 ++++--
 5 files changed, 106 insertions(+), 29 deletions(-)

-- 
2.32.0

[PATCH v2 1/4] i2c: core: Allow 255 byte transfers for SMBus 3.x

[Matt Johnston]

SMBus 3.0 increased the maximum block transfer size from 32 bytes to
255 bytes. We increase the size of struct i2c_smbus_data's block[]
member.

i2c_smbus_xfer() and i2c_smbus_xfer_emulated() now support 255 byte
block operations, other block functions remain limited to 32 bytes for
compatibility with existing callers.

We allow adapters to indicate support for the larger size with
I2C_FUNC_SMBUS3_BLOCKSIZE. Most emulated drivers should be able to use
255 byte blocks by replacing I2C_SMBUS_BLOCK_MAX with
I2C_SMBUS3_BLOCK_MAX though some will have hardware limitations that
need testing.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
 drivers/i2c/i2c-core-smbus.c | 20 +++++++++++++-------
 include/uapi/linux/i2c.h     | 16 +++++++++++-----
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index e5b2d1465e7e..5c0706e632f3 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -303,7 +303,8 @@ static void i2c_smbus_try_get_dmabuf(struct i2c_msg *msg, u8 init_val)
 	bool is_read = msg->flags & I2C_M_RD;
 	unsigned char *dma_buf;
 
-	dma_buf = kzalloc(I2C_SMBUS_BLOCK_MAX + (is_read ? 2 : 3), GFP_KERNEL);
+	dma_buf = kzalloc(I2C_SMBUS3_BLOCK_MAX + (is_read ? 2 : 3),
+		GFP_KERNEL);
 	if (!dma_buf)
 		return;
 
@@ -329,9 +330,10 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 	 * initialize most things with sane defaults, to keep the code below
 	 * somewhat simpler.
 	 */
-	unsigned char msgbuf0[I2C_SMBUS_BLOCK_MAX+3];
-	unsigned char msgbuf1[I2C_SMBUS_BLOCK_MAX+2];
+	unsigned char msgbuf0[I2C_SMBUS3_BLOCK_MAX+3];
+	unsigned char msgbuf1[I2C_SMBUS3_BLOCK_MAX+2];
 	int nmsgs = read_write == I2C_SMBUS_READ ? 2 : 1;
+	u16 block_max;
 	u8 partial_pec = 0;
 	int status;
 	struct i2c_msg msg[2] = {
@@ -350,6 +352,10 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 	bool wants_pec = ((flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK
 			  && size != I2C_SMBUS_I2C_BLOCK_DATA);
 
+	/* Drivers must opt in to 255 byte max block size */
+	block_max = i2c_check_functionality(adapter, I2C_FUNC_SMBUS3_BLOCKSIZE)
+			? I2C_SMBUS3_BLOCK_MAX : I2C_SMBUS_BLOCK_MAX;
+
 	msgbuf0[0] = command;
 	switch (size) {
 	case I2C_SMBUS_QUICK:
@@ -399,7 +405,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 			i2c_smbus_try_get_dmabuf(&msg[1], 0);
 		} else {
 			msg[0].len = data->block[0] + 2;
-			if (msg[0].len > I2C_SMBUS_BLOCK_MAX + 2) {
+			if (msg[0].len > block_max + 2) {
 				dev_err(&adapter->dev,
 					"Invalid block write size %d\n",
 					data->block[0]);
@@ -413,7 +419,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 	case I2C_SMBUS_BLOCK_PROC_CALL:
 		nmsgs = 2; /* Another special case */
 		read_write = I2C_SMBUS_READ;
-		if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+		if (data->block[0] > block_max) {
 			dev_err(&adapter->dev,
 				"Invalid block write size %d\n",
 				data->block[0]);
@@ -430,7 +436,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 		i2c_smbus_try_get_dmabuf(&msg[1], 0);
 		break;
 	case I2C_SMBUS_I2C_BLOCK_DATA:
-		if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
+		if (data->block[0] > block_max) {
 			dev_err(&adapter->dev, "Invalid block %s size %d\n",
 				read_write == I2C_SMBUS_READ ? "read" : "write",
 				data->block[0]);
@@ -498,7 +504,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
 			break;
 		case I2C_SMBUS_BLOCK_DATA:
 		case I2C_SMBUS_BLOCK_PROC_CALL:
-			if (msg[1].buf[0] > I2C_SMBUS_BLOCK_MAX) {
+			if (msg[1].buf[0] > block_max) {
 				dev_err(&adapter->dev,
 					"Invalid block size returned: %d\n",
 					msg[1].buf[0]);
diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index 92326ebde350..b7e1100ca3be 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -108,6 +108,7 @@ struct i2c_msg {
 #define I2C_FUNC_SMBUS_READ_I2C_BLOCK	0x04000000 /* I2C-like block xfer  */
 #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK	0x08000000 /* w/ 1-byte reg. addr. */
 #define I2C_FUNC_SMBUS_HOST_NOTIFY	0x10000000 /* SMBus 2.0 or later */
+#define I2C_FUNC_SMBUS3_BLOCKSIZE	0x20000000 /* Device supports 255 byte block */
 
 #define I2C_FUNC_SMBUS_BYTE		(I2C_FUNC_SMBUS_READ_BYTE | \
 					 I2C_FUNC_SMBUS_WRITE_BYTE)
@@ -137,11 +138,13 @@ struct i2c_msg {
 /*
  * Data for SMBus Messages
  */
-#define I2C_SMBUS_BLOCK_MAX	32	/* As specified in SMBus standard */
+#define I2C_SMBUS_BLOCK_MAX	32	/* As specified in SMBus standard < 3.0 */
+#define I2C_SMBUS3_BLOCK_MAX	255	/* As specified in SMBus 3.0 */
+
 union i2c_smbus_data {
 	__u8 byte;
 	__u16 word;
-	__u8 block[I2C_SMBUS_BLOCK_MAX + 2]; /* block[0] is used for length */
+	__u8 block[I2C_SMBUS3_BLOCK_MAX + 2]; /* block[0] is used for length */
 			       /* and one more for user-space compatibility */
 };
 
@@ -156,9 +159,12 @@ union i2c_smbus_data {
 #define I2C_SMBUS_BYTE_DATA	    2
 #define I2C_SMBUS_WORD_DATA	    3
 #define I2C_SMBUS_PROC_CALL	    4
-#define I2C_SMBUS_BLOCK_DATA	    5
+#define I2C_SMBUS1_BLOCK_DATA	    5 /* Legacy 32 byte block limit */
 #define I2C_SMBUS_I2C_BLOCK_BROKEN  6
-#define I2C_SMBUS_BLOCK_PROC_CALL   7		/* SMBus 2.0 */
-#define I2C_SMBUS_I2C_BLOCK_DATA    8
+#define I2C_SMBUS1_BLOCK_PROC_CALL  7 /* SMBus 2.0, legacy 32 byte block limit */
+#define I2C_SMBUS1_I2C_BLOCK_DATA   8 /* Legacy 32 byte block limit */
+#define I2C_SMBUS_BLOCK_DATA        9 /* Smbus 3.0, 255 byte limit */
+#define I2C_SMBUS_BLOCK_PROC_CALL  10 /* Smbus 3.0, 255 byte limit */
+#define I2C_SMBUS_I2C_BLOCK_DATA   11 /* Smbus 3.0, 255 byte limit */
 
 #endif /* _UAPI_LINUX_I2C_H */
-- 
2.32.0

[PATCH v2 2/4] i2c: dev: Support smbus3 block size of 255 bytes

[Matt Johnston]

From: Daniel Stodden <dns@arista.com>

* Add 3 new transaction types for
    - I2C_SMBUS_BLOCK_DATA,
    - I2C_SMBUS_BLOCK_PROC_CALL
    - I2C_SMBUS_I2C_BLOCK_DATA

* These take the names of the old types, not a "3" in a new
  transfer name. Meaning new builds will just adapt.

Old/existing binaries will pass the old transaction types. The i2c dev
code handles copying with a 32 byte block_max.

i2cdev_ioctl_smbus() can

1. Test for old transaction types

2. Adjust copy_from_user block length accordingly.

3. Proceed into i2c_smbus_xfer, with smbus3 limits established,
  and size likewise mapped to smbus3 transaction type names,
  for the remainder of the call.

4. upon return from i2c_smbus_xfer, a user block size conflict
  will result in -EMSGSIZE.

Original patch
https://lore.kernel.org/linux-i2c/20200729203658.411-1-daniel.stodden@gmail.com/

Modified by Matt Johnston to treat the transaction types as opaque values
rather than having special meaning for values > I2C_SMBUS1_BLOCK_DATA.
Constants I2C_SMBUS1_... were renamed to I2C_SMBUS2_...

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
 drivers/i2c/i2c-dev.c    | 91 +++++++++++++++++++++++++++++++++-------
 include/uapi/linux/i2c.h |  6 +--
 2 files changed, 80 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index cf5d049342ea..b524c62ccfb2 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -50,7 +50,7 @@ struct i2c_dev {
 static LIST_HEAD(i2c_dev_list);
 static DEFINE_SPINLOCK(i2c_dev_list_lock);
 
-static struct i2c_dev *i2c_dev_get_by_minor(unsigned index)
+static struct i2c_dev *i2c_dev_get_by_minor(unsigned int index)
 {
 	struct i2c_dev *i2c_dev;
 
@@ -233,9 +233,10 @@ static int i2cdev_check_addr(struct i2c_adapter *adapter, unsigned int addr)
 }
 
 static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
-		unsigned nmsgs, struct i2c_msg *msgs)
+		unsigned int nmsgs, struct i2c_msg *msgs)
 {
 	u8 __user **data_ptrs;
+	u16 user_len[I2C_RDWR_IOCTL_MAX_MSGS];
 	int i, res;
 
 	data_ptrs = kmalloc_array(nmsgs, sizeof(u8 __user *), GFP_KERNEL);
@@ -262,16 +263,34 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		msgs[i].flags |= I2C_M_DMA_SAFE;
 
 		/*
-		 * If the message length is received from the slave (similar
-		 * to SMBus block read), we must ensure that the buffer will
-		 * be large enough to cope with a message length of
-		 * I2C_SMBUS_BLOCK_MAX as this is the maximum underlying bus
-		 * drivers allow. The first byte in the buffer must be
-		 * pre-filled with the number of extra bytes, which must be
-		 * at least one to hold the message length, but can be
-		 * greater (for example to account for a checksum byte at
-		 * the end of the message.)
+		 * If the block length is received from the slave
+		 * (similar to SMBus block read), we ensure that
+		 *
+		 *  - the user buffer is large enough to hold a
+		 *    message length of I2C_SMBUS_BLOCK_MAX (32), as
+		 *    this is what any Smbus version allows.
+		 *
+		 *  - the kernel buffer is large enough to hold a
+		 *    message length of I2C_SMBUS3_BLOCK_MAX (255),
+		 *    which is what Smbus >= 3.0 allows.
+		 *
+		 * Kernel block lengths up to I2SMBUS3_BLOCK_MAX
+		 * ensure that drivers can always return up to 255
+		 * bytes safely.
+		 *
+		 * User block lengths up to only I2C_SMBUS_BLOCK_MAX
+		 * are supported for backward compatibility. If an
+		 * Smbus 3.0 slave produces a longer message than
+		 * userspace provides for, we truncate the user copy
+		 * and return -EMSGSIZE.
+		 *
+		 * The first byte in the user buffer must be
+		 * pre-filled with the number of extra bytes, at least
+		 * one to hold the message length, but can be greater
+		 * (for example to account for a checksum byte at the
+		 * end of the message.)
 		 */
+		user_len[i] = msgs[i].len;
 		if (msgs[i].flags & I2C_M_RECV_LEN) {
 			if (!(msgs[i].flags & I2C_M_RD) ||
 			    msgs[i].len < 1 || msgs[i].buf[0] < 1 ||
@@ -282,11 +301,26 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 				break;
 			}
 
+			if (msgs[i].len < msgs[i].buf[0] +
+					   I2C_SMBUS3_BLOCK_MAX) {
+				u8 *buf = krealloc(msgs[i].buf,
+						 msgs[i].buf[0] +
+						 I2C_SMBUS3_BLOCK_MAX,
+						 GFP_KERNEL);
+				if (!buf) {
+					i++;
+					res = -ENOMEM;
+					break;
+				}
+				msgs[i].buf = buf;
+			}
+
 			msgs[i].len = msgs[i].buf[0];
 		}
 	}
 	if (res < 0) {
 		int j;
+
 		for (j = 0; j < i; ++j)
 			kfree(msgs[j].buf);
 		kfree(data_ptrs);
@@ -298,8 +332,10 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 	while (i-- > 0) {
 		if (res >= 0 && (msgs[i].flags & I2C_M_RD)) {
 			if (copy_to_user(data_ptrs[i], msgs[i].buf,
-					 msgs[i].len))
+					 min(msgs[i].len, user_len[i])))
 				res = -EFAULT;
+			if (msgs[i].len > user_len[i])
+				res = res ? : -EMSGSIZE;
 		}
 		kfree(msgs[i].buf);
 	}
@@ -313,7 +349,28 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 		union i2c_smbus_data __user *data)
 {
 	union i2c_smbus_data temp = {};
-	int datasize, res;
+	int block_max, datasize, res;
+
+	/* Limit legacy transaction types to 32 blocksize */
+	switch (size) {
+	case I2C_SMBUS2_BLOCK_DATA:
+		size = I2C_SMBUS_BLOCK_DATA;
+		block_max = I2C_SMBUS_BLOCK_MAX;
+		break;
+	case I2C_SMBUS2_BLOCK_PROC_CALL:
+		size = I2C_SMBUS_BLOCK_PROC_CALL;
+		block_max = I2C_SMBUS_BLOCK_MAX;
+		break;
+	case I2C_SMBUS2_I2C_BLOCK_DATA:
+		size = I2C_SMBUS_I2C_BLOCK_DATA;
+		block_max = I2C_SMBUS_BLOCK_MAX;
+		break;
+	case I2C_SMBUS_I2C_BLOCK_BROKEN:
+		block_max = I2C_SMBUS_BLOCK_MAX;
+		break;
+	default:
+		block_max = I2C_SMBUS3_BLOCK_MAX;
+	}
 
 	if ((size != I2C_SMBUS_BYTE) &&
 	    (size != I2C_SMBUS_QUICK) &&
@@ -362,7 +419,7 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 		 (size == I2C_SMBUS_PROC_CALL))
 		datasize = sizeof(data->word);
 	else /* size == smbus block, i2c block, or block proc. call */
-		datasize = sizeof(data->block);
+		datasize = block_max + 2;
 
 	if ((size == I2C_SMBUS_PROC_CALL) ||
 	    (size == I2C_SMBUS_BLOCK_PROC_CALL) ||
@@ -385,6 +442,8 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 		     (read_write == I2C_SMBUS_READ))) {
 		if (copy_to_user(data, &temp, datasize))
 			return -EFAULT;
+		if (temp.block[0] > block_max)
+			return -EMSGSIZE;
 	}
 	return res;
 }
@@ -460,6 +519,7 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case I2C_SMBUS: {
 		struct i2c_smbus_ioctl_data data_arg;
+
 		if (copy_from_user(&data_arg,
 				   (struct i2c_smbus_ioctl_data __user *) arg,
 				   sizeof(struct i2c_smbus_ioctl_data)))
@@ -520,6 +580,7 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 {
 	struct i2c_client *client = file->private_data;
 	unsigned long funcs;
+
 	switch (cmd) {
 	case I2C_FUNCS:
 		funcs = i2c_get_functionality(client->adapter);
@@ -549,6 +610,7 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 		p = compat_ptr(rdwr_arg.msgs);
 		for (i = 0; i < rdwr_arg.nmsgs; i++) {
 			struct i2c_msg32 umsg;
+
 			if (copy_from_user(&umsg, p + i, sizeof(umsg))) {
 				kfree(rdwr_pa);
 				return -EFAULT;
@@ -565,6 +627,7 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 	}
 	case I2C_SMBUS: {
 		struct i2c_smbus_ioctl_data32	data32;
+
 		if (copy_from_user(&data32,
 				   (void __user *) arg,
 				   sizeof(data32)))
diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index b7e1100ca3be..7cd85594f287 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -159,10 +159,10 @@ union i2c_smbus_data {
 #define I2C_SMBUS_BYTE_DATA	    2
 #define I2C_SMBUS_WORD_DATA	    3
 #define I2C_SMBUS_PROC_CALL	    4
-#define I2C_SMBUS1_BLOCK_DATA	    5 /* Legacy 32 byte block limit */
+#define I2C_SMBUS2_BLOCK_DATA	    5 /* Legacy 32 byte block limit */
 #define I2C_SMBUS_I2C_BLOCK_BROKEN  6
-#define I2C_SMBUS1_BLOCK_PROC_CALL  7 /* SMBus 2.0, legacy 32 byte block limit */
-#define I2C_SMBUS1_I2C_BLOCK_DATA   8 /* Legacy 32 byte block limit */
+#define I2C_SMBUS2_BLOCK_PROC_CALL  7 /* SMBus 2.0, legacy 32 byte block limit */
+#define I2C_SMBUS2_I2C_BLOCK_DATA   8 /* Legacy 32 byte block limit */
 #define I2C_SMBUS_BLOCK_DATA        9 /* Smbus 3.0, 255 byte limit */
 #define I2C_SMBUS_BLOCK_PROC_CALL  10 /* Smbus 3.0, 255 byte limit */
 #define I2C_SMBUS_I2C_BLOCK_DATA   11 /* Smbus 3.0, 255 byte limit */
-- 
2.32.0

[PATCH v2 3/4] i2c: aspeed: Allow 255 byte block transfers

[Matt Johnston]

255 byte transfers have been tested on an AST2500 board

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 67e8b97c0c95..185cdf8a4518 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -533,7 +533,7 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		msg->buf[bus->buf_index++] = recv_byte;
 
 		if (msg->flags & I2C_M_RECV_LEN) {
-			if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
+			if (unlikely(recv_byte > I2C_SMBUS3_BLOCK_MAX)) {
 				bus->cmd_err = -EPROTO;
 				aspeed_i2c_do_stop(bus);
 				goto out_no_complete;
@@ -718,7 +718,8 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
 
 static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+		I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS3_BLOCKSIZE;
 }
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
-- 
2.32.0

[PATCH v2 4/4] i2c: npcm7xx: Allow 255 byte block SMBus transfers

[Matt Johnston]

255 byte support has been tested on a npcm750 board

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Reviewed-by: Tali Perry <tali.perry1@gmail.com>
Reviewed-by: Patrick Venture <venture@google.com>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
index 2ad166355ec9..01a1e96be4ba 100644
--- a/drivers/i2c/busses/i2c-npcm7xx.c
+++ b/drivers/i2c/busses/i2c-npcm7xx.c
@@ -1399,7 +1399,7 @@ static void npcm_i2c_irq_master_handler_read(struct npcm_i2c *bus)
 		if (bus->read_block_use) {
 			/* first byte in block protocol is the size: */
 			data = npcm_i2c_rd_byte(bus);
-			data = clamp_val(data, 1, I2C_SMBUS_BLOCK_MAX);
+			data = clamp_val(data, 1, I2C_SMBUS3_BLOCK_MAX);
 			bus->rd_size = data + block_extra_bytes_size;
 			bus->rd_buf[bus->rd_ind++] = data;
 
@@ -2187,6 +2187,7 @@ static u32 npcm_i2c_functionality(struct i2c_adapter *adap)
 	       I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_PEC |
+	       I2C_FUNC_SMBUS3_BLOCKSIZE |
 	       I2C_FUNC_SLAVE;
 }
 
-- 
2.32.0

Re: [PATCH v2 0/4] Increase SMBus max block size to 255

[James R T]

Hi folks,

Replying to the cover letter since this email refers to the whole patch series.

We were looking into support for SMBus v3.0 in Linux and found this
patch series. Could we kindly check if there has been any progress on
this?

From what I understand so far, a previous version of this patch series
was applied to netdev/net-next.git (master) but it was reverted via
commit 2f6a470d6545 due to concerns over ABI backward compatibility
from Wolfram. Then, Matt submitted this v2 patch series which has gone
unnoticed. It has been more than 2 years and there seems to have been
no responses to the series so far. I would like to gently bump up this
patch series so that we can hopefully move it forward.

Thank you.

Best regards,
James Raphael Tiovalen

Re: [PATCH v2 0/4] Increase SMBus max block size to 255

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

Hi James,

> We were looking into support for SMBus v3.0 in Linux and found this
> patch series. Could we kindly check if there has been any progress on
> this?

Well, on the drawing board level, yes. I gave two talks about how to
tackle this issue. The latest one being here, but, sorry, it is in
German only. Maybe you can get an idea out of the slides:

https://chemnitzer.linux-tage.de/2024/en/programm/beitrag/252

> From what I understand so far, a previous version of this patch series
> was applied to netdev/net-next.git (master) but it was reverted via
> commit 2f6a470d6545 due to concerns over ABI backward compatibility
> from Wolfram. Then, Matt submitted this v2 patch series which has gone
> unnoticed. It has been more than 2 years and there seems to have been
> no responses to the series so far. I would like to gently bump up this
> patch series so that we can hopefully move it forward.

True, it looks like it went unnoticed, but it wasn't. I need to update
the status of this series in Patchwork to 'Rejected' sadly. Renaming the
old transaction types in the UAPI header is a no-go and will break newer
userspace with older kernels. The new SMBus sizes need to be opt-in
completely.

Like I said, I have sketched this but both demand and work time have
been limited, so no actual implementation happened so far.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]