Re: [PATCH v5 3/9] dma-mapping: add dma_{map,unmap}_resource

From: Robin Murphy
Date: Fri Mar 11 2016 - 08:47:06 EST

Hi Dan,

On 11/03/16 06:47, Dan Williams wrote:
On Thu, Mar 10, 2016 at 8:05 AM, Niklas S??derlund
<niklas.soderlund@xxxxxxxxxxxx> wrote:
Hi Christoph,

On 2016-03-07 23:38:47 -0800, Christoph Hellwig wrote:
Please add some documentation on where/how this should be used. It's
not a very obvious interface.

Good idea, I have added the following to Documentation/DMA-API.txt and
folded it in to this patch. Do you feel it's adequate and do you know
anywhere else I should add documentation?

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 45ef3f2..248556a 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -277,14 +277,29 @@ and <size> parameters are provided to do partial page mapping, it is
recommended that you never use these unless you really know what the
cache width is.

+dma_map_resource(struct device *dev, phys_addr_t phys_addr, size_t size,
+ enum dma_data_direction dir, struct dma_attrs *attrs)
+Maps a MMIO region so it can be accessed by the device and returns the
+DMA address of the memory. API should only be used to map device MMIO,
+mapping of RAM is not permitted.

I think it is confusing to use the dma_ prefix for this peer-to-peer
mmio functionality. dma_addr_t is a device's view of host memory.
Something like bus_addr_t bus_map_resource(). Doesn't this routine
also need the source device in addition to the target device? The
resource address is from the perspective of the host cpu, it may be a
different address space in the view of two devices relative to each

Hmm, the trouble with that is that when the DMA master is behind an IOMMU, the address space as seen by the device is dynamic and whatever we decide it to be, so there is no distinction between a "DMA" address and a "bus" address.

In practice the dmaengine API has clearly worked for however long with slave MMIO addresses being a dma_addr_t, and it doesn't look like anyone objected to the change to phys_addr_t in -next either. If nothing is using bus_addr_t anyway, what's the right thing to do? Looking up through higher abstraction layers, we have the likes of struct snd_dmaengine_dai_dma_data also expecting the slave address to be a dma_addr_t, leading to things like the direct casting in bcm2835_i2s_probe() for the non-IOMMU dma != phys != bus case that could also be cleaned up with this proposed interface.