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

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

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@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.

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