Problems with dma_alloc_coherent()

[Adrian Cox]

On Fri, 2004-04-02 at 22:01, John Whitney wrote:

> I've been thinking about changes to the API that would help this, and
> have come up with the following (for the lists' consideration, with
> possible submission to the kernel mailing list):
>
> 1. dma_addr_t should be changed from a flat physical address variable
> to a structure, containing both the physical address and the device it
> corresponds to.

Why not leave dma_addr_t alone, and create a new type for this class of
DMA engine. The new type would be a structure. The API then would be:

dma_move(struct gen_dma *src, struct gen_dma *dest, unsigned len);

Then struct gen_dma contains a union, to allow it to refer to memory
(using the current DMA api), PCI devices, or on-chip peripherals:
struct gen_dma {
	int bus_type;
	union {
		dma_addr_t memory_address;
		unsigned long bus_address;
        }
}
#define DMA_BUS_TYPE_MEM    0
#define DMA_BUS_TYPE_PCI    1

And each platform can define extra bus types
#define DMA_BUS_TYPE_ODDBUS 2

> 2. A macro should exist to convert this to a flat physical address:
> unsigned long dma_addr_to_phys (dma_addr_t *address);

The macro to convert a struct gen_dma to a flat address would be
specific to the platform. It wouldn't be in any global header. The
conversion would only happen one line of code before writing the flat
address into the DMA engine's register.

> 3. A macro should exist to convert it from one device's address space
> to another's: dma_convert_addr (device *to_dev, dma_addr_t *to_address,
> dma_addr_t *from_address);  If, due to dma_mask reasons the address is
> not valid for the requested device, this could return an error
> indicator, indicating the caller would have to allocate new memory and
> copy the contents over.

Again, easiest to implement inside the platform specific DMA controller
code.

> 4. The processor should have a "device" associated with it, and this
> should be assumed to be the device when it is specified as NULL to the
> DMA API routines: dma_alloc_coherent (NULL, ...) would mean "allocate
> contiguous/coherent memory for me, with the processor's address space
> used for the physical address".

Actually, what might be useful would be a NULL device to indicate the
platform DMA engine.

> 5. There should be a way of taking a local bus physical address, and
> converting it to the device's address space (dma_addr_t):
> dma_convert_phys (device *to_dev, dma_addr_t *address, unsigned long
> phys_addr).  Again, this could return an error indicator if the
> dma_mask shows the address won't work.

I don't really see why we need this.

> The problem I see is that the current API assumes sysmem<->dev, with
> the device itself doing the transfer.  I'm wanting to add a generic
> address-space-translation API to it that will allow other kinds of DMA
> transfers, and handle busses other than the PCI bus.

I think that this generic API can live inside the platform specific
implementation of the DMA controller.

There are three important sorts of address:

local memory - the DMA api gives us a handle to this
PCI devices - addresses found through struct pci_dev
on-chip peripherals, platform buses - addresses defined in platform
header files

> The main impact I see this having on drivers written with the current
> API is that they need to call dma_addr_to_phys() instead of just using
> the dma_addr_t as a physical address.
>
> Comments?

Leave the current API alone. It's fine for the common class of
peripherals. This is a completely new API for embedded devices.

I've not encountered a chip with more than one platform DMA controller.
Rather than having a top level API with low-level providers, consider
picking the implementation at Kconfig time.

- Adrian Cox


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/