Re: [PATCH] dmaengine: add verification of DMA_INTERRUPT capability for dmatest

[Dave Jiang]

On 5/30/2022 10:24 PM, Vinod Koul wrote:
> On 30-05-22, 10:06, Geert Uytterhoeven wrote:
> > Hi Dave, Vinod,
> Hi Geert,
>
> > On Wed, Apr 13, 2022 at 12:58 AM Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
> > > Looks like I forgot to add DMA_INTERRUPT cap setting to the idxd driver and
> > > dmatest is still working regardless of this mistake. Add an explicit check
> > > of DMA_INTERRUPT capability for dmatest to make sure the DMA device being used
> > > actually supports interrupt before the test is launched and also that the
> > > driver is programmed correctly.
> > >
> > > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> > Thanks for your patch, which is now commit a8facc7b988599f8
> > ("dmaengine: add verification of DMA_INTERRUPT capability for
> > dmatest") upstream.
> >
> > > --- a/drivers/dma/dmatest.c
> > > +++ b/drivers/dma/dmatest.c
> > > @@ -675,10 +675,16 @@ static int dmatest_func(void *data)
> > >          /*
> > >           * src and dst buffers are freed by ourselves below
> > >           */
> > > -       if (params->polled)
> > > +       if (params->polled) {
> > >                  flags = DMA_CTRL_ACK;
> > > -       else
> > > -               flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> > > +       } else {
> > > +               if (dma_has_cap(DMA_INTERRUPT, dev->cap_mask)) {
> > > +                       flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> > > +               } else {
> > > +                       pr_err("Channel does not support interrupt!\n");
> > > +                       goto err_pq_array;
> > > +               }
> > > +       }
> > >
> > >          ktime = ktime_get();
> > >          while (!(kthread_should_stop() ||
> > > @@ -906,6 +912,7 @@ static int dmatest_func(void *data)
> > Shimoda-san reports that this commit breaks dmatest on rcar-dmac.
> > Like most DMA engine drivers, rcar-dmac does not set the DMA_INTERRUPT
> > capability flag, hence dmatest now fails to start:
> >
> >      dmatest: Channel does not support interrupt!
> >
> > To me, it looks like the new check is bogus, as I believe it confuses
> > two different concepts:
> >
> >    1. Documentation/driver-api/dmaengine/provider.rst says:
> >
> >         - DMA_INTERRUPT
> >
> >           - The device is able to trigger a dummy transfer that will
> >             generate periodic interrupts
> >
> >    2. In non-polled mode, dmatest sets DMA_PREP_INTERRUPT.
> >       include/linux/dmaengine.h says:
> >
> >         * @DMA_PREP_INTERRUPT - trigger an interrupt (callback) upon
> > completion of
> >         *  this transaction
> >
> > As dmatest uses real transfers, I think it does not depend on
> > the ability to use interrupts from dummy transfers.
> Yes this does not look right to me. DMA_INTERRUPT is for a specific
> capability which is linked to dma_prep_interrupt() which dmatest does
> not use so i think it is not correct for dmatest to use this...
>
> I can revert this patch... Dave?
Yes we can revert it.