[PATCH v2 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[PATCH v2 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[Biju Das]

Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC
use the same signal for both interrupt and DMA transfer requests.
The signal works as a DMA transfer request signal by setting
DMARS, and subsequent interrupt requests to the interrupt controller
are masked.

We can enable the interrupt by clearing the DMARS.

This patch adds device_synchronize callback for clearing
DMARS and thereby allowing DMA consumers to switch to
DMA mode.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change
---
 drivers/dma/sh/rz-dmac.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index ee2872e7d64c..675e6d5319d7 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -630,6 +630,14 @@ static void rz_dmac_virt_desc_free(struct virt_dma_desc *vd)
 	 */
 }
 
+static void rz_dmac_device_synchronize(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	struct rz_dmac *dmac = to_rz_dmac(chan->device);
+
+	rz_dmac_set_dmars_register(dmac, channel->index, 0);
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -909,6 +917,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	engine->device_config = rz_dmac_config;
 	engine->device_terminate_all = rz_dmac_terminate_all;
 	engine->device_issue_pending = rz_dmac_issue_pending;
+	engine->device_synchronize = rz_dmac_device_synchronize;
 
 	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
 	dma_set_max_seg_size(engine->dev, U32_MAX);
-- 
2.25.1

[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 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[Geert Uytterhoeven]

On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC
> use the same signal for both interrupt and DMA transfer requests.
> The signal works as a DMA transfer request signal by setting
> DMARS, and subsequent interrupt requests to the interrupt controller
> are masked.
>
> We can enable the interrupt by clearing the DMARS.

re-enable?

>
> This patch adds device_synchronize callback for clearing
> DMARS and thereby allowing DMA consumers to switch to
> DMA mode.

interrupt mode

>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * No change

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[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 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[Geert Uytterhoeven]

Hi Biju,

On Thu, Jul 21, 2022 at 11:47 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC
> > use the same signal for both interrupt and DMA transfer requests.
> > The signal works as a DMA transfer request signal by setting
> > DMARS, and subsequent interrupt requests to the interrupt controller
> > are masked.
> >
> > We can enable the interrupt by clearing the DMARS.
>
> re-enable?
>
> >
> > This patch adds device_synchronize callback for clearing
> > DMARS and thereby allowing DMA consumers to switch to
> > DMA mode.
>
> interrupt mode
>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * No change
>
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > +static void rz_dmac_device_synchronize(struct dma_chan *chan)
> > +{
> > +       struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > +       struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > +

Actually this should check if the DMA operation has been completed
or terminated, and wait (sleep) if needed.

> > +       rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > +}

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 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[Biju Das]

Hi Geert,

Thanks for the feedback

> Subject: Re: [PATCH v2 1/2] dmaengine: sh: rz-dmac: Add
> device_synchronize callback
> 
> On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use the
> > same signal for both interrupt and DMA transfer requests.
> > The signal works as a DMA transfer request signal by setting DMARS,
> > and subsequent interrupt requests to the interrupt controller are
> > masked.
> >
> > We can enable the interrupt by clearing the DMARS.
> 
> re-enable?

OK.

> 
> >
> > This patch adds device_synchronize callback for clearing DMARS and
> > thereby allowing DMA consumers to switch to DMA mode.
> 
> interrupt mode

OK. Will fix this in v2.

Cheers,
Biju

> 
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * No change
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> 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

RE: [PATCH v2 1/2] dmaengine: sh: rz-dmac: Add device_synchronize callback

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/2] dmaengine: sh: rz-dmac: Add
> device_synchronize callback
> 
> Hi Biju,
> 
> On Thu, Jul 21, 2022 at 11:47 AM Geert Uytterhoeven <geert@linux-
> m68k.org> wrote:
> > On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Some on-chip peripheral modules(for eg:- rspi) on RZ/G2L SoC use the
> > > same signal for both interrupt and DMA transfer requests.
> > > The signal works as a DMA transfer request signal by setting DMARS,
> > > and subsequent interrupt requests to the interrupt controller are
> > > masked.
> > >
> > > We can enable the interrupt by clearing the DMARS.
> >
> > re-enable?
> >
> > >
> > > This patch adds device_synchronize callback for clearing DMARS and
> > > thereby allowing DMA consumers to switch to DMA mode.
> >
> > interrupt mode
> >
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > v1->v2:
> > >  * No change
> >
> > With the above fixed:
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > +static void rz_dmac_device_synchronize(struct dma_chan *chan) {
> > > +       struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > > +       struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > > +
> 
> Actually this should check if the DMA operation has been completed or
> terminated, and wait (sleep) if needed.

OK will send v3 with these changes.

Cheers,
Biju