* [PATCH 0/7] I2C - detailed transfer reporting in case of a fault
@ 2026-06-23 16:31 Dmitry Guzman
2026-06-23 16:31 ` [PATCH 1/7] i2c: add I2C_XFER_V2 - support for detailed transfer reporting Dmitry Guzman
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw)
To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Linus Walleij
Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel,
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.
In this patchset, the introduced functionality is implemented in
`i2c-nomadik` driver. Several other improvements in this driver related
to fault handling are also included in this patchset. It has been tested
on EyeQ6H-EPM6 board.
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`.
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
Dmitry Guzman (7):
i2c: add I2C_XFER_V2 - support for detailed transfer reporting
i2c: nomadik: optimize layout of struct nmk_i2c_dev
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
Documentation/i2c/dev-interface.rst | 46 ++++++++++++++++
drivers/i2c/busses/i2c-nomadik.c | 105 ++++++++++++++++++++++++-----------
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 +++++++
8 files changed, 306 insertions(+), 79 deletions(-)
---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
change-id: 20260623-i2c-fault-reporting-9236c9affc2d
Best regards,
--
Dmitry Guzman <Dmitry.Guzman@mobileye.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/7] i2c: add I2C_XFER_V2 - support for detailed transfer reporting 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-23 16:31 ` [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev Dmitry Guzman ` (5 subsequent siblings) 6 siblings, 0 replies; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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 3ec04787a7373f113a15ee3fb35db425ae470427..c3694618b94fbdfd79a71d7cbd8d7c69c9638a17 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -2170,15 +2170,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; @@ -2229,31 +2231,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; @@ -2261,8 +2283,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 @@ -2283,8 +2315,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; @@ -2293,58 +2329,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 20fd41b51d5c85ee1665395c07345faafd8e2fca..0305d4daa157c27d700f31c15faf0c3984114ce0 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman 2026-06-23 16:31 ` [PATCH 1/7] i2c: add I2C_XFER_V2 - support for detailed transfer reporting Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:36 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors Dmitry Guzman ` (4 subsequent siblings) 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, Dmitry Guzman Put two bool variables `xfer_done` and `has_32b_bus` and two char 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c index b63ee51c1652080e414f4302bee16905914c1288..e4e5c6943c66144058fba857d7bf6c0be79ed5bd 100644 --- a/drivers/i2c/busses/i2c-nomadik.c +++ b/drivers/i2c/busses/i2c-nomadik.c @@ -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; @@ -206,13 +206,13 @@ struct nmk_i2c_dev { u32 clk_freq; unsigned char tft; unsigned char 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev 2026-06-23 16:31 ` [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev Dmitry Guzman @ 2026-06-24 22:36 ` Linus Walleij 2026-07-01 13:46 ` Dmitry Guzman 0 siblings, 1 reply; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:36 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel Hi Dmitry, thanks for your patch! Also nice to see some kernel contributions directly from MobilEye! On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote: > Put two bool variables `xfer_done` and `has_32b_bus` and two char > variables `tft` and `rft` together in order to reduce struct size > wasted for padding. > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> (...) > struct nmk_i2c_dev { > struct i2c_vendor_data *vendor; > @@ -206,13 +206,13 @@ struct nmk_i2c_dev { > u32 clk_freq; > unsigned char tft; > unsigned char rft; ^ Maybe you want to take the opportunity to change these two into u8 if you're anyway changing the layout of this struct? Either way: Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev 2026-06-24 22:36 ` Linus Walleij @ 2026-07-01 13:46 ` Dmitry Guzman 0 siblings, 0 replies; 15+ messages in thread From: Dmitry Guzman @ 2026-07-01 13:46 UTC (permalink / raw) To: Linus Walleij Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Thu, 25 Jun 2026 00:36:49 +0200 Linus Walleij <linusw@kernel.org> wrote: > Hi Dmitry, > > thanks for your patch! > > Also nice to see some kernel contributions directly from > MobilEye! Thanks for you review! > > struct nmk_i2c_dev { > > struct i2c_vendor_data *vendor; > > @@ -206,13 +206,13 @@ struct nmk_i2c_dev { > > u32 clk_freq; > > unsigned char tft; > > unsigned char rft; > > ^ > Maybe you want to take the opportunity to change these > two into u8 if you're anyway changing the layout of this > struct? I'm waiting for review of patch 1 in the set. If I need to submit next version of the patchset, I'll change these two unsigned chars, as well as `unsigned char *buffer` in `struct i2c_nmk_client`, into u8. Best Regards, -- Dmitry Guzman <Dmitry.Guzman@mobileye.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman 2026-06-23 16:31 ` [PATCH 1/7] i2c: add I2C_XFER_V2 - support for detailed transfer reporting Dmitry Guzman 2026-06-23 16:31 ` [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:51 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 4/7] i2c: nomadik: return proper fault codes Dmitry Guzman ` (3 subsequent siblings) 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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. 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 e4e5c6943c66144058fba857d7bf6c0be79ed5bd..3eb06988c026e5c573fcf55d83de7136b5ca7ac9 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors 2026-06-23 16:31 ` [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors Dmitry Guzman @ 2026-06-24 22:51 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:51 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote: > 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. > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> This originally came from: commit ebd10e0783d9fb92a147e60902e22c2d3f3ad69d Author: Virupax Sadashivpetimath <virupax.sadashivpetimath@stericsson.com> Date: Fri May 13 12:30:23 2011 +0200 i2c-nomadik: add code to retry on timeout failure It is seen that i2c-nomadik controller randomly stops generating the interrupts leading to a i2c timeout. As a workaround to this problem, add retries to the on going transfer on failure. Signed-off-by: Virupax Sadashivpetimath <virupax.sadashivpetimath@stericsson.com> Reviewed-by: Jonas ABERG <jonas.aberg@stericsson.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Ben Dooks <ben-linux@fluff.org> At that time the code looked very different: for (j = 0; j < 3; j++) { if (status || (dev->result)) { (...) break; } udelay(I2C_DELAY); } if (status == 0) break; We would only spin here if both status and dev->result (the number of sent bytes) was 0. This doesn't seem to be at all the case anymore! I suppose it's a bit dubious code, so: Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/7] i2c: nomadik: return proper fault codes 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman ` (2 preceding siblings ...) 2026-06-23 16:31 ` [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:53 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug Dmitry Guzman ` (2 subsequent siblings) 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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). 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 3eb06988c026e5c573fcf55d83de7136b5ca7ac9..e19ace904e79cd2d83171d9f38fc103a6c5e023b 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] i2c: nomadik: return proper fault codes 2026-06-23 16:31 ` [PATCH 4/7] i2c: nomadik: return proper fault codes Dmitry Guzman @ 2026-06-24 22:53 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:53 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote: > 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). > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman ` (3 preceding siblings ...) 2026-06-23 16:31 ` [PATCH 4/7] i2c: nomadik: return proper fault codes Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:54 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read Dmitry Guzman 2026-06-23 16:31 ` [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting Dmitry Guzman 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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. 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 e19ace904e79cd2d83171d9f38fc103a6c5e023b..7d93fb3876dc1324003dd19884e3fd5cbba9cfbb 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug 2026-06-23 16:31 ` [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug Dmitry Guzman @ 2026-06-24 22:54 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:54 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote: > 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. > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman ` (4 preceding siblings ...) 2026-06-23 16:31 ` [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:55 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting Dmitry Guzman 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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. 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 7d93fb3876dc1324003dd19884e3fd5cbba9cfbb..9cff0c2757fafeaf809395e02a5e754570f65e08 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); @@ -1162,6 +1171,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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read 2026-06-23 16:31 ` [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read Dmitry Guzman @ 2026-06-24 22:55 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:55 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote: > 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. > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> Excellent improvements, almost a Fixes: patch. Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman ` (5 preceding siblings ...) 2026-06-23 16:31 ` [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read Dmitry Guzman @ 2026-06-23 16:31 ` Dmitry Guzman 2026-06-24 22:56 ` Linus Walleij 6 siblings, 1 reply; 15+ messages in thread From: Dmitry Guzman @ 2026-06-23 16:31 UTC (permalink / raw) To: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Linus Walleij Cc: linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel, 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. 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 9cff0c2757fafeaf809395e02a5e754570f65e08..1cf03d634fdc856dc335a58597e0fd31ab077078 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting 2026-06-23 16:31 ` [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting Dmitry Guzman @ 2026-06-24 22:56 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2026-06-24 22:56 UTC (permalink / raw) To: Dmitry Guzman Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman <Dmitry.Guzman@mobileye.com> 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. > > This commit introduces support of this feature in i2c-nomadik driver. > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com> I don't fully understand patch 1 but if that is fine, this is fine: Acked-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-07-01 14:14 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-23 16:31 [PATCH 0/7] I2C - detailed transfer reporting in case of a fault Dmitry Guzman 2026-06-23 16:31 ` [PATCH 1/7] i2c: add I2C_XFER_V2 - support for detailed transfer reporting Dmitry Guzman 2026-06-23 16:31 ` [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev Dmitry Guzman 2026-06-24 22:36 ` Linus Walleij 2026-07-01 13:46 ` Dmitry Guzman 2026-06-23 16:31 ` [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors Dmitry Guzman 2026-06-24 22:51 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 4/7] i2c: nomadik: return proper fault codes Dmitry Guzman 2026-06-24 22:53 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug Dmitry Guzman 2026-06-24 22:54 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read Dmitry Guzman 2026-06-24 22:55 ` Linus Walleij 2026-06-23 16:31 ` [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting Dmitry Guzman 2026-06-24 22:56 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox