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

[Bounine, Alexandre]

On Mon, Oct 17, 2011 at 3:30 PM, Jassi Brar <jaswinder.singh@xxxxxxxxxx>
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
- 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.

> >>
> >> 1) The role of the 'extra' 2bits ?
> >>
> > Just upper bits of the RIO address.
> >
> I assume you mean any transfer could be targeted at any of 2^66 bits
> on the remote device.
Yes, this is correct. 

> 
> > 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).  

> > 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". They are expecting from RapidIO fabric to pass 66-bit
address and have right to do this. Tsi721 is capable to generate
66-bit address for RIO requests.

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.

I have 64-bit only support in Tsi721 DMA driver because my priority
is defining right upper layer interface to DMA Engine.
I published tsi721 DMA driver mostly as supporting example to
demonstrate what we are doing at RapidIO layer.
After the upper layer is defined it should not be a problem to
switch to 66-bit or just keep 64.

> >> We should try our best to avoid opening the can of worms by adding
> >> (void *) hook to each transfer, because any client driver could
want
> > to
> >> pass its own private data to dmac and there would be no way for a
> dmac
> >> driver to know what to cast the void pointer to.
> >
> > Do we really expect that clients will jump to use an extra parameter
> > without a valid reason and without knowing their hardware specifics?
> >
> The long term plan for dmaengine is to be able to have client drivers
> shared across dmac drivers. And that requires clients making no
> assumptions about the underlying dmac and the dmac driver expecting
> nothing from client via outside of common APIs.
In this case adding new values to dma_transaction_type (like
DMA_RAPIDIO)
may have a sense when using unified prep_slave_sg() with extra
parameter.
This will hide RapidIO dmac drivers from DMA_SLAVE clients (and vice
versa)
while keeping common APIs from growing in their numbers.

Alex.
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/