Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts
Mukesh Savaliya <[email protected]> Mon, 27 Jul 2026 09:45:23 +0530
| Newsgroups | org.kernel.vger.linux-i2c,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> > > ... > >> +/** [...] >> +} >> +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 > >> +