Re: [PATCH v2] dmaengine: Create symlinks between DMA channels and slaves

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	Vinod Koul <vkoul@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	dmaengine@vger.kernel.org,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] dmaengine: Create symlinks between DMA channels and slaves
Date: Thu, 30 Jan 2020 11:33:41 +0100	[thread overview]
Message-ID: <ab83c4e0-87d2-c60e-afa7-4549ffb15397@samsung.com> (raw)
In-Reply-To: <CAMuHMdXds2HuBAnLXmLVaCWKX77iZGvNSnD35-ysY9AnG9TKMw@mail.gmail.com>

Hi Geert,

On 30.01.2020 09:30, Geert Uytterhoeven wrote:
> On Wed, Jan 29, 2020 at 6:47 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> On 17.01.2020 16:30, Geert Uytterhoeven wrote:
>>> Currently it is not easy to find out which DMA channels are in use, and
>>> which slave devices are using which channels.
>>>
>>> Fix this by creating two symlinks between the DMA channel and the actual
>>> slave device when a channel is requested:
>>>     1. A "slave" symlink from DMA channel to slave device,
>>>     2. A "dma:<name>" symlink slave device to DMA channel.
>>> When the channel is released, the symlinks are removed again.
>>> The latter requires keeping track of the slave device and the channel
>>> name in the dma_chan structure.
>>>
>>> Note that this is limited to channel request functions for requesting an
>>> exclusive slave channel that take a device pointer (dma_request_chan()
>>> and dma_request_slave_channel*()).
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Tested-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>> This patch breaks booting on almost all Exynos based boards:
>>
>> https://lore.kernel.org/linux-samsung-soc/20200129161113.GE3928@sirena.org.uk/T/#u
> Sorry for the breakage.

No problem, that's why we have linux-next.

>> I've already sent a fix:
>>
>> https://protect2.fireeye.com/url?k=797fc496-24ab78fe-797e4fd9-0cc47a3356b2-edd084a6ee90e98a&u=https://lkml.org/lkml/2020/1/29/498
> Thanks a lot!
>
>> BTW, this patch reminds me some of my earlier work:
>>
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1329778.html
>>
>> I had similar need to keep a client's struct device pointer for every
>> requested channel, but it turned out to be much more complicated than
>> I've initially thought. I've abandoned that, due to lack of time, but
>> maybe some of that discussion and concerns are still valid (I hope that
>> links to earlier versions are still working)...
> Oh right, Runtime PM for DMA channels.
>
> As several DMA calls can be made from atomic context, probably the API
> should be split in a non-atomic and an atomic part, cfr. the difference
> between clk_prepare() and clk_enable().  Still, DMA slave drivers would need
> to be modified, to call the "prepare" to make use of this...

Well, I'm not a big fan for introducing this 2-levels of operation 
(prepare/enable). In most typical designs dmaengines have to operate 
from the atomic context anyway. I've made a workaround for that using 
device links. Usually dmaengine runtime PM can simply follow the runtime 
PM state of its client (master?) device. The main problem that time was 
to reliably find the device which requested the given channel. If you 
have some spare time, please read the thread and the discussions in the 
previous versions. This might be a bit related to the devices you create 
the symlinks.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland