BBB/I2C: Using ioctl(I2CRDWR) warns: interrupt storm detected on "intr70:"

[Ian Lepore]

On Fri, 2014-05-02 at 08:50 -0400, Winston Smith wrote:
> On Thu, May 1, 2014 at 9:17 PM, Ian Lepore <ian at freebsd.org> wrote:
> > Typically an interrupt storm will happen when a device driver's
> > interrupt handler fails to clear the condition that triggers the
> > interrupt, so it re-interrupts immediately, continuously.
> >
> > After spending a few minutes looking at arm/ti/ti_i2c.c, there's
> > something that seems not-quite-right... the interrupt handler reads the
> > I2C_STAT register and clears any interrupt bits it finds there, but:
> >
> > #define I2C_REG_STAT            0x88
> >
> > When I look in the AM335x reference manual that's listed as "I2C
> > interrupt status vector (legacy)."  There's another register at 0x28
> > described as "Per-event enabled interrupt status vector."
> >
> > I wonder if just changing the I2C_REG_STAT 0x28 (in ti_i2c.h) would fix
> > things?  If not, I'd recommend doing a more thorough version of what I
> > started doing... compare what you see in the manual with the code in
> > terms of register offsets and which bits are being checked and see if
> > there are other lurking glitches.
> 
> I spent some time looking over the documentation and didn't see
> anything obvious beyond what you pointed out (interesting, the latest
> version of the TRM doesn't even mention the legacy register at 0x88).
> 
> I changed I2C_REG_STAT to 0x28, rebuilt and there's no change in
> behavior from from before, the `bbb_eeprom` tool continues to work and
> also shows the same "interrupt storm detected".  This is the change I
> made:
> 
> --- ti_i2c.h 2014-05-02 06:58:17.000000000 -0400
> +++ /tmp/ti_i2c.h 2014-05-02 08:44:25.000000000 -0400
> @@ -52,7 +52,11 @@
>  #define I2C_IE_ARDY (1UL << 2)    /* Register Access Ready interrupt */
>  #define I2C_IE_NACK (1UL << 1)    /* No Acknowledgment interrupt */
>  #define I2C_IE_AL (1UL << 0)    /* Arbitration Lost interrupt */
> +#if defined(SOC_TI_AM335X)
> +#define I2C_REG_STAT 0x28
> +#else
>  #define I2C_REG_STAT 0x88
> +#endif
>  #define I2C_STAT_XDR (1UL << 14)
>  #define I2C_STAT_RDR (1UL << 13)
>  #define I2C_STAT_BB (1UL << 12)
> 
> 
> It may be worth adding it simply to keep the AM335X code up to date
> (rather than using the legacy register).  Interestingly, the ti_i2c
> code already uses the newer I2C_IRQENABLE_CLR and I2C_IRQENABLE_SET
> non legacy registers for enabling/disabling the IRQs (vs the old
> legacy I2C_REG_IE); see ti_i2c_set_intr_enable() in ti_i2c.c.
> 
> -W

The next thing I'd do at this point is start some printf-debugging.  For
starters, just printing the status value read in the interrupt handler
might give a clue on which way to go next.

I was going to look into this a bit this weekend.  I bought myself one
of these to play with: http://www.adafruit.com/products/264 and I was
going to assemble it and play around this weekend.  But now it looks
like I may spend the weekend getting my truck ready for some
early-season offroad snowbank-bashing. :)

-- Ian