Received-SPF: pass (google.com: domain of linux-kernel+bounces-198717-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 3 Jun 2024 09:34:58 +0200
From: Stefan Eichenberger <eichest@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
	jic23@kernel.org, lars@metafoo.de, nuno.sa@analog.com,
	andriy.shevchenko@linux.intel.com, u.kleine-koenig@pengutronix.de,
	marcelo.schmitt@analog.com, gnstark@salutedevices.com,
	francesco.dolcini@toradex.com, linux-i2c@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	Stefan Eichenberger <stefan.eichenberger@toradex.com>
Subject: Re: [RFC PATCH] i2c: imx: avoid rescheduling when waiting for bus
 not busy
Message-ID: <Zl1yIsE6f5JJY89S@eichest-laptop>
References: <20240531142437.74831-1-eichest@gmail.com>
 <f5d537e2-b102-415f-bc22-c949fd859344@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f5d537e2-b102-415f-bc22-c949fd859344@lunn.ch>

On Sun, Jun 02, 2024 at 04:31:27PM +0200, Andrew Lunn wrote:
> On Fri, May 31, 2024 at 04:24:37PM +0200, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@toradex.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).
> 
> Does the I2C specification say anything about this behaviour, or is it
> specific to this device?
> 

The timeouting mechanism is normally used in SMBus mode. However, for
this specific device they still call it I2C which is a bit confusing.
The difference between I2C and SMBus is that SMBus has a timeout while
the I2C uses a recovery mechanism. Besides that the two protocols are
identical.

> > 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.
> 
> 2. has the advantage you fix it for any system with this device, not
> just those using an IMX. Once question would be, is such a retry safe
> in all conditions. Does the timeout happen before any non idempotent
> operation is performed?
> 
> If the I2C specification allows this behaviour, maybe a more generic
> solution is needed, since it could affect more devices?

Maybe I could add a smbus_xfer function to the i2c driver and then
change the ti-ads1015 driver to use the smbus_xfer function instead of
i2c. However, I would still have to disable preemption while the SMBus
transfer is happening which concerns me a bit.

Regards,
Stefan