[PATCH 2/4] spi: bcm2835aux: disable tx fifo empty irq

[Stephan Olbrich]

Am Tuesday 09 February 2016, 15:45:32 schrieb Eric Anholt:
> stephanolbrich at gmx.de writes:
> > From: Stephan Olbrich <stephanolbrich at gmx.de>
> > 
> > The tx empty irq can be disabled when all data was copied.
> > This prevents unnecessary interrupts while the last bytes are sent.
> > 
> > Signed-off-by: Stephan Olbrich <stephanolbrich at gmx.de>
> > ---
> > 
> >  drivers/spi/spi-bcm2835aux.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c
> > index ecc73c0..d2f0067 100644
> > --- a/drivers/spi/spi-bcm2835aux.c
> > +++ b/drivers/spi/spi-bcm2835aux.c
> > @@ -212,6 +212,12 @@ static irqreturn_t bcm2835aux_spi_interrupt(int irq,
> > void *dev_id)> 
> >  		ret = IRQ_HANDLED;
> >  	
> >  	}
> > 
> > +	if (!bs->tx_len) {
> > +		/* disable tx fifo empty interrupt */
> > +		bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1] |
> > +			BCM2835_AUX_SPI_CNTL1_IDLE);
> > +	}
> > +
> > 
> >  	/* and if rx_len is 0 then wake up completion and disable spi */
> >  	if (!bs->rx_len) {
> >  	
> >  		bcm2835aux_spi_reset_hw(bs);
> 
> Right, we don't want to come back in here with a spurious TX empty
> interrupt while we wait for the RX bits to trickle in through the FIFO.
> I'm having a hard time reasoning through how likely this would be, but
> it seems like a good change.
I got annoyed by those interrupts during debugging, so yes they do occur.

> Reviewed-by: Eric Anholt <eric at anholt.net>
> 
> Aside: I think I see a problem that we reset the hardware before it has
> asserted SPI_STAT_BUSY, since we reset as soon as we've collected our RX
> data (!bs->rx_len).  That means we've potentially missed the trailing
> hold time on CS at the end of the transfer, and it's going to resume at
> the same point in its state machine when we reassert CNTL0_ENABLE for
> the next transfer.  That doesn't seem like what we want.

We could check the SPI_STAT_BUSY flag before resetting the hardware. But in the 
third patch in this series the reset gets moved to unprepare_message. How 
likely is it, that we're still not done by then?