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

Aniket RANDIVE <[email protected]> Thu, 30 Jul 2026 17:23:14 +0530
Newsgroups org.kernel.vger.linux-i2c,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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
>