dmaengine : xilinx_dma two issues

[Lars-Peter Clausen]

On 1/10/21 4:16 PM, Paul Thomas wrote:
> On Fri, Jan 8, 2021 at 1:36 PM Radhey Shyam Pandey <radheys at xilinx.com> wrote:
>>> -----Original Message-----
>>> From: Paul Thomas <pthomas8589 at gmail.com>
>>> Sent: Friday, January 8, 2021 9:27 PM
>>> To: Radhey Shyam Pandey <radheys at xilinx.com>
>>> Cc: Dan Williams <dan.j.williams at intel.com>; Vinod Koul
>>> <vkoul at kernel.org>; Michal Simek <michals at xilinx.com>; Matthew Murrian
>>> <matthew.murrian at goctsi.com>; Romain Perier
>>> <romain.perier at gmail.com>; Krzysztof Kozlowski <krzk at kernel.org>; Marc
>>> Ferland <ferlandm at amotus.ca>; Sebastian von Ohr
>>> <vonohr at smaract.com>; dmaengine at vger.kernel.org; Linux ARM <linux-
>>> arm-kernel at lists.infradead.org>; linux-kernel <linux-
>>> kernel at vger.kernel.org>; dave.jiang at intel.com; Shravya Kumbham
>>> <shravyak at xilinx.com>; git <git at xilinx.com>
>>> Subject: Re: dmaengine : xilinx_dma two issues
>>>
>>> Hi All,
>>>
>>> On Fri, Jan 8, 2021 at 2:13 AM Radhey Shyam Pandey <radheys at xilinx.com>
>>> wrote:
>>>>> -----Original Message-----
>>>>> From: Radhey Shyam Pandey
>>>>> Sent: Monday, January 4, 2021 10:50 AM
>>>>> To: Paul Thomas <pthomas8589 at gmail.com>; Dan Williams
>>>>> <dan.j.williams at intel.com>; Vinod Koul <vkoul at kernel.org>; Michal
>>>>> Simek <michals at xilinx.com>; Matthew Murrian
>>>>> <matthew.murrian at goctsi.com>; Romain Perier
>>>>> <romain.perier at gmail.com>; Krzysztof Kozlowski <krzk at kernel.org>;
>>>>> Marc Ferland <ferlandm at amotus.ca>; Sebastian von Ohr
>>>>> <vonohr at smaract.com>; dmaengine at vger.kernel.org; Linux ARM <linux-
>>>>> arm-kernel at lists.infradead.org>; linux-kernel <linux-
>>>>> kernel at vger.kernel.org>; Shravya Kumbham <shravyak at xilinx.com>; git
>>>>> <git at xilinx.com>
>>>>> Subject: RE: dmaengine : xilinx_dma two issues
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Paul Thomas <pthomas8589 at gmail.com>
>>>>>> Sent: Monday, December 28, 2020 10:14 AM
>>>>>> To: Dan Williams <dan.j.williams at intel.com>; Vinod Koul
>>>>>> <vkoul at kernel.org>; Michal Simek <michals at xilinx.com>; Radhey
>>>>>> Shyam Pandey <radheys at xilinx.com>; Matthew Murrian
>>>>>> <matthew.murrian at goctsi.com>; Romain Perier
>>>>> <romain.perier at gmail.com>;
>>>>>> Krzysztof Kozlowski <krzk at kernel.org>; Marc Ferland
>>>>>> <ferlandm at amotus.ca>; Sebastian von Ohr <vonohr at smaract.com>;
>>>>>> dmaengine at vger.kernel.org; Linux ARM <linux-
>>>>>> arm-kernel at lists.infradead.org>; linux-kernel <linux-
>>>>>> kernel at vger.kernel.org>
>>>>>> Subject: dmaengine : xilinx_dma two issues
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> I'm trying to get the 5.10 kernel up and running for our system,
>>>>>> and I'm running into a couple of issues with xilinx_dma.
>>>>> + (Xilinx mailing list)
>>>>>
>>>>> Thanks for bringing the issues to our notice. Replies inline.
>>>>>
>>>>>> First, commit 14ccf0aab46e 'dmaengine: xilinx_dma: In dma channel
>>>>>> probe fix node order dependency' breaks our usage. Before this
>>>>>> commit a
>>>>> call to:
>>>>>> dma_request_chan(&indio_dev->dev, "axi_dma_0"); returns fine, but
>>>>>> after that commit it returns -19. The reason for this seems to be
>>>>>> that the only channel that is setup is channel 1 (chan->id is 1 in
>>>>> xilinx_dma_chan_probe()).
>>>>>> However in
>>>>>> of_dma_xilinx_xlate() chan_id is gets set to 0 (int chan_id =
>>>>>> dma_spec-
>>>>>>> args[0];), which causes the:
>>>>>> !xdev->chan[chan_id]
>>>>>> test to fail in of_dma_xilinx_xlate()
>>>>> What is the channel number passed in dmaclient DT?
>>> Is this a question for me?
>> Yes, please also share the dmaclient DT client node. Need to see
>> channel number passed to dmas property. Something like below-
>>
>> dmas = <& axi_dma_0 1>
>> dma-names = "axi_dma_0"
> OK, I think I need to revisit this and clean it up some. Currently In
> the driver (a custom iio adc driver) it is hard coded:
> dma_request_chan(&indio_dev->dev, "axi_dma_0");
>
> However, the DT also has the entries (currently unused by the driver):
>          dmas = <&axi_dma_0 0>;
>          dma-names = "axi_dma_0";
>
> I'll go back and clean up our driver to do something like adi-axi-adc.c does:
>
>          if (!device_property_present(dev, "dmas"))
>                  return 0;
>
>          if (device_property_read_string(dev, "dma-names", &dma_name))
>                  dma_name = "axi_dma_0";
>
> Should the dmas node get used by the driver? I see the second argument
> is: '0' for write/tx and '1' for read/rx channel. So I should be
> setting this to 1 like this?
>          dmas = <&axi_dma_0 1>;
>          dma-names = "axi_dma_0";
>
> But where does that field get used?

This got broken in "dmaengine: xilinx_dma: In dma channel probe fix node 
order dependency" 
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=14ccf0aab46e1888e2f45b6e995c621c70b32651>. 
Before if there was only one channel that channel was always at index 0. 
Regardless of whether the channel was RX or TX. But after that change 
the RX channel is always at offset 1, regardless of whether the DMA has 
one or two channels. This is a breakage in ABI.

If you have the choice I'd recommend to not use the Xilinx DMA, it gets 
broken pretty much every other release.

- Lars