diff mbox series

[12/12] dma-mapping: move the dma_declare_coherent_memory documentation

Message ID 20200908164758.3177341-13-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series [01/12] MIPS: make dma_sync_*_for_cpu a little less overzealous | expand

Commit Message

Christoph Hellwig Sept. 8, 2020, 4:47 p.m. UTC
dma_declare_coherent_memory should not be in a DMA API guide aimed
at driver writers (that is consumers of the API).  Move it to a comment
near the function instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/core-api/dma-api.rst | 24 ------------------------
 kernel/dma/coherent.c              | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 24 deletions(-)

Comments

Robin Murphy Sept. 10, 2020, 1:51 p.m. UTC | #1
On 2020-09-08 17:47, Christoph Hellwig wrote:
> dma_declare_coherent_memory should not be in a DMA API guide aimed
> at driver writers (that is consumers of the API).  Move it to a comment
> near the function instead.

I still think there might be an occasional valid use for device-local 
memory outside the scope of platform code without the driver having to 
go full ZONE_DEVICE/HMM/TTM, e.g. with stuff like PCIe-based FPGA 
prototyping cards, but the kind of driver I'm imagining for that case 
would never be upstream anyway (if it were even written, rather than 
just using hard-coded hacks), so meh.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   Documentation/core-api/dma-api.rst | 24 ------------------------
>   kernel/dma/coherent.c              | 17 +++++++++++++++++
>   2 files changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst
> index 3b3abbbb4b9a6f..90239348b30f6f 100644
> --- a/Documentation/core-api/dma-api.rst
> +++ b/Documentation/core-api/dma-api.rst
> @@ -586,30 +586,6 @@ the DMA_ATTR_NON_CONSISTENT flag starting at virtual address vaddr and
>   continuing on for size.  Again, you *must* observe the cache line
>   boundaries when doing this.
>   
> -::
> -
> -	int
> -	dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
> -				    dma_addr_t device_addr, size_t size);
> -
> -Declare region of memory to be handed out by dma_alloc_coherent() when
> -it's asked for coherent memory for this device.
> -
> -phys_addr is the CPU physical address to which the memory is currently
> -assigned (this will be ioremapped so the CPU can access the region).
> -
> -device_addr is the DMA address the device needs to be programmed
> -with to actually address this memory (this will be handed out as the
> -dma_addr_t in dma_alloc_coherent()).
> -
> -size is the size of the area (must be multiples of PAGE_SIZE).
> -
> -As a simplification for the platforms, only *one* such region of
> -memory may be declared per device.
> -
> -For reasons of efficiency, most platforms choose to track the declared
> -region only at the granularity of a page.  For smaller allocations,
> -you should use the dma_pool() API.
>   
>   Part III - Debug drivers use of the DMA-API
>   -------------------------------------------
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index 2a0c4985f38e41..f85d14bbfcbe03 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -107,6 +107,23 @@ static int dma_assign_coherent_memory(struct device *dev,
>   	return 0;
>   }
>   
> +/*
> + * Declare a region of memory to be handed out by dma_alloc_coherent() when it
> + * is asked for coherent memory for this device.  This shall only be used
> + * from platform code, usually based on the device tree description.
> + *
> + * phys_addr is the CPU physical address to which the memory is currently
> + * assigned (this will be ioremapped so the CPU can access the region).
> + *
> + * device_addr is the DMA address the device needs to be programmed with to
> + * actually address this memory (this will be handed out as the dma_addr_t in
> + * dma_alloc_coherent()).
> + *
> + * size is the size of the area (must be a multiple of PAGE_SIZE).
> + *
> + * As a simplification for the platforms, only *one* such region of memory may
> + * be declared per device.
> + */
>   int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>   				dma_addr_t device_addr, size_t size)
>   {
>
Christoph Hellwig Sept. 11, 2020, 7:17 a.m. UTC | #2
On Thu, Sep 10, 2020 at 02:51:47PM +0100, Robin Murphy wrote:
> On 2020-09-08 17:47, Christoph Hellwig wrote:
>> dma_declare_coherent_memory should not be in a DMA API guide aimed
>> at driver writers (that is consumers of the API).  Move it to a comment
>> near the function instead.
>
> I still think there might be an occasional valid use for device-local 
> memory outside the scope of platform code without the driver having to go 
> full ZONE_DEVICE/HMM/TTM, e.g. with stuff like PCIe-based FPGA prototyping 
> cards, but the kind of driver I'm imagining for that case would never be 
> upstream anyway (if it were even written, rather than just using hard-coded 
> hacks), so meh.

And I'm not sure this would be the right interface for it.  E.g. NVMe
has the concept of a Controller Memory buffer (and a similar persistent
variant not supported by Linux), where the device can do this local DMA
(in a completely broken way that relies on correlating addresses seen
by the device and those by the host, but that's another disgression).
But that memory obviously can also be addresses by other devices using
PCIe P2P transactions which would also be useful for any HMM-ish devices,
so we'd need to expose it as P2P memory anyay..
diff mbox series

Patch

diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst
index 3b3abbbb4b9a6f..90239348b30f6f 100644
--- a/Documentation/core-api/dma-api.rst
+++ b/Documentation/core-api/dma-api.rst
@@ -586,30 +586,6 @@  the DMA_ATTR_NON_CONSISTENT flag starting at virtual address vaddr and
 continuing on for size.  Again, you *must* observe the cache line
 boundaries when doing this.
 
-::
-
-	int
-	dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
-				    dma_addr_t device_addr, size_t size);
-
-Declare region of memory to be handed out by dma_alloc_coherent() when
-it's asked for coherent memory for this device.
-
-phys_addr is the CPU physical address to which the memory is currently
-assigned (this will be ioremapped so the CPU can access the region).
-
-device_addr is the DMA address the device needs to be programmed
-with to actually address this memory (this will be handed out as the
-dma_addr_t in dma_alloc_coherent()).
-
-size is the size of the area (must be multiples of PAGE_SIZE).
-
-As a simplification for the platforms, only *one* such region of
-memory may be declared per device.
-
-For reasons of efficiency, most platforms choose to track the declared
-region only at the granularity of a page.  For smaller allocations,
-you should use the dma_pool() API.
 
 Part III - Debug drivers use of the DMA-API
 -------------------------------------------
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 2a0c4985f38e41..f85d14bbfcbe03 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -107,6 +107,23 @@  static int dma_assign_coherent_memory(struct device *dev,
 	return 0;
 }
 
+/*
+ * Declare a region of memory to be handed out by dma_alloc_coherent() when it
+ * is asked for coherent memory for this device.  This shall only be used
+ * from platform code, usually based on the device tree description.
+ * 
+ * phys_addr is the CPU physical address to which the memory is currently
+ * assigned (this will be ioremapped so the CPU can access the region).
+ *
+ * device_addr is the DMA address the device needs to be programmed with to
+ * actually address this memory (this will be handed out as the dma_addr_t in
+ * dma_alloc_coherent()).
+ *
+ * size is the size of the area (must be a multiple of PAGE_SIZE).
+ *
+ * As a simplification for the platforms, only *one* such region of memory may
+ * be declared per device.
+ */
 int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
 				dma_addr_t device_addr, size_t size)
 {