linux-kernel - Re: [PATCH v4 7/9] i2c: npcm: Handle spurious interrupts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACD3sJYPrQoWzOkOKM5tWuTWS0uEertBSrwMa5QmX0cMAPF4EQ@mail.gmail.com>
Date:   Wed, 11 May 2022 13:37:43 +0800
From:   Tyrone Ting <warp5tw@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     avifishman70@...il.com, tmaimon77@...il.com, tali.perry1@...il.com,
        venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
        robh+dt@...nel.org, wsa@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, jarkko.nikula@...ux.intel.com,
        semen.protsenko@...aro.org, sven@...npeter.dev, jsd@...ihalf.com,
        lukas.bulwahn@...il.com, olof@...om.net, arnd@...db.de,
        tali.perry@...oton.com, Avi.Fishman@...oton.com,
        tomer.maimon@...oton.com, KWLIU@...oton.com, JJLIU0@...oton.com,
        kfting@...oton.com, openbmc@...ts.ozlabs.org,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 7/9] i2c: npcm: Handle spurious interrupts

Hi Andy:

Thank you for your comments and they will be addressed.

Andy Shevchenko <andriy.shevchenko@...ux.intel.com> 於 2022年5月10日 週二 下午6:15寫道：
>
> On Tue, May 10, 2022 at 05:16:52PM +0800, Tyrone Ting wrote:
> > From: Tali Perry <tali.perry1@...il.com>
> >
> > On some platforms in rare cases (1 to 100,000 transactions),
> > the i2c gets a spurious interrupt which means that we enter an interrupt
> > but in the interrupt handler we don't find any status bit that points to
> > the reason we got this interrupt.
> >
> > This may be a case of a rare HW issue or signal integrity issue that is
> > still under investigation.
> >
> > In order to overcome this we are doing the following:
> > 1. Disable incoming interrupts in master mode only when slave mode is not
> >    enabled.
> > 2. Clear end of busy (EOB) after every interrupt.
> > 3. Clear other status bits (just in case since we found them cleared)
> > 4. Return correct status during the interrupt that will finish the
> >    transaction.
> >
> > On next xmit transaction if the bus is still busy the master will issue a
> > recovery process before issuing the new transaction.
>
> ...
>
> > +     /* clear status bits for spurious interrupts */
>
> Clear
>
> ...
>
> > +     /*
> > +      * if irq is not one of the above, make sure EOB is disabled and all
>
> If
>
> > +      * status bits are cleared.
> > +      */
>
> ...
>
> > +             /* verify no status bits are still set after bus is released */
>
> Verify
>
> ...
>
> > +     /* check HW is OK: SDA and SCL should be high at this point. */
>
> Check
>
> ...
>
> > +     if ((npcm_i2c_get_SDA(&bus->adap) == 0) ||
> > +         (npcm_i2c_get_SCL(&bus->adap) == 0)) {
>
> This fits one line
>
> > +             dev_err(bus->dev, "I2C%d init fail: lines are low", bus->num);
> > +             dev_err(bus->dev, "SDA=%d SCL=%d", npcm_i2c_get_SDA(&bus->adap),
> > +                     npcm_i2c_get_SCL(&bus->adap));
>
> No '\n' at the end of each?!
>
> > +             return -ENXIO;
> > +     }
>
> ...
>
> > +     /* clear status bits for spurious interrupts */
>
> Clear
>
> ...
>
> > +     /* after any xfer, successful or not, stall and EOB must be disabled */
>
> After
>
> ...
>
> Maybe you chose the small letter for one-liner comments, but I see even in the
> original code the inconsistent style. Please, add an explanation to the cover
> letter and follow it, assuming you add the patch at the end of the series that
> makes comment style consistent (for the one-liners, for the multi-line comments
> we have a clear understanding about the style).
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Best Regards,
Tyrone