Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <ba4ddd60-335b-b80a-68a2-0568c166f82d@intel.com>
Date:   Tue, 31 May 2022 07:49:16 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.9.1
Subject: Re: [PATCH] dmaengine: add verification of DMA_INTERRUPT capability
 for dmatest
Content-Language: en-US
To:     Vinod Koul <vkoul@kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        dmaengine <dmaengine@vger.kernel.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <164978679251.2361020.5856734256126725993.stgit@djiang5-desk3.ch.intel.com>
 <CAMuHMdVjDTAW-84c9Fh21f_GWOhnD4+VW2nqSTQ6EK-m+KG=vQ@mail.gmail.com>
 <YpWmcHtGzrv4oP5L@matsya>
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <YpWmcHtGzrv4oP5L@matsya>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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