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

From: Marek Szyprowski
Date: Fri Sep 01 2017 - 04:20:00 EST

Hi Christoph,

On 2017-08-26 09:26, Christoph Hellwig wrote:
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 <hch@xxxxxx>

There are 2 minor issues, besides that:

Reviewed-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>

---
Documentation/DMA-API.txt | 21 +---------
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 | 7 ++--
drivers/usb/host/ohci-tmio.c | 9 ++---
include/linux/dma-mapping.h | 6 +--
11 files changed, 50 insertions(+), 112 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index ded50aed1cd8..da58585c7b3a 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -590,30 +590,11 @@ 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
+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
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index dd75a4756761..c2818af18c55 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -245,7 +245,6 @@ static phys_addr_t mx2_camera_base __initdata;
static void __init visstrim_analog_camera_init(void)
{
struct platform_device *pdev;
- int dma;
gpio_set_value(TVP5150_PWDN, 1);
ndelay(1);
@@ -258,12 +257,9 @@ static void __init visstrim_analog_camera_init(void)
if (IS_ERR(pdev))
return;
- dma = dma_declare_coherent_memory(&pdev->dev,
- mx2_camera_base, mx2_camera_base,
- MX2_CAMERA_BUF_SIZE,
- DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
- if (!(dma & DMA_MEMORY_MAP))
- return;
+ dma_declare_coherent_memory(&pdev->dev, mx2_camera_base,
+ mx2_camera_base, MX2_CAMERA_BUF_SIZE,
+ DMA_MEMORY_EXCLUSIVE);
}
static void __init visstrim_reserve(void)
@@ -444,16 +440,13 @@ static const struct imx_ssi_platform_data visstrim_m10_ssi_pdata __initconst = {
static void __init visstrim_coda_init(void)
{
struct platform_device *pdev;
- int dma;
pdev = imx27_add_coda();
- dma = dma_declare_coherent_memory(&pdev->dev,
- mx2_camera_base + MX2_CAMERA_BUF_SIZE,
- mx2_camera_base + MX2_CAMERA_BUF_SIZE,
- MX2_CAMERA_BUF_SIZE,
- DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
- if (!(dma & DMA_MEMORY_MAP))
- return;
+ dma_declare_coherent_memory(&pdev->dev,
+ mx2_camera_base + MX2_CAMERA_BUF_SIZE,
+ mx2_camera_base + MX2_CAMERA_BUF_SIZE,
+ MX2_CAMERA_BUF_SIZE,
+ DMA_MEMORY_EXCLUSIVE);
}
/* DMA deinterlace */
@@ -466,24 +459,20 @@ static void __init visstrim_deinterlace_init(void)
{
int ret = -ENOMEM;
struct platform_device *pdev = &visstrim_deinterlace;
- int dma;
ret = platform_device_register(pdev);
- dma = dma_declare_coherent_memory(&pdev->dev,
- mx2_camera_base + 2 * MX2_CAMERA_BUF_SIZE,
- mx2_camera_base + 2 * MX2_CAMERA_BUF_SIZE,
- MX2_CAMERA_BUF_SIZE,
- DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
- if (!(dma & DMA_MEMORY_MAP))
- return;
+ dma_declare_coherent_memory(&pdev->dev,
+ mx2_camera_base + 2 * MX2_CAMERA_BUF_SIZE,
+ mx2_camera_base + 2 * MX2_CAMERA_BUF_SIZE,
+ MX2_CAMERA_BUF_SIZE,
+ DMA_MEMORY_EXCLUSIVE);
}
/* Emma-PrP for format conversion */
static void __init visstrim_emmaprp_init(void)
{
struct platform_device *pdev;
- int dma;
pdev = imx27_add_mx2_emmaprp();
if (IS_ERR(pdev))
@@ -493,12 +482,10 @@ static void __init visstrim_emmaprp_init(void)
* Use the same memory area as the analog camera since both
* devices are, by nature, exclusive.
*/
- dma = dma_declare_coherent_memory(&pdev->dev,
+ dma_declare_coherent_memory(&pdev->dev,
mx2_camera_base, mx2_camera_base,
MX2_CAMERA_BUF_SIZE,
- DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
- if (!(dma & DMA_MEMORY_MAP))
- pr_err("Failed to declare memory for emmaprp\n");
+ DMA_MEMORY_EXCLUSIVE);

Can we keep error message? It might be useful in case of any problems, because
the return value is not propagated otherwise.

> [...]

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9b12cb255063..92f7d84e51f5 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -708,9 +708,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,
@@ -723,7 +721,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;

ENOSYS ?

}
static inline void

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland