[PATCH RFC v6 4/5] dma: mpc512x: register for device tree channel lookup

[Alexander Popov]

Thanks for your replies, Gerhard and Vinod.

2014/1/9 Vinod Koul <vinod.koul at intel.com>:
> On Wed, Jan 08, 2014 at 05:47:19PM +0100, Gerhard Sittig wrote:
>> [ what is the semantics of DMA_PRIVATE capability flag?
>>   is documentation available beyond the initial commit message?
>>   need individual channels be handled instead of controllers? ]
>
> The DMA_PRIVATE means that your channels are not to be used for global memcpy,
> as one can do in async cases (this is hwere DMAengine came into existence)
>
> If the device has the capablity of doing genric memcpy then it should not set
> this. For slave dma usage the dam channel can transfer data to a specfic
> slave device(s), hence we should use this is geric fashion so setting
> DMA_PRIVATE makes sense in those cases.

Each DMA channel of MPC512x DMA controller can do _both_
mem-to-mem transfers and transfers between mem and some slave peripheral
(only one DMA channel is fully dedicated to DDR).
All DMA channels of MPC512x DMA controller belong to one dma_device.
So we _don't_ need setting DMA_PRIVATE flag for this dma_device at all, do we?

>> On Sat, Jan 04, 2014 at 00:54 +0400, Alexander Popov wrote:
>> > I've involved DMA_PRIVATE flag because new of_dma_xlate_by_chan_id()
>> > uses dma_get_slave_channel() instead of dma_request_channel()
>> > (PATCH RFC v6 3/5). This flag is implicitly set in dma_request_channel(),
>> > but is not set in dma_get_slave_channel().
> Which makes me thing you are targetting slave usages. Do you intend to use for
> mempcy too on all controllers you support. in that case you should set it
> selectively.

Vinod, please correct me if I'm wrong.
As I could understand from your comments and the code,
DMA_PRIVATE flag is needed for dma_devices with DMA channels
which _can_ work with slave peripheral but _can't_ do mem-to-mem transfers.
If DMA_PRIVATE flag is set for some dma_device before
dma_async_device_register()
then its DMA channels are not published in tables for kernel slab allocator
(because these channels are simply useless for memcpy).

>> Still I see a difference in the lookup approaches:  Yours applies
>> DMA_PRIVATE globally and in advance, preventing _any_ use of DMA
>> for memory transfers.  While the __dma_request_channel() routine
>> only applies it _temporarily_ around a dma_chan_get() operation.
>> Allowing for use of DMA channels by both individual peripherals
>> as well as memory transfers.
>>
> No it doesnt prevent. You can still use it for memcpy once you have the channel.

Excuse me, I don't completely understand why dma_request_channel()
needs to set DMA_PRIVATE flag.
If dma_request_channel() for some dma_device without DMA_PRIVATE
is called before the first dmaengine_get()
then no DMA channels of this dma_device will become available for memcpy
by slab allocator.
Could you give me a clue?

>> > > Consider the fact that this driver
>> > > handles both MPC5121 as well as MPC8308 hardware.
>> >
>> > Ah, yes, sorry. I should certainly fix this, if setting of DMA_PRIVATE flag
>> > is needed at all.
>>
>> What I meant here is that implications for all affected platforms
>> should be considered.  There is one driver source, but the driver
>> applies to more than one platform (another issue of the driver is
>> that this is not apparent from the doc nor the compat strings).

I'll add a comment with information about the supported platforms to
mpc512x_dma.c
in RFC PATCH 1/5. Ok?

>> So blocking memory transfers in mpc512x_dma.c is a total breakage
>> for MPC8308 (removes the only previous feature and adds nothing),
>> and is a regression for MPC512x (removes the previously supported
>> memory transfers, while it may add peripheral supports with very
>> few users).

Yes, I see. MPC512x and MPC8308 should be treated differently.

Thanks!
Alexander