From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752906AbdAZOnP (ORCPT ); Thu, 26 Jan 2017 09:43:15 -0500 Received: from mailout3.w1.samsung.com ([210.118.77.13]:42835 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752788AbdAZOnM (ORCPT ); Thu, 26 Jan 2017 09:43:12 -0500 X-AuditID: cbfec7f2-f790f6d000002555-98-588a0afb5147 Subject: Re: [PATCH v7 2/4] dmaengine: Forward slave device pointer to of_xlate callback To: 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 Cc: Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , Vinod Koul , Ulf Hansson , "Rafael J. Wysocki" , Kuninori Morimoto , Mark Brown , Inki Dae From: Marek Szyprowski Message-id: <49c9c23d-ff0a-268a-5edd-931e04eb98b5@samsung.com> Date: Thu, 26 Jan 2017 15:43:05 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-version: 1.0 In-reply-to: <26397455-1237-2a66-acf8-215aacc7c9ce@metafoo.de> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02SfSzUcRzH9/09+bl1+nWUb6S2s2qySGx9J7Ostn5/tNXWGqNWh5+H5Wl3 GNV0NMVVHlNCpe6UlOg8JAmd1rU7uTwWMzGqsQgXl4rl7sfmv9fn83l/Ht7ffWlcZCAd6MiY eE4aI4kSUwKi/t1Cx+5/AkXAnv4g1NOpwdDzwioSFYyMUehJwSKJ8r7kEMhgqLZCE22NAKny 75FIPdpHou7GEgoZr78FqNDQjKF2fReJtJX+aDyrhTiwnq02plGssmkcY9UVmRQ72NdEsTWq i6zqh4Zk59tzCba29wrBZtVWANao3npMECjwCeWiIhM5qbvvGUHEzOglLE4Pk2o6TJgc/LVV AGsaMl4wtawF43kT/DhURSmAgBYxZQAO6B5gfGAEUDv4CV/teFOcuqJ6CGBa3S+SD74DaPrw bLlC07ZMILzWG2bO25nz+nm9ZRTOdGLwfq7JspBiPKBiUkGZWcj4wqmSRcLMBLMdVpQPW5l5 I3MSynULVrxmA/ydP2TRWC/rC2ozLb044w2/LaWTPG+DNU8ncfMyyKTSUFfSZbkIMk5Q3bpi 4RAsmipZMW0LJ7S1Vjxvgd35Vwmes5etpbvyXAhgx6SQ5/2wTdu5sssG5tXfwvnxQphxWcQj Cx9rwni1HxxRFRP8+2RjsL/+BcgB24rWuCla46BojYNSgFcAOy5BFh3OyTzdZJJoWUJMuFtI bLQaLH8z/ZJ2tgHMvffWAIYG4nXC1oaMABEpSZQlR2sApHGxnfCPtSJAJAyVJJ/jpLGnpQlR nEwDHGlCbC9sKu3xFzHhknjuLMfFcdLVKkZbO8hBmatIr3Do8HJ28Uyp7IvdGzwvD3BXpnxu LncUD/oeTn+EKX+O7UjZmRTX03mnOETXfmrW3WZgs8qvvfuIrc/wjL7OpDtYNyAavumEiIEG +7m7LlHo/GSw+zCnlO8zThtUzq+CupNen5h+mXS0+vZXKvC4rzLTo/CGSUI7XuiZFhOyCInH Llwqk/wHhyGibGIDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrEIsWRmVeSWpSXmKPExsVy+t/xy7o/uLoiDFpu2lhcuXiIyWLjjPWs FlMfPmGzWD31L6vFpPsTWCzOn9/AbvHq8C5GiyWT57NabHp8jdXi8q45bBafe48wWsw4v4/J 4szpS6wWx9eGW7zs28/iwO+x4XMTm8fiPS+ZPDat6mTzuHNtD5vH5iX1HkveHGL1+HZmIovH lqvtLB59W1YxenzeJBfAFeVmk5GamJJapJCal5yfkpmXbqsUGuKma6GkkJeYm2qrFKHrGxKk pFCWmFMK5BkZoAEH5wD3YCV9uwS3jI+Pm5kKTktUbD73namB8bdwFyMnh4SAicTB2Y1sELaY xIV764FsLg4hgSWMElcvXWeGcJ4zSlzedo+li5GDQ1ggSuLV3AiQuAhIfMb/7YwQRf1MEhuP fmcFcZgFLjNJXJp7nBlkLpuAoUTX2y6wHbwCdhLv5vxlAbFZBFQlVq14wA5iiwrESLxdv5wd okZQ4sfke2A1nED1U7d0gvUyC5hJfHl5mBXClpfYvOYt8wRGgVlIWmYhKZuFpGwBI/MqRpHU 0uLc9NxiI73ixNzi0rx0veT83E2MwPjeduznlh2MXe+CDzEKcDAq8fBmbOuIEGJNLCuuzD3E KMHBrCTC68zaFSHEm5JYWZValB9fVJqTWnyI0RToiYnMUqLJ+cDUk1cSb2hiaG5paGRsYWFu ZKQkzjv1w5VwIYH0xJLU7NTUgtQimD4mDk6pBkYmHqXkN5+zuuWu6KiXfZG6fTI4WCZTy+Jq sdW0Hp530jNTonV/9s54oDnL89bvtlzRk2u6VroG21zVjbgi/YRRWjo0wPDFXl9rhWlRm9gC ZVx+qKS0z/p0w0slK19h7l7u/mydjel7EnoyGfYVb/Kc9f1Hq+GWUIFXRdVbfWLCKtmU1AUL lViKMxINtZiLihMBOOpCqAUDAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170126144307eucas1p278d5c5704e3e291ba49e96bf894b0dfa X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 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> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Lars, 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. 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. 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. I ignored other paths for other firmware handle to a DMA channel translation mechanism because for the current pl330 driver they are simply not used. I assume that if one needs to implement similar things for drivers relying on them, he will update the respective DMA engine core parts. Slave device assignments can be cleared in device_chan_release_resources if this is needed and that what existing DMA engine drivers do with the resources allocated in the of_xlate callback... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland