Messages in this thread | | | Subject | Re: [PATCH v7 2/4] dmaengine: Forward slave device pointer to of_xlate callback | From | Marek Szyprowski <> | Date | Fri, 03 Feb 2017 14:01:27 +0100 |
| |
Hi All,
On 2017-01-26 15:43, 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. > > 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...
Vinod: could you comment this patchset? Is there a chance to get it merged or at least give it a try in -next? If not, could you provide some hints what should I do?
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
| |