Re: [PATCH] spi: fsl-espi: Only process interrupts for expected events

[Joakim Tjernlund <Joakim.Tjernlund@xxxxxxxxxxxx>]

On Mon, 2020-09-14 at 12:28 +1000, Nicholas Piggin wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Excerpts from Chris Packham's message of September 14, 2020 8:03 am:
> > Hi All,
> > 
> > On 4/09/20 12:28 pm, Chris Packham wrote:
> > > The SPIE register contains counts for the TX FIFO so any time the irq
> > > handler was invoked we would attempt to process the RX/TX fifos. Use the
> > > SPIM value to mask the events so that we only process interrupts that
> > > were expected.
> > > 
> > > This was a latent issue exposed by commit 3282a3da25bd ("powerpc/64:
> > > Implement soft interrupt replay in C").
> > > 
> > > Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > ---
> > ping?
> 
> I don't know the code/hardware but thanks for tracking this down.
> 
> Was there anything more to be done with Jocke's observations, or would
> that be a follow-up patch if anything?

Patch is good IMHO, there may be more to fix w.r.t clearing the IRQs

> 
> If this patch fixes your problem it should probably go in, unless there
> are any objections.

It should go in I think.

 Jocke

> 
> Thanks,
> Nick
> 
> > > 
> > > Notes:
> > >      I've tested this on a T2080RDB and a custom board using the T2081 SoC. With
> > >      this change I don't see any spurious instances of the "Transfer done but
> > >      SPIE_DON isn't set!" or "Transfer done but rx/tx fifo's aren't empty!" messages
> > >      and the updates to spi flash are successful.
> > > 
> > >      I think this should go into the stable trees that contain 3282a3da25bd but I
> > >      haven't added a Fixes: tag because I think 3282a3da25bd exposed the issue as
> > >      opposed to causing it.
> > > 
> > >   drivers/spi/spi-fsl-espi.c | 5 +++--
> > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
> > > index 7e7c92cafdbb..cb120b68c0e2 100644
> > > --- a/drivers/spi/spi-fsl-espi.c
> > > +++ b/drivers/spi/spi-fsl-espi.c
> > > @@ -574,13 +574,14 @@ static void fsl_espi_cpu_irq(struct fsl_espi *espi, u32 events)
> > >   static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
> > >   {
> > >      struct fsl_espi *espi = context_data;
> > > -    u32 events;
> > > +    u32 events, mask;
> > > 
> > >      spin_lock(&espi->lock);
> > > 
> > >      /* Get interrupt events(tx/rx) */
> > >      events = fsl_espi_read_reg(espi, ESPI_SPIE);
> > > -    if (!events) {
> > > +    mask = fsl_espi_read_reg(espi, ESPI_SPIM);
> > > +    if (!(events & mask)) {
> > >              spin_unlock(&espi->lock);
> > >              return IRQ_NONE;
> > >      }