linux-kernel - Re: [PATCH v5 2/2] i2c: spacemit: introduce pio for k1

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: <aVKIDOj6x_BYEttg@aurel32.net>
Date: Mon, 29 Dec 2025 14:54:20 +0100
From: Aurelien Jarno <aurelien@...el32.net>
To: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
Cc: Alex Elder <elder@...cstar.com>, Andi Shyti <andi.shyti@...nel.org>,
	Yixun Lan <dlan@...too.org>,
	Michael Opdenacker <michael.opdenacker@...tcommit.com>,
	Troy Mitchell <troymitchell988@...il.com>,
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev
Subject: Re: [PATCH v5 2/2] i2c: spacemit: introduce pio for k1

Hi,

On 2025-12-29 10:07, Troy Mitchell wrote:
> > > @@ -474,6 +608,11 @@ static void spacemit_i2c_calc_timeout(struct spacemit_i2c_dev *i2c)
> > >   	unsigned long timeout;
> > >   	int idx = 0, cnt = 0;
> > > +	if (i2c->use_pio) {
> > > +		i2c->adapt.timeout = msecs_to_jiffies(SPACEMIT_WAIT_TIMEOUT);
> > 
> > Again, why is a rough 1000 millisecond timeout OK for PIO, while a
> > fairly precise timeout value based on the number of bytes to be
> > transferred and the transfer bit rate computed for interrupt mode?
> Sorry I didn't see this.
> 
> In interrupt-driven mode we wait for a single completion event, so the
> timeout needs to reflect the worst-case transfer duration to avoid
> spurious timeouts.
> In PIO mode the loop is driven by FIFO/status progress, and the timeout
> is only a safeguard against a stalled bus rather than an exact transfer
> time.
> Therefore a simple conservative value is sufficient there.

I do think the PIO code is correct wrt timeout. It is not possible to 
predict the time of a transaction as an I2C peripheral can slow down a 
transaction by pulling the SCL line down. Therefore a fixed timeout 
should be used in all cases not only for PIO, and this matches what is 
done in all other I2C drivers.

That said it's probably out of scope for this patch series.

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@...el32.net                     http://aurel32.net