Re: [PATCH v7 05/11] dmaengine: Introduce DMA-device device_caps callback

From: Dave Jiang
Date: Tue Jul 14 2020 - 12:49:16 EST




On 7/14/2020 9:29 AM, Serge Semin wrote:
On Tue, Jul 14, 2020 at 09:18:16AM -0700, Dave Jiang wrote:


On 7/14/2020 9:08 AM, Vinod Koul wrote:
On 13-07-20, 13:55, Dave Jiang wrote:


On 7/10/2020 2:38 AM, Serge Semin wrote:
On Fri, Jul 10, 2020 at 11:45:03AM +0300, Andy Shevchenko wrote:
On Fri, Jul 10, 2020 at 01:45:44AM +0300, Serge Semin wrote:
There are DMA devices (like ours version of Synopsys DW DMAC) which have
DMA capabilities non-uniformly redistributed between the device channels.
In order to provide a way of exposing the channel-specific parameters to
the DMA engine consumers, we introduce a new DMA-device callback. In case
if provided it gets called from the dma_get_slave_caps() method and is
able to override the generic DMA-device capabilities.


In light of recent developments consider not to add 'slave' and a such words to the kernel.

As long as the 'slave' word is used in the name of the dma_slave_caps
structure and in the rest of the DMA-engine subsystem, it will be ambiguous
to use some else terminology. If renaming needs to be done, then it should be
done synchronously for the whole subsystem.

What about just calling it dma_device_caps? Consider this is a useful
function not only slave DMA will utilize this. I can see this being useful
for some of my future code with idxd driver.

Some of the caps may make sense to generic dmaengine but few of them do
not :) While at it, am planning to make it dmaengine_periph_caps to
denote that these are dmaengine peripheral capabilities.



If the function only passes in periph_caps, how do we allow the non periph
DMA utilize this function?

Hello Dave. That seems reasonable. "dma_device_caps" or even "dma_chan_caps"
might be more suitable seeing after this patchset merged in the "dma_slave_caps"
may really provide the DMA channel-specific configs. Moreover that structure is
accessible only by means of the dma_chan descriptor:

int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);

which makes those caps being the channel-specific even without this patchset.

So as I see it "dma_chan_caps" might be the better choice.

Hi Sergey. Yes I think that sounds pretty good. Especially seeing there are DMA engines that have channels with different/asymmetric capabilities now.


-Sergey