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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux