Re: [PATCHv4] DMAEngine: Define interleaved transfer request api

[Jassi Brar <jaswinder.singh@linaro.org>]

On 18 October 2011 02:37, Bounine, Alexandre <Alexandre.Bounine@idt.com> wrote:
> On Mon, Oct 17, 2011 at 3:30 PM, Jassi Brar <jaswinder.singh@linaro.org>
> wrote:
> ... skip ...
>> > This is a source of the problem for RIO - DMA controller driver
>> creates
>> > virtual channels statically. RapidIO may use 8- or 16-bit destID.
>> > In this case we need to create 256 or 64K virtual channels if we
>> > want to cover all possible targets on single RIO port. Adding an
>> extra
>> > controller/net multiplies that number. Considering that not every
>> > device will need a data transfer from a given node static allocation
>> > will
>> > create even more wasted resources.
>> >
>> Please excuse my rudimentary knowledge of RapidIO but I am tempted
>> to ask why not register channels only for those targets that are
>> actually
>> detected and enumerated?
>>
> Two reasons:
> - possibility of hot device insertion/removal
... but the linux RIO stack doesn't seem to support hotplug.
Enumeration/discovery is done only once during boot.
Am I overlooking something ?

> - there is no advance knowledge of which target device may require DMA
>  service. A device driver for the particular target device is expected
>  to request DMA service if required.
>
IMHO 1 channel per real device is an acceptable 'overhead'.
Already many SoCs register dozens of channels but only a couple
of them are actually used.

>> > There is nothing that absence of full 66-bit addressing blocks now.
>> > So far we are not aware about implementations that use 66-bit
>> address.
>> >
>> Thanks for the important info.
>> If I were you, I would postpone enabling support for 66-bit addressing
>> esp when it soo affects the dmaengine API.
>> Otherwise, you don't just code unused feature, but also put
> constraints
>> on development/up-gradation of the API in future, possibly, nearer
> than
>> real need of the feature.
>>
>> If we postpone 66-bit addressing to when it arrives, we can
>> 1) Attach destID to the virtual channel's identity
>> 2) Use device_prep_dma_memcpy so as to be able to change
>>     target address for every transfer. Or use prep_slave, depending
>>     upon nature of address at target endpoint.
>> 3) Use slave_config to set wr_type if it remains same for enough
>>     consecutive transfers to the same target (only you could strike
>>     the balance between idealism and pragmatism).
>>
> With item #1 above being a separate topic, I may have a problem with #2
> as well: dma_addr_t is sized for the local platform and not guaranteed
> to be a 64-bit value (which may be required by a target).
> Agree with #3 (if #1 and #2 work).
>
Perhaps simply change dma_addr_t to u64 in dmaengine.h alone ?

>> > This does not prevent someone from designing RIO compliant endpoint
>> > device which gives interpretation to these two bits in addition to
>> full 64-bit
>> > addressing of their platform.
>> >
>> It sounds as if the 2bits are 'Vendor-Specific' ?
>>
> They are not 'Vendor-Specific' in the RIO spec (just address),
> but HW vendors may give them such meaning. I just wanted to say
> that HW designers may follow the same logic as we do here and
> decide to give those bits a special meaning because "no one uses
> them".
>
We should discount that. Irrespective of linux RIO stack, a vendor
would be at fault if it assigns different meaning to the upper 2bits
while the specs deem them just MSB of 66-bit addresses.

> So far this is a theoretical possibility and we are not
> aware about any designs of this type. We may put a big warning
> note about 64-bit limitation in RapidIO documentation section.
>
Yes, please.