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

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

On Tue, Oct 18, 2011 at 05:20:30PM +0530, Jassi Brar wrote:
> On 18 October 2011 15:19, Russell King <rmk@arm.linux.org.uk> 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.)
> >
> Thanks for the detailed explanation.
> 
> RapidIO is a packet switched interconnect with parallel or serial interface.
> Among other things, a packet contains 32, 48 or 64 bit offset into the
> remote-endpoint's address space. So I don't get how any of the above
> 6 points apply here.

So you can't see that redefining dma_addr_t to be 64-bit for the entire
DMA engine subsystem would be a bad idea...

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