From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752641AbdBIJyx (ORCPT ); Thu, 9 Feb 2017 04:54:53 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:37531 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752083AbdBIJys (ORCPT ); Thu, 9 Feb 2017 04:54:48 -0500 X-AuditID: cbfec7f5-f79d06d000004445-5b-589c34e0b7ad Subject: Re: [PATCH v7 2/4] dmaengine: Forward slave device pointer to of_xlate callback To: Vinod Koul Cc: Lars-Peter Clausen , linux-samsung-soc@vger.kernel.org, dmaengine@vger.kernel.org, alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , Ulf Hansson , "Rafael J. Wysocki" , Kuninori Morimoto , Mark Brown , Inki Dae From: Marek Szyprowski Message-id: Date: Thu, 09 Feb 2017 10:22:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-version: 1.0 In-reply-to: <20170209041123.GK19244@localhost> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTURzHObsPr6vVdWn90EpZRGRkSatOJj394xZB/qeYVCuvS3ImuxoW lVNw6Swzhw9USMpHaqHNYSZibYYylNZ8EWTlC5uRvUYyFS3nVfC/zzm/z/n9zvdwGELeQ/kz CUkpvDZJlaigpWRz54x997CyPHqvrT4Q9zusEvyipIHChSPjNK4vnKdwwZd8EtvtjV74W0cr wpXGRxQ2jQ1SuK+1nMau+28RLrG3S3BPdy+Fu55H4cm81+Sx9VyjK5PmnrRNSjhTXQ7NDQ22 0VxTZTpX+d1KcdM9D0nOPHCX5PLMdYhzmbZGSmOk4XF8YsJ1XrvnyEXpld9Ws1fyu6C0jAK3 RIcK/Q3ImwFWCZl9BkrkjfD+cwNtQFJGzlYhmHP+RuLChWCoU0+vnCjpnkAelrPVCPIrlqWv CCy5pkWJYTawMXBvIN7j+LLbwTyTveQQrJGEhq9thKdAs6FgmDIs+TL2CMy5Yz3b5KL/52PN kuLHxsJ9U9ESy1gfcBs/kx72ZveAcX5E4mGCDYOJhSxK5EBoejZFeGYBm8OAQ++kPP2B3QKm N4R4/wiw/8xbTrwBvnWZvUTeDDnZFonIDxBkZu0SuQTBuymZyIeho8uxPGsdFDQXE2J7GWTr 5SJyUGuNF+3jMFJZRoovNSkBhy0tHwWWrgpTuipA6aoAFYioQ758qqBR88L+EEGlEVKT1CGX r2lMaPGPdS90/W1BVZ1hVsQySLFWlnaoLFpOqa4LNzRWBAyh8JUVhZZHy2Vxqhs3ee21C9rU RF6wogCGVGyStVX0R8lZtSqFv8rzybx2pSphvP11aN8JfCBXdW4uYzrFHVRDWS6HbT59MmG9 aVvvuFl36dX3wNDZsafJOS+tPvtrd7Q4W0ednJLa+av/1uyaO1uPNs9MRIT/Gz0a8DiiPXih 2Ka23G45r2v/1Fh6agT7KW27IdsQRZ5wDA8L7g9n9ecz9/4YN0ZO9AanO2+fyT0IKdUKUrii Cg0mtILqP5NPxxJfAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrHIsWRmVeSWpSXmKPExsVy+t/xq7rapnMiDB7f47S4cvEQk8XGGetZ LaY+fMJmsXrqX1aLSfcnsFicP7+B3eLV4V2MFksmz2e12PT4GqvF5V1z2Cw+9x5htJhxfh+T xZnTl1gtjq8Nt3jZt5/Fgd9jw+cmNo/Fe14yeWxa1cnmcefaHjaPzUvqPZa8OcTq8e3MRBaP LVfbWTz6tqxi9Pi8SS6AK8rNJiM1MSW1SCE1Lzk/JTMv3VYpNMRN10JJIS8xN9VWKULXNyRI SaEsMacUyDMyQAMOzgHuwUr6dgluGR8PbWEvOKdQ0TjpB1MD41SpLkZODgkBE4kZp58xQthi EhfurWfrYuTiEBJYwihxet9NFpCEkMBzRomfc6y6GDk4hAWiJF7NjQAJiwioSmz52cEIUfKS SWLBy1CQXmaBySwS+1oPMoMk2AQMJbredrGB9PIK2En8/hEDEmYB6v10ezlYiahAjMTe/vtM IDavgKDEj8n3wNZyCuhLTP77ECzOLGAm8eXlYVYIW15i85q3zBMYBWYhaZmFpGwWkrIFjMyr GEVSS4tz03OLDfWKE3OLS/PS9ZLzczcxAuN627Gfm3cwXtoYfIhRgINRiYe3wnJ2hBBrYllx Ze4hRgkOZiUR3mmGcyKEeFMSK6tSi/Lji0pzUosPMZoCPTGRWUo0OR+YcvJK4g1NDM0tDY2M LSzMjYyUxHlLPlwJFxJITyxJzU5NLUgtgulj4uCUamBs8n8vOdXFosA142O8Ym8X/+f0P5P2 zV+zk5dlbqH+BIXWj57575syt3sULZJ9cKTz4jXZ96c5u169+pU/9ycXT0+jSunec7eLVr9a 5XfZtC1Qc29w/gNPXSmW2QJ1Szas/vrt2YKbQd4/nguv+r1oxt3udasuXrnmPHPCd8E5gls/ XzC5mR/3UImlOCPRUIu5qDgRAFDcGoEBAwAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170209092240eucas1p2689a441195bb1291ab0751ca892889e9 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRs=?= =?UTF-8?B?7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRtT?= =?UTF-8?B?YW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170125102824eucas1p1cf027184286131ff628eb7113c3f9e60 X-RootMTR: 20170125102824eucas1p1cf027184286131ff628eb7113c3f9e60 References: <1485340088-25481-1-git-send-email-m.szyprowski@samsung.com> <1485340088-25481-3-git-send-email-m.szyprowski@samsung.com> <26397455-1237-2a66-acf8-215aacc7c9ce@metafoo.de> <49c9c23d-ff0a-268a-5edd-931e04eb98b5@samsung.com> <20170209041123.GK19244@localhost> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Vinod, On 2017-02-09 05:11, Vinod Koul wrote: > On Thu, Jan 26, 2017 at 03:43:05PM +0100, Marek Szyprowski wrote: >> On 2017-01-25 14:12, Lars-Peter Clausen wrote: >>> On 01/25/2017 11:28 AM, Marek Szyprowski wrote: >>>> Add pointer to slave device to of_dma_xlate to let DMA engine driver >>>> to know which slave device is using given DMA channel. This will be >>>> later used to implement non-irq-safe runtime PM for DMA engine driver. >>> of_dma_xlate() is used to translate from a OF phandle and a specifier to a >>> DMA channel. On one hand this does not necessarily mean that the channel is >>> actually going to be used by the slave that called the xlate function. >>> Modifying the driver state when a lookup of the channel is done is a >>> layering violation. And this approach is also missing a way to disassociate >>> a slave from a DMA channel, e.g. when it is no longer used. >>> >>> On the other hand there are other mechanisms to translate between some kind >>> of firmware handle to a DMA channel which are completely ignored here. >>> >>> So this approach does not work. This is something that needs to be done at >>> the dmaengine level, not a the firmware resource translation level. And it >>> needs a matching method that is called when the channel is disassociated >> >from a device, when the device no longer uses the DMA channel. >> >> Frankly I agree that of_dma_xlate() should only return the requested channel >> to the dmaengine core and do not do any modification in the the >> driver state. > True.. > >> However the current dma engine design and implementation breaks this rule. >> Please check the drivers - how do they implement of_xlate callback. They >> usually call dma_get_any_slave_channel, dma_get_slave_channel or >> __dma_request_channel there, which in turn calls dma_chan_get, which then >> calls back to device_alloc_chan_resources callback. Some of the drivers also >> do a hardware configuration or other resource allocation in of_xlate. >> This is a bit messy design and leave no place in the core to set >> slave device >> before device_alloc_chan_resources callback, where one would expect to have >> it already set. > We shouldn't be doing much at this stage. We operate on a channel, so the > channel is returned to the client. We need to do these HW configurations > when the channel has to be prepred for a txn. IMHO, any HW configurations should be done in alloc_chan_resources callback and it would be best if a pointer of slave device will come as a parameter to it. >> The best place to add new calls to the dmaengine drivers to set slave device >> would be just before device_alloc_chan_resources(), what in turn means that >> the current dmaengine core should do in dma_chan_get(). This would >> require to >> forward the slave device pointer via even more layers including the of_xlate >> callback too. IMHO this is not worth the effort. >> >> DMA engine core and API definitely needs some cleanup. During such cleanup >> the slave device pointer might be moved out of xlate into separate callback >> when the core gets ready for such operation. > Yes agreed on that, plus the runtime handling needs to be built in, right > now the APIs dont work well with it, we disucssed these during the KS and > this goes without saying, patches are welcome :) Okay, so what is the conclusion? Do you want me to do the whole rework of dma engine core to get this runtime pm patchset for pl330 merged??? Is there any roadmap for this rework prepared, so I can at least take a look at the amount of work needed to be done? I'm rather new to dma engine framework and I only wanted to fix pl330 driver not to block turning off the power domain on Exynos5422/5433 SoCs. I can also check again if there is any other way to find the slave device in alloc_chan_resources, like for example scanning the device tree for phandles, to avoid changing dmaengine core as this turned out to be too problematic before one will do the proper dma engine core rework. > ... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland