Messages in this thread |  | | Subject | Re: dmaengine : xilinx_dma two issues | From | Lars-Peter Clausen <> | Date | Mon, 11 Jan 2021 16:33:12 +0100 |
| |
On 1/11/21 10:32 AM, Michal Simek wrote: > Hi Lars, > > On 10. 01. 21 16:43, Lars-Peter Clausen wrote: >> On 1/10/21 4:16 PM, Paul Thomas wrote: >>> On Fri, Jan 8, 2021 at 1:36 PM Radhey Shyam Pandey >>> <radheys@xilinx.com> wrote: >>>>> -----Original Message----- >>>>> From: Paul Thomas <pthomas8589@gmail.com> >>>>> Sent: Friday, January 8, 2021 9:27 PM >>>>> To: Radhey Shyam Pandey <radheys@xilinx.com> >>>>> Cc: Dan Williams <dan.j.williams@intel.com>; Vinod Koul >>>>> <vkoul@kernel.org>; Michal Simek <michals@xilinx.com>; Matthew Murrian >>>>> <matthew.murrian@goctsi.com>; Romain Perier >>>>> <romain.perier@gmail.com>; Krzysztof Kozlowski <krzk@kernel.org>; Marc >>>>> Ferland <ferlandm@amotus.ca>; Sebastian von Ohr >>>>> <vonohr@smaract.com>; dmaengine@vger.kernel.org; Linux ARM <linux- >>>>> arm-kernel@lists.infradead.org>; linux-kernel <linux- >>>>> kernel@vger.kernel.org>; dave.jiang@intel.com; Shravya Kumbham >>>>> <shravyak@xilinx.com>; git <git@xilinx.com> >>>>> Subject: Re: dmaengine : xilinx_dma two issues >>>>> >>>>> Hi All, >>>>> >>>>> On Fri, Jan 8, 2021 at 2:13 AM Radhey Shyam Pandey <radheys@xilinx.com> >>>>> wrote: >>>>>>> -----Original Message----- >>>>>>> From: Radhey Shyam Pandey >>>>>>> Sent: Monday, January 4, 2021 10:50 AM >>>>>>> To: Paul Thomas <pthomas8589@gmail.com>; Dan Williams >>>>>>> <dan.j.williams@intel.com>; Vinod Koul <vkoul@kernel.org>; Michal >>>>>>> Simek <michals@xilinx.com>; Matthew Murrian >>>>>>> <matthew.murrian@goctsi.com>; Romain Perier >>>>>>> <romain.perier@gmail.com>; Krzysztof Kozlowski <krzk@kernel.org>; >>>>>>> Marc Ferland <ferlandm@amotus.ca>; Sebastian von Ohr >>>>>>> <vonohr@smaract.com>; dmaengine@vger.kernel.org; Linux ARM <linux- >>>>>>> arm-kernel@lists.infradead.org>; linux-kernel <linux- >>>>>>> kernel@vger.kernel.org>; Shravya Kumbham <shravyak@xilinx.com>; git >>>>>>> <git@xilinx.com> >>>>>>> Subject: RE: dmaengine : xilinx_dma two issues >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Paul Thomas <pthomas8589@gmail.com> >>>>>>>> Sent: Monday, December 28, 2020 10:14 AM >>>>>>>> To: Dan Williams <dan.j.williams@intel.com>; Vinod Koul >>>>>>>> <vkoul@kernel.org>; Michal Simek <michals@xilinx.com>; Radhey >>>>>>>> Shyam Pandey <radheys@xilinx.com>; Matthew Murrian >>>>>>>> <matthew.murrian@goctsi.com>; Romain Perier >>>>>>> <romain.perier@gmail.com>; >>>>>>>> Krzysztof Kozlowski <krzk@kernel.org>; Marc Ferland >>>>>>>> <ferlandm@amotus.ca>; Sebastian von Ohr <vonohr@smaract.com>; >>>>>>>> dmaengine@vger.kernel.org; Linux ARM <linux- >>>>>>>> arm-kernel@lists.infradead.org>; linux-kernel <linux- >>>>>>>> kernel@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. > I expect that you are talking about Xilinx releases and I hope that this > has changed over times when most of changes are upstreamed already. The > patch above you are referencing has been applied by Vinod and he is > checking patches a lot. If there is a problem and any breakage it needs > to be fixed. And bugs happen all the time and we have a way how to work > with it.
I don't know if it has gotten better. When I upgrade to a new release what takes up most of the time is figuring out why the Xilinx DMA doesn't work anymore. Its been like this for years.
> If you see there any issue please report them and let's fix them and > continue on this topic from technical point of view. > In connection to this problem what are you suggesting? Just revert this > patch or fix ordering differently? Would be good to provide your > suggestion and fix it.
Reverting would re-introduce the issue the patch was supposed to fix.
The would have been to use index 0 for the channel if there is only one channel. If there are two channels use 0 for TX and 1 for RX.
The problem is that the change has been around for a while and restoring the previous behavior will break users that are expecting the new behavior. It is a bit of a catch-22.
- Lars
|  |