Re: [PATCH 22/22] arm64: use swiotlb_alloc and swiotlb_free

[Robin Murphy <robin.murphy@xxxxxxx>]

On 10/01/18 08:09, Christoph Hellwig wrote:
> The generic swiotlb_alloc and swiotlb_free routines already take care
> of CMA allocations and adding GFP_DMA32 where needed, so use them
> instead of the arm specific helpers.

It took a while to satisfy myself that the GFP_DMA(32) handling ends up 
equivalent to the current behaviour, but I think it checks out. This 
will certainly help with the long-overdue cleanup of this file that I've 
had sat around half-finished for ages.

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

> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>   arch/arm64/Kconfig          |  1 +
>   arch/arm64/mm/dma-mapping.c | 46 +++------------------------------------------
>   2 files changed, 4 insertions(+), 43 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6b6985f15d02..53205c02b18a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -59,6 +59,7 @@ config ARM64
>   	select COMMON_CLK
>   	select CPU_PM if (SUSPEND || CPU_IDLE)
>   	select DCACHE_WORD_ACCESS
> +	select DMA_DIRECT_OPS
>   	select EDAC_SUPPORT
>   	select FRAME_POINTER
>   	select GENERIC_ALLOCATOR
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 0d641875b20e..a96ec0181818 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -91,46 +91,6 @@ static int __free_from_pool(void *start, size_t size)
>   	return 1;
>   }
>   
> -static void *__dma_alloc_coherent(struct device *dev, size_t size,
> -				  dma_addr_t *dma_handle, gfp_t flags,
> -				  unsigned long attrs)
> -{
> -	if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
> -	    dev->coherent_dma_mask <= DMA_BIT_MASK(32))
> -		flags |= GFP_DMA32;
> -	if (dev_get_cma_area(dev) && gfpflags_allow_blocking(flags)) {
> -		struct page *page;
> -		void *addr;
> -
> -		page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT,
> -						 get_order(size), flags);
> -		if (!page)
> -			return NULL;
> -
> -		*dma_handle = phys_to_dma(dev, page_to_phys(page));
> -		addr = page_address(page);
> -		memset(addr, 0, size);
> -		return addr;
> -	} else {
> -		return swiotlb_alloc_coherent(dev, size, dma_handle, flags);
> -	}
> -}
> -
> -static void __dma_free_coherent(struct device *dev, size_t size,
> -				void *vaddr, dma_addr_t dma_handle,
> -				unsigned long attrs)
> -{
> -	bool freed;
> -	phys_addr_t paddr = dma_to_phys(dev, dma_handle);
> -
> -
> -	freed = dma_release_from_contiguous(dev,
> -					phys_to_page(paddr),
> -					size >> PAGE_SHIFT);
> -	if (!freed)
> -		swiotlb_free_coherent(dev, size, vaddr, dma_handle);
> -}
> -
>   static void *__dma_alloc(struct device *dev, size_t size,
>   			 dma_addr_t *dma_handle, gfp_t flags,
>   			 unsigned long attrs)
> @@ -152,7 +112,7 @@ static void *__dma_alloc(struct device *dev, size_t size,
>   		return addr;
>   	}
>   
> -	ptr = __dma_alloc_coherent(dev, size, dma_handle, flags, attrs);
> +	ptr = swiotlb_alloc(dev, size, dma_handle, flags, attrs);
>   	if (!ptr)
>   		goto no_mem;
>   
> @@ -173,7 +133,7 @@ static void *__dma_alloc(struct device *dev, size_t size,
>   	return coherent_ptr;
>   
>   no_map:
> -	__dma_free_coherent(dev, size, ptr, *dma_handle, attrs);
> +	swiotlb_free(dev, size, ptr, *dma_handle, attrs);
>   no_mem:
>   	return NULL;
>   }
> @@ -191,7 +151,7 @@ static void __dma_free(struct device *dev, size_t size,
>   			return;
>   		vunmap(vaddr);
>   	}
> -	__dma_free_coherent(dev, size, swiotlb_addr, dma_handle, attrs);
> +	swiotlb_free(dev, size, swiotlb_addr, dma_handle, attrs);
>   }
>   
>   static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,