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

[Krzysztof Kozlowski <k.kozlowski@samsung.com>]

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@samsung.com>
>> ---
>>   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@arm.com>

Right, thanks!

Best regards,
Krzysztof