linux-kernel - Re: [RFC PATCH] i2c: imx: avoid rescheduling when waiting for bus not busy

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 for Android: free password hash cracker in your pocket

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

Date: Fri, 21 Jun 2024 19:24:21 +0200
From: Francesco Dolcini <francesco@...cini.it>
To: Stefan Eichenberger <eichest@...il.com>
Cc: o.rempel@...gutronix.de, kernel@...gutronix.de, andi.shyti@...nel.org,
	shawnguo@...nel.org, s.hauer@...gutronix.de, festevam@...il.com,
	jic23@...nel.org, lars@...afoo.de, nuno.sa@...log.com,
	andriy.shevchenko@...ux.intel.com, marcelo.schmitt@...log.com,
	gnstark@...utedevices.com, francesco.dolcini@...adex.com,
	wsa+renesas@...g-engineering.com, andrew@...n.ch,
	linux-i2c@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org,
	Stefan Eichenberger <stefan.eichenberger@...adex.com>
Subject: Re: [RFC PATCH] i2c: imx: avoid rescheduling when waiting for bus
 not busy

Hello Stefan,

On Fri, Jun 21, 2024 at 05:22:46PM +0200, Stefan Eichenberger wrote:
> Hi Andi, Andrew, Wolfram, Oleksij,
> 
> After some internal discussion we still have some questions which are
> blocking us from solving the issue.
> 
> On Fri, May 31, 2024 at 04:24:37PM +0200, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@...adex.com>
> > 
> > On our i.MX8M Mini based module we have an ADS1015 I2C ADC connected to
> > the I2C bus. The ADS1015 I2C ADC will timeout after 25ms when the I2C
> > bus is idle. The imx i2c driver will call schedule when waiting for the
> > bus to become idle after switching to master mode. When the i2c
> > controller switches to master mode it pulls SCL and SDA low, if the
> > ADS1015 I2C ADC sees this for more than 25 ms without seeing SCL
> > clocking, it will timeout and ignore all signals until the next start
> > condition occurs (SCL and SDA low). This can occur when the system load
> > is high and schedule returns after more than 25 ms.
> > 
> > This rfc tries to solve the problem by using a udelay for the first 10
> > ms before calling schedule. This reduces the chance that we will
> > reschedule. However, it is still theoretically possible for the problem
> > to occur. To properly solve the problem, we would also need to disable
> > interrupts during the transfer.
> > 
> > After some internal discussion, we see three possible solutions:
> > 1. Use udelay as shown in this rfc and also disable the interrupts
> >    during the transfer. This would solve the problem but disable the
> >    interrupts. Also, we would have to re-enable the interrupts if the
> >    timeout is longer than 1ms (TBD).
> > 2. We use a retry mechanism in the ti-ads1015 driver. When we see a
> >    timeout, we try again.
> > 3. We use the suggested solution and accept that there is an edge case
> >    where the timeout can happen.
> > 
> > There may be a better way to do this, which is why this is an RFC.
> > 
> > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@...adex.com>
> > ---

...

> > On a multimaster bus system, the busy bus (I2C_I2SR[IBB]) must be
> > tested to determine whether the serial bus is free.
> We assume this means it is not necessary to test for IBB if we know we
> are in a single-master configuration.

To me this is a very interesting option. If we can confirm that this
busy-wait-loop is not required when we have a single master
configuration we can just solve the issue in a clean way.

And once the driver knows if it is multi-master mode or not we can also
get rid of the EAGAIN that does not make any sense with single-master.
There was an old discussion with some great contribution from Oleksij on
this topic.

Francesco