diff mbox series

[RESEND] firmware: qcom_scm: use correct parameter type for dma_alloc_coherent

Message ID 20180306123320.24523-1-benjamin.gaignard@st.com
State New
Headers show
Series [RESEND] firmware: qcom_scm: use correct parameter type for dma_alloc_coherent | expand

Commit Message

Benjamin Gaignard March 6, 2018, 12:33 p.m. UTC 
From: Benjamin Gaignard <benjamin.gaignard@linaro.org>


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.

Fixes: d82bd359972a7 ("firmware: scm: Add new SCM call API for switching memory ownership")

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

Andy Gross March 9, 2018, 10:56 p.m. UTC | #1 
On Tue, Mar 06, 2018 at 01:33:20PM +0100, Benjamin Gaignard wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> 

> 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.

> 

> Fixes: d82bd359972a7 ("firmware: scm: Add new SCM call API for switching memory ownership")


Benjamin,

This brings up an old discussion regarding the nature of the memory we are using
for SCM transactions (per Arnd's comments on the original).  I'd like to defer
taking this until we get some clarification on the long term plans for fixing
the underlying issues and not just this type issue.  I'd hope to have some
answer one way or the other in the next week and a half.

For now, I have this patch and will hold on to it.  Thanks for the contribution.

Regards,

Andy Gross
--
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 April 5, 2018, 1:02 p.m. UTC | #2 
2018-03-09 23:56 GMT+01:00 Andy Gross <andy.gross@linaro.org>:
> On Tue, Mar 06, 2018 at 01:33:20PM +0100, Benjamin Gaignard wrote:

>> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

>>

>> 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.

>>

>> Fixes: d82bd359972a7 ("firmware: scm: Add new SCM call API for switching memory ownership")

>

> Benjamin,

>

> This brings up an old discussion regarding the nature of the memory we are using

> for SCM transactions (per Arnd's comments on the original).  I'd like to defer

> taking this until we get some clarification on the long term plans for fixing

> the underlying issues and not just this type issue.  I'd hope to have some

> answer one way or the other in the next week and a half.


Hi Andy,

Any news on this topic ?

Regards,
Benjamin
>

> For now, I have this patch and will hold on to it.  Thanks for the contribution.

>

> Regards,

>

> Andy Gross

--
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
diff mbox series

Patch

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 5a7d693009ef..2847f9a93cfe 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);