[v2,05/19] spi: dw: Enable interrupts in accordance with DMA xfer mode

Commit Message

Serge Semin May 15, 2020, 10:47 a.m. UTC

It's pointless to track the Tx overrun interrupts if Rx-only SPI
transfer is issued. Similarly there is no need in handling the Rx
overrun/underrun interrupts if Tx-only SPI transfer is executed.
So lets unmask the interrupts only if corresponding SPI
transactions are implied.

Co-developed-by: Georgy Vlasov <[e-mail]>
Signed-off-by: Georgy Vlasov <[e-mail]>
Signed-off-by: Serge Semin <[e-mail]>
Cc: Ramil Zaripov <[e-mail]>
Cc: Alexey Malahov <[e-mail]>
Cc: Thomas Bogendoerfer <[e-mail]>
Cc: Paul Burton <[e-mail]>
Cc: Ralf Baechle <[e-mail]>
Cc: Arnd Bergmann <[e-mail]>
Cc: Allison Randal <[e-mail]>
Cc: Andy Shevchenko <[e-mail]>
Cc: Gareth Williams <[e-mail]>
Cc: Rob Herring <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
---
 drivers/spi/spi-dw-mid.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko May 15, 2020, 12:27 p.m. UTC | #1

On Fri, May 15, 2020 at 01:47:44PM +0300, Serge Semin wrote:
> It's pointless to track the Tx overrun interrupts if Rx-only SPI
> transfer is issued. Similarly there is no need in handling the Rx
> overrun/underrun interrupts if Tx-only SPI transfer is executed.
> So lets unmask the interrupts only if corresponding SPI
> transactions are implied.

My comments below.

> Co-developed-by: Georgy Vlasov <[e-mail]>
> Signed-off-by: Georgy Vlasov <[e-mail]>
> Signed-off-by: Serge Semin <[e-mail]>
> Cc: Ramil Zaripov <[e-mail]>
> Cc: Alexey Malahov <[e-mail]>
> Cc: Thomas Bogendoerfer <[e-mail]>
> Cc: Paul Burton <[e-mail]>
> Cc: Ralf Baechle <[e-mail]>
> Cc: Arnd Bergmann <[e-mail]>
> Cc: Allison Randal <[e-mail]>
> Cc: Andy Shevchenko <[e-mail]>
> Cc: Gareth Williams <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]

I think you really need to revisit Cc list in all patches (DT people hardly
interested in this one, though ones where properties are being used might be
point of interest).

...

>  	/* Set the interrupt mask */
> -	spi_umask_intr(dws, SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI);
> +	spi_umask_intr(dws, imr);

Can we rather do like this

	/* Set the interrupt mask */
	if (xfer->tx_buf)
		imr |= SPI_INT_TXOI;
	if (xfer->rx_buf)
		imr |= SPI_INT_RXUI | SPI_INT_RXOI;
	spi_umask_intr(dws, imr);

?

(First block sets DMA, second one IRQ)

Serge Semin May 16, 2020, 2:06 p.m. UTC | #2

On Fri, May 15, 2020 at 03:27:00PM +0300, Andy Shevchenko wrote:
> On Fri, May 15, 2020 at 01:47:44PM +0300, Serge Semin wrote:
> > It's pointless to track the Tx overrun interrupts if Rx-only SPI
> > transfer is issued. Similarly there is no need in handling the Rx
> > overrun/underrun interrupts if Tx-only SPI transfer is executed.
> > So lets unmask the interrupts only if corresponding SPI
> > transactions are implied.
> 
> My comments below.
> 
> > Co-developed-by: Georgy Vlasov <[e-mail]>
> > Signed-off-by: Georgy Vlasov <[e-mail]>
> > Signed-off-by: Serge Semin <[e-mail]>
> > Cc: Ramil Zaripov <[e-mail]>
> > Cc: Alexey Malahov <[e-mail]>
> > Cc: Thomas Bogendoerfer <[e-mail]>
> > Cc: Paul Burton <[e-mail]>
> > Cc: Ralf Baechle <[e-mail]>
> > Cc: Arnd Bergmann <[e-mail]>
> > Cc: Allison Randal <[e-mail]>
> > Cc: Andy Shevchenko <[e-mail]>
> > Cc: Gareth Williams <[e-mail]>
> > Cc: Rob Herring <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> 
> I think you really need to revisit Cc list in all patches (DT people hardly
> interested in this one, though ones where properties are being used might be
> point of interest).
> 
> ...
> 
> >  	/* Set the interrupt mask */
> > -	spi_umask_intr(dws, SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI);
> > +	spi_umask_intr(dws, imr);
> 

> Can we rather do like this
> 
> 	/* Set the interrupt mask */
> 	if (xfer->tx_buf)
> 		imr |= SPI_INT_TXOI;
> 	if (xfer->rx_buf)
> 		imr |= SPI_INT_RXUI | SPI_INT_RXOI;
> 	spi_umask_intr(dws, imr);
> 
> ?
> 
> (First block sets DMA, second one IRQ)

I'd rather leave it as is.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>

Patch

diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 0c597b6bb154..1902336cb8d6 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -293,19 +293,23 @@  static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
 
 static int mid_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
 {
-	u16 dma_ctrl = 0;
+	u16 imr = 0, dma_ctrl = 0;
 
 	dw_writel(dws, DW_SPI_DMARDLR, 0xf);
 	dw_writel(dws, DW_SPI_DMATDLR, 0x10);
 
-	if (xfer->tx_buf)
+	if (xfer->tx_buf) {
 		dma_ctrl |= SPI_DMA_TDMAE;
-	if (xfer->rx_buf)
+		imr |= SPI_INT_TXOI;
+	}
+	if (xfer->rx_buf) {
 		dma_ctrl |= SPI_DMA_RDMAE;
+		imr |= SPI_INT_RXUI | SPI_INT_RXOI;
+	}
 	dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
 
 	/* Set the interrupt mask */
-	spi_umask_intr(dws, SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI);
+	spi_umask_intr(dws, imr);
 
 	dws->transfer_handler = dma_transfer;