Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case

[Sebastian Reichel <sebastian.reichel@collabora.co.uk>]

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

Hi,

On Mon, Oct 02, 2017 at 06:01:19PM -0500, Grygorii Strashko wrote:
> 
> 
> On 09/29/2017 11:37 AM, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Fri, Sep 29, 2017 at 05:17:47PM +0200, Claudio Foellmi wrote:
> >>>> Sebastian, can you please check if this helps with your problems on N950?
> >>>> If it does, I'll turn it into a proper standalone patch.
> >>>
> >>> No, it does not. Also no interrupts ignoring messages appearing
> >>> in dmesg:
> >>>
> >>> n950# dmesg | grep -E "48072000.i2c|lp5523x"
> >>> [    0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz
> >>> [    4.934265] lp5523x 1-0032: reset command sent (no ACK)!
> >>> [    6.003875] omap_i2c 48072000.i2c: controller timed out
> >>> [    6.033874] lp5523x 1-0032: device detection err: -110
> >>> [    6.039154] lp5523x: probe of 1-0032 failed with error -110
> >>>
> >>
> >> Hi Sebastian
> >>
> >> Thank you for trying it out.
> >> It seems that your symptoms are quite different from the ones that Vignesh
> >> described earlier. He had never-ending storms of spurious interrupts
> >> (which that patch would have addressed), but you don't seem to get
> >> any interrupts at all. Not even the NACK one, which just looks wrong.
> 
> regarding spurious interrupts during recovery. In my opinion,
> I2C IRQs should be disabled on entering recovery and re-enabled after, as
> bus state is unpredictable during recovery and OMAP I2C driver expect to
> receive IRQs only and only when omap_i2c_xfer() is called.
> (omap->receiver will have correct value only in above case)
> 
> >>
> >> If you want to still dig deeper, you can enable debug logs for i2c-omap,
> >> so you can see every single interrupt. But if there are none, I don't see
> >> what we could possibly do to fix it.
> >>
> >>
> >> Vignesh, do you still have access to any of those devices with interrupt
> >> floods? If so, could you try the previous patch on one of them?
> >>
> >> Also note that Sebastian's issue is definitely not caused (or helped)
> >> by bus recovery, the timeout he sees resets the adapter right away.
> >> So he is not affected by my original patch either way.
> >>
> >> -- Claudio
> > 
> > Yeah, I don't see IRQ storm, so this might be a different issue.
> > FWIW this is what I see on N950:
> > 
> > n950# dmesg | grep -E "48072000.i2c|lp55"
> > [    2.136383] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz
> > [    8.212951] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1
> > [    8.213287] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
> > [    8.213592] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)
> > [    8.213897] lp5523x 1-0032: reset command sent (no ACK)!
> > [    8.242462] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1
> > [    8.243255] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)
> > [    8.243469] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
> 
> this second XRDY looks suspicious, as it's received after ARDY.
> 
> > [    8.253387] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0
> > [    8.253631] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
> > [    9.272735] omap_i2c 48072000.i2c: controller timed out
> > [    9.278167] lp5523x 1-0032: device detection err: -110
> > [    9.283843] lp5523x: probe of 1-0032 failed with error -110
> > [    9.662780] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0
> > [    9.670166] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
> > [    9.675659] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
> > [    9.682617] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1
> > [    9.689819] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
> > [    9.695007] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
> 
>  
> Wouldn't below diff help:
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 1ebb5e9..e52bdae 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -959,7 +959,7 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,
>  {
>         u16             w;
>  
> -       while (num_bytes--) {
> +       while (omap->buf_len && num_bytes--) {
>                 w = *omap->buf++;
>                 omap->buf_len--;

that did not change anything as far as I can see:

n950# dmesg | grep -E "48072000.i2c|lp5523x"
[    2.139801] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz
[    8.236968] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1
[    8.237457] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
[    8.237792] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)
[    8.238189] lp5523x 1-0032: reset command sent (no ACK)!
[    8.256500] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1
[    8.256835] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)
[    8.257080] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
[    8.258941] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0
[    8.259246] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
[    9.276062] omap_i2c 48072000.i2c: controller timed out
[    9.306060] lp5523x 1-0032: device detection err: -110
[    9.311492] lp5523x: probe of 1-0032 failed with error -110
[    9.705993] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0
[    9.713256] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
[    9.718536] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
[    9.723754] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1
[    9.731079] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
[    9.736297] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]