Re: i2c-imx.c: Unnecessary delay slowing down i2c communication

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi Ian,

On Thu, Mar 03, 2022 at 10:33:10PM +0100, Wolfram Sang wrote:
> On Thu, Mar 03, 2022 at 03:19:00PM +0000, Ian Dannapel wrote:
> > Hello I²C Driver Maintainers,
> 
> Adding the i2c-imx maintainer to CC.
> 
> > 
> > please excuse me if I am not following the right steps to report a question. I did not find consensus between all instructions that I read.
> > 
> > We noted that on the IMX i2c driver, at the i2c_imx_start funtion, some sleep/delay was introduced without any apparent reason:
> > Line 448 at:  https://github.com/Freescale/linux-fslc/commit/3a5ee18d2a32bda6b9a1260136f6805848e3839d

In case of atomic use, for example on poweroff or system panic, we can't
schedule, polling the register at CPU speed makes no sense too. May be
this part can be optimized. I assume you don't care about atomic path. Correct?

> > Line 528 at:  https://github.com/Freescale/linux-fslc/commit/2b899f34e1db9adef8716d07e872a800dfa60790

This commit provides enough description in the commit log. But I assume, you
refer more to the next commit. Since this commit is changing only existing
sleep.

> > Line 200 at:  https://github.com/Freescale/linux-fslc/commit/43309f3b521302bb66c4c9e66704dd3675e4d725

Good question. We have "Enable I2C controller" instruction and next step
is "Wait controller to be stable". It looks like some SoC/IP specific
workaround.

Different iMX* documentation say:
....
Master mode is not aware that the bus is busy, so when a START cycle is
initiated, the current bus cycle can become corrupted and cause either
the current bus master or the I2C module to lose arbitration, after
which bus operation returns to normal.
...

In this case it would make no real sense to start, but the question is,
In what case should we care and can we optimize it?

> > This sleep causes a pretty big latency overhead on I²C writes and no
> > IMX8MP document states the need of this delay on the controller.

I would be careful using one SoC and apply same rule to all of them. 

> > NXP Support also informed that this delay might not be needed.  Some
> > early tests with removing this delay completely showed a great
> > reduction on the write latency and no problems with the
> > communication.

I assume, the test was done on iMX8MP only with only one use case?

> > But since we want a stable and fast communication, I ask here again
> > if someone knows the reason or the need for this delay when starting
> > to communicate on the I2C bus.

Since this delay was added as part of completely different functionality
it is hard to say, what issue it is addressing.

Removing this sleep may affect different devices. We can disable it SoC
by SoC or disable it for all SoC and wait for the feedback. Last option
is probably only way to get usable results. So, please send patches and
be ready to respond on possible regressions.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |