firmware: qcom_scm: use correct parameter type for dma_alloc_coherent

Message ID 20180208122146.28399-1-benjamin.gaignard@linaro.org
State New
Headers show
Series
  • firmware: qcom_scm: use correct parameter type for dma_alloc_coherent
Related show

Commit Message

Benjamin Gaignard Feb. 8, 2018, 12:21 p.m.
dma_alloc_coherent expects it third argument type to be dma_addr_t and
not phys_addr_t.
When phys_addr_t is defined as u32 and dma_addr_t as u64 that generate
a compilation issue.
Change the variable name because dma_alloc_coherent returns a bus
address not a physical address.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

---
 drivers/firmware/qcom_scm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Arnd Bergmann Feb. 8, 2018, 2:09 p.m. | #1
On Thu, Feb 8, 2018 at 1:21 PM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> dma_alloc_coherent expects it third argument type to be dma_addr_t and

> not phys_addr_t.

> When phys_addr_t is defined as u32 and dma_addr_t as u64 that generate

> a compilation issue.

> Change the variable name because dma_alloc_coherent returns a bus

> address not a physical address.

>


> @@ -480,14 +480,14 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,

>

>         /* Fill details of mem buff to map */

>         mem_to_map = ptr + ALIGN(src_sz, SZ_64);

> -       mem_to_map_phys = ptr_phys + ALIGN(src_sz, SZ_64);

> +       mem_to_map_phys = handle + ALIGN(src_sz, SZ_64);

>         mem_to_map[0].mem_addr = cpu_to_le64(mem_addr);

>         mem_to_map[0].mem_size = cpu_to_le64(mem_sz);

>

>         next_vm = 0;

>         /* Fill details of next vmid detail */

>         destvm = ptr + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);

> -       dest_phys = ptr_phys + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);

> +       dest_phys = handle + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);


This still assigns from a 'dma' address to a 'phys' address without a
long comment
explaining why you do it. There is also another instance of the naming confusion
in qcom_scm_pas_init_image and __qcom_scm_pas_init_image, so maybe they
should be addressed at the same time.

       Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Gaignard Feb. 8, 2018, 2:26 p.m. | #2
2018-02-08 15:09 GMT+01:00 Arnd Bergmann <arnd@arndb.de>:
> On Thu, Feb 8, 2018 at 1:21 PM, Benjamin Gaignard

> <benjamin.gaignard@linaro.org> wrote:

>> dma_alloc_coherent expects it third argument type to be dma_addr_t and

>> not phys_addr_t.

>> When phys_addr_t is defined as u32 and dma_addr_t as u64 that generate

>> a compilation issue.

>> Change the variable name because dma_alloc_coherent returns a bus

>> address not a physical address.

>>

>

>> @@ -480,14 +480,14 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,

>>

>>         /* Fill details of mem buff to map */

>>         mem_to_map = ptr + ALIGN(src_sz, SZ_64);

>> -       mem_to_map_phys = ptr_phys + ALIGN(src_sz, SZ_64);

>> +       mem_to_map_phys = handle + ALIGN(src_sz, SZ_64);

>>         mem_to_map[0].mem_addr = cpu_to_le64(mem_addr);

>>         mem_to_map[0].mem_size = cpu_to_le64(mem_sz);

>>

>>         next_vm = 0;

>>         /* Fill details of next vmid detail */

>>         destvm = ptr + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);

>> -       dest_phys = ptr_phys + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);

>> +       dest_phys = handle + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);

>

> This still assigns from a 'dma' address to a 'phys' address without a

> long comment

> explaining why you do it. There is also another instance of the naming confusion

> in qcom_scm_pas_init_image and __qcom_scm_pas_init_image, so maybe they

> should be addressed at the same time.


This patch just fix a compilation issue, nothing else.

>

>        Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index af4c75217ea6..a405a540c464 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -448,7 +448,7 @@  int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
 	struct qcom_scm_mem_map_info *mem_to_map;
 	phys_addr_t mem_to_map_phys;
 	phys_addr_t dest_phys;
-	phys_addr_t ptr_phys;
+	dma_addr_t handle;
 	size_t mem_to_map_sz;
 	size_t dest_sz;
 	size_t src_sz;
@@ -466,7 +466,7 @@  int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
 	ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(mem_to_map_sz, SZ_64) +
 			ALIGN(dest_sz, SZ_64);
 
-	ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_phys, GFP_KERNEL);
+	ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &handle, GFP_KERNEL);
 	if (!ptr)
 		return -ENOMEM;
 
@@ -480,14 +480,14 @@  int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
 
 	/* Fill details of mem buff to map */
 	mem_to_map = ptr + ALIGN(src_sz, SZ_64);
-	mem_to_map_phys = ptr_phys + ALIGN(src_sz, SZ_64);
+	mem_to_map_phys = handle + ALIGN(src_sz, SZ_64);
 	mem_to_map[0].mem_addr = cpu_to_le64(mem_addr);
 	mem_to_map[0].mem_size = cpu_to_le64(mem_sz);
 
 	next_vm = 0;
 	/* Fill details of next vmid detail */
 	destvm = ptr + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);
-	dest_phys = ptr_phys + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);
+	dest_phys = handle + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(src_sz, SZ_64);
 	for (i = 0; i < dest_cnt; i++) {
 		destvm[i].vmid = cpu_to_le32(newvm[i].vmid);
 		destvm[i].perm = cpu_to_le32(newvm[i].perm);
@@ -497,8 +497,8 @@  int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
 	}
 
 	ret = __qcom_scm_assign_mem(__scm->dev, mem_to_map_phys, mem_to_map_sz,
-				    ptr_phys, src_sz, dest_phys, dest_sz);
-	dma_free_coherent(__scm->dev, ALIGN(ptr_sz, SZ_64), ptr, ptr_phys);
+				    handle, src_sz, dest_phys, dest_sz);
+	dma_free_coherent(__scm->dev, ALIGN(ptr_sz, SZ_64), ptr, handle);
 	if (ret) {
 		dev_err(__scm->dev,
 			"Assign memory protection call failed %d.\n", ret);