From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Andi Shyti <andi.shyti@kernel.org>,
Aniket Randive <aniket.randive@oss.qualcomm.com>,
Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Wolfram Sang <wsa@kernel.org>
Subject: Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts
Date: Mon, 3 Aug 2026 17:22:59 +0530 [thread overview]
Message-ID: <9dcc47cd-2470-45b2-9106-51d4636cf60e@oss.qualcomm.com> (raw)
In-Reply-To: <amoaEGX8Q4zwHF2i@shikoro>
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.
>
Yes, 1HZ it's not reasonable for smaller transfers. Agree too that
changing this may cause regression to few others.
>> 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?
>
Thinking to give precedence to user space here in such case. if no
userspace setting timeout, then default will continue with core set timeout.
>> 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?
>
So, does it mean user space can override kernel/core calculated timeout
? if yes, i agree to this idea.
>>
>> 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.
>
In that case, let's decide if userspace configured timeout wins. If not
set by user, then set calculated timeout by core layer.
I was thinking, user space may not always set the timeout but core layer
will always need some timeout value based on formulae aniket has kept.
>>>> 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.
>
Understood now, it may cause regression.
>> 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...
>
This looks like a reasonable compromise to discuss and converge on. A
polished version:
1. Userspace-configured timeout takes precedence over any timeout
configured by the kernel.
2. When userspace does not configure a timeout, the kernel-computed
timeout is used.
3. To avoid regressions, introduce a Kconfig option for formula-based
timeout calculation:
A. If the Kconfig option is enabled, derive the timeout using
the proposed formula-based approach.
B. If the Kconfig option is disabled, retain the existing
default timeout behavior (currently 1 second) to preserve
backward compatibility.
> Happy hacking,
>
> Wolfram
>
next prev parent reply other threads:[~2026-08-03 11:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-20 11:41 [PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message length and frequency Aniket Randive
2026-07-20 11:41 ` [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts Aniket Randive
2026-07-22 5:25 ` Mukesh Savaliya
2026-07-24 11:46 ` Aniket RANDIVE
2026-07-26 20:11 ` Andi Shyti
2026-07-27 4:15 ` Mukesh Savaliya
2026-07-27 20:42 ` Andi Shyti
2026-07-28 4:56 ` Mukesh Savaliya
2026-07-28 9:42 ` Wolfram Sang
2026-07-28 10:14 ` Mukesh Savaliya
2026-07-29 15:19 ` Wolfram Sang
2026-07-30 11:53 ` Aniket RANDIVE
2026-07-30 20:23 ` Wolfram Sang
2026-08-01 21:02 ` Wolfram Sang
2026-08-03 10:53 ` Aniket RANDIVE
2026-08-03 11:52 ` Mukesh Savaliya [this message]
2026-07-20 11:41 ` [PATCH v6 2/2] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency Aniket Randive
2026-07-22 5:26 ` Mukesh Savaliya
2026-07-24 11:51 ` Aniket RANDIVE
2026-07-22 5:25 ` [PATCH v6 0/2] i2c: Add dynamic transfer timeout based on message " Mukesh Savaliya
2026-07-24 11:56 ` Aniket RANDIVE
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9dcc47cd-2470-45b2-9106-51d4636cf60e@oss.qualcomm.com \
--to=mukesh.savaliya@oss.qualcomm.com \
--cc=andi.shyti@kernel.org \
--cc=aniket.randive@oss.qualcomm.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viken.dadhaniya@oss.qualcomm.com \
--cc=wsa+renesas@sang-engineering.com \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.