[PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message length and frequency

[PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message length and frequency

[Aniket Randive]

The I2C core and most controller drivers use a static 1-second timeout
for all transfers regardless of message length or bus frequency.  This
causes unnecessarily long delays on error paths for short transfers, and
may be tight for very long transfers at low bus frequencies.

This series introduces a generic helper in i2c-core that computes a
transfer-specific timeout and stores it in the standard adap->timeout
field, making the dynamic value visible to the core retry loop in
__i2c_transfer() as well as to the driver's own wait sites.

The helper accepts a safety coefficient and a minimum floor as parameters
so each driver retains control over its own timing policy without those
values becoming public API.

The second patch converts the Qualcomm GENI I2C controller to use this
helper.  The 10x safety margin over the theoretical wire time and the
300ms minimum floor (to budget for clock stretching) remain private to
the qcom-geni driver.

Changes in v6:
- Split into two patches: core helper + driver consumer
- Moved timeout calculation to i2c-core as i2c_update_timeout(), which
  writes directly into adap->timeout so all consumers of that field
  (including the __i2c_transfer() retry loop) benefit automatically
- Driver supplies safety coefficient and minimum floor as parameters,
  keeping I2C_TIMEOUT_SAFETY_COEFFICIENT and I2C_TIMEOUT_MIN_USEC
  internal to i2c-qcom-geni.c
- Compute timeout once per batch in geni_i2c_xfer() using max message
  length, all internal wait sites read adap->timeout directly

Link: https://lore.kernel.org/r/20260715101805.3615166-1-aniket.randive@oss.qualcomm.com

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
Aniket Randive (2):
      i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts
      i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency

 drivers/i2c/busses/i2c-qcom-geni.c | 37 +++++++++++++++++++++++++++++--------
 drivers/i2c/i2c-core-base.c        | 24 ++++++++++++++++++++++++
 include/linux/i2c.h                |  3 +++
 3 files changed, 56 insertions(+), 8 deletions(-)
---
base-commit: b8809969e1d7a591e0f49dd464a5d04b3cf02ab1
change-id: 20260716-master-f7da57c7529a

Best regards,
--  
Aniket Randive <aniket.randive@oss.qualcomm.com>

[PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Aniket Randive]

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().

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
 drivers/i2c/i2c-core-base.c | 24 ++++++++++++++++++++++++
 include/linux/i2c.h         |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 3ec04787a737..3280652e20d8 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -29,6 +29,7 @@
 #include <linux/irq.h>
 #include <linux/jump_label.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
@@ -2000,6 +2001,29 @@ void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_de
 }
 EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
 
+/**
+ * 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.
+ */
+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 = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz);
+
+	adap->timeout = usecs_to_jiffies(bit_usec * safety_coeff + min_usec);
+}
+EXPORT_SYMBOL_GPL(i2c_update_timeout);
+
 /* ------------------------------------------------------------------------- */
 
 int i2c_for_each_dev(void *data, int (*fn)(struct device *dev, void *data))
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 14ab4d3055af..b52974eb7e58 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -912,6 +912,9 @@ void i2c_put_adapter(struct i2c_adapter *adap);
 unsigned int i2c_adapter_depth(struct i2c_adapter *adapter);
 
 void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults);
+void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz,
+			size_t len, unsigned int safety_coeff,
+			unsigned int min_usec);
 
 /* Return the functionality mask */
 static inline u32 i2c_get_functionality(struct i2c_adapter *adap)

-- 
2.34.1

[PATCH v6 2/2] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency

[Aniket Randive]

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() from i2c-core to compute the timeout dynamically
from the maximum message length across the transfer batch and the bus
frequency, then read adap->timeout at each wait site.  The timeout is
updated once per transfer batch in geni_i2c_xfer() so all internal paths
(FIFO rx/tx and both GPI modes) use a consistent value.

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.  Both constants remain private to this driver.

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 96dbf04138be..d6fab3aa8468 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -74,9 +74,14 @@ enum geni_i2c_err_code {
 #define PACKING_BYTES_PW	4
 
 #define ABORT_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
+
 struct geni_i2c_desc {
 	bool no_dma_support;
 	unsigned int tx_fifo_depth;
@@ -471,7 +476,9 @@ 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)
 		geni_i2c_abort_xfer(gi2c);
 
@@ -513,7 +520,9 @@ 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)
 		geni_i2c_abort_xfer(gi2c);
 
@@ -591,7 +600,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
+ * @transfer_timeout_msecs: Per-message completion timeout in jiffies
  * @transfer_comp: Completion object of the transfer
  *
  * This function waits for the completion of each processed transfer messages
@@ -601,7 +610,7 @@ 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;
@@ -612,7 +621,7 @@ static int geni_i2c_gpi_multi_xfer_timeout_handler(struct device *dev,
 
 		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;
@@ -736,8 +745,17 @@ 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 max_len = 0;
+			int j;
+
+			for (j = 0; j < gi2c->num_msgs; j++)
+				max_len = max_t(size_t, max_len, msgs[j].len);
+			i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, max_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",
@@ -852,7 +870,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;

-- 
2.34.1

Re: [PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message length and frequency

[Mukesh Savaliya]

On 7/20/2026 5:11 PM, Aniket Randive wrote:
> The I2C core and most controller drivers use a static 1-second timeout
> for all transfers regardless of message length or bus frequency.  This
> causes unnecessarily long delays on error paths for short transfers, and
> may be tight for very long transfers at low bus frequencies.
> 
OR can be more than 1 sec for big transfer at low frequency. Hence 
better to make it generic saying depends on data length and frequency 
which you have mentioned. So limit it that.
> This series introduces a generic helper in i2c-core that computes a
generic helper function
> transfer-specific timeout and stores it in the standard adap->timeout
> field, making the dynamic value visible to the core retry loop in
> __i2c_transfer() as well as to the driver's own wait sites.
> 
> The helper accepts a safety coefficient and a minimum floor as parameters
what's the policy to accept ? i think should explain here.
read from dtsi or hard coded ?
> so each driver retains control over its own timing policy without those
> values becoming public API.
> 
> The second patch converts the Qualcomm GENI I2C controller to use this
> helper.  The 10x safety margin over the theoretical wire time and the
> 300ms minimum floor (to budget for clock stretching) remain private to
> the qcom-geni driver.
geni i2c driver
> 
> Changes in v6:
> - Split into two patches: core helper + driver consumer
> - Moved timeout calculation to i2c-core as i2c_update_timeout(), which
>    writes directly into adap->timeout so all consumers of that field
>    (including the __i2c_transfer() retry loop) benefit automatically
please Add suggested-by: dmitry guzman < >
Also add reviewer into to/cc list specifically.
> - Driver supplies safety coefficient and minimum floor as parameters,
>    keeping I2C_TIMEOUT_SAFETY_COEFFICIENT and I2C_TIMEOUT_MIN_USEC
>    internal to i2c-qcom-geni.c
> - Compute timeout once per batch in geni_i2c_xfer() using max message
>    length, all internal wait sites read adap->timeout directly
> 
> Link: https://lore.kernel.org/r/20260715101805.3615166-1-aniket.randive@oss.qualcomm.com
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---

[...]
> 

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Mukesh Savaliya]

On 7/20/2026 5:11 PM, Aniket Randive wrote:
> 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.
> 
in short write as should be relative to the bus speed and data length 
instead of any hard coded values.
> 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().
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
>   drivers/i2c/i2c-core-base.c | 24 ++++++++++++++++++++++++
>   include/linux/i2c.h         |  3 +++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 3ec04787a737..3280652e20d8 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -29,6 +29,7 @@
>   #include <linux/irq.h>
>   #include <linux/jump_label.h>
>   #include <linux/kernel.h>
> +#include <linux/math64.h>
>   #include <linux/module.h>
>   #include <linux/mutex.h>
>   #include <linux/of_device.h>
> @@ -2000,6 +2001,29 @@ void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_de
>   }
>   EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
>   
can i2c-core-base.c calculate within and store into adap->timeout ? 
Current adap->timeout = HZ can be removed if we add coefficient and 
safety factor default ?

Any other thoughts ?

> +/**
> + * 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.
> + */
> +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 = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz);
> +
> +	adap->timeout = usecs_to_jiffies(bit_usec * safety_coeff + min_usec);
> +}
> +EXPORT_SYMBOL_GPL(i2c_update_timeout);
why is this exported ? you have alrady added in i2c.h ?
> +
>   /* ------------------------------------------------------------------------- */
>   
>   int i2c_for_each_dev(void *data, int (*fn)(struct device *dev, void *data))
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 14ab4d3055af..b52974eb7e58 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -912,6 +912,9 @@ void i2c_put_adapter(struct i2c_adapter *adap);
>   unsigned int i2c_adapter_depth(struct i2c_adapter *adapter);
>   
>   void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_defaults);
> +void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz,
> +			size_t len, unsigned int safety_coeff,
> +			unsigned int min_usec);
>   
>   /* Return the functionality mask */
>   static inline u32 i2c_get_functionality(struct i2c_adapter *adap)
> 

Re: [PATCH v6 2/2] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency

[Mukesh Savaliya]

On 7/20/2026 5:11 PM, Aniket Randive wrote:
[...]

> ---
>   drivers/i2c/busses/i2c-qcom-geni.c | 37 +++++++++++++++++++++++++++++--------
>   1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 96dbf04138be..d6fab3aa8468 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -74,9 +74,14 @@ enum geni_i2c_err_code {
>   #define PACKING_BYTES_PW	4
>   
>   #define ABORT_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
> +
Let other expert suggest if pass from DTSI or hardcode here. because 
this let all other controller drivers to make change. I request to 
conclude on discussion, i kept my point in earlier patch.
>   struct geni_i2c_desc {
>   	bool no_dma_support;
>   	unsigned int tx_fifo_depth;
> @@ -471,7 +476,9 @@ 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)
>   		geni_i2c_abort_xfer(gi2c);
>   
> @@ -513,7 +520,9 @@ 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);
Expect some discussion or conclusion from other experts if need to hard 
code these coefficient or pass from DTSI. if core can read like existing 
i2c_parse_fw_timings(), it would become generic to every i2c adapter driver.
> +	time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout);
>   	if (!time_left)
>   		geni_i2c_abort_xfer(gi2c);
>   [...]

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Aniket RANDIVE]

On 7/22/2026 10:55 AM, Mukesh Savaliya wrote:
> 
> 
> On 7/20/2026 5:11 PM, Aniket Randive wrote:
>> 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.
>>
> in short write as should be relative to the bus speed and data length 
> instead of any hard coded values.

OK. will update the commit message.
Thanks,
Aniket

>> 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().
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>>   drivers/i2c/i2c-core-base.c | 24 ++++++++++++++++++++++++
>>   include/linux/i2c.h         |  3 +++
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
>> index 3ec04787a737..3280652e20d8 100644
>> --- a/drivers/i2c/i2c-core-base.c
>> +++ b/drivers/i2c/i2c-core-base.c
>> @@ -29,6 +29,7 @@
>>   #include <linux/irq.h>
>>   #include <linux/jump_label.h>
>>   #include <linux/kernel.h>
>> +#include <linux/math64.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>>   #include <linux/of_device.h>
>> @@ -2000,6 +2001,29 @@ void i2c_parse_fw_timings(struct device *dev, 
>> struct i2c_timings *t, bool use_de
>>   }
>>   EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
> can i2c-core-base.c calculate within and store into adap->timeout ? 
> Current adap->timeout = HZ can be removed if we add coefficient and 
> safety factor default ?
> 
> Any other thoughts ?
> 

I think keeping the timeout calculation in the core layer is reasonable, 
but the core may not have enough information to derive a universally 
correct timeout value. Different controllers and platforms can operate 
at different bus frequencies and may require different timeout margins 
depending on hardware characteristics, clocking, latency. Because of 
this, a single default coefficient or safety factor in the core could be 
too aggressive for some platforms and unnecessarily large for others.

My preference would be for the core to perform the timeout calculation 
while allowing bus drivers to provide the required parameters (e.g. 
floor timeout and safety coefficient). This keeps the calculation 
centralized while still giving individual controllers the flexibility to 
tune the timeout based on their hardware requirements.
Using a fixed default coefficient/safety factor in the core and removing 
the existing adap->timeout = HZ fallback may risk regressions on 
platforms that require a larger timeout margin.

Thanks,
Aniket

>> +/**
>> + * 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.
>> + */
>> +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 = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz);
>> +
>> +    adap->timeout = usecs_to_jiffies(bit_usec * safety_coeff + 
>> min_usec);
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_update_timeout);
> why is this exported ? you have alrady added in i2c.h ?

Adding in i2c.h file only provides the prototype required for 
compilation. Since the implementation present in i2c-core-base.c and is 
called from i2c Geni driver, the symbol must also be exported when the 
core and driver are built as separate modules. Otherwise modpost reports 
an undefined symbol for i2c_update_timeout(). Therefore the header 
declaration and EXPORT_SYMBOL_GPL() serve different purposes and both 
are required.

Thanks,
Aniket

>> +
>>   /* 
>> ------------------------------------------------------------------------- */
>>   int i2c_for_each_dev(void *data, int (*fn)(struct device *dev, void 
>> *data))
>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>> index 14ab4d3055af..b52974eb7e58 100644
>> --- a/include/linux/i2c.h
>> +++ b/include/linux/i2c.h
>> @@ -912,6 +912,9 @@ void i2c_put_adapter(struct i2c_adapter *adap);
>>   unsigned int i2c_adapter_depth(struct i2c_adapter *adapter);
>>   void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, 
>> bool use_defaults);
>> +void i2c_update_timeout(struct i2c_adapter *adap, u32 bus_freq_hz,
>> +            size_t len, unsigned int safety_coeff,
>> +            unsigned int min_usec);
>>   /* Return the functionality mask */
>>   static inline u32 i2c_get_functionality(struct i2c_adapter *adap)
>>
> 

Re: [PATCH v6 2/2] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency

[Aniket RANDIVE]

On 7/22/2026 10:56 AM, Mukesh Savaliya wrote:
> 
> 
> On 7/20/2026 5:11 PM, Aniket Randive wrote:
> [...]
> 
>> ---
>>   drivers/i2c/busses/i2c-qcom-geni.c | 37 ++++++++++++++++++++++++++++ 
>> +--------
>>   1 file changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/ 
>> i2c-qcom-geni.c
>> index 96dbf04138be..d6fab3aa8468 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -74,9 +74,14 @@ enum geni_i2c_err_code {
>>   #define PACKING_BYTES_PW    4
>>   #define ABORT_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
>> +
> Let other expert suggest if pass from DTSI or hardcode here. because 
> this let all other controller drivers to make change. I request to 
> conclude on discussion, i kept my point in earlier patch.

Sure, let's wait for comments from other reviewers (Wolfram, Dmitry, 
Andi) before concluding the discussion.

I have already shared my concerns in the previous thread regarding 
keeping this policy in the core versus allowing controller-specific 
configuration.

Thanks,
Aniket

>>   struct geni_i2c_desc {
>>       bool no_dma_support;
>>       unsigned int tx_fifo_depth;
>> @@ -471,7 +476,9 @@ 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)
>>           geni_i2c_abort_xfer(gi2c);
>> @@ -513,7 +520,9 @@ 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);
> Expect some discussion or conclusion from other experts if need to hard 
> code these coefficient or pass from DTSI. if core can read like existing 
> i2c_parse_fw_timings(), it would become generic to every i2c adapter 
> driver.

Agreed. This likely needs input from the maintainers before we finalize 
the approach.

Thanks,
Aniket

>> +    time_left = wait_for_completion_timeout(&gi2c->done, gi2c- 
>> >adap.timeout);
>>       if (!time_left)
>>           geni_i2c_abort_xfer(gi2c);
>>   [...]

Re: [PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message length and frequency

[Aniket RANDIVE]

On 7/22/2026 10:55 AM, Mukesh Savaliya wrote:
> 
> 
> On 7/20/2026 5:11 PM, Aniket Randive wrote:
>> The I2C core and most controller drivers use a static 1-second timeout
>> for all transfers regardless of message length or bus frequency.  This
>> causes unnecessarily long delays on error paths for short transfers, and
>> may be tight for very long transfers at low bus frequencies.
>>
> OR can be more than 1 sec for big transfer at low frequency. Hence 
> better to make it generic saying depends on data length and frequency 
> which you have mentioned. So limit it that.
>> This series introduces a generic helper in i2c-core that computes a
> generic helper function

I will update the comment.

Thanks,
Aniket

>> transfer-specific timeout and stores it in the standard adap->timeout
>> field, making the dynamic value visible to the core retry loop in
>> __i2c_transfer() as well as to the driver's own wait sites.
>>
>> The helper accepts a safety coefficient and a minimum floor as parameters
> what's the policy to accept ? i think should explain here.
> read from dtsi or hard coded ?

Agreed.
The policy is still being discussed (hard-coded vs driver-supplied vs DT 
based values). Once we converge on an approach with the other reviewers 
and maintainers, I'll update the commit message to reflect the final 
decision and rationale.

Thanks,
Aniket

>> so each driver retains control over its own timing policy without those
>> values becoming public API.
>>
>> The second patch converts the Qualcomm GENI I2C controller to use this
>> helper.  The 10x safety margin over the theoretical wire time and the
>> 300ms minimum floor (to budget for clock stretching) remain private to
>> the qcom-geni driver.
> geni i2c driver

I will update the comment.

Thanks,
Aniket

>>
>> Changes in v6:
>> - Split into two patches: core helper + driver consumer
>> - Moved timeout calculation to i2c-core as i2c_update_timeout(), which
>>    writes directly into adap->timeout so all consumers of that field
>>    (including the __i2c_transfer() retry loop) benefit automatically
> please Add suggested-by: dmitry guzman < >
> Also add reviewer into to/cc list specifically.

I will add the tag in next patch.

Thanks,
Aniket

>> - Driver supplies safety coefficient and minimum floor as parameters,
>>    keeping I2C_TIMEOUT_SAFETY_COEFFICIENT and I2C_TIMEOUT_MIN_USEC
>>    internal to i2c-qcom-geni.c
>> - Compute timeout once per batch in geni_i2c_xfer() using max message
>>    length, all internal wait sites read adap->timeout directly
>>
>> Link: https://lore.kernel.org/r/20260715101805.3615166-1- 
>> aniket.randive@oss.qualcomm.com
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
> 
> [...]
>>
> 

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Andi Shyti]

Hi Aniket,

On Mon, Jul 20, 2026 at 05:11:19PM +0530, Aniket Randive wrote:
> 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().

this patch does not take into account a timeout set by userspace
through the I2C_TIMEOUT ioctl. That value would be silently
overwritten by the driver.

> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>

...

> +/**
> + * 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.
> + */
> +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 = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz);

bus_freq_hz must be checked before the division, otherwise a zero
value will cause a division by zero.

> +	adap->timeout = usecs_to_jiffies(bit_usec * safety_coeff + min_usec);

If min_usec is a minimum timeout, why is it added to the
calculated value?

I would expect something like:

   max(bit_usec * safety_coeff, min_usec)

Otherwise, min_usec is an additional margin, not a minimum.

> +}
> +EXPORT_SYMBOL_GPL(i2c_update_timeout);

I'm wondering whether changing the adapter timeout from the I2C
core is the right approach.

Perhaps i2c_update_timeout() could return the calculated value
instead, but then  do not see much value in exposing such a small
calculation through the I2C core.

I know you were advised to move this into the core, but it does
not seem particularly useful there and, as it stands, it looks
somewhat controversial.

Unless there is another good reason for keeping it in the core,
I would drop this helper and keep the calculation in the driver.

Thanks,
Andi

> +

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Mukesh Savaliya]

Hi Andi,

On 7/27/2026 1:41 AM, Andi Shyti wrote:
> Hi Aniket,
> 
> On Mon, Jul 20, 2026 at 05:11:19PM +0530, Aniket Randive wrote:
>> 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().
> 
> this patch does not take into account a timeout set by userspace
> through the I2C_TIMEOUT ioctl. That value would be silently
> overwritten by the driver.
> 

Can we make timeout handling a kernel responsibility and avoid placing 
this burden on userspace ? Every userspace client already accesses the 
bus through the adapter/core layer, so the kernel has full visibility 
into factors such as bus frequency, transfer size, and controller 
characteristics to compute a sensible default timeout.

Also, if both a userspace provided timeout and a kernel-calculated 
timeout are specified, which one should win? Having two independent 
timeout mechanisms could easily lead to ambiguity and inconsistent behavior.

>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> 
> ...
> 
>> +/**

[...]

>> +}
>> +EXPORT_SYMBOL_GPL(i2c_update_timeout);
> 
> I'm wondering whether changing the adapter timeout from the I2C
> core is the right approach.
> 
> Perhaps i2c_update_timeout() could return the calculated value
> instead, but then  do not see much value in exposing such a small
> calculation through the I2C core.
> 
> I know you were advised to move this into the core, but it does
> not seem particularly useful there and, as it stands, it looks
> somewhat controversial.
> 
> Unless there is another good reason for keeping it in the core,
> I would drop this helper and keep the calculation in the driver.
> 

There may not always be a userspace application available to configure 
the timeout.

My suggestion was that the I²C core could derive the timeout based on 
the bus frequency, transfer length, and a reasonable safety margin. This 
would provide a generic solution that works across all clients without 
requiring userspace involvement.

To account for variations in system load, we could optionally expose a 
DT property for an offset or scaling coefficient to tune the computed 
timeout. Even with such tuning, I believe this approach would be 
significantly better than the current fixed 1-second (HZ) timeout.

> Thanks,
> Andi
> 
>> +

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Andi Shyti]

Hi Mukesh,

thanks for the nice discussion!

On Mon, Jul 27, 2026 at 09:45:23AM +0530, Mukesh Savaliya wrote:
> On 7/27/2026 1:41 AM, Andi Shyti wrote:
> > On Mon, Jul 20, 2026 at 05:11:19PM +0530, Aniket Randive wrote:
> > > 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().
> > 
> > this patch does not take into account a timeout set by userspace
> > through the I2C_TIMEOUT ioctl. That value would be silently
> > overwritten by the driver.
> > 
> 
> Can we make timeout handling a kernel responsibility and avoid placing this
> burden on userspace ? Every userspace client already accesses the bus
> through the adapter/core layer, so the kernel has full visibility into
> factors such as bus frequency, transfer size, and controller characteristics
> to compute a sensible default timeout.
> 
> Also, if both a userspace provided timeout and a kernel-calculated timeout
> are specified, which one should win? Having two independent timeout
> mechanisms could easily lead to ambiguity and inconsistent behavior.

There is no explicit policy for this today, and I am not against
a driver selecting a timeout according to its own requirements.

However, once userspace explicitly sets I2C_TIMEOUT, I would
expect that value to take precedence. Otherwise, the ioctl
succeeds but the value can later be silently overwritten by
the driver.

> > > Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> > 
> > ...
> > 
> > > +/**
> 
> [...]
> 
> > > +}
> > > +EXPORT_SYMBOL_GPL(i2c_update_timeout);
> > 
> > I'm wondering whether changing the adapter timeout from the I2C
> > core is the right approach.
> > 
> > Perhaps i2c_update_timeout() could return the calculated value
> > instead, but then  do not see much value in exposing such a small
> > calculation through the I2C core.
> > 
> > I know you were advised to move this into the core, but it does
> > not seem particularly useful there and, as it stands, it looks
> > somewhat controversial.
> > 
> > Unless there is another good reason for keeping it in the core,
> > I would drop this helper and keep the calculation in the driver.
> > 
> 
> There may not always be a userspace application available to configure the
> timeout.

That is fine. I do not expect userspace to configure the timeout
during normal operation. The ioctl is mostly useful for
debugging

The driver can still provide its own default when userspace has
not selected one.

> My suggestion was that the I²C core could derive the timeout based on the
> bus frequency, transfer length, and a reasonable safety margin. This would
> provide a generic solution that works across all clients without requiring
> userspace involvement.

Is there a standard formula which works for every I2C controller?

The timeout may depend on controller specific behaviour, hardware
limitations, clock stretching, FIFO handling and whether the
driver uses interrupts or polling. A calculation based only on
the transfer length and bus frequency does not seem generic
enough to cover all of that.

> To account for variations in system load, we could optionally expose a DT
> property for an offset or scaling coefficient to tune the computed timeout.
> Even with such tuning, I believe this approach would be significantly better
> than the current fixed 1-second (HZ) timeout.

I do not think a safety margin or scaling coefficient belongs in
devicetree. Devicetree should describe the hardware, not a
software timeout policy.

If a controller has a real hardware timeout parameter, that can
be described by the DT. Otherwise, I think the timeout policy
should remain in the driver.

Thanks,
Andi

> > Thanks,
> > Andi
> > 
> > > +
> 

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Mukesh Savaliya]

Thanks Andi !

On 7/28/2026 2:12 AM, Andi Shyti wrote:
> Hi Mukesh,
> 
> thanks for the nice discussion!
> 
> On Mon, Jul 27, 2026 at 09:45:23AM +0530, Mukesh Savaliya wrote:
>> On 7/27/2026 1:41 AM, Andi Shyti wrote:
>>> On Mon, Jul 20, 2026 at 05:11:19PM +0530, Aniket Randive wrote:
>>>> 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().
>>>
>>> this patch does not take into account a timeout set by userspace
>>> through the I2C_TIMEOUT ioctl. That value would be silently
>>> overwritten by the driver.
>>>
>>
>> Can we make timeout handling a kernel responsibility and avoid placing this
>> burden on userspace ? Every userspace client already accesses the bus
>> through the adapter/core layer, so the kernel has full visibility into
>> factors such as bus frequency, transfer size, and controller characteristics
>> to compute a sensible default timeout.
>>
>> Also, if both a userspace provided timeout and a kernel-calculated timeout
>> are specified, which one should win? Having two independent timeout
>> mechanisms could easily lead to ambiguity and inconsistent behavior.
> 
> There is no explicit policy for this today, and I am not against
> a driver selecting a timeout according to its own requirements.
> 
> However, once userspace explicitly sets I2C_TIMEOUT, I would
> expect that value to take precedence. Otherwise, the ioctl
> succeeds but the value can later be silently overwritten by
> the driver.
> 
>>>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>>>
>>> ...
>>>
>>>> +/**
>>
>> [...]
>>
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(i2c_update_timeout);
>>>
>>> I'm wondering whether changing the adapter timeout from the I2C
>>> core is the right approach.
>>>
>>> Perhaps i2c_update_timeout() could return the calculated value
>>> instead, but then  do not see much value in exposing such a small
>>> calculation through the I2C core.
>>>
>>> I know you were advised to move this into the core, but it does
>>> not seem particularly useful there and, as it stands, it looks
>>> somewhat controversial.
>>>
>>> Unless there is another good reason for keeping it in the core,
>>> I would drop this helper and keep the calculation in the driver.
>>>
>>
>> There may not always be a userspace application available to configure the
>> timeout.
> 

Agree.

> That is fine. I do not expect userspace to configure the timeout
> during normal operation. The ioctl is mostly useful for
> debugging
> 
> The driver can still provide its own default when userspace has
> not selected one.
> 

yes, agree.

>> My suggestion was that the I²C core could derive the timeout based on the
>> bus frequency, transfer length, and a reasonable safety margin. This would
>> provide a generic solution that works across all clients without requiring
>> userspace involvement.
> 
> Is there a standard formula which works for every I2C controller?
> 

My thinking was that we are trying to derive a timeout for transfer 
completion, so the transfer length and bus frequency should already give
us the theoretical on-the-wire transfer time.

For example, the transfer time could be estimated from the number of its 
transferred, including protocol overhead, and then converted into a 
timeout value:

bit_usec = mul_u64_u32_div(len * 9, USEC_PER_SEC, bus_freq_hz);

On top of that, we could add a fixed margin to account for interrupt and
system scheduling latency before converting the result to jiffies.

The exact margin is open for discussion. I was considering something on
the order of a few hundred milliseconds (e.g. 500 ms), but perhaps that
is still too optimistic on some systems?

Alternatively, the core could provide a calculated baseline timeout
(transfer time + fixed margin) and allow userspace to add an optional
extra offset when needed. That way the default behavior remains 
automatic and works for most clients, while systems with unusual latency
requirements can still increase the timeout without every userspace 
client having to determine an appropriate value itself.

Do you see cases where a transfer-time-based timeout with a generous
system-latency margin would still be insufficient?

> The timeout may depend on controller specific behaviour, hardware
> limitations, clock stretching, FIFO handling and whether the
> driver uses interrupts or polling. A calculation based only on
> the transfer length and bus frequency does not seem generic
> enough to cover all of that.
> 
>> To account for variations in system load, we could optionally expose a DT
>> property for an offset or scaling coefficient to tune the computed timeout.
>> Even with such tuning, I believe this approach would be significantly better
>> than the current fixed 1-second (HZ) timeout.
> 
> I do not think a safety margin or scaling coefficient belongs in
> devicetree. Devicetree should describe the hardware, not a
> software timeout policy.
> 
> If a controller has a real hardware timeout parameter, that can
> be described by the DT. Otherwise, I think the timeout policy
> should remain in the driver.
> 

Fully agree, this DT property option is not valid unless its controller 
implemented.

> Thanks,
> Andi
> 
>>> Thanks,
>>> Andi
>>>
>>>> +
>>

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Wolfram Sang]

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

Hi,

> My thinking was that we are trying to derive a timeout for transfer
> completion, so the transfer length and bus frequency should already give
> us the theoretical on-the-wire transfer time.

With my experience in all these years with I2C, this is exactly true. It
is a _theoretical_ value, and the practical value is at least board(!)
dependant. It may also depend on the environment in other cases. So, the
theoretical value may supply a minimum but IMO this doesn't help.
Because we want a precise value, but we don't know it.

> On top of that, we could add a fixed margin to account for interrupt and
> system scheduling latency before converting the result to jiffies.
> 
> The exact margin is open for discussion. I was considering something on
> the order of a few hundred milliseconds (e.g. 500 ms), but perhaps that
> is still too optimistic on some systems?

See, you simply cannot know. So, why not leaving it to those who do know
for their system?

> Alternatively, the core could provide a calculated baseline timeout
> (transfer time + fixed margin) and allow userspace to add an optional
> extra offset when needed. That way the default behavior remains automatic
> and works for most clients, while systems with unusual latency
> requirements can still increase the timeout without every userspace client
> having to determine an appropriate value itself.

We already have a mechanism for userspace to set a timeout.

> Do you see cases where a transfer-time-based timeout with a generous
> system-latency margin would still be insufficient?

Regressions. You could time out too early on boards which worked before.

Happy hacking,

   Wolfram


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

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Mukesh Savaliya]

Thanks a lot Wolfram !

On 7/28/2026 3:12 PM, Wolfram Sang wrote:
> Hi,
> 
>> My thinking was that we are trying to derive a timeout for transfer
>> completion, so the transfer length and bus frequency should already give
>> us the theoretical on-the-wire transfer time.
> 
> With my experience in all these years with I2C, this is exactly true. It
> is a _theoretical_ value, and the practical value is at least board(!)
> dependant. It may also depend on the environment in other cases. So, the
> theoretical value may supply a minimum but IMO this doesn't help.
> Because we want a precise value, but we don't know it.
> 
I agree that the precise timeout is platform dependent and cannot be 
derived exactly from the transfer parameters alone. My intention is not 
to determine the perfect value, but rather to provide a reasonable 
kernel-side default for cases where no timeout has been configured 
explicitly.

Since kernel-space clients have no generic mechanism to tune adapter 
timeouts on a per-system basis, deriving a baseline from the transfer 
length and bus frequency, combined with a conservative margin, seems 
preferable to relying solely on a fixed constant.

I am also suggesting let userspace add something on top of this if the 
core derived final timeout is not sufficient.

>> On top of that, we could add a fixed margin to account for interrupt and
>> system scheduling latency before converting the result to jiffies.
>>
>> The exact margin is open for discussion. I was considering something on
>> the order of a few hundred milliseconds (e.g. 500 ms), but perhaps that
>> is still too optimistic on some systems?
> 
> See, you simply cannot know. So, why not leaving it to those who do know
> for their system?
> 

This is an option for userspace. Should we expose device attributes for 
kernel space ? if no, then there has to be some calculated value with 
reasonable offset.

>> Alternatively, the core could provide a calculated baseline timeout
>> (transfer time + fixed margin) and allow userspace to add an optional
>> extra offset when needed. That way the default behavior remains automatic
>> and works for most clients, while systems with unusual latency
>> requirements can still increase the timeout without every userspace client
>> having to determine an appropriate value itself.
> 
> We already have a mechanism for userspace to set a timeout.
> 

Yes, and I fully support keeping I2C_TIMEOUT as the mechanism for 
userspace adjustment. What I am proposing is complementary rather than a 
replacement. The core could calculate a baseline timeout from the 
transfer characteristics and apply a conservative margin, while 
I2C_TIMEOUT would remain available for systems that require additional 
headroom beyond the default calculation.

>> Do you see cases where a transfer-time-based timeout with a generous
>> system-latency margin would still be insufficient?
> 
> Regressions. You could time out too early on boards which worked before.
> 

That is a valid concern. My assumption is that any calculated timeout 
would include a sufficiently conservative margin, based on measurements 
across a range of systems, so that existing working platforms would not 
regress. If a platform still requires significantly larger values due to 
exceptional latency characteristics, I would expect that requirement to 
be addressed through the existing timeout override mechanism rather than 
by forcing every client to use a large fixed timeout.

Looking further for common approach to be finalized considering both 
userspace, kernel space.

> Happy hacking,
> 
>     Wolfram
> 

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Wolfram Sang]

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

Hi,

> I agree that the precise timeout is platform dependent and cannot be derived
> exactly from the transfer parameters alone. My intention is not to determine
> the perfect value, but rather to provide a reasonable kernel-side default
> for cases where no timeout has been configured explicitly.

We have that already. From the I2C core:

1572         /* Set default timeout to 1 second if not already set */
1573         if (adap->timeout == 0)
1574                 adap->timeout = HZ;

This may not meet your definition of 'reasonable', though, I understand
that. But you need to be aware that you immediately enter
regression-area if you change this behaviour.

> Since kernel-space clients have no generic mechanism to tune adapter
> timeouts on a per-system basis, deriving a baseline from the transfer length

This would be easy to add. We could introduce
i2c_client_request_timeout_margin(client, desired_timeout) or something alike
with basically doing:

  client->adapter->timeout = max(client->adapter->timeout, desired_timeout);

Or? Then we would get the theoretical value of a client. Which is maybe
exceeded by the board specific timeout set by the board designer. It
gets tricky, though, with userspace. Who has precedence then?

> I am also suggesting let userspace add something on top of this if the core
> derived final timeout is not sufficient.

Why can't userspace set an absolute value like now?

> 
> This is an option for userspace. Should we expose device attributes for
> kernel space ?

See above. adap->timeout is easily accessible.

> Yes, and I fully support keeping I2C_TIMEOUT as the mechanism for userspace
> adjustment. What I am proposing is complementary rather than a replacement.
> The core could calculate a baseline timeout from the transfer
> characteristics and apply a conservative margin, while I2C_TIMEOUT would
> remain available for systems that require additional headroom beyond the
> default calculation.

If you have two ways of setting a timeout, people might get confused.

> > > Do you see cases where a transfer-time-based timeout with a generous
> > > system-latency margin would still be insufficient?
> > 
> > Regressions. You could time out too early on boards which worked before.
> 
> That is a valid concern. My assumption is that any calculated timeout would
> include a sufficiently conservative margin, based on measurements across a
> range of systems, so that existing working platforms would not regress.

You simply cannot guarantee this.

> platform still requires significantly larger values due to exceptional
> latency characteristics, I would expect that requirement to be addressed
> through the existing timeout override mechanism rather than by forcing every
> client to use a large fixed timeout.

The only way to deal with this is 'opt_in', not 'opt_out'. If you want
to provide different defaults than the existing ones, I think you should
make this available via a kernel config option, so somebody has to make
an active decision "I want that and I know it can regress".

I am still not convinced this is all worth the hazzle, but let's keep
discussing...

Happy hacking,

   Wolfram


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

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Aniket RANDIVE]

Thanks Andi and Wolfram for the detailed feedback.

 From the discussion, I understand there are concerns about introducing 
a generic timeout policy in the I2C core due to potential regressions, 
platform-specific behavior and interaction with existing timeout mechanisms.

My primary goal is to reduce the excessive timeout delays rather than to 
change the generic I2C core timeout policy.

Given the feedback so far, would maintainers prefer that if i drop the 
core helper entirely and keep the dynamic timeout calculation local to 
the qcom-geni driver or should we continue exploring an opt-in 
core-based approach?

I'd appreciate maintainers' feedback on the preferred direction so I can 
align the next revision accordingly.

Thanks,
Aniket

On 7/29/2026 8:49 PM, Wolfram Sang wrote:
> Hi,
> 
>> I agree that the precise timeout is platform dependent and cannot be derived
>> exactly from the transfer parameters alone. My intention is not to determine
>> the perfect value, but rather to provide a reasonable kernel-side default
>> for cases where no timeout has been configured explicitly.
> 
> We have that already. From the I2C core:
> 
> 1572         /* Set default timeout to 1 second if not already set */
> 1573         if (adap->timeout == 0)
> 1574                 adap->timeout = HZ;
> 
> This may not meet your definition of 'reasonable', though, I understand
> that. But you need to be aware that you immediately enter
> regression-area if you change this behaviour.
> 
>> Since kernel-space clients have no generic mechanism to tune adapter
>> timeouts on a per-system basis, deriving a baseline from the transfer length
> 
> This would be easy to add. We could introduce
> i2c_client_request_timeout_margin(client, desired_timeout) or something alike
> with basically doing:
> 
>    client->adapter->timeout = max(client->adapter->timeout, desired_timeout);
> 
> Or? Then we would get the theoretical value of a client. Which is maybe
> exceeded by the board specific timeout set by the board designer. It
> gets tricky, though, with userspace. Who has precedence then?
> 
>> I am also suggesting let userspace add something on top of this if the core
>> derived final timeout is not sufficient.
> 
> Why can't userspace set an absolute value like now?
> 
>>
>> This is an option for userspace. Should we expose device attributes for
>> kernel space ?
> 
> See above. adap->timeout is easily accessible.
> 
>> Yes, and I fully support keeping I2C_TIMEOUT as the mechanism for userspace
>> adjustment. What I am proposing is complementary rather than a replacement.
>> The core could calculate a baseline timeout from the transfer
>> characteristics and apply a conservative margin, while I2C_TIMEOUT would
>> remain available for systems that require additional headroom beyond the
>> default calculation.
> 
> If you have two ways of setting a timeout, people might get confused.
> 
>>>> Do you see cases where a transfer-time-based timeout with a generous
>>>> system-latency margin would still be insufficient?
>>>
>>> Regressions. You could time out too early on boards which worked before.
>>
>> That is a valid concern. My assumption is that any calculated timeout would
>> include a sufficiently conservative margin, based on measurements across a
>> range of systems, so that existing working platforms would not regress.
> 
> You simply cannot guarantee this.
> 
>> platform still requires significantly larger values due to exceptional
>> latency characteristics, I would expect that requirement to be addressed
>> through the existing timeout override mechanism rather than by forcing every
>> client to use a large fixed timeout.
> 
> The only way to deal with this is 'opt_in', not 'opt_out'. If you want
> to provide different defaults than the existing ones, I think you should
> make this available via a kernel config option, so somebody has to make
> an active decision "I want that and I know it can regress".
> 
> I am still not convinced this is all worth the hazzle, but let's keep
> discussing...
> 
> Happy hacking,
> 
>     Wolfram
> 

Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts

[Wolfram Sang]

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


> Given the feedback so far, would maintainers prefer that if i drop the core
> helper entirely and keep the dynamic timeout calculation local to the
> qcom-geni driver or should we continue exploring an opt-in core-based
> approach?

Well, I suggested a potential way in my last mail (Kconfig option). So,
what do you think about it? Does it handle your use case?


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