Skip to contentSkip to conversation

clean up dma_declare_coherent_memory calling conventions

PATCH In review Christoph Hellwig 2 participants 5 messages Jun 27, 2017 – Jun 28, 2017

Subsystems (13)
Documentation Arm (32-bit) Media (V4L/DVB) SCSI +9 more subsystems
lore
Share & tools
lore
author
#1 lore raw

PATCH 1/2 dma-coherent: remove the DMA_MEMORY_INCLUDES_CHILDREN flag

This flag was never implemented or used.

Signed-off-by Christoph Hellwig <[email protected]>
---
 Documentation/DMA-API.txt     | 4 ----
 drivers/base/dma-coherent.c   | 2 --
 drivers/char/virtio_console.c | 3 ---
 include/linux/dma-mapping.h   | 3 +--
 4 files changed, 1 insertion(+), 11 deletions(-)
Documentation/DMA-API.txt +0 -4
index 6b20128fab8a..03072bc7ff74 100644
@@ -524,10 +524,6 @@ dma_alloc_coherent() be addressable using read()/write()/memcpy_toio() etc.
 
 One or both of these flags must be present.
 
-DMA_MEMORY_INCLUDES_CHILDREN - make the declared memory be allocated by
-dma_alloc_coherent of any child devices of this one (for memory residing
-on a bridge).
-
 DMA_MEMORY_EXCLUSIVE - only allocate memory from the declared regions. 
 Do not allow dma_alloc_coherent() to fall back to system memory when
 it's out of memory in the declared region.
drivers/base/dma-coherent.c +0 -2
index 2ae24c28e70c..e70e33d5c902 100644
@@ -109,8 +109,6 @@ static int dma_assign_coherent_memory(struct device *dev,
 		return -EBUSY;
 
 	dev->dma_mem = mem;
-	/* FIXME: this routine just ignores DMA_MEMORY_INCLUDES_CHILDREN */
-
 	return 0;
 }
drivers/char/virtio_console.c +0 -3
index ad843eb02ae7..48e86ce9fac8 100644
@@ -451,9 +451,6 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
 		 * device is created by remoteproc, the DMA memory is
 		 * associated with the grandparent device:
 		 * vdev => rproc => platform-dev.
-		 * The code here would have been less quirky if
-		 * DMA_MEMORY_INCLUDES_CHILDREN had been supported
-		 * in dma-coherent.c
 		 */
 		if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
 			goto free_buf;
include/linux/dma-mapping.h +1 -2
index 843ab866e0f4..fb66fe98e61d 100644
@@ -685,8 +685,7 @@ static inline int dma_get_cache_alignment(void)
 /* flags for the coherent memory api */
 #define	DMA_MEMORY_MAP			0x01
 #define DMA_MEMORY_IO			0x02
-#define DMA_MEMORY_INCLUDES_CHILDREN	0x04
-#define DMA_MEMORY_EXCLUSIVE		0x08
+#define DMA_MEMORY_EXCLUSIVE		0x04
 
 #ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
 int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
Signature · 1 line
2.11.0
reviewer Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] dma-coherent: remove the DMA_MEMORY_INCLUDES_CHILDREN flag

Quoted 1 line · Christoph Hellwig

This flag was never implemented or used.

And at worst, we can always bring it back.

Reviewed-by Robin Murphy <[email protected]>
Quoted 65 lines
Signed-off-by Christoph Hellwig <[email protected]>
---
 Documentation/DMA-API.txt     | 4 ----
 drivers/base/dma-coherent.c   | 2 --
 drivers/char/virtio_console.c | 3 ---
 include/linux/dma-mapping.h   | 3 +--
 4 files changed, 1 insertion(+), 11 deletions(-)
Documentation/DMA-API.txt +0 -4
index 6b20128fab8a..03072bc7ff74 100644
@@ -524,10 +524,6 @@ dma_alloc_coherent() be addressable using read()/write()/memcpy_toio() etc.
 
 One or both of these flags must be present.
 
-DMA_MEMORY_INCLUDES_CHILDREN - make the declared memory be allocated by
-dma_alloc_coherent of any child devices of this one (for memory residing
-on a bridge).
-
 DMA_MEMORY_EXCLUSIVE - only allocate memory from the declared regions. 
 Do not allow dma_alloc_coherent() to fall back to system memory when
 it's out of memory in the declared region.
drivers/base/dma-coherent.c +0 -2
index 2ae24c28e70c..e70e33d5c902 100644
@@ -109,8 +109,6 @@ static int dma_assign_coherent_memory(struct device *dev,
 		return -EBUSY;
 
 	dev->dma_mem = mem;
-	/* FIXME: this routine just ignores DMA_MEMORY_INCLUDES_CHILDREN */
-
 	return 0;
 }
drivers/char/virtio_console.c +0 -3
index ad843eb02ae7..48e86ce9fac8 100644
@@ -451,9 +451,6 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
 		 * device is created by remoteproc, the DMA memory is
 		 * associated with the grandparent device:
 		 * vdev => rproc => platform-dev.
-		 * The code here would have been less quirky if
-		 * DMA_MEMORY_INCLUDES_CHILDREN had been supported
-		 * in dma-coherent.c
 		 */
 		if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
 			goto free_buf;
include/linux/dma-mapping.h +1 -2
index 843ab866e0f4..fb66fe98e61d 100644
@@ -685,8 +685,7 @@ static inline int dma_get_cache_alignment(void)
 /* flags for the coherent memory api */
 #define	DMA_MEMORY_MAP			0x01
 #define DMA_MEMORY_IO			0x02
-#define DMA_MEMORY_INCLUDES_CHILDREN	0x04
-#define DMA_MEMORY_EXCLUSIVE		0x08
+#define DMA_MEMORY_EXCLUSIVE		0x04
 
 #ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
 int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
author
#3 lore raw

PATCH 2/2 dma-coherent: remove the DMA_MEMORY_MAP and DMA_MEMORY_IO flags

In reply to Christoph Hellwig

DMA_MEMORY_IO was never used in the tree, so remove it. That means there is no need for the DMA_MEMORY_MAP flag either now, so remove it as well and change dma_declare_coherent_memory to return a normal errno value.

Signed-off-by Christoph Hellwig <[email protected]>
---
 Documentation/DMA-API.txt                          | 19 ---------
 arch/arm/mach-imx/mach-imx27_visstrim_m10.c        | 43 +++++++-------------
 arch/arm/mach-imx/mach-mx31moboard.c               | 12 +++---
 arch/sh/drivers/pci/fixups-dreamcast.c             |  3 +-
 drivers/base/dma-coherent.c                        | 46 +++++++---------------
 drivers/base/dma-mapping.c                         |  7 +---
 .../platform/soc_camera/sh_mobile_ceu_camera.c     |  5 +--
 drivers/scsi/NCR_Q720.c                            |  3 +-
 drivers/usb/host/ohci-sm501.c                      |  6 +--
 drivers/usb/host/ohci-tmio.c                       |  9 ++---
 include/linux/dma-mapping.h                        |  6 +--
 11 files changed, 49 insertions(+), 110 deletions(-)
Documentation/DMA-API.txt +0 -19
index 03072bc7ff74..d464530c0ada 100644
@@ -516,29 +516,10 @@ size is the size of the area (must be multiples of PAGE_SIZE).
 
 flags can be ORed together and are:
 
-DMA_MEMORY_MAP - request that the memory returned from
-dma_alloc_coherent() be directly writable.
-
-DMA_MEMORY_IO - request that the memory returned from
-dma_alloc_coherent() be addressable using read()/write()/memcpy_toio() etc.
-
-One or both of these flags must be present.
-
 DMA_MEMORY_EXCLUSIVE - only allocate memory from the declared regions. 
 Do not allow dma_alloc_coherent() to fall back to system memory when
 it's out of memory in the declared region.
 
-The return value will be either DMA_MEMORY_MAP or DMA_MEMORY_IO and
-must correspond to a passed in flag (i.e. no returning DMA_MEMORY_IO
-if only DMA_MEMORY_MAP were passed in) for success or zero for
-failure.
-
-Note, for DMA_MEMORY_IO returns, all subsequent memory returned by
-dma_alloc_coherent() may no longer be accessed directly, but instead
-must be accessed using the correct bus functions.  If your driver
-isn't prepared to handle this contingency, it should not specify
-DMA_MEMORY_IO in the input flags.
-
 As a simplification for the platforms, only *one* such region of
 memory may be declared per device.
arch/arm/mach-imx/mach-imx27_visstrim_m10.c +15 -28

86 lines not on this page · raw message

arch/arm/mach-imx/mach-mx31moboard.c +6 -6
index bde9a9af6714..3cd030960c98 100644
@@ -475,7 +475,7 @@ static phys_addr_t mx3_camera_base __initdata;
 
 static int __init mx31moboard_init_cam(void)
 {
-	int dma, ret = -ENOMEM;
+	int dma, ret;
 	struct platform_device *pdev;
 
 	imx31_add_ipu_core();

@@ -484,11 +484,11 @@ static int __init mx31moboard_init_cam(void)
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
 
-	dma = dma_declare_coherent_memory(&pdev->dev,
-					mx3_camera_base, mx3_camera_base,
-					MX3_CAMERA_BUF_SIZE,
-					DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
-	if (!(dma & DMA_MEMORY_MAP))
+	ret = dma_declare_coherent_memory(&pdev->dev,
+					  mx3_camera_base, mx3_camera_base,
+					  MX3_CAMERA_BUF_SIZE,
+					  DMA_MEMORY_EXCLUSIVE);
+	if (ret)
 		goto err;
 
 	ret = platform_device_add(pdev);
arch/sh/drivers/pci/fixups-dreamcast.c +1 -2
index 1d1c5a227e50..c931d5872ffe 100644
@@ -63,11 +63,10 @@ static void gapspci_fixup_resources(struct pci_dev *dev)
 		res.end = GAPSPCI_DMA_BASE + GAPSPCI_DMA_SIZE - 1;
 		res.flags = IORESOURCE_MEM;
 		pcibios_resource_to_bus(dev->bus, &region, &res);
-		BUG_ON(!dma_declare_coherent_memory(&dev->dev,
+		BUG_ON(dma_declare_coherent_memory(&dev->dev,
 						res.start,
 						region.start,
 						resource_size(&res),
-						DMA_MEMORY_MAP |
 						DMA_MEMORY_EXCLUSIVE));
 		break;
 	default:
drivers/base/dma-coherent.c +14 -32

94 lines not on this page · raw message

drivers/base/dma-mapping.c +2 -5
index 10e7c022e8cf..b96bfffd314e 100644
@@ -176,13 +176,10 @@ int dmam_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
 
 	rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
 					 flags);
-	if (rc) {
+	if (rc)
 		devres_add(dev, res);
-		rc = 0;
-	} else {
+	else
 		devres_free(res);
-		rc = -ENOMEM;
-	}
 
 	return rc;
 }
drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c +2 -3
index 96dc01750bc0..36762ec954e7 100644
@@ -1708,11 +1708,10 @@ static int sh_mobile_ceu_probe(struct platform_device *pdev)
 		err = dma_declare_coherent_memory(&pdev->dev, res->start,
 						  res->start,
 						  resource_size(res),
-						  DMA_MEMORY_MAP |
 						  DMA_MEMORY_EXCLUSIVE);
-		if (!err) {
+		if (err) {
 			dev_err(&pdev->dev, "Unable to declare CEU memory.\n");
-			return -ENXIO;
+			return err;
 		}
 
 		pcdev->video_limit = resource_size(res);
drivers/scsi/NCR_Q720.c +1 -2
index 05835bf1bf9c..54e7d26908ee 100644
@@ -217,8 +217,7 @@ NCR_Q720_probe(struct device *dev)
 	}
 	
 	if (dma_declare_coherent_memory(dev, base_addr, base_addr,
-					mem_size, DMA_MEMORY_MAP)
-	    != DMA_MEMORY_MAP) {
+					mem_size, 0)) {
 		printk(KERN_ERR "NCR_Q720: DMA declare memory failed\n");
 		goto out_release_region;
 	}
drivers/usb/host/ohci-sm501.c +3 -3
index a8b8d8b8d9f3..21302a0f82e2 100644
@@ -123,11 +123,11 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 	 * regular memory. The HCD_LOCAL_MEM flag does just that.
 	 */
 
-	if (!dma_declare_coherent_memory(dev, mem->start,
+	retval = dma_declare_coherent_memory(dev, mem->start,
 					 mem->start - mem->parent->start,
 					 resource_size(mem),
-					 DMA_MEMORY_MAP |
-					 DMA_MEMORY_EXCLUSIVE)) {
+					 DMA_MEMORY_EXCLUSIVE);
+	if (retval) {
 		dev_err(dev, "cannot declare coherent memory\n");
 		retval = -ENXIO;
 		goto err1;
drivers/usb/host/ohci-tmio.c +3 -6
index cfcfadfc94fc..16d081a093bb 100644
@@ -227,13 +227,10 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device *dev)
 		goto err_ioremap_regs;
 	}
 
-	if (!dma_declare_coherent_memory(&dev->dev, sram->start,
-				sram->start,
-				resource_size(sram),
-				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE)) {
-		ret = -EBUSY;
+	ret = dma_declare_coherent_memory(&dev->dev, sram->start, sram->start,
+				resource_size(sram), DMA_MEMORY_EXCLUSIVE);
+	if (ret)
 		goto err_dma_declare;
-	}
 
 	if (cell->enable) {
 		ret = cell->enable(dev);
include/linux/dma-mapping.h +2 -4
index fb66fe98e61d..e33feccc35d1 100644
@@ -683,9 +683,7 @@ static inline int dma_get_cache_alignment(void)
 #endif
 
 /* flags for the coherent memory api */
-#define	DMA_MEMORY_MAP			0x01
-#define DMA_MEMORY_IO			0x02
-#define DMA_MEMORY_EXCLUSIVE		0x04
+#define DMA_MEMORY_EXCLUSIVE		0x01
 
 #ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
 int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,

@@ -698,7 +696,7 @@ static inline int
 dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
 			    dma_addr_t device_addr, size_t size, int flags)
 {
-	return 0;
+	return -EINVAL;
 }
 
 static inline void
Signature · 1 line
2.11.0
reviewer
#4 lore raw

Re: [PATCH 2/2] dma-coherent: remove the DMA_MEMORY_MAP and DMA_MEMORY_IO flags

Quoted 6 lines · Christoph Hellwig

DMA_MEMORY_IO was never used in the tree, so remove it. That means there is no need for the DMA_MEMORY_MAP flag either now, so remove it as well and change dma_declare_coherent_memory to return a normal errno value.

Signed-off-by Christoph Hellwig <[email protected]>

---

[...]

drivers/base/dma-mapping.c int dmam_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr, · line 179

Quoted 10 lines
drivers/base/dma-mapping.c +1 -1
index 10e7c022e8cf..b96bfffd314e 100644
@@ -176,13 +176,10 @@ int dmam_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
 
 	rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
 					 flags);
-	if (rc) {
+	if (rc)

if (!rc)

Quoted 10 lines
drivers/base/dma-mapping.c +1 -4
 		devres_add(dev, res);
-		rc = 0;
-	} else {
+	else
 		devres_free(res);
-		rc = -ENOMEM;
-	}
 
 	return rc;
 }

[...]

Quoted 8 lines
drivers/usb/host/ohci-sm501.c +0 -0
index a8b8d8b8d9f3..21302a0f82e2 100644
@@ -123,11 +123,11 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 	 * regular memory. The HCD_LOCAL_MEM flag does just that.
 	 */

drivers/usb/host/ohci-sm501.c static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev) · line 132

Quoted 10 lines
drivers/usb/host/ohci-sm501.c +3 -3
-	if (!dma_declare_coherent_memory(dev, mem->start,
+	retval = dma_declare_coherent_memory(dev, mem->start,
 					 mem->start - mem->parent->start,
 					 resource_size(mem),
-					 DMA_MEMORY_MAP |
-					 DMA_MEMORY_EXCLUSIVE)) {
+					 DMA_MEMORY_EXCLUSIVE);
+	if (retval) {
 		dev_err(dev, "cannot declare coherent memory\n");
 		retval = -ENXIO;

It looks like we could just let retval propagate here, rather than rewriting it, since we're only bailing out of a probe routine.

Robin.

Quoted 1 line
drivers/usb/host/ohci-sm501.c +0 -0
 		goto err1;