linux-kernel - Re: [PATCH 1/2] i2c: ocores: replace 1ms poll iteration timeout with total transfer timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0007bc39777fc1e09f953be943fd8ce474b054da.camel@ew.tq-group.com>
Date: Tue, 07 Oct 2025 16:41:00 +0200
From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Peter Korsgaard <peter@...sgaard.com>, Andi Shyti
 <andi.shyti@...nel.org>,  linux-i2c@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux@...tq-group.com
Subject: Re: [PATCH 1/2] i2c: ocores: replace 1ms poll iteration timeout
 with total transfer timeout

On Tue, 2025-10-07 at 16:20 +0200, Andrew Lunn wrote:
> On Tue, Oct 07, 2025 at 04:06:36PM +0200, Matthias Schiffer wrote:
> > On Tue, 2025-10-07 at 14:34 +0200, Andrew Lunn wrote:
> > > On Tue, Oct 07, 2025 at 02:09:24PM +0200, Matthias Schiffer wrote:
> > > > When a target makes use of clock stretching, a timeout of 1ms may not be
> > > > enough. One extreme example is the NXP PTN3460 eDP to LVDS bridge, which
> > > > takes ~320ms to send its ACK after a flash command has been
> > > > submitted.
> > > > 
> > > > Replace the per-iteration timeout of 1ms with limiting the total
> > > > transfer time to the timeout set in struct i2c_adapter (defaulting to
> > > > 1s, configurable through the I2C_TIMEOUT ioctl). While we're at it, also
> > > > add a cpu_relax() to the busy poll loop.
> > > 
> > > 1s is a long time to spin. Maybe it would be better to keep with the
> > > current spin for 1ms, and then use one of the helpers from iopoll.h to
> > > do a sleeping wait? Say with 10ms sleeps, up to the 1s maximum?
> > > 
> > > 	Andrew
> > 
> > Makes sense. I don't think I can use something from iopoll.h directly, as i2c-
> > ocores has its own ioreadX abstraction to deal with different register widths
> > and endianesses, but a combination of spin + sleep is probably the way to go.
> 
> I think iopoll.h should work.
> 
> 
> 		u8 status = oc_getreg(i2c, reg);
> 
> 		if ((status & mask) == val)
> 			break;
> 
> This maps to
> 
> u8 status;
>      
> ret = read_poll_timeout(oc_getreg, status, (status & mask) == val,
>                        10000, 1000000, false, i2c, reg);
> 
> Andrew

Ah, you are right, that should work.

If we want to keep the spin case for short waits, not duplicating the read and
mask check seems preferable to me though - maybe something like the following
(which could also be extended to exponentially increasing sleeps or similar if
we want to start with something smaller than 10ms):

	unsigned long spin_timeout = jiffies + msecs_to_jiffies(1);
	...

		if (time_before(jiffies, spin_timeout))
			cpu_relax();
		else
			msleep(10);

Best,
Matthias


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/