Re: [PATCH v6 1/2] i2c: core: Add i2c_update_timeout() helper for dynamic transfer timeouts
Wolfram Sang <[email protected]> Wed, 29 Jul 2026 17:19:44 +0200
| Newsgroups | org.kernel.vger.linux-i2c,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <amoaEGX8Q4zwHF2i@shikoro> |
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
signature.asc
(application/pgp-signature, 833 B)
-----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAmpqGgkACgkQFA3kzBSg KbZXSxAAgw+bXawtzSmPKUNc2txknub8t038XxD+4R2DYbrcklEirwTz9d57/wJN o+Va+Sth4SPhpU6Vf/ro9Xg457PsYGPt1hL/K/EsgDa3qzUJNcMtKpcYv1VjFl8p DL3zHGJGem+6oubojT7Bi2osbq2RjszpQqHxz99TBATuIerC1CZ6dPJgYWw3ZtEu AzlgzVzMOj67CO71JY9gLWem/NphCPNXubp4kmUEul5wh2yw+KN2La0j1+hbQANm nOj/62+Jz9HuTEjYtDqYxsLgF/hIVkAss7epAEpp8NAzEfFOgY55Lti5OG+y34qd SX+3eVProWKxiSZueRelQ/Xrymn2erpqkMAFQ40XMQkYU+DL0MGeA5A4YZV3H94V rZhyA/qDwdDVo2+bUou68rny/RPY5wqUUkN1ef1KMHF4b8F7F+dFWeAyM6U8RVSC jt7f3fDWTd4wxaVbZIuyH8aOy7zJIwiDioCUwlAVf5tZ/pewOZBeY7xnC3/FABYW WBY7DfhCP25jsLerh8XFLQ4SO/JuY9cmDYpi9qd3mmQcHc85yKSfLy8n9VhbDa7a sObiPj8LS7CQggNOOo0r0eYdLaJpM4+KvIHtYV0gDDMx+HF7rV9HY40g4M53aYXh 3BPsCuwwg6HOpHDh0Twv9iGipJDJcDU0xOpw3xoDMcl1cgFgxR4= =rCdS -----END PGP SIGNATURE-----