| Message ID | 20190517210923.202131-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcom_scm: Fix some dma mapping things | expand |
Stephen Boyd writes ("[PATCH 1/3] firmware: qcom_scm: Use proper types for dma mappings"):
> We need to use the proper types and convert between physical addresses
> and dma addresses here to avoid mismatch warnings. This is especially
> important on systems with a different size for dma addresses and
> physical addresses. Otherwise, we get the following warning:
Thanks. Do you expect this to be a backport candidate and if so how
far back do you think it will go ? To be honest, I am not really
convinced that backporting this would be a service to users. The
situation I have, where I changed the compiler but kept the old kernel
code and old configuration, is going to be fairly rare.
I think I should probably therefore disable this driver in the config
on stable branches of Linux, at least.
Thanks,
Ian.
Hi Ian, On 20/05/2019 10:41, Ian Jackson wrote: > Stephen Boyd writes ("[PATCH 1/3] firmware: qcom_scm: Use proper types for dma mappings"): >> We need to use the proper types and convert between physical addresses >> and dma addresses here to avoid mismatch warnings. This is especially >> important on systems with a different size for dma addresses and >> physical addresses. Otherwise, we get the following warning: > > Thanks. Do you expect this to be a backport candidate and if so how > far back do you think it will go ? To be honest, I am not really > convinced that backporting this would be a service to users. The > situation I have, where I changed the compiler but kept the old kernel > code and old configuration, is going to be fairly rare. I will leave the maintainers answering to that. > > I think I should probably therefore disable this driver in the config > on stable branches of Linux, at least. If we decide to disable the driver, then we would need to add in our .config, then we would need to disable completely the support for Qualcomm (i.e CONFIG_ARCH_QCOM=n) on Arm32. This should not be an issue in osstest as we don't test any qualcomm board so far. Cheers,
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index af4eee86919d..0c63495cf269 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -18,6 +18,7 @@ #include <linux/init.h> #include <linux/cpumask.h> #include <linux/export.h> +#include <linux/dma-direct.h> #include <linux/dma-mapping.h> #include <linux/module.h> #include <linux/types.h> @@ -449,6 +450,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, phys_addr_t mem_to_map_phys; phys_addr_t dest_phys; phys_addr_t ptr_phys; + dma_addr_t ptr_dma; size_t mem_to_map_sz; size_t dest_sz; size_t src_sz; @@ -466,9 +468,10 @@ 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, &ptr_dma, GFP_KERNEL); if (!ptr) return -ENOMEM; + ptr_phys = dma_to_phys(__scm->dev, ptr_dma); /* Fill source vmid detail */ src = ptr; @@ -498,7 +501,7 @@ 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); + dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_dma); if (ret) { dev_err(__scm->dev, "Assign memory protection call failed %d.\n", ret);
We need to use the proper types and convert between physical addresses and dma addresses here to avoid mismatch warnings. This is especially important on systems with a different size for dma addresses and physical addresses. Otherwise, we get the following warning: drivers/firmware/qcom_scm.c: In function "qcom_scm_assign_mem": drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of "dma_alloc_coherent" from incompatible pointer type [-Werror=incompatible-pointer-types] We also fix the size argument to dma_free_coherent() because that size doesn't need to be aligned after it's already aligned on the allocation size. In fact, dma debugging expects the same arguments to be passed to both the allocation and freeing sides of the functions so changing the size is incorrect regardless. Reported-by: Ian Jackson <ian.jackson@citrix.com> Cc: Ian Jackson <ian.jackson@citrix.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/firmware/qcom_scm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)