[PATCH v2 00/12] I2C - detailed transfer reporting in case of a fault

[PATCH v2 00/12] I2C - detailed transfer reporting in case of a fault

[Dmitry Guzman]

The existing API has function `i2c_xfer` that transfers one or more
messages, and it only returns a single error code if the transfer was
failed. It doesn't allow to know how many of the messages were
transferred successfully, neither how many bytes were transferred in the
message that caused the fault, and also it drops all data received from
target device before the fault. There is a comment about this in
drivers/i2c/i2c-core-base.c: "REVISIT the fault reporting model here is
weak".

This patch series implements new API function `i2c_xfer_v2` that does
the same as `i2c_xfer` but also returns detailed transfer report, including
number of messages and bytes transferred before the fault. This also allows
client to get the bytes read from the target before the fault occurred.

For user space clients, new ioctl `I2C_RDWR_V2` is introduced.

Patch for `i2c-tools` user space utility that introduces support of this
feature:

https://lore.kernel.org/linux-i2c/20260623-i2c_detailed_report-v1-1-b67259a31a9d@mobileye.com/

A usage example (the first message changes the target address, so the
second message is not ACKed):

    # i2ctransfer -r -y 0 w2@0x20 0 0x55 r1
    Error: No such device or address at message 1
    1 out of 2 messages sent
    #

In this patchset, the introduced functionality is implemented in
`i2c-nomadik` and `i2c-designware` drivers. Several other improvements in
these drivers related to fault handling are also included in this patchset.

`i2c-nomadik` driver has been tested on EyeQ6H.

`i2c-designware` driver has been tested on EyeQ6L+.

The implementation is split up into patches:

Patch #1 Introduce callback `xfer_v2` in struct `i2c_algorithm`,
	 function `i2c_xfer_v2`, ioctl `I2C_RDWR_V2`, structures for I2C
	 transfer reporting and implement all driver-independent functionality.
Patch #2 Optimize struct layout in `i2c-nomadik`.
Patch #3 Remove automatic retransfer in `i2c-nomadik`.
Patch #4 Fix error codes returned by `xfer` callback in `i2c-nomadik`.
Patch #5 Replace `dev_err` with `dev_dbg` on I2C faults in `i2c-nomadik`.
Patch #6 Add quirks that describe some limitations of `i2c-nomadik`.
Patch #7 Add support for `xfer_v2` in `i2c-nomadik`.
Patch #8 Fix error codes returned by `xfer` callback in
	  `i2c-designware`.
Patch #9 Reset SMBUS_READ_BLOCK_DATA bit in functionality mask for
	  `designware` variants without EMPTYFIFO_HOLD_MASTER feature.
Patch #10 Terminate transfer if spurious STOP is detected in
	  `i2c-designware` driver.
Patch #11 Separate `i2c_algorithm` structure for AMD_NAVI_GPU variant of
	  designware controller.
Patch #12 Add support for `xfer_v2` in `i2c-designware` driver.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
Changes in v2:
- Add support for detailed transfer reporting in designware driver (patches
  #8..#12).
- Change `unsigned char` into `u8` in patch #2.
- Added code-review trailers for patches #3..#7.
- Rebase on new kernel version (no conflicting code changes).
- Link to v1: https://lore.kernel.org/r/20260623-i2c-fault-reporting-v1-0-6db1a8aabf18@mobileye.com

---
Dmitry Guzman (12):
      i2c: core: add I2C_XFER_V2 - support for detailed transfer reporting
      i2c: nomadik: optimize struct layout, change unsigned char into u8
      i2c: nomadik: do not try to retransmit I2C message series on errors
      i2c: nomadik: return proper fault codes
      i2c: nomadik: change print level for fault messages to debug
      i2c: nomadik: add quirks max_len=2047 and no_zero_len_read
      i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting
      i2c: designware: return proper fault codes
      i2c: designware: no SMBUS_READ_BLOCK_DATA without EMPTYFIFO_HOLD_MASTER
      i2c: designware: stop transfer if spurious STOP is detected
      i2c: designware: use separate `i2c_algorithm` for AMD_NAVI_GPU
      i2c: designware: add support for I2C_XFER_V2 - detailed fault reporting

 Documentation/i2c/dev-interface.rst        |  46 +++++++++
 drivers/i2c/busses/i2c-designware-common.c |  23 ++++-
 drivers/i2c/busses/i2c-designware-core.h   |  14 ++-
 drivers/i2c/busses/i2c-designware-master.c | 160 ++++++++++++++++++++++++++---
 drivers/i2c/busses/i2c-nomadik.c           | 111 ++++++++++++++------
 drivers/i2c/i2c-core-base.c                | 107 +++++++++++++------
 drivers/i2c/i2c-dev.c                      |  79 +++++++++++---
 include/linux/i2c.h                        |  12 +++
 include/trace/events/i2c.h                 |   6 +-
 include/uapi/linux/i2c-dev.h               |   9 ++
 include/uapi/linux/i2c.h                   |  21 ++++
 11 files changed, 489 insertions(+), 99 deletions(-)
---
base-commit: 940de590b839f71d6dc846160534bf202401b8b7
change-id: 20260623-i2c-fault-reporting-9236c9affc2d

Best regards,
-- 
Dmitry Guzman <Dmitry.Guzman@mobileye.com>

[PATCH v2 01/12] i2c: core: add I2C_XFER_V2 - support for detailed transfer reporting

[Dmitry Guzman]

In I2C subsystem there is API that allows sending/receiving a number of
messages in a single call. I2C_RDWR ioctl, as well as i2c_transfer kernel
API function, returns only a single error code. In case of a fault,
there is no way to know which message in the series caused a fault, and
how many bytes have been sent or received before the fault.

This commit introduces i2c_transfer_v2 kernel API function and
I2C_RDWR_V2 ioctl. They provide the same functionality as the old ones,
but also accept additional pointer to `i2c_transfer_report` structure
and fill it with detailed fault report: number of messages transferred
successfully, index of message that caused fault, number of bytes
transferred (if a fault occurred in the middle of the last message).

I2C bus controller driver may implement either both callbacks or any one
of them. The implementation of both callbacks may make sense if the
precise detection of the fault position requires different handling with
the hardware that causes to extra CPU load or other consequences that
may be unwanted if the precise fault report is not required. If the
precise fault detection is free, the driver may implement only `xfer_v2`
callback - the infrastructure will provide pointer to a dummy fault
report that will be dropped if the client uses old API.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 Documentation/i2c/dev-interface.rst |  46 ++++++++++++++++
 drivers/i2c/i2c-core-base.c         | 107 +++++++++++++++++++++++++-----------
 drivers/i2c/i2c-dev.c               |  79 ++++++++++++++++++++++----
 include/linux/i2c.h                 |  12 ++++
 include/trace/events/i2c.h          |   6 +-
 include/uapi/linux/i2c-dev.h        |   9 +++
 include/uapi/linux/i2c.h            |  21 +++++++
 7 files changed, 232 insertions(+), 48 deletions(-)

diff --git a/Documentation/i2c/dev-interface.rst b/Documentation/i2c/dev-interface.rst
index c277a8e1202b51403a8d00d6c92fca13da1afc58..45a8b94f585b57889c153fbb110a5826879484f6 100644
--- a/Documentation/i2c/dev-interface.rst
+++ b/Documentation/i2c/dev-interface.rst
@@ -140,6 +140,52 @@ The following IOCTLs are defined:
   The slave address and whether to use ten bit address mode has to be
   set in each message, overriding the values set with the above ioctl's.
 
+``ioctl(file, I2C_RDWR_V2, struct i2c_rdwr_v2_ioctl_data *msgset)``
+  Does the same combined read/write transaction as I2C_RDWR, but also
+  provides detailed fault report. The argument is a pointer to a::
+
+    struct i2c_rdwr_v2_ioctl_data {
+	    struct i2c_rdwr_ioctl_data rdwr_data;
+	    struct i2c_transfer_report report;
+    };
+
+  The rdwr_data is the same structure as the argument for I2C_RDWR ioctl.
+  The report is the structure that the transfer report is written to::
+
+    struct i2c_transfer_report {
+      __s32 fault_msg_idx;
+      __s32 msgs_cplt;
+      __s32 bytes_cplt;
+    };
+
+  msgs_cplt is the number of messages that has been sent or received
+  successfully. If there are read messages within this range, the returned
+  data is guaranteed to be valid. If a message has been read from the
+  device but the read data is lost (for example, FIFO is flushed before
+  CPU read it), this message must not be counted.  If the controller cannot
+  determine the number of completed messages, the value is -EOPNOTSUPP.
+
+  fault_msg_idx is the number of message that caused a fault. In case of a
+  fault, it is not necessary equal to msgs_cplt. For example, if the driver
+  validates the whole batch before starting transmission, detects that it
+  cannot send it, it returns -EOPNOTSUPP error immediately, so msgs_cplt is 0,
+  while fault_msg_idx points to the message that cannot be sent. Another
+  example when these two value may be different is I2C controller that
+  flushes RX FIFO when an error is detected before CPU reads data from it.
+
+  If there is no fault, the fault_msg_idx value is equal to msgs_cplt.
+
+  bytes_cplt indicates the number of bytes sent/received in the message at
+  index msgs_cplt. If this is a read message, it is guaranteed that these
+  bytes in the message data buffer are valid. If the controller cannot
+  determine the byte number, the value should be -EOPNOTSUPP. If there was
+  no fault, the value should be 0.
+
+  To discover if the device supports detailed fault reporting, use I2C_RDWR_V2
+  ioctl with nmsgs = 0. If the driver supports it, the return value shall be 0.
+  If the driver supports only legacy I2C_RDWR, the return value shall be
+  -EOPNOTSUPP. In any case, nothing is done on the bus.
+
 ``ioctl(file, I2C_SMBUS, struct i2c_smbus_ioctl_data *args)``
   If possible, use the provided ``i2c_smbus_*`` methods described below instead
   of issuing direct ioctls.
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ddaacf876dada92c1ffafd2d5d1e4b83c8f0101f..a6515868f97baeacb4fffa3bdad2f1365a00dffe 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2177,15 +2177,17 @@ module_exit(i2c_exit);
 /* Check if val is exceeding the quirk IFF quirk is non 0 */
 #define i2c_quirk_exceeded(val, quirk) ((quirk) && ((val) > (quirk)))
 
-static int i2c_quirk_error(struct i2c_adapter *adap, struct i2c_msg *msg, char *err_msg)
+static struct i2c_msg *i2c_quirk_error(struct i2c_adapter *adap,
+				       struct i2c_msg *msg, char *err_msg)
 {
 	dev_err_ratelimited(&adap->dev, "adapter quirk: %s (addr 0x%04x, size %u, %s)\n",
 			    err_msg, msg->addr, msg->len,
 			    str_read_write(msg->flags & I2C_M_RD));
-	return -EOPNOTSUPP;
+	return msg;
 }
 
-static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+static struct i2c_msg *i2c_check_for_quirks(struct i2c_adapter *adap,
+					    struct i2c_msg *msgs, int num)
 {
 	const struct i2c_adapter_quirks *q = adap->quirks;
 	int max_num = q->max_num_msgs, i;
@@ -2236,31 +2238,51 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		}
 	}
 
-	return 0;
+	return NULL;
 }
 
 /**
- * __i2c_transfer - unlocked flavor of i2c_transfer
+ * __i2c_transfer_v2 - unlocked flavor of i2c_transfer_v2
  * @adap: Handle to I2C bus
  * @msgs: One or more messages to execute before STOP is issued to
  *	terminate the operation; each message begins with a START.
  * @num: Number of messages to be executed.
+ * @report: The buffer for detailed transfer report (may be NULL if not required)
  *
  * Returns negative errno, else the number of messages executed.
+ * Writes the detailed transfer report to the structure pointed by 'report'.
  *
  * Adapter lock must be held when calling this function. No debug logging
  * takes place.
  */
-int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+int __i2c_transfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		   struct i2c_transfer_report *report)
 {
+	struct i2c_transfer_report dummy_report;
 	unsigned long orig_jiffies;
 	int ret, try;
 
-	if (!adap->algo->master_xfer) {
+	if (report) {
+		report->msgs_cplt = -EOPNOTSUPP;
+		report->bytes_cplt = -EOPNOTSUPP;
+		report->fault_msg_idx = -EOPNOTSUPP;
+
+		if (!adap->algo->xfer_v2)
+			return -EOPNOTSUPP;
+	}
+
+	if (!adap->algo->master_xfer && !adap->algo->xfer_v2) {
 		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
 		return -EOPNOTSUPP;
 	}
 
+	/*
+	 * If the controller only supports "v2" callback and the report is not requested,
+	 * provide pointer to a dummy report.
+	 */
+	if (!(adap->algo->master_xfer) && (!report))
+		report = &dummy_report;
+
 	if (WARN_ON(!msgs || num < 1))
 		return -EINVAL;
 
@@ -2268,8 +2290,18 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	if (ret)
 		return ret;
 
-	if (adap->quirks && i2c_check_for_quirks(adap, msgs, num))
-		return -EOPNOTSUPP;
+	if (adap->quirks) {
+		struct i2c_msg *bad_msg = i2c_check_for_quirks(adap, msgs, num);
+
+		if (bad_msg) {
+			if (report) {
+				report->msgs_cplt = 0;
+				report->bytes_cplt = 0;
+				report->fault_msg_idx = bad_msg - msgs;
+			}
+			return -EOPNOTSUPP;
+		}
+	}
 
 	/*
 	 * i2c_trace_msg_key gets enabled when tracepoint i2c_transfer gets
@@ -2290,8 +2322,12 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	for (ret = 0, try = 0; try <= adap->retries; try++) {
 		if (i2c_in_atomic_xfer_mode() && adap->algo->master_xfer_atomic)
 			ret = adap->algo->master_xfer_atomic(adap, msgs, num);
-		else
-			ret = adap->algo->master_xfer(adap, msgs, num);
+		else {
+			if (report)
+				ret = adap->algo->xfer_v2(adap, msgs, num, report);
+			else
+				ret = adap->algo->master_xfer(adap, msgs, num);
+		}
 
 		if (ret != -EAGAIN)
 			break;
@@ -2300,58 +2336,63 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	}
 
 	if (static_branch_unlikely(&i2c_trace_msg_key)) {
-		int i;
-		for (i = 0; i < ret; i++)
+		int n;
+
+		if (report)
+			n = report->msgs_cplt;
+		else
+			n = ret;
+		for (int i = 0; i < n; i++)
 			if (msgs[i].flags & I2C_M_RD)
-				trace_i2c_reply(adap, &msgs[i], i);
+				trace_i2c_reply(adap, &msgs[i], msgs[i].len, i);
+		if (report && report->bytes_cplt > 0 && msgs[n].flags & I2C_M_RD)
+			trace_i2c_reply(adap, &msgs[n], report->bytes_cplt, n);
 		trace_i2c_result(adap, num, ret);
 	}
 
 	return ret;
 }
+EXPORT_SYMBOL(__i2c_transfer_v2);
+
+int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	return __i2c_transfer_v2(adap, msgs, num, NULL);
+}
 EXPORT_SYMBOL(__i2c_transfer);
 
 /**
- * i2c_transfer - execute a single or combined I2C message
+ * i2c_transfer_v2 - execute a single or combined I2C message
  * @adap: Handle to I2C bus
  * @msgs: One or more messages to execute before STOP is issued to
  *	terminate the operation; each message begins with a START.
  * @num: Number of messages to be executed.
+ * @report: Pointer for transmission fault report.
  *
  * Returns negative errno, else the number of messages executed.
  *
  * Note that there is no requirement that each message be sent to
  * the same slave address, although that is the most common model.
  */
-int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+int i2c_transfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		 struct i2c_transfer_report *report)
 {
 	int ret;
 
-	/* REVISIT the fault reporting model here is weak:
-	 *
-	 *  - When we get an error after receiving N bytes from a slave,
-	 *    there is no way to report "N".
-	 *
-	 *  - When we get a NAK after transmitting N bytes to a slave,
-	 *    there is no way to report "N" ... or to let the master
-	 *    continue executing the rest of this combined message, if
-	 *    that's the appropriate response.
-	 *
-	 *  - When for example "num" is two and we successfully complete
-	 *    the first message but get an error part way through the
-	 *    second, it's unclear whether that should be reported as
-	 *    one (discarding status on the second message) or errno
-	 *    (discarding status on the first one).
-	 */
 	ret = __i2c_lock_bus_helper(adap);
 	if (ret)
 		return ret;
 
-	ret = __i2c_transfer(adap, msgs, num);
+	ret = __i2c_transfer_v2(adap, msgs, num, report);
 	i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
 
 	return ret;
 }
+EXPORT_SYMBOL(i2c_transfer_v2);
+
+int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	return i2c_transfer_v2(adap, msgs, num, NULL);
+}
 EXPORT_SYMBOL(i2c_transfer);
 
 /**
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index ccaac5e29f906bec0bf3b0e4a259391053469c2f..90456e6c04b4131dde9a9c2a5bd2075dd32b4baf 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -240,12 +240,18 @@ static int i2cdev_check_addr(struct i2c_adapter *adapter, unsigned int addr)
 	return result;
 }
 
-static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
-		unsigned nmsgs, struct i2c_msg *msgs)
+static noinline int i2cdev_ioctl_rdwr_v2(struct i2c_client *client,
+				      unsigned int nmsgs, struct i2c_msg *msgs,
+				      struct i2c_transfer_report __user *user_report)
 {
+	struct i2c_transfer_report report;
 	u8 __user **data_ptrs;
 	int i, res;
 
+	report.msgs_cplt = -EOPNOTSUPP;
+	report.fault_msg_idx = -EOPNOTSUPP;
+	report.bytes_cplt = -EOPNOTSUPP;
+
 	/* Adapter must support I2C transfers */
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -EOPNOTSUPP;
@@ -259,6 +265,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		/* Limit the size of the message to a sane amount */
 		if (msgs[i].len > 8192) {
 			res = -EINVAL;
+			report.fault_msg_idx = i;
 			break;
 		}
 
@@ -266,6 +273,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		msgs[i].buf = memdup_user(data_ptrs[i], msgs[i].len);
 		if (IS_ERR(msgs[i].buf)) {
 			res = PTR_ERR(msgs[i].buf);
+			report.fault_msg_idx = i;
 			break;
 		}
 		/* memdup_user allocates with GFP_KERNEL, so DMA is ok */
@@ -289,6 +297,7 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 					     I2C_SMBUS_BLOCK_MAX) {
 				i++;
 				res = -EINVAL;
+				report.fault_msg_idx = i;
 				break;
 			}
 
@@ -303,9 +312,34 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		return res;
 	}
 
-	res = i2c_transfer(client->adapter, msgs, nmsgs);
+	if (user_report) {
+		res = i2c_transfer_v2(client->adapter, msgs, nmsgs, &report);
+		i = report.msgs_cplt;
+	} else {
+		res = i2c_transfer(client->adapter, msgs, nmsgs);
+		if (res < 0)
+			i = 0;
+		else
+			i = nmsgs;
+	}
+
+	if (user_report && copy_to_user(user_report, &report, sizeof(report)))
+		res = -EFAULT;
+
+	/* Number of messages transferred completely or partially */
+	if (report.bytes_cplt > 0) {
+		i++;
+		msgs[i].len = report.bytes_cplt;
+	}
+
+	if (i > (int)nmsgs) {
+		pr_err("Bad i2c_transfer_report: msgs_cplt = %i, bytes_cplt = %i, nmsgs = %i\n",
+		       report.msgs_cplt, report.bytes_cplt, nmsgs);
+		i = nmsgs;
+	}
+
 	while (i-- > 0) {
-		if (res >= 0 && (msgs[i].flags & I2C_M_RD)) {
+		if (msgs[i].flags & I2C_M_RD) {
 			if (copy_to_user(data_ptrs[i], msgs[i].buf,
 					 msgs[i].len))
 				res = -EFAULT;
@@ -439,18 +473,39 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		funcs = i2c_get_functionality(client->adapter);
 		return put_user(funcs, (unsigned long __user *)arg);
 
-	case I2C_RDWR: {
+	case I2C_RDWR:
+	case I2C_RDWR_V2:
+	{
+		struct i2c_rdwr_ioctl_data __user *user_arg;
+		struct i2c_transfer_report __user *user_rep;
 		struct i2c_rdwr_ioctl_data rdwr_arg;
 		struct i2c_msg *rdwr_pa;
 		int res;
 
-		if (copy_from_user(&rdwr_arg,
-				   (struct i2c_rdwr_ioctl_data __user *)arg,
-				   sizeof(rdwr_arg)))
+		if (cmd == I2C_RDWR_V2) {
+			user_arg = &((struct i2c_rdwr_v2_ioctl_data __user *)arg)->rdwr_data;
+			user_rep = &((struct i2c_rdwr_v2_ioctl_data __user *)arg)->report;
+		} else {
+			user_arg = (struct i2c_rdwr_ioctl_data __user *)arg;
+			user_rep = NULL;
+		}
+
+		if (copy_from_user(&rdwr_arg, user_arg, sizeof(rdwr_arg)))
 			return -EFAULT;
 
-		if (!rdwr_arg.msgs || rdwr_arg.nmsgs == 0)
-			return -EINVAL;
+		if (!rdwr_arg.msgs || rdwr_arg.nmsgs == 0) {
+			/*
+			 * I2C_RDWR_V2 ioctl with nmsgs == 0 is used for
+			 * discovering of the controller capability to return
+			 * detailed fault reports.
+			 */
+			if (cmd == I2C_RDWR)
+				return -EINVAL;
+			if (client->adapter->algo->xfer_v2)
+				return 0;
+			else
+				return -EOPNOTSUPP;
+		}
 
 		/*
 		 * Put an arbitrary limit on the number of messages that can
@@ -464,7 +519,7 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (IS_ERR(rdwr_pa))
 			return PTR_ERR(rdwr_pa);
 
-		res = i2cdev_ioctl_rdwr(client, rdwr_arg.nmsgs, rdwr_pa);
+		res = i2cdev_ioctl_rdwr_v2(client, rdwr_arg.nmsgs, rdwr_pa, user_rep);
 		kfree(rdwr_pa);
 		return res;
 	}
@@ -572,7 +627,7 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
 			};
 		}
 
-		res = i2cdev_ioctl_rdwr(client, rdwr_arg.nmsgs, rdwr_pa);
+		res = i2cdev_ioctl_rdwr_v2(client, rdwr_arg.nmsgs, rdwr_pa, NULL);
 		kfree(rdwr_pa);
 		return res;
 	}
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 14ab4d3055aff624cdb3fccb1dc39ae738164fb7..8ef14b326d84723850d552fbefd75df8062f5cd8 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -131,6 +131,14 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
 /* Unlocked flavor */
 int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
 
+/* Transfer with detailed transfer reporting.
+ */
+int i2c_transfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		    struct i2c_transfer_report *report);
+/* Unlocked flavor */
+int __i2c_transfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		      struct i2c_transfer_report *report);
+
 /* This is the very generalized SMBus access routine. You probably do not
    want to use this, though; one of the functions below may be much easier,
    and probably just as fast.
@@ -567,6 +575,10 @@ struct i2c_algorithm {
 				 unsigned short flags, char read_write,
 				 u8 command, int size, union i2c_smbus_data *data);
 
+	/* Same as xfer with detailed reporting */
+	int (*xfer_v2)(struct i2c_adapter *adap, struct i2c_msg *msgs,
+		       int num, struct i2c_transfer_report *report);
+
 	/* To determine what the adapter supports */
 	u32 (*functionality)(struct i2c_adapter *adap);
 
diff --git a/include/trace/events/i2c.h b/include/trace/events/i2c.h
index 142a23c6593c611de9abc2a89a146b95550b23cd..2ea8e9805edf591d63dcb589340b0704fd6d38f7 100644
--- a/include/trace/events/i2c.h
+++ b/include/trace/events/i2c.h
@@ -88,8 +88,8 @@ TRACE_EVENT_FN(i2c_read,
  */
 TRACE_EVENT_FN(i2c_reply,
 	       TP_PROTO(const struct i2c_adapter *adap, const struct i2c_msg *msg,
-			int num),
-	       TP_ARGS(adap, msg, num),
+			int data_len, int num),
+	       TP_ARGS(adap, msg, data_len, num),
 	       TP_STRUCT__entry(
 		       __field(int,	adapter_nr		)
 		       __field(__u16,	msg_nr			)
@@ -102,7 +102,7 @@ TRACE_EVENT_FN(i2c_reply,
 		       __entry->msg_nr = num;
 		       __entry->addr = msg->addr;
 		       __entry->flags = msg->flags;
-		       __entry->len = msg->len;
+		       __entry->len = data_len;
 		       memcpy(__get_dynamic_array(buf), msg->buf, msg->len);
 			      ),
 	       TP_printk("i2c-%d #%u a=%03x f=%04x l=%u [%*phD]",
diff --git a/include/uapi/linux/i2c-dev.h b/include/uapi/linux/i2c-dev.h
index 1c4cec4ddd84d739193b234d33cae7860856738e..5097568a31490e2c9c2036a7d94ab47588413beb 100644
--- a/include/uapi/linux/i2c-dev.h
+++ b/include/uapi/linux/i2c-dev.h
@@ -11,11 +11,13 @@
 
 #include <linux/types.h>
 #include <linux/compiler.h>
+#include <linux/i2c.h>
 
 /* /dev/i2c-X ioctl commands.  The ioctl's parameter is always an
  * unsigned long, except for:
  *	- I2C_FUNCS, takes pointer to an unsigned long
  *	- I2C_RDWR, takes pointer to struct i2c_rdwr_ioctl_data
+ *	- I2C_RDWR_V2, takes pointer to struct i2c_rdwr_v2_ioctl_data
  *	- I2C_SMBUS, takes pointer to struct i2c_smbus_ioctl_data
  */
 #define I2C_RETRIES	0x0701	/* number of times a device address should
@@ -33,6 +35,7 @@
 #define I2C_FUNCS	0x0705	/* Get the adapter functionality mask */
 
 #define I2C_RDWR	0x0707	/* Combined R/W transfer (one STOP only) */
+#define I2C_RDWR_V2	0x0709  /* I2C_RDWR with detailed fault reporting */
 
 #define I2C_PEC		0x0708	/* != 0 to use PEC with SMBus */
 #define I2C_SMBUS	0x0720	/* SMBus transfer */
@@ -52,6 +55,12 @@ struct i2c_rdwr_ioctl_data {
 	__u32 nmsgs;			/* number of i2c_msgs */
 };
 
+/* This is the structure as used in the I2C_RDWR_V2 ioctl call */
+struct i2c_rdwr_v2_ioctl_data {
+	struct i2c_rdwr_ioctl_data rdwr_data;
+	struct i2c_transfer_report report;
+};
+
 #define  I2C_RDWR_IOCTL_MAX_MSGS	42
 /* Originally defined with a typo, keep it for compatibility */
 #define  I2C_RDRW_IOCTL_MAX_MSGS	I2C_RDWR_IOCTL_MAX_MSGS
diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index 2a226657d9f8238365453121321fd70dc11dac02..5e8e7d3536c85f2fe604a285258b070f2efffbb2 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -135,6 +135,27 @@ struct i2c_msg {
 					 I2C_FUNC_SMBUS_READ_BLOCK_DATA | \
 					 I2C_FUNC_SMBUS_BLOCK_PROC_CALL)
 
+/* Detailed transfer report */
+
+struct i2c_transfer_report {
+	__s32 fault_msg_idx;		/* In case of a fault, index of the message that caused
+					 * the fault. If the bus driver cannot determine it, it
+					 * puts a negative error code. If there is no fault, the
+					 * value is equal to number of messages transferred.
+					 */
+	__s32 msgs_cplt;		/* Number of messages that are known to be transferred
+					 * successfully. If the bus driver cannot determine it, it
+					 * puts a negative error code. If there is no fault, the
+					 * value is equal to number of messages transferred.
+					 */
+	__s32 bytes_cplt;		/* In case of a fault, number of bytes in the message at
+					 * index `msgs_cplt` that are known to be transferred
+					 * successfully. If the bus driver cannot determine the
+					 * number of bytes, it puts a negative error value.
+					 * If there is no fault, the value is 0.
+					 */
+};
+
 /*
  * Data for SMBus Messages
  */

-- 
2.43.0

[PATCH v2 02/12] i2c: nomadik: optimize struct layout, change unsigned char into u8

[Dmitry Guzman]

Change type of `tft` and `rft` fields of `nmk_i2c_dev` structure from
`unsigned char` to `u8` and `buffer` field of `i2c_nmk_client` structure
from `unsigned char *` to `u8 *`.

Put two bool variables `xfer_done` and `has_32b_bus` and two u8 variables
`tft` and `rft` together in order to reduce struct size wasted for padding.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index c3878670da9cfa462b66919ddc58b55cc78db6e5..45b4815b094524d9a8de69376e2933171541611a 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -170,7 +170,7 @@ enum i2c_operating_mode {
 struct i2c_nmk_client {
 	unsigned short		slave_adr;
 	unsigned long		count;
-	unsigned char		*buffer;
+	u8			*buffer;
 	unsigned long		xfer_bytes;
 	enum i2c_operation	operation;
 };
@@ -187,13 +187,13 @@ struct i2c_nmk_client {
  * @clk_freq: clock frequency for the operation mode
  * @tft: Tx FIFO Threshold in bytes
  * @rft: Rx FIFO Threshold in bytes
+ * @xfer_done: xfer done boolean.
+ * @has_32b_bus: controller is on a bus that only supports 32-bit accesses.
  * @timeout_usecs: Slave response timeout
  * @sm: speed mode
  * @stop: stop condition.
  * @xfer_wq: xfer done wait queue.
- * @xfer_done: xfer done boolean.
  * @result: controller propogated result.
- * @has_32b_bus: controller is on a bus that only supports 32-bit accesses.
  */
 struct nmk_i2c_dev {
 	struct i2c_vendor_data		*vendor;
@@ -204,15 +204,15 @@ struct nmk_i2c_dev {
 	struct clk			*clk;
 	struct i2c_nmk_client		cli;
 	u32				clk_freq;
-	unsigned char			tft;
-	unsigned char			rft;
+	u8				tft;
+	u8				rft;
+	bool				xfer_done;
+	bool				has_32b_bus;
 	u32				timeout_usecs;
 	enum i2c_freq_mode		sm;
 	int				stop;
 	struct wait_queue_head		xfer_wq;
-	bool				xfer_done;
 	int				result;
-	bool				has_32b_bus;
 };
 
 /* controller's abort causes */

-- 
2.43.0

[PATCH v2 03/12] i2c: nomadik: do not try to retransmit I2C message series on errors

[Dmitry Guzman]

i2c-nomadik driver of I2C bus controller in `xfer` callback retransmits
the whole message series in cause of any fault, and returns fault only
after third failed attempt. This behavior contradicts with API because
not only it hides hardware faults, but also re-sends messages, while
they are not guaranteed to be idempotent.

Remove the triple attempt to send messages in `xfer` callback.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 45b4815b094524d9a8de69376e2933171541611a..ca9366e09e7125b9c86dd0f995162018df56634d 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -716,27 +716,21 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	int status = 0;
 	int i;
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
-	int j;
 
 	pm_runtime_get_sync(&priv->adev->dev);
 
-	/* Attempt three times to send the message queue */
-	for (j = 0; j < 3; j++) {
-		/* setup the i2c controller */
-		setup_i2c_controller(priv);
-
-		for (i = 0; i < num_msgs; i++) {
-			priv->cli.slave_adr	= msgs[i].addr;
-			priv->cli.buffer		= msgs[i].buf;
-			priv->cli.count		= msgs[i].len;
-			priv->stop = (i < (num_msgs - 1)) ? 0 : 1;
-			priv->result = 0;
-
-			status = nmk_i2c_xfer_one(priv, msgs[i].flags);
-			if (status != 0)
-				break;
-		}
-		if (status == 0)
+	/* setup the i2c controller */
+	setup_i2c_controller(priv);
+
+	for (i = 0; i < num_msgs; i++) {
+		priv->cli.slave_adr	= msgs[i].addr;
+		priv->cli.buffer		= msgs[i].buf;
+		priv->cli.count		= msgs[i].len;
+		priv->stop = (i < (num_msgs - 1)) ? 0 : 1;
+		priv->result = 0;
+
+		status = nmk_i2c_xfer_one(priv, msgs[i].flags);
+		if (status != 0)
 			break;
 	}
 

-- 
2.43.0

[PATCH v2 04/12] i2c: nomadik: return proper fault codes

[Dmitry Guzman]

I2C documentation Documentation/i2c/fault-codes.rst defines fault codes
for different negative results in I2C transmittion. Previously,
i2c-nomadik driver didn't implement them properly - it returned
ETIMEDOUT on most errors and EIO on master arbitration lost.

To comply with the documentation, return the proper fault codes for
different conditions, namely:

- EAGAIN if arbitration lost
- EOVERFLOW if message is too long (>2047 bytes)
- ENXIO if target address is not acknowledged
- EIO on other errors detected by controller (for example, NACK on data)
- ETIMEDOUT if driver gets timeout waiting for message completion
  without any fault condition detected by the controller (for example,
too long message, or SDA/SCL line stuck on 0).

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index ca9366e09e7125b9c86dd0f995162018df56634d..a3cef4b66190a9e45898e25867a3ceefce5d3d0a 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -226,6 +226,18 @@ static const char *abort_causes[] = {
 	"overflow, maxsize is 2047 bytes",
 };
 
+/* Linux fault codes for controller abort causes */
+static int fault_codes[] = {
+	ENXIO,
+	EIO,
+	EIO,
+	EAGAIN,
+	EIO,
+	EIO,
+	EOVERFLOW,
+	EIO
+};
+
 static inline void i2c_set_bit(void __iomem *reg, u32 mask)
 {
 	writel(readl(reg) | mask, reg);
@@ -653,6 +665,8 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 				cause >= ARRAY_SIZE(abort_causes) ?
 				"unknown reason" :
 				abort_causes[cause]);
+			priv->result = -fault_codes[cause];
+			status = priv->result;
 		}
 
 		init_hw(priv);
@@ -865,7 +879,7 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 
 	/* Master Arbitration lost interrupt */
 	case I2C_IT_MAL:
-		priv->result = -EIO;
+		priv->result = -EAGAIN;
 		init_hw(priv);
 
 		i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_MAL);

-- 
2.43.0

[PATCH v2 05/12] i2c: nomadik: change print level for fault messages to debug

[Dmitry Guzman]

i2c-nomadik driver prints error message on every faulted message. This
is not a good practice, because in I2C a fault not always is an error,
sometimes it is the expected result. For example, scanning bus with
`i2cdetects` prints over 100 messages in dmesg (two messages per each
target address).

To avoid excessive prints in the log, change the print level from err to
debug.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index a3cef4b66190a9e45898e25867a3ceefce5d3d0a..9c7658148bd45ecdaef1f998558315580233555d 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -627,7 +627,7 @@ static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
 
 	if (!xfer_done) {
 		/* Controller timed out */
-		dev_err(&priv->adev->dev, "write to slave 0x%x timed out\n",
+		dev_dbg(&priv->adev->dev, "write to slave 0x%x timed out\n",
 			priv->cli.slave_adr);
 		status = -ETIMEDOUT;
 	}
@@ -661,7 +661,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 		i2c_sr = readl(priv->virtbase + I2C_SR);
 		if (FIELD_GET(I2C_SR_STATUS, i2c_sr) == I2C_ABORT) {
 			cause = FIELD_GET(I2C_SR_CAUSE, i2c_sr);
-			dev_err(&priv->adev->dev, "%s\n",
+			dev_dbg(&priv->adev->dev, "%s\n",
 				cause >= ARRAY_SIZE(abort_causes) ?
 				"unknown reason" :
 				abort_causes[cause]);

-- 
2.43.0

[PATCH v2 06/12] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read

[Dmitry Guzman]

In Nomadik I2c controller, register I2C_MCR has 11-bit wide LENGTH
field. Its maximum value is 2047, so this is the maximum length of a
single message. It is less than the common maximum I2C message length in
I2C subsystem (8192), so define a quirk in order to report the
unsupported message without any attempt to transfer it.

Zero length reading doesn't work properly on this controller, so add
`I2C_AQ_NO_ZERO_LEN_READ` quirk flag.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 9c7658148bd45ecdaef1f998558315580233555d..c36e9961e0a39e76386a6e20f6e0b6fec3d5b5a9 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -79,6 +79,9 @@
 #define I2C_MCR_STOP		BIT(14)		/* Stop condition */
 #define I2C_MCR_LENGTH		GENMASK(25, 15)	/* Transaction length */
 
+/* Controller hardware limitation of the message length */
+#define I2C_MAX_MSG_LENGTH	(I2C_MCR_LENGTH >> 15)
+
 /* Status register (SR) */
 #define I2C_SR_OP		GENMASK(1, 0)	/* Operation */
 #define I2C_SR_STATUS		GENMASK(3, 2)	/* controller status */
@@ -238,6 +241,12 @@ static int fault_codes[] = {
 	EIO
 };
 
+static const struct i2c_adapter_quirks nmk_i2c_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+	.max_read_len  = I2C_MAX_MSG_LENGTH,
+	.max_write_len = I2C_MAX_MSG_LENGTH,
+};
+
 static inline void i2c_set_bit(void __iomem *reg, u32 mask)
 {
 	writel(readl(reg) | mask, reg);
@@ -1161,6 +1170,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	adap->class = I2C_CLASS_DEPRECATED;
 	adap->algo = &nmk_i2c_algo;
 	adap->timeout = usecs_to_jiffies(priv->timeout_usecs);
+	adap->quirks = &nmk_i2c_quirks;
 	snprintf(adap->name, sizeof(adap->name),
 		 "Nomadik I2C at %pR", &adev->res);
 

-- 
2.43.0

[PATCH v2 07/12] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting

[Dmitry Guzman]

I2C_XFER_V2 is a new API that allows I2C clients to get the detailed
report in case of transmission failure. Previously, the only information
returned by I2C bus controller was the error code; there was no way to
find out how many messages or bytes in a certain message have been sent
or received until the fault condition occurred.

This commit introduces support of this feature in i2c-nomadik driver.

Acked-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index c36e9961e0a39e76386a6e20f6e0b6fec3d5b5a9..8b0ab191f7c1fc120f81319c1b1689193e2b9ea2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -197,6 +197,7 @@ struct i2c_nmk_client {
  * @stop: stop condition.
  * @xfer_wq: xfer done wait queue.
  * @result: controller propogated result.
+ * @bytes_cplt: number of bytes completed in the message that caused a fault.
  */
 struct nmk_i2c_dev {
 	struct i2c_vendor_data		*vendor;
@@ -216,6 +217,7 @@ struct nmk_i2c_dev {
 	int				stop;
 	struct wait_queue_head		xfer_wq;
 	int				result;
+	int				bytes_cplt;
 };
 
 /* controller's abort causes */
@@ -529,6 +531,8 @@ static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
 	int status = 0;
 	bool xfer_done;
 
+	priv->cli.xfer_bytes = 0;
+
 	mcr = load_i2c_mcr_reg(priv, flags);
 	writel(mcr, priv->virtbase + I2C_MCR);
 
@@ -653,6 +657,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 {
 	int status;
 
+	priv->bytes_cplt = 0;
 	if (flags & I2C_M_RD) {
 		/* read operation */
 		priv->cli.operation = I2C_READ;
@@ -678,6 +683,16 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 			status = priv->result;
 		}
 
+		if (flags & I2C_M_RD) {
+			/* For READ messages, return the number of bytes read from FIFO */
+			priv->bytes_cplt = priv->cli.xfer_bytes;
+		} else {
+			/* For WRITE messages, return the number of bytes sent on bus */
+			priv->bytes_cplt = FIELD_GET(I2C_SR_LENGTH, i2c_sr);
+			/* LENGTH value includes the last byte that has not been sent or ACKed */
+			if (priv->bytes_cplt > 0)
+				priv->bytes_cplt--;
+		}
 		init_hw(priv);
 
 		status = status ? status : priv->result;
@@ -687,10 +702,11 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 }
 
 /**
- * nmk_i2c_xfer() - I2C transfer function used by kernel framework
+ * nmk_i2c_xfer_v2() - I2C transfer function used by kernel framework
  * @i2c_adap: Adapter pointer to the controller
  * @msgs: Pointer to data to be written.
  * @num_msgs: Number of messages to be executed
+ * @report: Pointer to transfer report to be written.
  *
  * This is the function called by the generic kernel i2c_transfer()
  * or i2c_smbus...() API calls. Note that this code is protected by the
@@ -733,14 +749,16 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
  * please use the i2c_smbus_read_i2c_block_data()
  * or i2c_smbus_write_i2c_block_data() API
  */
-static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
-		struct i2c_msg msgs[], int num_msgs)
+static int nmk_i2c_xfer_v2(struct i2c_adapter *i2c_adap,
+		struct i2c_msg msgs[], int num_msgs,
+		struct i2c_transfer_report *report)
 {
 	int status = 0;
 	int i;
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
 
 	pm_runtime_get_sync(&priv->adev->dev);
+	priv->bytes_cplt = 0;
 
 	/* setup the i2c controller */
 	setup_i2c_controller(priv);
@@ -760,10 +778,17 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	pm_runtime_put_sync(&priv->adev->dev);
 
 	/* return the no. messages processed */
-	if (status)
+	if (status) {
+		report->msgs_cplt = i;
+		report->bytes_cplt = priv->bytes_cplt;
+		report->fault_msg_idx = i;
 		return status;
-	else
+	} else {
+		report->msgs_cplt = num_msgs;
+		report->bytes_cplt = 0;
+		report->fault_msg_idx = num_msgs;
 		return num_msgs;
+	}
 }
 
 /**
@@ -1014,7 +1039,7 @@ static unsigned int nmk_i2c_functionality(struct i2c_adapter *adap)
 }
 
 static const struct i2c_algorithm nmk_i2c_algo = {
-	.xfer = nmk_i2c_xfer,
+	.xfer_v2 = nmk_i2c_xfer_v2,
 	.functionality = nmk_i2c_functionality
 };
 

-- 
2.43.0

[PATCH v2 08/12] i2c: designware: return proper fault codes

[Dmitry Guzman]

I2C documentation Documentation/i2c/fault-codes.rst defines fault codes
for different negative results in I2C transmittion. Previously,
i2c-designware driver didn't implement them properly - it returned
EREMOTEIO on NACK either on address or data phase instead of ENXIO and
EIO respectively, and EINVAL instead of EOPNOTSUPP if a message cannot be
sent due to controller hardware limitations.

To comply with the documentation, return the proper fault codes for
different conditions, namely:

- EOPNOTSUPP if the controller cannot transfer the message sequence
  requested (for example, target address change without STOP condition)
- ENXIO if target address is not acknowledged
- EIO on other faults detected by controller (for example, NACK on data)

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 8 ++++++--
 drivers/i2c/busses/i2c-designware-core.h   | 3 +--
 drivers/i2c/busses/i2c-designware-master.c | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index a1eca6cd4b75e9a76f2fc10afba8877d2b45417d..e934bcbe766042483874b3e43d10ffff30c59e04 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -776,11 +776,15 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	unsigned long abort_source = dev->abort_source;
 	int i;
 
-	if (abort_source & DW_IC_TX_ABRT_NOACK) {
+	if (abort_source & (DW_IC_TX_ABRT_ADDR_NOACK | DW_IC_TX_ABRT_TXDATA_NOACK)) {
 		for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
 			dev_dbg(dev->dev,
 				"%s: %s\n", __func__, abort_sources[i]);
-		return -EREMOTEIO;
+
+		if (abort_source & DW_IC_TX_ABRT_TXDATA_NOACK)
+			return -EIO;
+		else
+			return -ENXIO;
 	}
 
 	for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 2c929a6e8da2a35b3b2d8ad886e326e2df285927..025311c8662c612aaaf772fb223af38278217c90 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -112,10 +112,9 @@
 #define DW_IC_RX_ABRT_SLAVE_ARBLOST		BIT(ABRT_SLAVE_ARBLOST)
 #define DW_IC_RX_ABRT_SLAVE_FLUSH_TXFIFO	BIT(ABRT_SLAVE_FLUSH_TXFIFO)
 
-#define DW_IC_TX_ABRT_NOACK			(DW_IC_TX_ABRT_7B_ADDR_NOACK | \
+#define DW_IC_TX_ABRT_ADDR_NOACK		(DW_IC_TX_ABRT_7B_ADDR_NOACK | \
 						 DW_IC_TX_ABRT_10ADDR1_NOACK | \
 						 DW_IC_TX_ABRT_10ADDR2_NOACK | \
-						 DW_IC_TX_ABRT_TXDATA_NOACK | \
 						 DW_IC_TX_ABRT_GCALL_NOACK)
 
 struct clk;
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index a1bcc3797e4ffef0edb9d3e978eab20a3c09d6aa..d10f46cf4aa44344fa1c53b9b8798a8b0f0c193c 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -894,7 +894,7 @@ i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
 		 */
 		for (cnt = 1; ; cnt++) {
 			if (!i2c_dw_msg_is_valid(dev, msgs_part, cnt - 1)) {
-				ret = -EINVAL;
+				ret = -EOPNOTSUPP;
 				break;
 			}
 

-- 
2.43.0

[PATCH v2 09/12] i2c: designware: no SMBUS_READ_BLOCK_DATA without EMPTYFIFO_HOLD_MASTER

[Dmitry Guzman]

Controllers without EMPTYFIFO_HOLD_MASTER feature cannot hold bus with
empty FIFO. This makes impossible to issue SMBUS block read command:
it is not known how many bytes must be read until it reads the  first byte
indicates the data length is read. Without EMPTYFIFO_HOLD_MASTER, when
the first byte is read, TX FIFO gets empty, this causes STOP condition
and the transfer is terminated.

Reset SMBUS_READ_BLOCK_DATA bit in functionality mask on such controllers.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-designware-master.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index d10f46cf4aa44344fa1c53b9b8798a8b0f0c193c..76f28e1696bdc81326cf149dfe88c52dd2f0731f 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -940,6 +940,13 @@ void i2c_dw_configure_master(struct dw_i2c_dev *dev)
 	if ((dev->flags & MODEL_MASK) != MODEL_AMD_NAVI_GPU)
 		dev->functionality |= I2C_FUNC_PROTOCOL_MANGLING;
 
+	/* Controllers without EMPTYFIFO_HOLD_MASTER feature cannot issue SMBUS
+	 * block read because after reading the data length FIFO gets empty and
+	 * STOP is generated automatically
+	 */
+	if (!dev->emptyfifo_hold_master)
+		dev->functionality &= ~I2C_FUNC_SMBUS_READ_BLOCK_DATA;
+
 	dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 			  DW_IC_CON_RESTART_EN;
 

-- 
2.43.0

[PATCH v2 10/12] i2c: designware: stop transfer if spurious STOP is detected

[Dmitry Guzman]

In `i2c_dw_process_transfer()`, if STOP_DET flag is set while read or
write is in progress, the transfer should be aborted. If this flag is
handled the same interrupt as TX_EMPTY flag, the driver first handles
TX_EMPTY flag and transmits the following messages, and only afterwards
checks STOP_DET flag.

Also, if STOP_DET flag is set, interrupts are not masked, so if TX_EMPTY
interrupts arrives later, the driver will also send further messages.

To abort transmitting data after spurious STOP, check STOP_DET flag
before TX_EMPTY flag, and if STOP_DET flag is set while transfer is in
progress, disable interrupts as it is done in case of TX_ABRT flag set.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-designware-master.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 76f28e1696bdc81326cf149dfe88c52dd2f0731f..936b9150754e85a1bac2d17a0fa65f6963e4ef3e 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -651,17 +651,19 @@ static void i2c_dw_process_transfer(struct dw_i2c_dev *dev, unsigned int stat)
 	if (stat & DW_IC_INTR_RX_FULL)
 		i2c_dw_read(dev);
 
-	if (stat & DW_IC_INTR_TX_EMPTY)
-		i2c_dw_xfer_msg(dev);
-
 	/* Abort if we detect a STOP in the middle of a read or a write */
 	if ((stat & DW_IC_INTR_STOP_DET) &&
 	    (dev->status & (STATUS_READ_IN_PROGRESS | STATUS_WRITE_IN_PROGRESS))) {
 		dev_err(dev->dev, "spurious STOP detected\n");
 		dev->rx_outstanding = 0;
 		dev->msg_err = -EIO;
+		__i2c_dw_write_intr_mask(dev, 0);
+		goto tx_aborted;
 	}
 
+	if (stat & DW_IC_INTR_TX_EMPTY)
+		i2c_dw_xfer_msg(dev);
+
 	/*
 	 * No need to modify or disable the interrupt mask here.
 	 * i2c_dw_xfer_msg() will take care of it according to

-- 
2.43.0

[PATCH v2 11/12] i2c: designware: use separate `i2c_algorithm` for AMD_NAVI_GPU

[Dmitry Guzman]

AMD_NAVI_GPU variant of designware I2C controller uses its own
implementation of `xfer` method. Instead of checking flags in runtime,
create a separate `i2c_algorithm` structure and put pointer to
`amd_i2c_dw_xfer_quirk` directly into it.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 14 +++++++++++++-
 drivers/i2c/busses/i2c-designware-core.h   |  1 +
 drivers/i2c/busses/i2c-designware-master.c |  6 ++----
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e934bcbe766042483874b3e43d10ffff30c59e04..4274bf809b3f4bff90139d186447256b01317951 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -886,6 +886,15 @@ static const struct i2c_algorithm i2c_dw_algo = {
 #endif
 };
 
+static const struct i2c_algorithm i2c_dw_amd_navi_gpu_algo = {
+	.xfer = amd_i2c_dw_xfer_quirk,
+	.functionality = i2c_dw_func,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_slave = i2c_dw_reg_slave,
+	.unreg_slave = i2c_dw_unreg_slave,
+#endif
+};
+
 static const struct i2c_adapter_quirks i2c_dw_quirks = {
 	.flags = I2C_AQ_NO_ZERO_LEN,
 };
@@ -922,7 +931,10 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 		strscpy(adap->name, "Synopsys DesignWare I2C adapter");
 
 	adap->retries = 3;
-	adap->algo = &i2c_dw_algo;
+	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
+		adap->algo = &i2c_dw_amd_navi_gpu_algo;
+	else
+		adap->algo = &i2c_dw_algo;
 	adap->quirks = &i2c_dw_quirks;
 	adap->dev.parent = dev->dev;
 	i2c_set_adapdata(adap, dev);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 025311c8662c612aaaf772fb223af38278217c90..9f3492717e18fe104d307cfc65293ae4b41251f0 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -316,6 +316,7 @@ extern void i2c_dw_configure_master(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
 
 int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
+int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs, int num_msgs);
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 extern void i2c_dw_configure_slave(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 936b9150754e85a1bac2d17a0fa65f6963e4ef3e..f029aa1eadbf83d304fc73b038e92f258e2148a1 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -291,8 +291,9 @@ static int i2c_dw_status(struct dw_i2c_dev *dev)
  * Initiate and continue master read/write transaction with polling
  * based transfer routine afterward write messages into the Tx buffer.
  */
-static int amd_i2c_dw_xfer_quirk(struct dw_i2c_dev *dev, struct i2c_msg *msgs, int num_msgs)
+int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs, int num_msgs)
 {
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	int msg_wrt_idx, msg_itr_lmt, buf_len, data_idx;
 	int cmd = 0, status;
 	u8 *tx_buf;
@@ -926,9 +927,6 @@ int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 
-	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
-		return amd_i2c_dw_xfer_quirk(dev, msgs, num);
-
 	return i2c_dw_xfer_common(dev, msgs, num);
 }
 

-- 
2.43.0

[PATCH v2 12/12] i2c: designware: add support for I2C_XFER_V2 - detailed fault reporting

[Dmitry Guzman]

I2C_XFER_V2 is a new API that allows I2C clients to get the detailed
report in case of transmission failure. Previously, the only information
returned by I2C bus controller was the error code; there was no way to
find out how many messages or bytes in a certain message have been sent
or received until the fault condition occurred, or to receive the data
read before the fault.

This patch introduces support for this feature in i2c-designware driver.
For AMD_NAVI_GPU model that has its own `xfer` callback function, this
feature is currently not implemented.

The driver uses FLUSH_CNT field of ABORT_SOURCE register to detect
how many command/data bytes written to FIFO were not actually sent.

Also, if the detailed fault report is required, the driver checks
status after each read/write from/to FIFO. If ABORT flag is set, the
driver cannot detect what happened first - FIFO read/write or
communication abort, so it assumes that abort happened first, otherwise
it either falsely reports one extra byte sent or returns the byte read
from FIFO flushed after abort as a valid read byte. So, in unfortunate
conditions, the fault report may be too pessimistic by one byte.

This check increases amount of time spent in interrupt handler, so if
client doesn't need detailed fault report, this check should be avoided.
Because of this, both `xfer` and `xfer_v2` callbacks are implemented in
struct i2c_algorithm: the former does not generate detailed fault
report, while the latter does.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-designware-common.c |   1 +
 drivers/i2c/busses/i2c-designware-core.h   |  10 +++
 drivers/i2c/busses/i2c-designware-master.c | 139 +++++++++++++++++++++++++++--
 3 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4274bf809b3f4bff90139d186447256b01317951..42fc5989c917686a89802d7ef027a6d89aea6213 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -879,6 +879,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
 static const struct i2c_algorithm i2c_dw_algo = {
 	.xfer = i2c_dw_xfer,
+	.xfer_v2 = i2c_dw_xfer_v2,
 	.functionality = i2c_dw_func,
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	.reg_slave = i2c_dw_reg_slave,
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9f3492717e18fe104d307cfc65293ae4b41251f0..108fc1d1132c0aa91723de83198204a0dde42484 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -117,6 +117,8 @@
 						 DW_IC_TX_ABRT_10ADDR2_NOACK | \
 						 DW_IC_TX_ABRT_GCALL_NOACK)
 
+#define DW_IC_TX_ABRT_SOURCE_FLUSH_CNT_MASK	GENMASK(31, 23)
+
 struct clk;
 struct device;
 struct reset_control;
@@ -180,6 +182,9 @@ struct reset_control;
  *	to generate the high period and low period of SCL line.
  * @emptyfifo_hold_master: true if the controller acting as master holds
  *	the clock when the Tx FIFO is empty instead of emitting a stop.
+ * @need_precise_report: true if client needs precise fault report
+ * @bytes_written: number of bytes written to FIFO. Used only for precise
+	fault report.
  *
  * HCNT and LCNT parameters can be used if the platform knows more accurate
  * values than the one computed based only on the input clock frequency.
@@ -239,6 +244,8 @@ struct dw_i2c_dev {
 	u32			bus_capacitance_pF;
 	bool			clk_freq_optimized;
 	bool			emptyfifo_hold_master;
+	bool			need_precise_report;
+	u32			bytes_written;
 };
 
 #define ACCESS_INTR_MASK			BIT(0)
@@ -318,6 +325,9 @@ extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
 int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
 int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs, int num_msgs);
 
+int i2c_dw_xfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		   struct i2c_transfer_report *report);
+
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 extern void i2c_dw_configure_slave(struct dw_i2c_dev *dev);
 irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index f029aa1eadbf83d304fc73b038e92f258e2148a1..bc23c3fdaaf2e0629e02016aef428cc9f8bffda2 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -11,6 +11,7 @@
 
 #define DEFAULT_SYMBOL_NAMESPACE	"I2C_DW"
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/errno.h>
@@ -33,6 +34,16 @@
 #define AMD_TIMEOUT_MAX_US	250
 #define AMD_MASTERCFG_MASK	GENMASK(15, 0)
 
+static int i2c_dw_fault_report(struct dw_i2c_dev *dev, struct i2c_transfer_report *report);
+
+static inline bool i2c_dw_check_abort_flag(struct dw_i2c_dev *dev)
+{
+	u32 stat;
+
+	regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
+	return (!!(stat & DW_IC_INTR_TX_ABRT));
+}
+
 static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 {
 	unsigned int comp_param1;
@@ -198,6 +209,8 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 
 	i2c_dw_set_mode(dev, DW_IC_MASTER);
 
+	dev->rx_buf_len = 0;
+
 	/* If the slave address is ten bit address, enable 10BITADDR */
 	if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
 		ic_con = DW_IC_CON_10BITADDR_MASTER;
@@ -451,6 +464,21 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 					     cmd | *buf++);
 			}
 			tx_limit--; buf_len--;
+
+			/* If precise fault reporting is required, check if the transfer
+			 * is aborted after writing each byte.
+			 * Otherwise, if it is aborted during filling FIFO, there is no way
+			 * to know how many bytes was written to FIFO after transfer abort
+			 * and thus are not counted in FLUSH_CNT.
+			 * If we are checking abort flag after each byte, we can lose only 1 byte.
+			 */
+			if (dev->need_precise_report) {
+				if (i2c_dw_check_abort_flag(dev)) {
+					dev_warn_ratelimited(dev->dev, "Transfer aborted during FIFO writing. Report may be imprecise.");
+					break;
+				}
+				dev->bytes_written++;
+			}
 		}
 
 		dev->tx_buf = buf;
@@ -542,6 +570,13 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
 			regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+			if (dev->need_precise_report) {
+				if (i2c_dw_check_abort_flag(dev)) {
+					dev_warn_ratelimited(dev->dev, "Transfer aborted during FIFO reading. Data byte may be lost");
+					/* It is unknown if the read byte is valid. Drop it. */
+					break;
+				}
+			}
 			tmp &= DW_IC_DATA_CMD_DAT;
 			/* Ensure length byte is a valid value */
 			if (flags & I2C_M_RECV_LEN) {
@@ -568,8 +603,10 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 			dev->rx_buf_len = len;
 			dev->rx_buf = buf;
 			return;
-		} else
+		} else {
 			dev->status &= ~STATUS_READ_IN_PROGRESS;
+			dev->rx_buf_len = 0;
+		}
 	}
 }
 
@@ -765,6 +802,7 @@ __i2c_dw_xfer_one_part(struct dw_i2c_dev *dev, struct i2c_msg *msgs, size_t num)
 	dev->status = 0;
 	dev->abort_source = 0;
 	dev->rx_outstanding = 0;
+	dev->bytes_written = 0;
 
 	ret = i2c_dw_wait_bus_not_busy(dev);
 	if (ret < 0)
@@ -868,12 +906,25 @@ i2c_dw_msg_is_valid(struct dw_i2c_dev *dev, const struct i2c_msg *msgs, size_t i
 }
 
 static int
-i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
+i2c_dw_xfer_common_v2(struct dw_i2c_dev *dev, struct i2c_msg *msgs, int num,
+		      struct i2c_transfer_report *report)
 {
 	struct i2c_msg *msgs_part;
-	size_t cnt;
+	int msgs_in_prev_parts = 0;
+	size_t cnt = 0;
 	int ret;
 
+	if (!report) {
+		dev->need_precise_report = false;
+	} else {
+		dev->need_precise_report = true;
+		report->msgs_cplt = -EOPNOTSUPP;
+		report->bytes_cplt = -EOPNOTSUPP;
+		report->fault_msg_idx = -EOPNOTSUPP;
+	}
+
+	dev->msg_read_idx = 0;
+
 	dev_dbg(dev->dev, "msgs: %d\n", num);
 
 	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev->dev, pm);
@@ -889,6 +940,8 @@ i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
 	 * we do one transaction for each part up to the STOP.
 	 */
 	for (msgs_part = msgs; msgs_part < msgs + num; msgs_part += cnt) {
+		/* Count previously transferred messages*/
+		msgs_in_prev_parts += cnt;
 		/*
 		 * Count the messages in a transaction, up to a STOP or
 		 * the end of the msgs. The last if below guarantees that
@@ -897,6 +950,15 @@ i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
 		 */
 		for (cnt = 1; ; cnt++) {
 			if (!i2c_dw_msg_is_valid(dev, msgs_part, cnt - 1)) {
+				if (report) {
+					report->fault_msg_idx = msgs_in_prev_parts + cnt - 1;
+					report->msgs_cplt = msgs_in_prev_parts;
+					report->bytes_cplt = 0;
+					/* Reset report pointer to avoid
+					 * calling i2c_dw_fault_report later
+					 */
+					report = NULL;
+				}
 				ret = -EOPNOTSUPP;
 				break;
 			}
@@ -918,16 +980,83 @@ i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
 
 	i2c_dw_release_lock(dev);
 
-	if (ret < 0)
+	if (ret < 0) {
+		if (report) {
+			i2c_dw_fault_report(dev, report);
+			report->msgs_cplt += msgs_in_prev_parts;
+			report->fault_msg_idx += msgs_in_prev_parts;
+		}
 		return ret;
+	}
+	if (report) {
+		report->msgs_cplt = num;
+		report->fault_msg_idx = num;
+		report->bytes_cplt = 0;
+	}
+
 	return num;
 }
 
+int i2c_dw_xfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
+		   struct i2c_transfer_report *report)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
+	return i2c_dw_xfer_common_v2(dev, msgs, num, report);
+}
+
 int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 
-	return i2c_dw_xfer_common(dev, msgs, num);
+	return i2c_dw_xfer_common_v2(dev, msgs, num, NULL);
+}
+
+static int i2c_dw_fault_report(struct dw_i2c_dev *dev, struct i2c_transfer_report *report)
+{
+	int idx;
+	int n_flushed = FIELD_GET(DW_IC_TX_ABRT_SOURCE_FLUSH_CNT_MASK, dev->abort_source);
+
+	report->msgs_cplt = -1;
+
+	if (n_flushed <= dev->bytes_written)
+		dev->bytes_written -= n_flushed;
+	else
+		dev->bytes_written = 0;
+
+	/* The last byte that the transmission was interrupted on is not counted as "flushed".
+	 * We should not reported it as transferred successfully, so decrement the counter.
+	 */
+	if (dev->bytes_written)
+		dev->bytes_written--;
+
+	for (idx = 0; idx < dev->msg_write_idx; idx++) {
+		if (dev->msgs[idx].len <= dev->bytes_written)
+			dev->bytes_written -= dev->msgs[idx].len;
+		else
+			break;
+	}
+
+	report->fault_msg_idx = idx;
+	for (int i = dev->msg_read_idx; i <= idx && i < dev->msgs_num; i++) {
+		if (dev->msgs[i].flags & I2C_M_RD) {
+			report->msgs_cplt = i;
+			if (!(dev->rx_buf_len))
+				report->bytes_cplt = 0;
+			else
+				report->bytes_cplt = dev->rx_buf - dev->msgs[i].buf;
+			if ((i < idx) || (report->bytes_cplt < dev->bytes_written))
+				dev_warn_ratelimited(dev->dev, "Read data lost due to FIFO flush");
+			break;
+		}
+	}
+
+	if (report->msgs_cplt < 0) {
+		report->msgs_cplt = idx;
+		report->bytes_cplt = dev->bytes_written;
+	}
+
+	return idx;
 }
 
 void i2c_dw_configure_master(struct dw_i2c_dev *dev)

-- 
2.43.0

Re: [PATCH v2 01/12] i2c: core: add I2C_XFER_V2 - support for detailed transfer reporting

[Andy Shevchenko]

On Thu, Sep 03, 2026 at 08:38:08AM +0300, Dmitry Guzman wrote:
> In I2C subsystem there is API that allows sending/receiving a number of
> messages in a single call. I2C_RDWR ioctl, as well as i2c_transfer kernel
> API function, returns only a single error code. In case of a fault,
> there is no way to know which message in the series caused a fault, and
> how many bytes have been sent or received before the fault.
> 
> This commit introduces i2c_transfer_v2 kernel API function and
> I2C_RDWR_V2 ioctl. They provide the same functionality as the old ones,
> but also accept additional pointer to `i2c_transfer_report` structure
> and fill it with detailed fault report: number of messages transferred
> successfully, index of message that caused fault, number of bytes
> transferred (if a fault occurred in the middle of the last message).
> 
> I2C bus controller driver may implement either both callbacks or any one
> of them. The implementation of both callbacks may make sense if the
> precise detection of the fault position requires different handling with
> the hardware that causes to extra CPU load or other consequences that
> may be unwanted if the precise fault report is not required. If the
> precise fault detection is free, the driver may implement only `xfer_v2`
> callback - the infrastructure will provide pointer to a dummy fault
> report that will be dropped if the client uses old API.

What tool do you use?
The problem with the submission is that:
- it has no cover letter
- it has been send with In-Reply-To set to the previous version of the set.

I have noticed the same issue in another patch series which is heavily relies
on AI. You should read the coding-assistants.rst and act accordingly.

...

> -int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +int __i2c_transfer_v2(struct i2c_adapter *adap, struct i2c_msg *msgs, int num,
> +		   struct i2c_transfer_report *report)
>  {
> +	struct i2c_transfer_report dummy_report;
>  	unsigned long orig_jiffies;
>  	int ret, try;
>  
> -	if (!adap->algo->master_xfer) {
> +	if (report) {
> +		report->msgs_cplt = -EOPNOTSUPP;
> +		report->bytes_cplt = -EOPNOTSUPP;
> +		report->fault_msg_idx = -EOPNOTSUPP;

Why all three?! Why even a single one as long as we return an error code?

> +		if (!adap->algo->xfer_v2)
> +			return -EOPNOTSUPP;
> +	}
> +
> +	if (!adap->algo->master_xfer && !adap->algo->xfer_v2) {
>  		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/*
> +	 * If the controller only supports "v2" callback and the report is not requested,
> +	 * provide pointer to a dummy report.
> +	 */
> +	if (!(adap->algo->master_xfer) && (!report))

It's not a line in the macro.

> +		report = &dummy_report;


> +	if (adap->quirks) {
> +		struct i2c_msg *bad_msg = i2c_check_for_quirks(adap, msgs, num);
> +
> +		if (bad_msg) {

This style is bad for maintenance. Whenever you need to validate something,
never assign it in the definition.

> +			if (report) {
> +				report->msgs_cplt = 0;
> +				report->bytes_cplt = 0;
> +				report->fault_msg_idx = bad_msg - msgs;
> +			}
> +			return -EOPNOTSUPP;
> +		}
> +	}

...

On a brief look this patch made with AI and it's a complete unreviewable mess.
Make sure you do a series of a small logically finished pieces. For example,
introducing a v2 of a hook with a new prototype. This can be done in a separate
patch. Adding report is another, and so on...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 09/12] i2c: designware: no SMBUS_READ_BLOCK_DATA without EMPTYFIFO_HOLD_MASTER

[Andy Shevchenko]

On Thu, Sep 03, 2026 at 08:38:16AM +0300, Dmitry Guzman wrote:
> Controllers without EMPTYFIFO_HOLD_MASTER feature cannot hold bus with
> empty FIFO. This makes impossible to issue SMBUS block read command:
> it is not known how many bytes must be read until it reads the  first byte

Too many spaces.

> indicates the data length is read. Without EMPTYFIFO_HOLD_MASTER, when
> the first byte is read, TX FIFO gets empty, this causes STOP condition
> and the transfer is terminated.
> 
> Reset SMBUS_READ_BLOCK_DATA bit in functionality mask on such controllers.

...

> +	/* Controllers without EMPTYFIFO_HOLD_MASTER feature cannot issue SMBUS
> +	 * block read because after reading the data length FIFO gets empty and
> +	 * STOP is generated automatically
> +	 */

/*
 * Please, use the regular style for the
 * multi-line comments. This reply can be used
 * as an example.
 */

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 12/12] i2c: designware: add support for I2C_XFER_V2 - detailed fault reporting

[Andy Shevchenko]

On Thu, Sep 03, 2026 at 08:38:19AM +0300, Dmitry Guzman wrote:
> I2C_XFER_V2 is a new API that allows I2C clients to get the detailed
> report in case of transmission failure. Previously, the only information
> returned by I2C bus controller was the error code; there was no way to
> find out how many messages or bytes in a certain message have been sent
> or received until the fault condition occurred, or to receive the data
> read before the fault.
> 
> This patch introduces support for this feature in i2c-designware driver.
> For AMD_NAVI_GPU model that has its own `xfer` callback function, this
> feature is currently not implemented.
> 
> The driver uses FLUSH_CNT field of ABORT_SOURCE register to detect
> how many command/data bytes written to FIFO were not actually sent.
> 
> Also, if the detailed fault report is required, the driver checks
> status after each read/write from/to FIFO. If ABORT flag is set, the
> driver cannot detect what happened first - FIFO read/write or
> communication abort, so it assumes that abort happened first, otherwise
> it either falsely reports one extra byte sent or returns the byte read
> from FIFO flushed after abort as a valid read byte. So, in unfortunate
> conditions, the fault report may be too pessimistic by one byte.
> 
> This check increases amount of time spent in interrupt handler, so if
> client doesn't need detailed fault report, this check should be avoided.
> Because of this, both `xfer` and `xfer_v2` callbacks are implemented in
> struct i2c_algorithm: the former does not generate detailed fault
> report, while the latter does.

...

> +static int i2c_dw_fault_report(struct dw_i2c_dev *dev, struct i2c_transfer_report *report);

Why forward declaration?

> +static inline bool i2c_dw_check_abort_flag(struct dw_i2c_dev *dev)

Why 'inline'?

> +{
> +	u32 stat;
> +
> +	regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
> +	return (!!(stat & DW_IC_INTR_TX_ABRT));

Not a macro style.

> +}

...

> +			/* If precise fault reporting is required, check if the transfer
> +			 * is aborted after writing each byte.
> +			 * Otherwise, if it is aborted during filling FIFO, there is no way
> +			 * to know how many bytes was written to FIFO after transfer abort
> +			 * and thus are not counted in FLUSH_CNT.
> +			 * If we are checking abort flag after each byte, we can lose only 1 byte.
> +			 */

Style of the comments...

...

> +	if (!report) {

Use positive conditional instead.

> +		dev->need_precise_report = false;
> +	} else {
> +		dev->need_precise_report = true;
> +		report->msgs_cplt = -EOPNOTSUPP;
> +		report->bytes_cplt = -EOPNOTSUPP;
> +		report->fault_msg_idx = -EOPNOTSUPP;
> +	}

Will each driver use this? Needs a helper with explanation of the defaults.
Also note, the helper itself can be made in a separate patch.

...

> +					/* Reset report pointer to avoid
> +					 * calling i2c_dw_fault_report later
> +					 */

Style of the comment, also respect English punctuation and kernel standard
references, for instance the function can be referred as func().

...

> +	if (ret < 0) {
> +		if (report) {
> +			i2c_dw_fault_report(dev, report);
> +			report->msgs_cplt += msgs_in_prev_parts;
> +			report->fault_msg_idx += msgs_in_prev_parts;
> +		}
>  		return ret;
> +	}
> +	if (report) {
> +		report->msgs_cplt = num;
> +		report->fault_msg_idx = num;
> +		report->bytes_cplt = 0;
> +	}

If we going to have many lines like these you need to introduce an I²C report
handling APIs instead of these. This will help a lot. Currently this bulk of
stuff hard to review and follow.

...

> +	int idx;
> +	int n_flushed = FIELD_GET(DW_IC_TX_ABRT_SOURCE_FLUSH_CNT_MASK, dev->abort_source);

Reversed xmas tree order.

...

> +	for (int i = dev->msg_read_idx; i <= idx && i < dev->msgs_num; i++) {

Why is 'i' signed?

> +		if (dev->msgs[i].flags & I2C_M_RD) {
> +			report->msgs_cplt = i;
> +			if (!(dev->rx_buf_len))
> +				report->bytes_cplt = 0;
> +			else
> +				report->bytes_cplt = dev->rx_buf - dev->msgs[i].buf;
> +			if ((i < idx) || (report->bytes_cplt < dev->bytes_written))
> +				dev_warn_ratelimited(dev->dev, "Read data lost due to FIFO flush");
> +			break;
> +		}
> +	}

-- 
With Best Regards,
Andy Shevchenko