[PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops

[PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops

[Biju Das]

RSPI IP on RZ/{A, G2L} SoC's has the same signal for both interrupt and
DMA transfer request. Setting DMARS register for DMA transfer
makes the signal to work as a DMA transfer request signal and
subsequent interrupt requests to the interrupt controller
are masked.

Currently, DMA to interrupt mode switching does not work because of this
masking.

This patch adds need_dmar_clr device configuration flag to spi_ops
and it makes the signal to work as an interrupt request by clearing
DMARS after DMA callback.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Fixed the typo need_dmar_clr->rspi->ops->need_dmar_clr.
---
 drivers/spi/spi-rspi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 7a014eeec2d0..8e316f0025e4 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -249,6 +249,7 @@ struct spi_ops {
 	u16 flags;
 	u16 fifo_size;
 	u8 num_hw_ss;
+	bool need_dmar_clr;
 };
 
 static void rspi_set_rate(struct rspi_data *rspi)
@@ -613,6 +614,12 @@ static int rspi_dma_transfer(struct rspi_data *rspi, struct sg_table *tx,
 					       rspi->dma_callbacked, HZ);
 	if (ret > 0 && rspi->dma_callbacked) {
 		ret = 0;
+		if (rspi->ops->need_dmar_clr) {
+			if (tx)
+				dmaengine_synchronize(rspi->ctlr->dma_tx);
+			if (rx)
+				dmaengine_synchronize(rspi->ctlr->dma_rx);
+		}
 	} else {
 		if (!ret) {
 			dev_err(&rspi->ctlr->dev, "DMA timeout\n");
@@ -1196,6 +1203,7 @@ static const struct spi_ops rspi_rz_ops = {
 	.flags =		SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX,
 	.fifo_size =		8,	/* 8 for TX, 32 for RX */
 	.num_hw_ss =		1,
+	.need_dmar_clr =	true,
 };
 
 static const struct spi_ops qspi_ops = {
-- 
2.25.1

Re: [PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On Tue, Jul 19, 2022 at 04:00:00PM +0100, Biju Das wrote:
> RSPI IP on RZ/{A, G2L} SoC's has the same signal for both interrupt and
> DMA transfer request. Setting DMARS register for DMA transfer
> makes the signal to work as a DMA transfer request signal and
> subsequent interrupt requests to the interrupt controller
> are masked.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops

[Geert Uytterhoeven]

Hi Biju,

On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RSPI IP on RZ/{A, G2L} SoC's has the same signal for both interrupt and
> DMA transfer request. Setting DMARS register for DMA transfer
> makes the signal to work as a DMA transfer request signal and
> subsequent interrupt requests to the interrupt controller
> are masked.
>
> Currently, DMA to interrupt mode switching does not work because of this
> masking.
>
> This patch adds need_dmar_clr device configuration flag to spi_ops
> and it makes the signal to work as an interrupt request by clearing
> DMARS after DMA callback.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Fixed the typo need_dmar_clr->rspi->ops->need_dmar_clr.

Thanks for your patch!

> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -249,6 +249,7 @@ struct spi_ops {
>         u16 flags;
>         u16 fifo_size;
>         u8 num_hw_ss;
> +       bool need_dmar_clr;

Do you need this flag? See below.

>  };
>
>  static void rspi_set_rate(struct rspi_data *rspi)
> @@ -613,6 +614,12 @@ static int rspi_dma_transfer(struct rspi_data *rspi, struct sg_table *tx,
>                                                rspi->dma_callbacked, HZ);
>         if (ret > 0 && rspi->dma_callbacked) {
>                 ret = 0;
> +               if (rspi->ops->need_dmar_clr) {
> +                       if (tx)
> +                               dmaengine_synchronize(rspi->ctlr->dma_tx);
> +                       if (rx)
> +                               dmaengine_synchronize(rspi->ctlr->dma_rx);
> +               }

Why not call it unconditionally?
If the DMAC driver does not provide a .device_synchronize(), it is
a no-op anyway.

BTW, I don't think there is a hard dependency on patch 1/2, so
I think this patch can go in through the SPI tree.

>         } else {
>                 if (!ret) {
>                         dev_err(&rspi->ctlr->dev, "DMA timeout\n");

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/2] spi: spi-rspi: Add need_dmar_clr to spi_ops
> 
> Hi Biju,
> 
> On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > RSPI IP on RZ/{A, G2L} SoC's has the same signal for both interrupt
> > and DMA transfer request. Setting DMARS register for DMA transfer
> > makes the signal to work as a DMA transfer request signal and
> > subsequent interrupt requests to the interrupt controller are masked.
> >
> > Currently, DMA to interrupt mode switching does not work because of
> > this masking.
> >
> > This patch adds need_dmar_clr device configuration flag to spi_ops and
> > it makes the signal to work as an interrupt request by clearing DMARS
> > after DMA callback.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * Fixed the typo need_dmar_clr->rspi->ops->need_dmar_clr.
> 
> Thanks for your patch!
> 
> > --- a/drivers/spi/spi-rspi.c
> > +++ b/drivers/spi/spi-rspi.c
> > @@ -249,6 +249,7 @@ struct spi_ops {
> >         u16 flags;
> >         u16 fifo_size;
> >         u8 num_hw_ss;
> > +       bool need_dmar_clr;
> 
> Do you need this flag? See below.

Ok.

> 
> >  };
> >
> >  static void rspi_set_rate(struct rspi_data *rspi) @@ -613,6 +614,12
> > @@ static int rspi_dma_transfer(struct rspi_data *rspi, struct
> sg_table *tx,
> >                                                rspi->dma_callbacked,
> HZ);
> >         if (ret > 0 && rspi->dma_callbacked) {
> >                 ret = 0;
> > +               if (rspi->ops->need_dmar_clr) {
> > +                       if (tx)
> > +                               dmaengine_synchronize(rspi->ctlr-
> >dma_tx);
> > +                       if (rx)
> > +                               dmaengine_synchronize(rspi->ctlr-
> >dma_rx);
> > +               }
> 
> Why not call it unconditionally?
> If the DMAC driver does not provide a .device_synchronize(), it is a no-
> op anyway.

OK, currently rcar has this callback and it calls synchronize_irq();
So I guess it should be ok as we are calling this after wait_event
Synchronization with DMA callback.

> 
> BTW, I don't think there is a hard dependency on patch 1/2, so I think
> this patch can go in through the SPI tree.

OK.

Cheers,
Biju

> 
> >         } else {
> >                 if (!ret) {
> >                         dev_err(&rspi->ctlr->dev, "DMA timeout\n");
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds