diff mbox

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

Message ID 1455041435-8015-3-git-send-email-stephanolbrich@gmx.de (mailing list archive)
State Accepted
Commit f29ab1845f3e2684ba1c6de6c3bd5198e4b1459c
Headers show

Commit Message

Stephan Olbrich Feb. 9, 2016, 6:10 p.m. UTC
From: Stephan Olbrich <stephanolbrich@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@gmx.de>
---
 drivers/spi/spi-bcm2835aux.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Anholt Feb. 9, 2016, 11:45 p.m. UTC | #1
stephanolbrich@gmx.de writes:

> From: Stephan Olbrich <stephanolbrich@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@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);
> -- 
> 2.5.0

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.

Reviewed-by: Eric Anholt <eric@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.
diff mbox

Patch

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);