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

[Russell King <rmk@arm.linux.org.uk>]

On Tue, Oct 18, 2011 at 02:00:45PM +0530, Jassi Brar wrote:
> On 18 October 2011 13:12, Russell King <rmk@arm.linux.org.uk> wrote:
> > On Tue, Oct 18, 2011 at 11:15:29AM +0530, Jassi Brar wrote:
> >> On 18 October 2011 02:37, Bounine, Alexandre <Alexandre.Bounine@idt.com> wrote:
> >> > 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 ?
> >
> > That's just an idiotic suggestion - there's no other way to put that.
> > Let's have some sanity here.
> >
> Yeah, I am not proud of the workaround, so I only probed the option.
> I think I need to explain myself.
> 
> The case here is that even a 32-bit RapidIO host could ask transfer against
> 64-bit address space on a remote device. And vice versa 64->32.
> 
> > dma_addr_t is the size of a DMA address for the CPU architecture being
> > built.  This has no relationship to what any particular DMA engine uses.
> >
> Yes, so far the dmaengine ever only needed to transfer within platform's
> address-space. So the assumption that src and dst addresses could
> be contained within dma_addr_t, worked.
> If the damengine is to get rid of that assumption/constraint, the memcpy,
> slave_sg etc need to accept addresses specified in bigger of the host and
> remote address space, and u64 is the safe option.
> Ultimately dma_addr_t is either u32 or u64.

Let me spell it out:

1. Data structures read by the DMA engine hardware should not be defined
   using the 'dma_addr_t' type, but one of the [bl]e{8,16,32,64} types,
   or at a push the u{8,16,32,64} types if they're always host-endian.

   This helps to ensure that the layout of the structures read by the
   hardware are less dependent of the host architecture and each element
   is appropriately sized (and, with sparse and the endian-sized types,
   can be endian-checked at compile time.)

2. dma_addr_t is the size of the DMA address for the host architecture.
   This may be 32-bit or 64-bit depending on the host architecture.

The following points are my opinion:

3. For architectures where there are only 32-bit DMA addresses, dma_addr_t
   will be a 32-bit type.  For architectures where there are 64-bit DMA
   addresses, it will be a 64-bit type.

4. If RIO can accept 64-bit DMA addresses but is only connected to 32-bit
   busses, then the top 32 address bits are not usable (it's truncated in
   hardware.)  So there's no point passing around a 64-bit DMA address.

5. In the case of a 64-bit dma_addr_t and a 32-bit DMA engine host being
   asked to transfer >= 4GB, this needs error handing in the DMA engine
   driver (I don't think its checked for - I know amba-pl08x doesn't.)

6. 32-bit dma_addr_t with 64-bit DMA address space is a problem and is
   probably a bug in itself - the platform should be using a 64-bit
   dma_addr_t in this case.  (see 3.)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of: