Hi JFD,

Please send v2 with correction suggested by Hyun

Thanks,
Manju
From: [email protected] 
[mailto:[email protected]] On Behalf Of Jean-Francois 
Dagenais
Sent: Friday, February 22, 2019 5:04 AM
To: Hyun Kwon <[email protected]>
Cc: [email protected]; Hyun Kwon <[email protected]>
Subject: Re: [meta-xilinx] [PATCH] kernel-module-mali: check and honour 
dma_unmap_page exit code

Hi Hyun,


On Feb 21, 2019, at 17:25, Hyun Kwon 
<[email protected]<mailto:[email protected]>> wrote:

Hi Jean-Francois,

Thanks for the patch.

On Tue, 2019-02-19 at 09:16:46 -0800, Jean-Francois Dagenais wrote:

Sorry, the subject should be:

kernel-module-mali: check and honour dma_map_page exit code

instead of "dma_UNmap_page"

Why using upper case, 'UN'?

It was just emphasis on what the mistake on the commit headline was... trying 
to remove the subtlety.






On Feb 19, 2019, at 10:39, Jean-Francois Dagenais 
<[email protected]<mailto:[email protected]>> wrote:

This fixes an error when using the module in a kernel configured with
the CONFIG_DMA_API_DEBUG flag.

utgard fd4b0000.gpu: DMA-API: device driver failed to check map error[device 
address=0x00000000325b0000] [size=4096 bytes] [mapped as page]
...
[<ffffff80082f72bc>] check_unmap+0x44c/0x7e8
[<ffffff80082f76b8>] debug_dma_unmap_page+0x60/0x68
[<ffffff8000622e60>] mali_mem_os_alloc_pages+0x230/0x498 [mali]
...

Signed-off-by: Jean-Francois Dagenais 
<[email protected]<mailto:[email protected]>>

I believe the Yocto patch requires the upstreaming status. Please refer to
other patches. You can mark it as pending. Our team can share this to ARM
internally.

noted.




---
.../recipes-graphics/mali/https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fkernel-module-mali.bb&c=E,1,7H1c4ml0nbgV5Y-a8ex5Nz-45BClZi0HAb8rSq_LVyywEvSS_hmK5Elx6jiQ69wE9_E1QeXVcbkuLM3wVzF94BmsSb6qcaxlV72Yhev6VOpD-Va8ik99UVY,&typo=1
          |  1 +
.../0007-fix-driver-failed-to-check-map-error.patch      | 16 ++++++++++++++++
2 files changed, 17 insertions(+)
create mode 100644 
meta-xilinx-bsp/recipes-graphics/mali/kernel-module-mali/0007-fix-driver-failed-to-check-map-error.patch

diff --git 
a/meta-xilinx-bsp/recipes-graphics/mali/https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fkernel-module-mali.bb&c=E,1,_43jl_v77hUeZ715j6nn3vef5fYyi5F8RndfWf7jFiMkSoYFNOFmaBY-eCWB04sK6Rs2xuAQXQQLPfB3DR-4vTWdAeyhs1FG88HNCPpxRT4IBMFxfpo,&typo=1
 
b/meta-xilinx-bsp/recipes-graphics/mali/https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fkernel-module-mali.bb&c=E,1,JaZ3yJLlmgR_Qe9c3t82B8aU2fH7lMmTxdGS2rEmZdyNWYlZXRBMelmmmJNcH07ltvr8DLau5iRr3aHww_esitrJ74H-z6N3l499VbXohkosRapNSkht&typo=1
index 0f44d25..5833239 100644
--- 
a/meta-xilinx-bsp/recipes-graphics/mali/https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fkernel-module-mali.bb&c=E,1,JqkVEZw5PLC_acDxcaxe3EmRTDzQtpfOF0S4ihWN30-lP3msD4vK_acCQ9sKuS5qgR16izABptBPu3V1ZiG5Asv_MlTX6zV8I0Sis_DkCyxAetfsx3tIR_xQvQ,,&typo=1
+++ 
b/meta-xilinx-bsp/recipes-graphics/mali/https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fkernel-module-mali.bb&c=E,1,BrH3PiHKnMb_dJfDtP8QdTfn_a_1u4u6-Op5vJEWvlZOpwm8o6Girdnwajqk5AnN4y9nDiNOQlDSJVCmnVqNSjABPOdFOeXlvF-4gujGyfw,&typo=1
@@ -16,6 +16,7 @@ SRC_URI = " \
             
file://0004-staging-mali-r8p0-01rel0-Don-t-include-mali_read_phy.patch<file:///\\0004-staging-mali-r8p0-01rel0-Don-t-include-mali_read_phy.patch>
 \
             
file://0005-linux-mali_kernel_linux.c-Handle-clock-when-probed-a.patch<file:///\\0005-linux-mali_kernel_linux.c-Handle-clock-when-probed-a.patch>
 \
             
file://0006-arm.c-global-variable-dma_ops-is-removed-from-the-ke.patch<file:///\\0006-arm.c-global-variable-dma_ops-is-removed-from-the-ke.patch>
 \
+           
file://0007-fix-driver-failed-to-check-map-error.patch<file:///\\0007-fix-driver-failed-to-check-map-error.patch>
 \

This list is different from what I see in the head. There's no empty index
there. Then this can be 0012. Maybe we are looking at differnt branches. Please
confirm.

You mean meta-xilinx/master? I will check and apply there with proper sequence 
number.




             
file://0010-common-mali_pm.c-Add-PM-runtime-barrier-after-removi.patch<file:///\\0010-common-mali_pm.c-Add-PM-runtime-barrier-after-removi.patch>
 \
             
file://0011-linux-mali_kernel_linux.c-Enable-disable-clock-for-r.patch\<file:///\\0011-linux-mali_kernel_linux.c-Enable-disable-clock-for-r.patch\>
             "
diff --git 
a/meta-xilinx-bsp/recipes-graphics/mali/kernel-module-mali/0007-fix-driver-failed-to-check-map-error.patch
 
b/meta-xilinx-bsp/recipes-graphics/mali/kernel-module-mali/0007-fix-driver-failed-to-check-map-error.patch

It would be better to make this as a git commit than just diff. I'm not sure if
there's any guideline for that though.

Indeed, however what the kernel-module-mali recipe downloads is not a git repo 
but a tarball (.tgz). If the tar extracted directory structure is the same as 
their git repo's, I can create it as a commit patch and cross my fingers.




new file mode 100644
index 0000000..f553e58
--- /dev/null
+++ 
b/meta-xilinx-bsp/recipes-graphics/mali/kernel-module-mali/0007-fix-driver-failed-to-check-map-error.patch
@@ -0,0 +1,16 @@
+Index: mali/linux/mali_memory_os_alloc.c
+===================================================================
+--- mali.orig/linux/mali_memory_os_alloc.c
++++ mali/linux/mali_memory_os_alloc.c
+@@ -239,8 +239,9 @@ int mali_mem_os_alloc_pages(mali_mem_os_
+                          /* Ensure page is flushed from CPU caches. */
+                          dma_addr = dma_map_page(&mali_platform_device->dev, 
new_page,
+                                                                       0, 
_MALI_OSK_MALI_PAGE_SIZE, DMA_BIDIRECTIONAL);
+-                       dma_unmap_page(&mali_platform_device->dev, dma_addr,
+-                                             _MALI_OSK_MALI_PAGE_SIZE, 
DMA_BIDIRECTIONAL);
++                        if (!dma_mapping_error(&mali_platform_device->dev, 
dma_addr))

I'd use the style consistent with existing code, but I let you decide.

             err = dma_mapping_error(&mali_platform_device->dev, dma_addr);
             if (unlikely(err)) {
sure.


The change itself looks fine. Please make sure on some format related comments
and let's wait for Yocto folks to confirm on a few.

Not sure I understand what you mean here. You mean wait for yocto folks 
monitoring this meta-xilinx list?

I will wait for your feedback before sending a V2 with the discussed 
corrections.

-- 
_______________________________________________
meta-xilinx mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/meta-xilinx

Reply via email to