From: Aniket RANDIVE <aniket.randive@oss.qualcomm.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>,
Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
Cc: Andi Shyti <andi.shyti@kernel.org>,
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: Thu, 30 Jul 2026 17:23:14 +0530 [thread overview]
Message-ID: <be5ba8e8-15d0-4fe4-a66c-fe7ce8da6cc3@oss.qualcomm.com> (raw)
In-Reply-To: <amoaEGX8Q4zwHF2i@shikoro>
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
>
next prev parent reply other threads:[~2026-07-30 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 [this message]
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
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=be5ba8e8-15d0-4fe4-a66c-fe7ce8da6cc3@oss.qualcomm.com \
--to=aniket.randive@oss.qualcomm.com \
--cc=andi.shyti@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mukesh.savaliya@oss.qualcomm.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®