[PATCH] i2c: omap: fix i207 errata handling

[PATCH] i2c: omap: fix i207 errata handling

[Alexander Kochetkov]

commit 6d9939f651419a63e091105663821f9c7d3fec37 (i2c: omap: split out [XR]DR
and [XR]RDY) changed the way how errata i207 (I2C: RDR Flag May Be Incorrectly
Set) get handled. 6d9939f6514 code doesn't correspond to workaround provided by
errata.

According to errata ISR must filter out spurious RDR before data read not after.
ISR must read RXSTAT to get number of bytes available to read. Because RDR
could be set while there could no data in the receive FIFO.

Restored pre 6d9939f6514 way of handling errata.

Found by code review. Real impact haven't seen.
Tested on Beagleboard XM C.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
Fixes: 6d9939f651419a63e09110 i2c: omap: split out [XR]DR and [XR]RDY
---
 drivers/i2c/busses/i2c-omap.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 90dcc2e..e7cbcb0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -958,11 +958,13 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 			if (dev->fifo_size)
 				num_bytes = dev->buf_len;
 
-			omap_i2c_receive_data(dev, num_bytes, true);
-
-			if (dev->errata & I2C_OMAP_ERRATA_I207)
+			if (dev->errata & I2C_OMAP_ERRATA_I207) {
 				i2c_omap_errata_i207(dev, stat);
+				num_bytes = (omap_i2c_read_reg(dev,
+					OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F;
+			}
 
+			omap_i2c_receive_data(dev, num_bytes, true);
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RDR);
 			continue;
 		}
-- 
1.7.9.5

Re: [PATCH] i2c: omap: fix i207 errata handling

[Alexander Kochetkov]

21 нояб. 2014 г., в 3:16, Alexander Kochetkov <al.kochet@gmail.com> написал(а):

> commit 6d9939f651419a63e091105663821f9c7d3fec37 (i2c: omap: split out [XR]DR
> and [XR]RDY) changed the way how errata i207 (I2C: RDR Flag May Be Incorrectly
> Set) get handled.

It's is seen if open 6d9939f651419a63e09^ and 6d9939f651419a63e09 side by side:

6d9939f651419a63e09^:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-omap.c?id=540a4790f7da0d3ca5ad9d726f198a5eb4db05ec#n800

6d9939f651419a63e09:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-omap.c?id=6d9939f651419a63e091105663821f9c7d3fec37#n800

Errata "2C: RDR Flag May Be Incorrectly Set"
http://www.ti.com/lit/er/sprz319f/sprz319f.pdf

21 нояб. 2014 г., в 3:16, Alexander Kochetkov <al.kochet@gmail.com> написал(а):

> Found by code review. Real impact haven't seen.
> Tested on Beagleboard XM C.

Does anybody know the "certain rare conditions" when RDR errata appears?
I tested without luck (Beagleboard XM C).

Re: [PATCH] i2c: omap: fix i207 errata handling

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

On Fri, Nov 21, 2014 at 04:16:51AM +0400, Alexander Kochetkov wrote:
> commit 6d9939f651419a63e091105663821f9c7d3fec37 (i2c: omap: split out [XR]DR
> and [XR]RDY) changed the way how errata i207 (I2C: RDR Flag May Be Incorrectly
> Set) get handled. 6d9939f6514 code doesn't correspond to workaround provided by
> errata.
> 
> According to errata ISR must filter out spurious RDR before data read not after.
> ISR must read RXSTAT to get number of bytes available to read. Because RDR
> could be set while there could no data in the receive FIFO.
> 
> Restored pre 6d9939f6514 way of handling errata.
> 
> Found by code review. Real impact haven't seen.
> Tested on Beagleboard XM C.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Fixes: 6d9939f651419a63e09110 i2c: omap: split out [XR]DR and [XR]RDY

Tested on BBB and AM437x Starter Kit

Tested-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  drivers/i2c/busses/i2c-omap.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 90dcc2e..e7cbcb0 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -958,11 +958,13 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
>  			if (dev->fifo_size)
>  				num_bytes = dev->buf_len;
>  
> -			omap_i2c_receive_data(dev, num_bytes, true);
> -
> -			if (dev->errata & I2C_OMAP_ERRATA_I207)
> +			if (dev->errata & I2C_OMAP_ERRATA_I207) {
>  				i2c_omap_errata_i207(dev, stat);
> +				num_bytes = (omap_i2c_read_reg(dev,
> +					OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F;
> +			}
>  
> +			omap_i2c_receive_data(dev, num_bytes, true);
>  			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RDR);
>  			continue;
>  		}
> -- 
> 1.7.9.5
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: fix i207 errata handling

[Alexander Kochetkov]

21 нояб. 2014 г., в 19:08, Felipe Balbi <balbi@ti.com> написал(а):

> Tested on BBB and AM437x Starter Kit
> 
> Tested-by: Felipe Balbi <balbi@ti.com>
> Reviewed-by: Felipe Balbi <balbi@ti.com>

21 нояб. 2014 г., в 0:10, Aaro Koskinen <aaro.koskinen@iki.fi> написал(а):

> I could not see any breakage or anything wrong on OMAP2 & OMAP3.
> On OMAP1 I don't have anything on the OMAP I2C bus, so cannot really
> test anything there.
> 
> Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>


21 нояб. 2014 г., в 21:11, Wolfram Sang <wsa@the-dreams.de> написал(а):

> The errno for AL is -EAGAIN. Curly braces are not needed.


Guys, I really appreciate you help.
So much testing and review.
I could not have done one.
Thank you!

Alexander.

Re: [PATCH] i2c: omap: fix i207 errata handling

[Alexander Kochetkov]

21 нояб. 2014 г., в 3:29, Alexander Kochetkov <al.kochet@gmail.com> написал(а):

>> 
>> Found by code review. Real impact haven't seen.
>> Tested on Beagleboard XM C.
> 
> Does anybody know the "certain rare conditions" when RDR errata appears?
> I tested without luck (Beagleboard XM C).

Spent half a day trying to catch the errata without luck.
Tried to simulate noise on the bus in hope it may happen.
Tried to run with OMAP_I2C_FLAG_NO_FIFO flag.

What a mystery errata. Hiding.

Anyway, thanks!
Have a nice weekend!

Re: [PATCH] i2c: omap: fix i207 errata handling

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

On Fri, Nov 21, 2014 at 04:16:51AM +0400, Alexander Kochetkov wrote:
> commit 6d9939f651419a63e091105663821f9c7d3fec37 (i2c: omap: split out [XR]DR
> and [XR]RDY) changed the way how errata i207 (I2C: RDR Flag May Be Incorrectly
> Set) get handled. 6d9939f6514 code doesn't correspond to workaround provided by
> errata.
> 
> According to errata ISR must filter out spurious RDR before data read not after.
> ISR must read RXSTAT to get number of bytes available to read. Because RDR
> could be set while there could no data in the receive FIFO.
> 
> Restored pre 6d9939f6514 way of handling errata.
> 
> Found by code review. Real impact haven't seen.
> Tested on Beagleboard XM C.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Fixes: 6d9939f651419a63e09110 i2c: omap: split out [XR]DR and [XR]RDY

Applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: fix i207 errata handling

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]


> Guys, I really appreciate you help.
> So much testing and review.
> I could not have done one.
> Thank you!

Well, thank YOU, for your hard work in verifying this controller at
low-level and in corner-cases. This is very much appreciated. Plus, you
have been very responsive to comments. Really good work! Hope you'll be
around for more :)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]