Re: [PATCH v4 05/44] arm64: dma-mapping: Use unsigned long for dma_attrs

From: Krzysztof Kozlowski
Date: Fri Jun 10 2016 - 06:55:24 EST

On 06/10/2016 12:51 PM, Robin Murphy wrote:
> Hi Krzysztof,
>
> On 10/06/16 11:11, Krzysztof Kozlowski wrote:
>> Split out subsystem specific changes for easier reviews. This will be
>> squashed with main commit.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
>> ---
>> arch/arm64/mm/dma-mapping.c | 57
>> +++++++++++++++++++++++----------------------
>> 1 file changed, 29 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index 0ef620a34c4e..a7686028dfeb 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -29,7 +29,7 @@
>>
>> #include <asm/cacheflush.h>
>>
>> -static pgprot_t __get_dma_pgprot(const struct dma_attrs *attrs,
>> pgprot_t prot,
>> +static pgprot_t __get_dma_pgprot(unsigned long attrs, pgprot_t prot,
>> bool coherent)
>
> Is this series intended to replace the patches currently in -next adding
> that "const"? (I was momentarily puzzled why this wouldn't apply locally)

Yes, the series replace completely my previous const changes (applied by
Andrew Morton).

I rebased the patchset on linux-next so I could grab all recent changes
around dma_attrs. Probably that should be changed before applying -
choose one tree to base on (Andrew's?).

If you would like to play with the entire series (on top of next):
https://github.com/krzk/linux/tree/for-next/dma-attrs-const-v4


>
>> {
>> if (!coherent || dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs))
>> @@ -88,7 +88,7 @@ static int __free_from_pool(void *start, size_t size)
>>
>> static void *__dma_alloc_coherent(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t flags,
>> - const struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> if (dev == NULL) {
>> WARN_ONCE(1, "Use an actual device structure for DMA
>> allocation\n");
>> @@ -118,7 +118,7 @@ static void *__dma_alloc_coherent(struct device
>> *dev, size_t size,
>>
>> static void __dma_free_coherent(struct device *dev, size_t size,
>> void *vaddr, dma_addr_t dma_handle,
>> - const struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> bool freed;
>> phys_addr_t paddr = dma_to_phys(dev, dma_handle);
>> @@ -137,7 +137,7 @@ static void __dma_free_coherent(struct device
>> *dev, size_t size,
>>
>> static void *__dma_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t flags,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> struct page *page;
>> void *ptr, *coherent_ptr;
>> @@ -185,7 +185,7 @@ no_mem:
>>
>> static void __dma_free(struct device *dev, size_t size,
>> void *vaddr, dma_addr_t dma_handle,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> void *swiotlb_addr = phys_to_virt(dma_to_phys(dev, dma_handle));
>>
>> @@ -202,7 +202,7 @@ static void __dma_free(struct device *dev, size_t
>> size,
>> static dma_addr_t __swiotlb_map_page(struct device *dev, struct page
>> *page,
>> unsigned long offset, size_t size,
>> enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> dma_addr_t dev_addr;
>>
>> @@ -216,7 +216,7 @@ static dma_addr_t __swiotlb_map_page(struct device
>> *dev, struct page *page,
>>
>> static void __swiotlb_unmap_page(struct device *dev, dma_addr_t
>> dev_addr,
>> size_t size, enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> if (!is_device_dma_coherent(dev))
>> __dma_unmap_area(phys_to_virt(dma_to_phys(dev, dev_addr)),
>> size, dir);
>> @@ -225,7 +225,7 @@ static void __swiotlb_unmap_page(struct device
>> *dev, dma_addr_t dev_addr,
>>
>> static int __swiotlb_map_sg_attrs(struct device *dev, struct
>> scatterlist *sgl,
>> int nelems, enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> struct scatterlist *sg;
>> int i, ret;
>> @@ -242,7 +242,7 @@ static int __swiotlb_map_sg_attrs(struct device
>> *dev, struct scatterlist *sgl,
>> static void __swiotlb_unmap_sg_attrs(struct device *dev,
>> struct scatterlist *sgl, int nelems,
>> enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> struct scatterlist *sg;
>> int i;
>> @@ -303,7 +303,7 @@ static void __swiotlb_sync_sg_for_device(struct
>> device *dev,
>> static int __swiotlb_mmap(struct device *dev,
>> struct vm_area_struct *vma,
>> void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> int ret = -ENXIO;
>> unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >>
>> @@ -330,7 +330,7 @@ static int __swiotlb_mmap(struct device *dev,
>>
>> static int __swiotlb_get_sgtable(struct device *dev, struct sg_table
>> *sgt,
>> void *cpu_addr, dma_addr_t handle, size_t size,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> int ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
>>
>> @@ -425,21 +425,21 @@ out:
>>
>> static void *__dummy_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t flags,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> return NULL;
>> }
>>
>> static void __dummy_free(struct device *dev, size_t size,
>> void *vaddr, dma_addr_t dma_handle,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> }
>>
>> static int __dummy_mmap(struct device *dev,
>> struct vm_area_struct *vma,
>> void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> return -ENXIO;
>> }
>> @@ -447,20 +447,20 @@ static int __dummy_mmap(struct device *dev,
>> static dma_addr_t __dummy_map_page(struct device *dev, struct page
>> *page,
>> unsigned long offset, size_t size,
>> enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> return DMA_ERROR_CODE;
>> }
>>
>> static void __dummy_unmap_page(struct device *dev, dma_addr_t dev_addr,
>> size_t size, enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> }
>>
>> static int __dummy_map_sg(struct device *dev, struct scatterlist *sgl,
>> int nelems, enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> return 0;
>> }
>> @@ -468,7 +468,7 @@ static int __dummy_map_sg(struct device *dev,
>> struct scatterlist *sgl,
>> static void __dummy_unmap_sg(struct device *dev,
>> struct scatterlist *sgl, int nelems,
>> enum dma_data_direction dir,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> }
>>
>> @@ -540,7 +540,7 @@ static void flush_page(struct device *dev, const
>> void *virt, phys_addr_t phys)
>>
>> static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>> dma_addr_t *handle, gfp_t gfp,
>> - struct dma_attrs *attrs)
>> + unsigned long attrs)
>> {
>> bool coherent = is_device_dma_coherent(dev);
>> int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>> @@ -600,7 +600,8 @@ static void *__iommu_alloc_attrs(struct device
>> *dev, size_t size,
>> }
>>
>> static void __iommu_free_attrs(struct device *dev, size_t size, void
>> *cpu_addr,
>> - dma_addr_t handle, struct dma_attrs *attrs)
>> + dma_addr_t handle,
>> + unsigned long attrs)
>
> Nit: unnecessary line break?
>
> Otherwise, though, I'm all for the idea;
>
> Acked-by: Robin Murphy <robin.murphy@xxxxxxx>

Right, thanks!

Best regards,
Krzysztof