| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 01 Sep 2017 10:19:45 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Christoph Hellwig <hch@....de>, linux-kernel@...r.kernel.org
Cc: Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH 2/2] dma-coherent: remove the DMA_MEMORY_MAP and
DMA_MEMORY_IO flags
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@....de>
There are 2 minor issues, besides that:
Reviewed-by: Marek Szyprowski <m.szyprowski@...sung.com>
> ---
> 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
Powered by blists - more mailing lists