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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Biju,

On Tue, Jul 19, 2022 at 5:00 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx

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