From e7755b5f29ccbfd1222dd52dc2d6eebf39d02289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Antinori?= Date: Mon, 19 Oct 2026 12:23:15 -0300 Subject: [PATCH 1/8] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the `TryFrom<&device::Device>` implementation for `&I2cClient` since it has had no users since its introduction. Fixes: 57c5bd9aee94 ("rust: i2c: add basic I2C device and driver abstractions") Reported-by: Georgios Androutsopoulos Closes: https://github.com/Rust-for-Linux/linux/issues/1264 Signed-off-by: Nicolás Antinori Signed-off-by: Igor Korotin --- rust/kernel/i2c.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs index 0487bae811fb..b4ff25c830fc 100644 --- a/rust/kernel/i2c.rs +++ b/rust/kernel/i2c.rs @@ -5,7 +5,6 @@ // I2C Driver abstractions. use crate::{ acpi, - container_of, device, device_id::{ RawDeviceId, @@ -517,26 +516,6 @@ fn as_ref(&self) -> &device::Device { } } -impl TryFrom<&device::Device> for &I2cClient { - type Error = kernel::error::Error; - - fn try_from(dev: &device::Device) -> Result { - // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a - // `struct device`. - if unsafe { bindings::i2c_verify_client(dev.as_raw()).is_null() } { - return Err(EINVAL); - } - - // SAFETY: We've just verified that the type of `dev` equals to - // `bindings::i2c_client_type`, hence `dev` must be embedded in a valid - // `struct i2c_client` as guaranteed by the corresponding C code. - let idev = unsafe { container_of!(dev.as_raw(), bindings::i2c_client, dev) }; - - // SAFETY: `idev` is a valid pointer to a `struct i2c_client`. - Ok(unsafe { &*idev.cast() }) - } -} - // SAFETY: A `I2cClient` is always reference-counted and can be released from any thread. unsafe impl Send for I2cClient {} From 6d49edd9a348b2d6549dac420152d30aeb970953 Mon Sep 17 00:00:00 2001 From: Aniket Randive Date: Fri, 11 Sep 2026 12:29:51 +0530 Subject: [PATCH 2/8] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts The transfer timeout for an I2C controller should reflect the actual message length and bus frequency rather than a static 1-second value. A static timeout causes unnecessary delays on error paths for short messages, and may be insufficient for very long transfers. Add i2c_update_timeout() to i2c-core which computes a transfer-specific timeout and stores it directly in the standard adap->timeout field. The formula accounts for 9 bits per byte (8 data + 1 ACK) at the configured bus frequency. The caller supplies a safety multiplier and a minimum floor so that each driver retains full control over its timing policy without those values becoming public API. Storing the result in adap->timeout makes it visible to all consumers of that field, including the arbitration-loss retry loop in __i2c_transfer(). The function is gated by CONFIG_I2C_DYNAMIC_TIMEOUT. When the config is disabled, i2c_update_timeout() compiles to a no-op inline stub so drivers that call it build cleanly and the existing static 1-second default is preserved unchanged. A timeout explicitly configured by userspace via the I2C_TIMEOUT ioctl is stored in a new adap->user_timeout field and always takes precedence over the kernel-computed value. When userspace has not configured a timeout, the computed value is used. The ioctl keeps writing adap->timeout as well, so adapters that never call i2c_update_timeout() continue to honour it exactly as before. As i2c_update_timeout() is an exported helper, guard against a zero bus frequency from a misbehaving caller with WARN_ON_ONCE() and return early, leaving the existing timeout untouched as a safe fallback rather than dividing by zero. Signed-off-by: Aniket Randive Reviewed-by: Mukesh Kumar Savaliya Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20260911-master-v9-1-77ac458344e2@oss.qualcomm.com --- drivers/i2c/Kconfig | 13 +++++++++++ drivers/i2c/i2c-core-base.c | 43 +++++++++++++++++++++++++++++++++++++ drivers/i2c/i2c-dev.c | 3 +++ include/linux/i2c.h | 13 +++++++++++ 4 files changed, 72 insertions(+) diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index c232054fddd6..1d9691ec0e7e 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -136,6 +136,19 @@ config I2C_SLAVE_TESTUNIT endif +config I2C_DYNAMIC_TIMEOUT + bool "Dynamic per-transfer timeout based on message length" + depends on I2C + help + When enabled, the I2C core computes a per-transfer timeout from the + message length and bus frequency instead of using a static 1-second + default. A timeout explicitly configured via the I2C_TIMEOUT userspace + interface always takes precedence over the computed value. + + When disabled, the existing static 1-second timeout is preserved. + + If unsure, say N. + config I2C_DEBUG_CORE bool "I2C Core debugging messages" help diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 852d6a08518c..165ec14260c8 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -2010,6 +2011,48 @@ void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_de } EXPORT_SYMBOL_GPL(i2c_parse_fw_timings); +#ifdef CONFIG_I2C_DYNAMIC_TIMEOUT +/** + * i2c_update_timeout - compute and set a dynamic transfer timeout on an adapter + * @adap: the i2c_adapter whose timeout field will be updated + * @bus_freq_hz: I2C bus clock frequency in Hz + * @len: transfer length in bytes + * @safety_coeff: multiplier applied over the theoretical wire time + * @min_usec: minimum timeout floor in microseconds + * + * Computes a transfer-specific timeout from the message length and bus + * frequency, applies a safety multiplier and a minimum floor, then stores + * the result in adap->timeout (in jiffies). The caller supplies the policy + * constants so they remain internal to the driver. + * + * A timeout explicitly configured by userspace via the I2C_TIMEOUT ioctl + * always takes precedence; the computed value is used only when userspace + * has not configured one. + */ +void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz, + size_t len, unsigned int safety_coeff, + unsigned int min_usec) +{ + u64 bit_usec, total_usec; + unsigned long jiffies_val; + + if (adap->user_timeout > 0) { + adap->timeout = adap->user_timeout; + return; + } + + if (WARN_ON_ONCE(!bus_freq_hz)) + return; + + bit_usec = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz); + total_usec = bit_usec * safety_coeff + min_usec; + + jiffies_val = usecs_to_jiffies((unsigned int)min_t(u64, total_usec, UINT_MAX)); + adap->timeout = (int)min_t(unsigned long, jiffies_val, INT_MAX); +} +EXPORT_SYMBOL_GPL(i2c_update_timeout); +#endif /* CONFIG_I2C_DYNAMIC_TIMEOUT */ + /* ------------------------------------------------------------------------- */ int i2c_for_each_dev(void *data, int (*fn)(struct device *dev, void *data)) diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c index ccaac5e29f90..d219eb89f978 100644 --- a/drivers/i2c/i2c-dev.c +++ b/drivers/i2c/i2c-dev.c @@ -495,6 +495,9 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return -EINVAL; client->adapter->timeout = msecs_to_jiffies(arg * 10); +#ifdef CONFIG_I2C_DYNAMIC_TIMEOUT + client->adapter->user_timeout = client->adapter->timeout; +#endif break; default: /* NOTE: returning a fault code here could cause trouble diff --git a/include/linux/i2c.h b/include/linux/i2c.h index f3192ab1982e..6a847fc88d25 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -757,6 +757,9 @@ struct i2c_adapter { struct rt_mutex mux_lock; int timeout; /* in jiffies */ +#ifdef CONFIG_I2C_DYNAMIC_TIMEOUT + int user_timeout; /* I2C_TIMEOUT ioctl value in jiffies */ +#endif int retries; struct device dev; /* the adapter device */ unsigned long locked_flags; /* owned by the I2C core */ @@ -928,6 +931,16 @@ unsigned int i2c_adapter_depth(struct i2c_adapter *adapter); void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults); +#ifdef CONFIG_I2C_DYNAMIC_TIMEOUT +void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz, + size_t len, unsigned int safety_coeff, + unsigned int min_usec); +#else +static inline void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz, + size_t len, unsigned int safety_coeff, + unsigned int min_usec) {} +#endif + /* Return the functionality mask */ static inline u32 i2c_get_functionality(struct i2c_adapter *adap) { From 02d9f7b8324d3a09b01c9a4c0ab0e6846e6691dc Mon Sep 17 00:00:00 2001 From: Aniket Randive Date: Fri, 11 Sep 2026 12:29:52 +0530 Subject: [PATCH 3/8] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency The driver uses a static XFER_TIMEOUT of HZ (1 second) for all transfers regardless of message length or bus frequency, causing unnecessary delays on error paths. Use i2c_update_timeout() API to calculate transfer timeouts dynamically based on message length and bus frequency. update the timeout per message for FIFO, SE-DMA and GPI single descriptor transfers. For GPI multi descriptor transfers, calculate the timeout from the combined length of all messages, as completion is reported only after the entire batch has finished. A 10x safety margin over the theoretical wire time is applied, with a 300ms floor to account for I2C clock stretching and other situations where a slave may keep SCL asserted for an extended period, including faulty devices holding the bus. Signed-off-by: Aniket Randive Reviewed-by: Mukesh Kumar Savaliya Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20260911-master-v9-2-77ac458344e2@oss.qualcomm.com --- drivers/i2c/busses/i2c-qcom-geni.c | 48 ++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index a431e98723ef..50f4fc133085 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -79,9 +79,14 @@ enum geni_i2c_err_code { #define ABORT_TIMEOUT HZ #define CANCEL_TIMEOUT HZ -#define XFER_TIMEOUT HZ #define RST_TIMEOUT HZ +/* 9 bits per byte (8 data + 1 ACK), 10x safety margin */ +#define I2C_TIMEOUT_SAFETY_COEFFICIENT 10 + +/* 300ms floor: budget for clock stretching; slave may hold SCL low indefinitely */ +#define I2C_TIMEOUT_MIN_USEC 300000 + #define GENI_SE_CLK_32MHZ (32 * HZ_PER_MHZ) #define GENI_SE_CLK_19P2MHZ 19200000UL @@ -545,7 +550,10 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, } cur = gi2c->cur; - time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); + i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, len, + I2C_TIMEOUT_SAFETY_COEFFICIENT, + I2C_TIMEOUT_MIN_USEC); + time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout); if (!time_left || (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err)) geni_i2c_cancel_xfer(gi2c); @@ -587,7 +595,10 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, writel_relaxed(1, se->base + SE_GENI_TX_WATERMARK_REG); cur = gi2c->cur; - time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); + i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, len, + I2C_TIMEOUT_SAFETY_COEFFICIENT, + I2C_TIMEOUT_MIN_USEC); + time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout); if (!time_left || (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err)) geni_i2c_cancel_xfer(gi2c); @@ -665,7 +676,7 @@ static void geni_i2c_gpi_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_ * geni_i2c_gpi_multi_xfer_timeout_handler() - Handles multi message transfer timeout * @dev: Pointer to the corresponding dev node * @multi_xfer: Pointer to the geni_i2c_gpi_multi_desc_xfer - * @transfer_timeout_msecs: Timeout value in milliseconds + * @timeout_jiffies: Per-message completion timeout in jiffies * @transfer_comp: Completion object of the transfer * * This function waits for the completion of each processed transfer messages @@ -675,18 +686,18 @@ static void geni_i2c_gpi_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_ */ static int geni_i2c_gpi_multi_xfer_timeout_handler(struct device *dev, struct geni_i2c_gpi_multi_desc_xfer *multi_xfer, - u32 transfer_timeout_msecs, + unsigned long timeout_jiffies, struct completion *transfer_comp) { int i; - u32 time_left; + unsigned long time_left; for (i = 0; i < multi_xfer->msg_idx_cnt - 1; i++) { reinit_completion(transfer_comp); if (multi_xfer->msg_idx_cnt != multi_xfer->irq_cnt) { time_left = wait_for_completion_timeout(transfer_comp, - transfer_timeout_msecs); + timeout_jiffies); if (!time_left) { dev_err(dev, "%s: Transfer timeout\n", __func__); return -ETIMEDOUT; @@ -810,8 +821,24 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], dma_async_issue_pending(gi2c->tx_c); if ((msg_idx == (gi2c->num_msgs - 1)) || flags & DMA_PREP_INTERRUPT) { + size_t total_len = 0; + int j; + + /* + * All TREs except the last carry the BEI bit, so a single + * completion interrupt fires only after the entire batch has + * drained on the wire. The timeout budget must therefore cover + * the combined wire time of every message in the batch. + */ + for (j = 0; j < gi2c->num_msgs; j++) + total_len += msgs[j].len; + + i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, total_len, + I2C_TIMEOUT_SAFETY_COEFFICIENT, + I2C_TIMEOUT_MIN_USEC); ret = geni_i2c_gpi_multi_xfer_timeout_handler(gi2c->se.dev, gi2c_gpi_xfer, - XFER_TIMEOUT, &gi2c->done); + gi2c->adap.timeout, + &gi2c->done); if (ret) { dev_err(gi2c->se.dev, "I2C multi write msg transfer timeout: %d\n", @@ -931,7 +958,10 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i if (!gi2c->is_tx_multi_desc_xfer) { dma_async_issue_pending(gi2c->tx_c); - time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); + i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, msgs[i].len, + I2C_TIMEOUT_SAFETY_COEFFICIENT, + I2C_TIMEOUT_MIN_USEC); + time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout); if (!time_left) { dev_err(gi2c->se.dev, "%s:I2C timeout\n", __func__); gi2c->err = -ETIMEDOUT; From a65917d0a22f9b10694853b725f9cd399fc3ef4d Mon Sep 17 00:00:00 2001 From: Ma Ke Date: Mon, 12 Oct 2026 09:34:21 +0800 Subject: [PATCH 4/8] i2c: acpi: Fix adapter reference leak in i2c_acpi_new_device_by_fwnode() i2c_acpi_new_device_by_fwnode() obtains an i2c_adapter through i2c_acpi_find_adapter_by_handle(). That helper uses bus_find_device() internally, so it returns the adapter with a reference held: adapter = i2c_verify_adapter(dev); if (!adapter) put_device(dev); return adapter; The reference is only put on the failure path inside that helper. The caller owns it otherwise. i2c_acpi_new_device_by_fwnode() never drops it, neither on the -EPROBE_DEFER path nor after i2c_new_client_device(), so every call leaks one reference on the adapter and the adapter can never be released. i2c_new_client_device() does not consume that reference. It registers the new client with the adapter as its parent, and device_add() takes its own reference on the parent, which device_del() releases when the client goes away. The reference obtained by the lookup is a separate one and has to be dropped here. i2c_acpi_notify() in the same file already does exactly this. It takes the adapter from the same helper, registers the client through i2c_acpi_register_device(), which calls i2c_new_client_device(), and then puts the reference. i2c_new_client_device() gives the client its own reference on the adapter, so ours can be dropped unconditionally. Found by code review. Fixes: 605f8fc22442 ("i2c: core: Add new i2c_acpi_new_device helper function") Signed-off-by: Ma Ke Cc: # v4.12+ Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20261012013421.3597430-1-make_ruc2021@163.com --- drivers/i2c/i2c-core-acpi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index 703588e9990a..84841ed13487 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -549,6 +549,7 @@ struct i2c_client *i2c_acpi_new_device_by_fwnode(struct fwnode_handle *fwnode, { struct i2c_acpi_lookup lookup; struct i2c_adapter *adapter; + struct i2c_client *client; struct acpi_device *adev; LIST_HEAD(resource_list); int ret; @@ -576,7 +577,10 @@ struct i2c_client *i2c_acpi_new_device_by_fwnode(struct fwnode_handle *fwnode, if (!adapter) return ERR_PTR(-EPROBE_DEFER); - return i2c_new_client_device(adapter, info); + client = i2c_new_client_device(adapter, info); + put_device(&adapter->dev); + + return client; } EXPORT_SYMBOL_GPL(i2c_acpi_new_device_by_fwnode); From 73dae605cad7921800677d982bfaeb81cc930dec Mon Sep 17 00:00:00 2001 From: Navon John Lukose Date: Mon, 21 Sep 2026 04:30:17 +0530 Subject: [PATCH 5/8] i2c: designware: queue only one read command for block reads i2c_dw_xfer_msg() pre-queues up to rx_fifo_depth read commands based on the initial msg.len. For I2C_M_RECV_LEN transfers the real length is unknown until the first byte arrives and i2c_dw_recv_len() runs. When a large initial length is passed, the surplus pre-queued reads leave rx_outstanding non-zero after i2c_dw_recv_len() adjusts msg.len, so the completion condition is never met and the transfer times out. In a loop, this repeatedly times out the controller and disrupts unrelated driver-bound devices on the same adapter. Queue only one read command while I2C_M_RECV_LEN is pending. i2c_dw_recv_len() re-enables TX_EMPTY, and the rest are queued once the device-reported length is known. Fixes: c3ae106050b9 ("i2c: designware: Implement support for SMBus block read and write") Signed-off-by: Navon John Lukose Cc: # v4.10+ Acked-by: Mika Westerberg Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-designware-master.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index f4ad20799045..7e1d9a2e7733 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -441,6 +441,14 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev) if (dev->rx_outstanding >= dev->rx_fifo_depth) break; + /* + * For block reads, queue only the length + * byte until i2c_dw_recv_len() runs. + */ + if ((flags & I2C_M_RECV_LEN) && + buf_len < msgs[dev->msg_write_idx].len) + break; + regmap_write(dev->map, DW_IC_DATA_CMD, cmd | 0x100); rx_limit--; From b7fd977fec0d3f2bdd4f13bb59949c817a365652 Mon Sep 17 00:00:00 2001 From: Christian Marangi Date: Thu, 22 Oct 2026 21:38:51 +0200 Subject: [PATCH 6/8] i2c: mt7621: fix copying uninitialized kernel stack with reduced page_len When a page_len is less than 5 bytes in mtk_i2c_xfer, uninitialized kernel stack data is copied and written to REG_SM0D1_REG. Equally on the read part, an unnecessary read is done. Guard these extra copy/write and read for the page_len to fix leaking uninitialized stack memory. Fixes: d04913ec5f89 ("i2c: mt7621: Add MediaTek MT7621/7628/7688 I2C driver") Signed-off-by: Christian Marangi Cc: # v5.3+ Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20261022193853.670625-1-ansuelsmth@gmail.com --- drivers/i2c/busses/i2c-mt7621.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c index 0a288c998419..9ea5e3f9a763 100644 --- a/drivers/i2c/busses/i2c-mt7621.c +++ b/drivers/i2c/busses/i2c-mt7621.c @@ -144,7 +144,6 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, u16 addr; int i, j, ret, len, page_len; u32 cmd; - u32 data[2]; i2c = i2c_get_adapdata(adap); @@ -186,6 +185,8 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, /* transfer data */ for (len = pmsg->len, j = 0; len > 0; len -= 8, j += 8) { + u32 data[2] = { }; + page_len = (len >= 8) ? 8 : len; if (pmsg->flags & I2C_M_RD) { @@ -194,7 +195,8 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } else { memcpy(data, &pmsg->buf[j], page_len); iowrite32(data[0], i2c->base + REG_SM0D0_REG); - iowrite32(data[1], i2c->base + REG_SM0D1_REG); + if (page_len > 4) + iowrite32(data[1], i2c->base + REG_SM0D1_REG); cmd = SM0CTL1_WRITE; } @@ -204,7 +206,8 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, if (pmsg->flags & I2C_M_RD) { data[0] = ioread32(i2c->base + REG_SM0D0_REG); - data[1] = ioread32(i2c->base + REG_SM0D1_REG); + if (page_len > 4) + data[1] = ioread32(i2c->base + REG_SM0D1_REG); memcpy(&pmsg->buf[j], data, page_len); } else { if (!(pmsg->flags & I2C_M_IGNORE_NAK)) { From c2867bb0fa1dd1932a7a382b44df376d2085a66f Mon Sep 17 00:00:00 2001 From: Vincent Jardin Date: Fri, 23 Oct 2026 16:20:19 +0200 Subject: [PATCH 7/8] i2c: imx: add I2C_M_REV_DIR_ADDR and I2C_M_NOSTART The Realtek RTL8366SE SMI read frame per: S Addr Rd [A] reg[7:0] [A] reg[15:8] [A] [data[7:0]] A [data[15:8]] NA P Linux support it using I2C_M_REV_DIR_ADDR on a write message that inverts the transmitted R/W bit, and I2C_M_NOSTART on the following read message that continues the frame without re-addressing. This NXP i2c-imx is missing such support, so on an LX2160A the Realtek switch could not be used. Signed-off-by: Vincent Jardin Acked-by: Carlos Song Reviewed-by: Frank Li Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20261023-for-upstream-i2c-imx-lx2160-reverse-v3-1-a07b249df381@free.fr --- drivers/i2c/busses/i2c-imx.c | 157 ++++++++++++++++++++++++++--------- 1 file changed, 117 insertions(+), 40 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 62359a01a18b..ccf08cd63151 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -1000,16 +1000,10 @@ static inline int i2c_imx_isr_write(struct imx_i2c_struct *i2c_imx) return 1; } -static inline int i2c_imx_isr_read(struct imx_i2c_struct *i2c_imx) +static inline void i2c_imx_setup_read(struct imx_i2c_struct *i2c_imx) { - int result; unsigned int temp; - result = i2c_imx_isr_acked(i2c_imx); - if (result) - return result; - - /* setup bus to read data */ temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); temp &= ~I2CR_MTX; if ((i2c_imx->msg->len - 1) || (i2c_imx->msg->flags & I2C_M_RECV_LEN)) @@ -1017,6 +1011,18 @@ static inline int i2c_imx_isr_read(struct imx_i2c_struct *i2c_imx) imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */ +} + +static inline int i2c_imx_isr_read(struct imx_i2c_struct *i2c_imx) +{ + int result; + + result = i2c_imx_isr_acked(i2c_imx); + if (result) + return result; + + /* setup bus to read data */ + i2c_imx_setup_read(i2c_imx); return 0; } @@ -1172,6 +1178,16 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id) return i2c_imx_master_isr(i2c_imx, status); } +static u8 i2c_imx_addr_byte(struct i2c_msg *msg) +{ + u8 addr = i2c_8bit_addr_from_msg(msg); + + if (msg->flags & I2C_M_REV_DIR_ADDR) + addr ^= 1; + + return addr; +} + static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs) { @@ -1200,7 +1216,7 @@ static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx, * Write slave address. * The first byte must be transmitted by the CPU. */ - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR); + imx_i2c_write_reg(i2c_imx_addr_byte(msgs), i2c_imx, IMX_I2C_I2DR); time_left = wait_for_completion_timeout( &i2c_imx->dma->cmd_complete, msecs_to_jiffies(DMA_TIMEOUT)); @@ -1242,14 +1258,20 @@ static int i2c_imx_prepare_read(struct imx_i2c_struct *i2c_imx, int result; unsigned int temp = 0; - /* write slave address */ - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR); - result = i2c_imx_trx_complete(i2c_imx, !use_dma); - if (result) - return result; - result = i2c_imx_acked(i2c_imx); - if (result) - return result; + /* + * I2C_M_NOSTART continues a frame that is already open, so there is + * no address phase: go straight to turning the bus around. + */ + if (!(msgs->flags & I2C_M_NOSTART)) { + /* write slave address */ + imx_i2c_write_reg(i2c_imx_addr_byte(msgs), i2c_imx, IMX_I2C_I2DR); + result = i2c_imx_trx_complete(i2c_imx, !use_dma); + if (result) + return result; + result = i2c_imx_acked(i2c_imx); + if (result) + return result; + } dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__); @@ -1371,16 +1393,18 @@ static int i2c_imx_atomic_write(struct imx_i2c_struct *i2c_imx, int i, result; dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n", - __func__, i2c_8bit_addr_from_msg(msgs)); + __func__, i2c_imx_addr_byte(msgs)); - /* write slave address */ - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR); - result = i2c_imx_trx_complete(i2c_imx, true); - if (result) - return result; - result = i2c_imx_acked(i2c_imx); - if (result) - return result; + if (!(msgs->flags & I2C_M_NOSTART)) { + /* write slave address */ + imx_i2c_write_reg(i2c_imx_addr_byte(msgs), i2c_imx, IMX_I2C_I2DR); + result = i2c_imx_trx_complete(i2c_imx, true); + if (result) + return result; + result = i2c_imx_acked(i2c_imx); + if (result) + return result; + } dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__); /* write data */ @@ -1402,7 +1426,7 @@ static int i2c_imx_atomic_write(struct imx_i2c_struct *i2c_imx, static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs) { dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n", - __func__, i2c_8bit_addr_from_msg(msgs)); + __func__, i2c_imx_addr_byte(msgs)); i2c_imx->state = IMX_I2C_STATE_WRITE; i2c_imx->msg = msgs; @@ -1411,8 +1435,16 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs) /* * By writing the device address we start the state machine in the ISR. * The ISR will report when it is done or when it fails. + * + * I2C_M_NOSTART continues a frame that is already open and so has no + * address byte: push the first data byte instead. That raises the same + * interrupt and the ISR carries on from the second byte. */ - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR); + if (msgs->flags & I2C_M_NOSTART) + imx_i2c_write_reg(msgs->buf[i2c_imx->msg_buf_idx++], i2c_imx, + IMX_I2C_I2DR); + else + imx_i2c_write_reg(i2c_imx_addr_byte(msgs), i2c_imx, IMX_I2C_I2DR); wait_event_timeout(i2c_imx->queue, i2c_imx->state == IMX_I2C_STATE_DONE || i2c_imx->state == IMX_I2C_STATE_FAILED, @@ -1529,22 +1561,29 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n", - __func__, i2c_8bit_addr_from_msg(msgs)); + __func__, i2c_imx_addr_byte(msgs)); i2c_imx->is_lastmsg = is_lastmsg; - if (block_data) - i2c_imx->state = IMX_I2C_STATE_READ_BLOCK_DATA; - else - i2c_imx->state = IMX_I2C_STATE_READ; i2c_imx->msg = msgs; i2c_imx->msg_buf_idx = 0; - /* - * By writing the device address we start the state machine in the ISR. - * The ISR will report when it is done or when it fails. - */ - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR); + if (msgs->flags & I2C_M_NOSTART) { + i2c_imx->state = block_data ? IMX_I2C_STATE_READ_BLOCK_DATA_LEN + : IMX_I2C_STATE_READ_CONTINUE; + i2c_imx_setup_read(i2c_imx); + } else { + if (block_data) + i2c_imx->state = IMX_I2C_STATE_READ_BLOCK_DATA; + else + i2c_imx->state = IMX_I2C_STATE_READ; + + /* + * By writing the device address we start the state machine in the ISR. + * The ISR will report when it is done or when it fails. + */ + imx_i2c_write_reg(i2c_imx_addr_byte(msgs), i2c_imx, IMX_I2C_I2DR); + } wait_event_timeout(i2c_imx->queue, i2c_imx->state == IMX_I2C_STATE_DONE || i2c_imx->state == IMX_I2C_STATE_FAILED, @@ -1574,6 +1613,37 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, return ret; } +#define I2C_IMX_UNSUPPORTED_PROTOCOL_MANGLING_FLAGS \ + (I2C_M_IGNORE_NAK | I2C_M_NO_RD_ACK | I2C_M_STOP) + +static int i2c_imx_check_msgs(struct i2c_msg *msgs, int num) +{ + int i; + + for (i = 0; i < num; i++) { + /* Reject rather than silently transfer */ + if (msgs[i].flags & I2C_IMX_UNSUPPORTED_PROTOCOL_MANGLING_FLAGS) + return -EOPNOTSUPP; + + if (msgs[i].flags & I2C_M_NOSTART) { + /* + * NOSTART continues an already open frame. The first + * message has nothing to continue from, + * a 0 length continuation leaves the controller with + * no byte to clock. + */ + if (i == 0 || msgs[i].len == 0) + return -EOPNOTSUPP; + + if ((msgs[i].flags & I2C_M_RD) && + (msgs[i - 1].flags & I2C_M_RD)) + return -EOPNOTSUPP; + } + } + + return 0; +} + static int i2c_imx_xfer_common(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num, bool atomic) { @@ -1583,6 +1653,10 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter, struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(adapter); int use_dma = 0; + result = i2c_imx_check_msgs(msgs, num); + if (result) + return result; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx, atomic); if (result) { @@ -1604,7 +1678,7 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter, if (i == num - 1) is_lastmsg = true; - if (i) { + if (i && !(msgs[i].flags & I2C_M_NOSTART)) { dev_dbg(&i2c_imx->adapter.dev, "<%s> repeated start\n", __func__); temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); @@ -1636,7 +1710,8 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter, #endif use_dma = i2c_imx->dma && msgs[i].len >= DMA_THRESHOLD && - msgs[i].flags & I2C_M_DMA_SAFE; + msgs[i].flags & I2C_M_DMA_SAFE && + !(msgs[i].flags & I2C_M_NOSTART); if (msgs[i].flags & I2C_M_RD) { int block_data = msgs->flags & I2C_M_RECV_LEN; @@ -1730,7 +1805,9 @@ static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx, static u32 i2c_imx_func(struct i2c_adapter *adapter) { return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL - | I2C_FUNC_SMBUS_READ_BLOCK_DATA; + | I2C_FUNC_SMBUS_READ_BLOCK_DATA + | I2C_FUNC_NOSTART + | I2C_FUNC_PROTOCOL_MANGLING; } static const struct i2c_algorithm i2c_imx_algo = { From f7b70efba26963bf725f10574cc071f86cd5ee1d Mon Sep 17 00:00:00 2001 From: Myeonghun Pak Date: Sat, 17 Oct 2026 23:35:16 -0400 Subject: [PATCH 8/8] i2c: sh_mobile: request IRQs after mapping registers The managed IRQ handlers are acquired before the managed register mapping, so devres unmaps the registers before freeing the IRQs. A transfer wakes its waiter before the ISR's final register read, and unregistering the adapter does not synchronize these IRQs. An in-flight handler can therefore still access the mapping when it is released during driver detach. With forced-threaded IRQs, the handler can be preempted after waking the transfer waiter and before its final ICSR read. Map the registers before requesting the IRQs so that managed cleanup frees and synchronizes all handlers before unmapping their registers. Keep IRQs available while unregistering the adapter because client removal may perform transfers. This issue was identified as part of our ongoing research while reviewing kernel code. Co-developed-by: Ijae Kim Signed-off-by: Ijae Kim Signed-off-by: Myeonghun Pak Signed-off-by: Andi Shyti Link: https://patch.msgid.link/20261017233416.47b09ffff48f.i2c-sh-mobile@gmail.com --- drivers/i2c/busses/i2c-sh_mobile.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c index dae8967f8749..3a55f803e4f1 100644 --- a/drivers/i2c/busses/i2c-sh_mobile.c +++ b/drivers/i2c/busses/i2c-sh_mobile.c @@ -880,10 +880,6 @@ static int sh_mobile_i2c_probe(struct platform_device *dev) return PTR_ERR(pd->clk); } - ret = sh_mobile_i2c_hook_irqs(dev, pd); - if (ret) - return ret; - pd->dev = &dev->dev; platform_set_drvdata(dev, pd); @@ -891,6 +887,10 @@ static int sh_mobile_i2c_probe(struct platform_device *dev) if (IS_ERR(pd->reg)) return PTR_ERR(pd->reg); + ret = sh_mobile_i2c_hook_irqs(dev, pd); + if (ret) + return ret; + ret = of_property_read_u32(dev->dev.of_node, "clock-frequency", &bus_speed); pd->bus_speed = (ret || !bus_speed) ? I2C_MAX_STANDARD_MODE_FREQ : bus_speed; pd->clks_per_count = 1;