Revert "Merge branch 'mctp-i2c-driver'"

This reverts commit 71812af7, reversing
changes made to cc0be1ad.

Wolfram Sang says:

Please revert. Besides the driver in net, it modifies the I2C core
code. This has not been acked by the I2C maintainer (in this case me).
So, please don't pull this in via the net tree. The question raised here
(extending SMBus calls to 255 byte) is complicated because we need ABI
backwards compatibility.

Link: https://lore.kernel.org/all/YZJ9H4eM%2FM7OXVN0@shikoro/Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Documentation/devicetree/bindings/i2c/i2c.txt

@@ -95,10 +95,6 @@ wants to support one of the below features, it should adapt these bindings.
 - smbus-alert
 	states that the optional SMBus-Alert feature apply to this bus.
 
-- mctp-controller
-	indicates that the system is accessible via this bus as an endpoint for
-	MCTP over I2C transport.
-
 Required properties (per child device)
 --------------------------------------
 

Documentation/devicetree/bindings/net/mctp-i2c-controller.yaml

-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/net/mctp-i2c-controller.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: MCTP I2C transport binding
-
-maintainers:
-  - Matt Johnston <matt@codeconstruct.com.au>
-
-description: |
-  An mctp-i2c-controller defines a local MCTP endpoint on an I2C controller.
-  MCTP I2C is specified by DMTF DSP0237.
-
-  An mctp-i2c-controller must be attached to an I2C adapter which supports
-  slave functionality. I2C busses (either directly or as subordinate mux
-  busses) are attached to the mctp-i2c-controller with a 'mctp-controller'
-  property on each used bus. Each mctp-controller I2C bus will be presented
-  to the host system as a separate MCTP I2C instance.
-
-properties:
-  compatible:
-    const: mctp-i2c-controller
-
-  reg:
-    minimum: 0x40000000
-    maximum: 0x4000007f
-    description: |
-      7 bit I2C address of the local endpoint.
-      I2C_OWN_SLAVE_ADDRESS (1<<30) flag must be set.
-
-additionalProperties: false
-
-required:
-  - compatible
-  - reg
-
-examples:
-  - |
-    // Basic case of a single I2C bus
-    #include <dt-bindings/i2c/i2c.h>
-
-    i2c {
-      #address-cells = <1>;
-      #size-cells = <0>;
-      mctp-controller;
-
-      mctp@30 {
-        compatible = "mctp-i2c-controller";
-        reg = <(0x30 | I2C_OWN_SLAVE_ADDRESS)>;
-      };
-    };
-
-  - |
-    // Mux topology with multiple MCTP-handling busses under
-    // a single mctp-i2c-controller.
-    // i2c1 and i2c6 can have MCTP devices, i2c5 does not.
-    #include <dt-bindings/i2c/i2c.h>
-
-    i2c1: i2c {
-      #address-cells = <1>;
-      #size-cells = <0>;
-      mctp-controller;
-
-      mctp@50 {
-        compatible = "mctp-i2c-controller";
-        reg = <(0x50 | I2C_OWN_SLAVE_ADDRESS)>;
-      };
-    };
-
-    i2c-mux {
-      #address-cells = <1>;
-      #size-cells = <0>;
-      i2c-parent = <&i2c1>;
-
-      i2c5: i2c@0 {
-        #address-cells = <1>;
-        #size-cells = <0>;
-        reg = <0>;
-        eeprom@33 {
-          reg = <0x33>;
-        };
-      };
-
-      i2c6: i2c@1 {
-        #address-cells = <1>;
-        #size-cells = <0>;
-        reg = <1>;
-        mctp-controller;
-      };
-    };

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_V3_BLOCK_MAX)) {
+			if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
 				bus->cmd_err = -EPROTO;
 				aspeed_i2c_do_stop(bus);
 				goto out_no_complete;
@@ -718,8 +718,7 @@ 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 | I2C_FUNC_SMBUS_V3_BLOCK;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)

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_V3_BLOCK_MAX);
+			data = clamp_val(data, 1, I2C_SMBUS_BLOCK_MAX);
 			bus->rd_size = data + block_extra_bytes_size;
 			bus->rd_buf[bus->rd_ind++] = data;
 
@@ -2187,7 +2187,6 @@ static u32 npcm_i2c_functionality(struct i2c_adapter *adap)
 	       I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_PEC |
-	       I2C_FUNC_SMBUS_V3_BLOCK |
 	       I2C_FUNC_SLAVE;
 }
 

drivers/i2c/i2c-core-smbus.c

@@ -303,8 +303,7 @@ 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_V3_BLOCK_MAX + (is_read ? 2 : 3),
-		GFP_KERNEL);
+	dma_buf = kzalloc(I2C_SMBUS_BLOCK_MAX + (is_read ? 2 : 3), GFP_KERNEL);
 	if (!dma_buf)
 		return;
 
@@ -330,10 +329,9 @@ 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_V3_BLOCK_MAX+3];
-	unsigned char msgbuf1[I2C_SMBUS_V3_BLOCK_MAX+2];
+	unsigned char msgbuf0[I2C_SMBUS_BLOCK_MAX+3];
+	unsigned char msgbuf1[I2C_SMBUS_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] = {
@@ -352,10 +350,6 @@ 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_SMBUS_V3_BLOCK)
-			? I2C_SMBUS_V3_BLOCK_MAX : I2C_SMBUS_BLOCK_MAX;
-
 	msgbuf0[0] = command;
 	switch (size) {
 	case I2C_SMBUS_QUICK:
@@ -405,7 +399,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 > block_max + 2) {
+			if (msg[0].len > I2C_SMBUS_BLOCK_MAX + 2) {
 				dev_err(&adapter->dev,
 					"Invalid block write size %d\n",
 					data->block[0]);
@@ -419,7 +413,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] > block_max) {
+		if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
 			dev_err(&adapter->dev,
 				"Invalid block write size %d\n",
 				data->block[0]);
@@ -436,7 +430,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] > block_max) {
+		if (data->block[0] > I2C_SMBUS_BLOCK_MAX) {
 			dev_err(&adapter->dev, "Invalid block %s size %d\n",
 				read_write == I2C_SMBUS_READ ? "read" : "write",
 				data->block[0]);
@@ -504,7 +498,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] > block_max) {
+			if (msg[1].buf[0] > I2C_SMBUS_BLOCK_MAX) {
 				dev_err(&adapter->dev,
 					"Invalid block size returned: %d\n",
 					msg[1].buf[0]);

drivers/i2c/i2c-dev.c

@@ -46,24 +46,6 @@ struct i2c_dev {
 	struct cdev cdev;
 };
 
-/* The userspace union i2c_smbus_data for I2C_SMBUS ioctl is limited
- * to 32 bytes (I2C_SMBUS_BLOCK_MAX) for compatibility.
- */
-union compat_i2c_smbus_data {
-	__u8 byte;
-	__u16 word;
-	__u8 block[I2C_SMBUS_BLOCK_MAX + 2]; /* block[0] is used for length */
-			       /* and one more for user-space compatibility */
-};
-
-/* Must match i2c-dev.h definition with compat .data member */
-struct i2c_smbus_ioctl_data {
-	__u8 read_write;
-	__u8 command;
-	__u32 size;
-	union compat_i2c_smbus_data __user *data;
-};
-
 #define I2C_MINORS	(MINORMASK + 1)
 static LIST_HEAD(i2c_dev_list);
 static DEFINE_SPINLOCK(i2c_dev_list_lock);
@@ -253,17 +235,14 @@ 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)
 {
-	u8 __user **data_ptrs = NULL;
-	u16 *orig_lens = NULL;
+	u8 __user **data_ptrs;
 	int i, res;
 
-	res = -ENOMEM;
 	data_ptrs = kmalloc_array(nmsgs, sizeof(u8 __user *), GFP_KERNEL);
-	if (data_ptrs == NULL)
-		goto out;
-	orig_lens = kmalloc_array(nmsgs, sizeof(u16), GFP_KERNEL);
-	if (orig_lens == NULL)
-		goto out;
+	if (data_ptrs == NULL) {
+		kfree(msgs);
+		return -ENOMEM;
+	}
 
 	res = 0;
 	for (i = 0; i < nmsgs; i++) {
@@ -274,30 +253,12 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		}
 
 		data_ptrs[i] = (u8 __user *)msgs[i].buf;
-		msgs[i].buf = NULL;
-		if (msgs[i].len < 1) {
-			/* Sanity check */
-			res = -EINVAL;
-			break;
-
-		}
-		/* Allocate a larger buffer to accommodate possible 255 byte
-		 * blocks. Read results will be dropped later
-		 * if they are too large for the original length.
-		 */
-		orig_lens[i] = msgs[i].len;
-		msgs[i].buf = kmalloc(msgs[i].len + I2C_SMBUS_V3_BLOCK_MAX,
-			GFP_USER | __GFP_NOWARN);
+		msgs[i].buf = memdup_user(data_ptrs[i], msgs[i].len);
 		if (IS_ERR(msgs[i].buf)) {
 			res = PTR_ERR(msgs[i].buf);
 			break;
 		}
-		if (copy_from_user(msgs[i].buf, data_ptrs[i], msgs[i].len)) {
-			kfree(msgs[i].buf);
-			res = -EFAULT;
-			break;
-		}
-		/* Buffer from kmalloc, so DMA is ok */
+		/* memdup_user allocates with GFP_KERNEL, so DMA is ok */
 		msgs[i].flags |= I2C_M_DMA_SAFE;
 
 		/*
@@ -313,7 +274,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		 */
 		if (msgs[i].flags & I2C_M_RECV_LEN) {
 			if (!(msgs[i].flags & I2C_M_RD) ||
-			    msgs[i].buf[0] < 1 ||
+			    msgs[i].len < 1 || msgs[i].buf[0] < 1 ||
 			    msgs[i].len < msgs[i].buf[0] +
 					     I2C_SMBUS_BLOCK_MAX) {
 				i++;
@@ -336,16 +297,12 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 	res = i2c_transfer(client->adapter, msgs, nmsgs);
 	while (i-- > 0) {
 		if (res >= 0 && (msgs[i].flags & I2C_M_RD)) {
-			if (orig_lens[i] < msgs[i].len)
-				res = -EINVAL;
-			else if (copy_to_user(data_ptrs[i], msgs[i].buf,
-						 msgs[i].len))
+			if (copy_to_user(data_ptrs[i], msgs[i].buf,
+					 msgs[i].len))
 				res = -EFAULT;
 		}
 		kfree(msgs[i].buf);
 	}
-out:
-	kfree(orig_lens);
 	kfree(data_ptrs);
 	kfree(msgs);
 	return res;
@@ -353,7 +310,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 
 static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 		u8 read_write, u8 command, u32 size,
-		union compat_i2c_smbus_data __user *data)
+		union i2c_smbus_data __user *data)
 {
 	union i2c_smbus_data temp = {};
 	int datasize, res;
@@ -414,16 +371,6 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 		if (copy_from_user(&temp, data, datasize))
 			return -EFAULT;
 	}
-	if ((size == I2C_SMBUS_BLOCK_PROC_CALL ||
-	    size == I2C_SMBUS_I2C_BLOCK_DATA ||
-	    size == I2C_SMBUS_BLOCK_DATA) &&
-	    read_write == I2C_SMBUS_WRITE &&
-	    temp.block[0] > I2C_SMBUS_BLOCK_MAX) {
-		/* Don't accept writes larger than the buffer size */
-		dev_dbg(&client->adapter->dev, "block write is too large");
-		return -EINVAL;
-
-	}
 	if (size == I2C_SMBUS_I2C_BLOCK_BROKEN) {
 		/* Convert old I2C block commands to the new
 		   convention. This preserves binary compatibility. */
@@ -433,21 +380,9 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
 	}
 	res = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
 	      read_write, command, size, &temp);
-	if (res)
-		return res;
-	if ((size == I2C_SMBUS_BLOCK_PROC_CALL ||
-	    size == I2C_SMBUS_I2C_BLOCK_DATA ||
-	    size == I2C_SMBUS_BLOCK_DATA) &&
-	    read_write == I2C_SMBUS_READ &&
-	    temp.block[0] > I2C_SMBUS_BLOCK_MAX) {
-		/* Don't accept reads larger than the buffer size */
-		dev_dbg(&client->adapter->dev, "block read is too large");
-		return -EINVAL;
-
-	}
-	if ((size == I2C_SMBUS_PROC_CALL) ||
-	    (size == I2C_SMBUS_BLOCK_PROC_CALL) ||
-	    (read_write == I2C_SMBUS_READ)) {
+	if (!res && ((size == I2C_SMBUS_PROC_CALL) ||
+		     (size == I2C_SMBUS_BLOCK_PROC_CALL) ||
+		     (read_write == I2C_SMBUS_READ))) {
 		if (copy_to_user(data, &temp, datasize))
 			return -EFAULT;
 	}

drivers/net/mctp/Kconfig

@@ -3,18 +3,6 @@ if MCTP
 
 menu "MCTP Device Drivers"
 
-config MCTP_TRANSPORT_I2C
-	tristate "MCTP SMBus/I2C transport"
-	# i2c-mux is optional, but we must build as a module if i2c-mux is a module
-	depends on I2C_MUX || !I2C_MUX
-	depends on I2C
-	depends on I2C_SLAVE
-	select MCTP_FLOWS
-	help
-	  Provides a driver to access MCTP devices over SMBus/I2C transport,
-	  from DMTF specification DSP0237. A MCTP protocol network device is
-	  created for each I2C bus that has been assigned a mctp-i2c device.
-
 endmenu
 
 endif

drivers/net/mctp/Makefile

-obj-$(CONFIG_MCTP_TRANSPORT_I2C) += mctp-i2c.o

include/linux/i2c.h

@@ -52,19 +52,6 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client,
 struct module;
 struct property_entry;
 
-/* SMBus 3.0 extends the maximum block read/write size to 255 (from 32).
- * The larger size is only supported by some drivers, indicated by
- * the I2C_FUNC_SMBUS_V3_BLOCK functionality bit.
- */
-#define I2C_SMBUS_V3_BLOCK_MAX	255	/* As specified in SMBus 3.0 standard */
-
-/* Note compatibility definition in uapi header with 32 byte block */
-union i2c_smbus_data {
-	__u8 byte;
-	__u16 word;
-	__u8 block[I2C_SMBUS_V3_BLOCK_MAX + 1]; /* block[0] is used for length */
-};
-
 #if IS_ENABLED(CONFIG_I2C)
 /* Return the Frequency mode string based on the bus frequency */
 const char *i2c_freq_mode_string(u32 bus_freq_hz);

include/uapi/linux/i2c-dev.h

@@ -39,14 +39,12 @@
 
 
 /* This is the structure as used in the I2C_SMBUS ioctl call */
-#ifndef __KERNEL__
 struct i2c_smbus_ioctl_data {
 	__u8 read_write;
 	__u8 command;
 	__u32 size;
 	union i2c_smbus_data __user *data;
 };
-#endif
 
 /* This is the structure as used in the I2C_RDWR ioctl call */
 struct i2c_rdwr_ioctl_data {

include/uapi/linux/i2c.h

@@ -108,9 +108,6 @@ 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_SMBUS_V3_BLOCK		0x20000000 /* Device supports 255 byte block */
-						   /* Note that I2C_SMBUS ioctl only */
-						   /* supports a 32 byte block */
 
 #define I2C_FUNC_SMBUS_BYTE		(I2C_FUNC_SMBUS_READ_BYTE | \
 					 I2C_FUNC_SMBUS_WRITE_BYTE)
@@ -140,15 +137,13 @@ struct i2c_msg {
 /*
  * Data for SMBus Messages
  */
-#define I2C_SMBUS_BLOCK_MAX	32	/* As specified in SMBus 2.0 standard */
-#ifndef __KERNEL__
+#define I2C_SMBUS_BLOCK_MAX	32	/* As specified in SMBus standard */
 union i2c_smbus_data {
 	__u8 byte;
 	__u16 word;
 	__u8 block[I2C_SMBUS_BLOCK_MAX + 2]; /* block[0] is used for length */
 			       /* and one more for user-space compatibility */
 };
-#endif
 
 /* i2c_smbus_xfer read or write markers */
 #define I2C_SMBUS_READ	1