On Tue, Apr 22, 2025 at 07:33:03PM +0530, Hardik Gohil wrote:
I'm sorry, I have no idea what to do here :(
please add all the patches 1/3,2/3 and 3/3 to v5.4.y.
Please do not post in html format :(
Anyway, I do not see patches 2/3 or 3/3 at all.
Please resend them all as a full patch series, don't just give links.
thanks,
greg k-h
From: Chuhong Yuan hslester96@gmail.com
The driver forgets to call pm_runtime_disable and pm_runtime_put_sync in probe failure and remove. Add the calls and modify probe failure handling to fix it.
To simplify the fix, the patch adjusts the calling order and merges checks for devm_kcalloc.
Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/") Signed-off-by: Chuhong Yuan hslester96@gmail.com Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191124052855.6472-1-hslester96@gmail.com Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/dma/ti/edma.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c index 756a3c951dc7..0628ee4bf1b4 100644 --- a/drivers/dma/ti/edma.c +++ b/drivers/dma/ti/edma.c @@ -2289,13 +2289,6 @@ static int edma_probe(struct platform_device *pdev) if (!info) return -ENODEV;
- pm_runtime_enable(dev); - ret = pm_runtime_get_sync(dev); - if (ret < 0) { - dev_err(dev, "pm_runtime_get_sync() failed\n"); - return ret; - } - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (ret) return ret; @@ -2326,27 +2319,31 @@ static int edma_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ecc);
+ pm_runtime_enable(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "pm_runtime_get_sync() failed\n"); + pm_runtime_disable(dev); + return ret; + } + /* Get eDMA3 configuration from IP */ ret = edma_setup_from_hw(dev, info, ecc); if (ret) - return ret; + goto err_disable_pm;
/* Allocate memory based on the information we got from the IP */ ecc->slave_chans = devm_kcalloc(dev, ecc->num_channels, sizeof(*ecc->slave_chans), GFP_KERNEL); - if (!ecc->slave_chans) - return -ENOMEM;
ecc->slot_inuse = devm_kcalloc(dev, BITS_TO_LONGS(ecc->num_slots), sizeof(unsigned long), GFP_KERNEL); - if (!ecc->slot_inuse) - return -ENOMEM;
ecc->channels_mask = devm_kcalloc(dev, BITS_TO_LONGS(ecc->num_channels), sizeof(unsigned long), GFP_KERNEL); - if (!ecc->channels_mask) - return -ENOMEM; + if (!ecc->slave_chans || !ecc->slot_inuse || !ecc->channels_mask) + goto err_disable_pm;
/* Mark all channels available initially */ bitmap_fill(ecc->channels_mask, ecc->num_channels); @@ -2388,7 +2385,7 @@ static int edma_probe(struct platform_device *pdev) ecc); if (ret) { dev_err(dev, "CCINT (%d) failed --> %d\n", irq, ret); - return ret; + goto err_disable_pm; } ecc->ccint = irq; } @@ -2404,7 +2401,7 @@ static int edma_probe(struct platform_device *pdev) ecc); if (ret) { dev_err(dev, "CCERRINT (%d) failed --> %d\n", irq, ret); - return ret; + goto err_disable_pm; } ecc->ccerrint = irq; } @@ -2412,7 +2409,8 @@ static int edma_probe(struct platform_device *pdev) ecc->dummy_slot = edma_alloc_slot(ecc, EDMA_SLOT_ANY); if (ecc->dummy_slot < 0) { dev_err(dev, "Can't allocate PaRAM dummy slot\n"); - return ecc->dummy_slot; + ret = ecc->dummy_slot; + goto err_disable_pm; }
queue_priority_mapping = info->queue_priority_mapping; @@ -2512,6 +2510,9 @@ static int edma_probe(struct platform_device *pdev)
err_reg1: edma_free_slot(ecc, ecc->dummy_slot); +err_disable_pm: + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); return ret; }
@@ -2542,6 +2543,8 @@ static int edma_remove(struct platform_device *pdev) if (ecc->dma_memcpy) dma_async_device_unregister(ecc->dma_memcpy); edma_free_slot(ecc, ecc->dummy_slot); + pm_runtime_put_sync(dev); + pm_runtime_disable(dev);
return 0; }
From: Kunwu Chan chentao@kylinos.cn
[ Upstream commit 6e2276203ac9ff10fc76917ec9813c660f627369 ]
devm_kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Ensure the allocation was successful by checking the pointer validity.
Signed-off-by: Kunwu Chan chentao@kylinos.cn Link: https://lore.kernel.org/r/20240118031929.192192-1-chentao@kylinos.cn Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/dma/ti/edma.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c index a1adc8d91fd8..69292d4a0c44 100644 --- a/drivers/dma/ti/edma.c +++ b/drivers/dma/ti/edma.c @@ -2462,6 +2462,11 @@ static int edma_probe(struct platform_device *pdev) if (irq > 0) { irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccint", dev_name(dev)); + if (!irq_name) { + ret = -ENOMEM; + goto err_disable_pm; + } + ret = devm_request_irq(dev, irq, dma_irq_handler, 0, irq_name, ecc); if (ret) { @@ -2478,6 +2483,11 @@ static int edma_probe(struct platform_device *pdev) if (irq > 0) { irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccerrint", dev_name(dev)); + if (!irq_name) { + ret = -ENOMEM; + goto err_disable_pm; + } + ret = devm_request_irq(dev, irq, dma_ccerr_handler, 0, irq_name, ecc); if (ret) {
On Tue, Apr 22, 2025 at 03:17:09PM +0000, Hardik Gohil wrote:
From: Kunwu Chan chentao@kylinos.cn
[ Upstream commit 6e2276203ac9ff10fc76917ec9813c660f627369 ]
devm_kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Ensure the allocation was successful by checking the pointer validity.
Signed-off-by: Kunwu Chan chentao@kylinos.cn Link: https://lore.kernel.org/r/20240118031929.192192-1-chentao@kylinos.cn Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org
Sasha did not sign off on the original commit here.
And you didn't either. Please read the documentation for what this means, and what you are doing when you add it.
thanks,
greg k-h
Sasha did not sign off on the original commit here.
And you didn't either. Please read the documentation for what this means, and what you are doing when you add it.
thanks,
greg k-h
There have been no changes from my side for those patches. Do we still need to add a signed-off sign?
On Wed, Apr 23, 2025 at 03:30:39PM +0530, Hardik Gohil wrote:
Sasha did not sign off on the original commit here.
And you didn't either. Please read the documentation for what this means, and what you are doing when you add it.
thanks,
greg k-h
There have been no changes from my side for those patches. Do we still need to add a signed-off sign?
Please take some time and work with the kernel developers on your team and your group's corporate lawyer, to understand what a signed-off-by means and why it is required here.
thanks,
greg k-h
From: Chuhong Yuan hslester96@gmail.com
The driver forgets to call pm_runtime_disable and pm_runtime_put_sync in probe failure and remove. Add the calls and modify probe failure handling to fix it.
To simplify the fix, the patch adjusts the calling order and merges checks for devm_kcalloc.
Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/") Signed-off-by: Chuhong Yuan hslester96@gmail.com Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191124052855.6472-1-hslester96@gmail.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com --- drivers/dma/ti/edma.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c index 47423bbd7bc7..4fea8688b596 100644 --- a/drivers/dma/ti/edma.c +++ b/drivers/dma/ti/edma.c @@ -2290,13 +2290,6 @@ static int edma_probe(struct platform_device *pdev) if (!info) return -ENODEV;
- pm_runtime_enable(dev); - ret = pm_runtime_get_sync(dev); - if (ret < 0) { - dev_err(dev, "pm_runtime_get_sync() failed\n"); - return ret; - } - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (ret) return ret; @@ -2327,27 +2320,31 @@ static int edma_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ecc);
+ pm_runtime_enable(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "pm_runtime_get_sync() failed\n"); + pm_runtime_disable(dev); + return ret; + } + /* Get eDMA3 configuration from IP */ ret = edma_setup_from_hw(dev, info, ecc); if (ret) - return ret; + goto err_disable_pm;
/* Allocate memory based on the information we got from the IP */ ecc->slave_chans = devm_kcalloc(dev, ecc->num_channels, sizeof(*ecc->slave_chans), GFP_KERNEL); - if (!ecc->slave_chans) - return -ENOMEM;
ecc->slot_inuse = devm_kcalloc(dev, BITS_TO_LONGS(ecc->num_slots), sizeof(unsigned long), GFP_KERNEL); - if (!ecc->slot_inuse) - return -ENOMEM;
ecc->channels_mask = devm_kcalloc(dev, BITS_TO_LONGS(ecc->num_channels), sizeof(unsigned long), GFP_KERNEL); - if (!ecc->channels_mask) - return -ENOMEM; + if (!ecc->slave_chans || !ecc->slot_inuse || !ecc->channels_mask) + goto err_disable_pm;
/* Mark all channels available initially */ bitmap_fill(ecc->channels_mask, ecc->num_channels); @@ -2397,7 +2394,7 @@ static int edma_probe(struct platform_device *pdev) ecc); if (ret) { dev_err(dev, "CCINT (%d) failed --> %d\n", irq, ret); - return ret; + goto err_disable_pm; } ecc->ccint = irq; } @@ -2413,7 +2410,7 @@ static int edma_probe(struct platform_device *pdev) ecc); if (ret) { dev_err(dev, "CCERRINT (%d) failed --> %d\n", irq, ret); - return ret; + goto err_disable_pm; } ecc->ccerrint = irq; } @@ -2421,7 +2418,8 @@ static int edma_probe(struct platform_device *pdev) ecc->dummy_slot = edma_alloc_slot(ecc, EDMA_SLOT_ANY); if (ecc->dummy_slot < 0) { dev_err(dev, "Can't allocate PaRAM dummy slot\n"); - return ecc->dummy_slot; + ret = ecc->dummy_slot; + goto err_disable_pm; }
queue_priority_mapping = info->queue_priority_mapping; @@ -2521,6 +2519,9 @@ static int edma_probe(struct platform_device *pdev)
err_reg1: edma_free_slot(ecc, ecc->dummy_slot); +err_disable_pm: + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); return ret; }
@@ -2551,6 +2552,8 @@ static int edma_remove(struct platform_device *pdev) if (ecc->dma_memcpy) dma_async_device_unregister(ecc->dma_memcpy); edma_free_slot(ecc, ecc->dummy_slot); + pm_runtime_put_sync(dev); + pm_runtime_disable(dev);
return 0; }
From: Kunwu Chan chentao@kylinos.cn
[ Upstream commit 6e2276203ac9ff10fc76917ec9813c660f627369 ]
devm_kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Ensure the allocation was successful by checking the pointer validity.
Signed-off-by: Kunwu Chan chentao@kylinos.cn Link: https://lore.kernel.org/r/20240118031929.192192-1-chentao@kylinos.cn Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com --- drivers/dma/ti/edma.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c index 4fea8688b596..625f04cbbcd4 100644 --- a/drivers/dma/ti/edma.c +++ b/drivers/dma/ti/edma.c @@ -2390,6 +2390,11 @@ static int edma_probe(struct platform_device *pdev) if (irq > 0) { irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccint", dev_name(dev)); + if (!irq_name) { + ret = -ENOMEM; + goto err_disable_pm; + } + ret = devm_request_irq(dev, irq, dma_irq_handler, 0, irq_name, ecc); if (ret) { @@ -2406,6 +2411,11 @@ static int edma_probe(struct platform_device *pdev) if (irq > 0) { irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccerrint", dev_name(dev)); + if (!irq_name) { + ret = -ENOMEM; + goto err_disable_pm; + } + ret = devm_request_irq(dev, irq, dma_ccerr_handler, 0, irq_name, ecc); if (ret) {
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ❌ Build failures detected
The upstream commit SHA1 provided is correct: 6e2276203ac9ff10fc76917ec9813c660f627369
WARNING: Author mismatch between patch and upstream commit: Backport author: Hardik Gohilhgohil@mvista.com Commit author: Kunwu Chanchentao@kylinos.cn
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 7b24760f3a3c) 6.1.y | Present (different SHA1: 9d508c897153) 5.15.y | Present (different SHA1: 4fe4e5adc7d2) 5.10.y | Present (different SHA1: c432094aa7c9)
Note: The patch differs from the upstream commit: --- 1: 6e2276203ac9f ! 1: 518abacf56db8 dmaengine: ti: edma: Add some null pointer checks to the edma_probe @@ Metadata ## Commit message ## dmaengine: ti: edma: Add some null pointer checks to the edma_probe
+ [ Upstream commit 6e2276203ac9ff10fc76917ec9813c660f627369 ] + devm_kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Ensure the allocation was successful by checking the pointer validity. @@ Commit message Signed-off-by: Kunwu Chan chentao@kylinos.cn Link: https://lore.kernel.org/r/20240118031929.192192-1-chentao@kylinos.cn Signed-off-by: Vinod Koul vkoul@kernel.org + Signed-off-by: Sasha Levin sashal@kernel.org + Signed-off-by: Hardik Gohil hgohil@mvista.com
## drivers/dma/ti/edma.c ## @@ drivers/dma/ti/edma.c: static int edma_probe(struct platform_device *pdev) ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.4.y | Success | Failed |
Build Errors: Build error for stable/linux-5.4.y: arch/x86/entry/entry_64.o: warning: objtool: .entry.text+0x1e1: stack state mismatch: cfa1=7+56 cfa2=7+40 arch/x86/kvm/vmx/vmenter.o: warning: objtool: __vmx_vcpu_run()+0x12a: return with modified stack frame In file included from ./include/linux/list.h:9, from ./include/linux/kobject.h:19, from ./include/linux/of.h:17, from ./include/linux/clk-provider.h:9, from drivers/clk/qcom/clk-rpmh.c:6: drivers/clk/qcom/clk-rpmh.c: In function 'clk_rpmh_bcm_send_cmd': ./include/linux/kernel.h:843:43: warning: comparison of distinct pointer types lacks a cast [-Wcompare-distinct-pointer-types] 843 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/kernel.h:857:18: note: in expansion of macro '__typecheck' 857 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/kernel.h:867:31: note: in expansion of macro '__safe_cmp' 867 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/kernel.h:876:25: note: in expansion of macro '__careful_cmp' 876 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ drivers/clk/qcom/clk-rpmh.c:273:21: note: in expansion of macro 'min' 273 | cmd_state = min(cmd_state, BCM_TCS_CMD_VOTE_MASK); | ^~~ drivers/dma/ti/edma.c: In function 'edma_probe': drivers/dma/ti/edma.c:2389:25: error: label 'err_disable_pm' used but not defined 2389 | goto err_disable_pm; | ^~~~ make[3]: *** [scripts/Makefile.build:262: drivers/dma/ti/edma.o] Error 1 make[3]: Target '__build' not remade because of errors. make[2]: *** [scripts/Makefile.build:497: drivers/dma/ti] Error 2 make[2]: Target '__build' not remade because of errors. make[1]: *** [scripts/Makefile.build:497: drivers/dma] Error 2 fs/xfs/libxfs/xfs_inode_fork.c: In function 'xfs_ifork_verify_attr': fs/xfs/libxfs/xfs_inode_fork.c:735:13: warning: the comparison will always evaluate as 'true' for the address of 'i_df' will never be NULL [-Waddress] 735 | if (!XFS_IFORK_PTR(ip, XFS_ATTR_FORK)) | ^ In file included from fs/xfs/libxfs/xfs_inode_fork.c:14: ./fs/xfs/xfs_inode.h:38:33: note: 'i_df' declared here 38 | struct xfs_ifork i_df; /* data fork */ | ^~~~ drivers/gpu/drm/i915/display/intel_dp.c: In function 'intel_dp_mode_valid': drivers/gpu/drm/i915/display/intel_dp.c:639:33: warning: 'drm_dp_dsc_sink_max_slice_count' reading 16 bytes from a region of size 0 [-Wstringop-overread] 639 | drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 640 | true); | ~~~~~ drivers/gpu/drm/i915/display/intel_dp.c:639:33: note: referencing argument 1 of type 'const u8[16]' {aka 'const unsigned char[16]'} In file included from drivers/gpu/drm/i915/display/intel_dp.c:39: ./include/drm/drm_dp_helper.h:1174:4: note: in a call to function 'drm_dp_dsc_sink_max_slice_count' 1174 | u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE], | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/net/dsa/microchip/ksz9477.c: In function 'ksz9477_reset_switch': drivers/net/dsa/microchip/ksz9477.c:198:12: warning: unused variable 'data8' [-Wunused-variable] 198 | u8 data8; | ^~~~~ In file included from ./include/linux/bitops.h:5, from ./include/linux/kernel.h:12, from ./include/linux/list.h:9, from ./include/linux/module.h:9, from drivers/net/ethernet/qlogic/qed/qed_debug.c:6: drivers/net/ethernet/qlogic/qed/qed_debug.c: In function 'qed_grc_dump_addr_range': ./include/linux/bits.h:8:33: warning: overflow in conversion from 'long unsigned int' to 'u8' {aka 'unsigned char'} changes value from '(long unsigned int)((int)vf_id << 8 | 128)' to '128' [-Woverflow] 8 | #define BIT(nr) (UL(1) << (nr)) | ^ drivers/net/ethernet/qlogic/qed/qed_debug.c:2572:31: note: in expansion of macro 'BIT' 2572 | fid = BIT(PXP_PRETEND_CONCRETE_FID_VFVALID_SHIFT) | | ^~~ drivers/gpu/drm/nouveau/dispnv50/wndw.c:628:1: warning: conflicting types for 'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, u32, enum nv50_disp_interlock_type, u32, struct nv50_wndw **)' {aka 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const unsigned int *, unsigned int, enum nv50_disp_interlock_type, unsigned int, struct nv50_wndw **)'} [-Wenum-int-mismatch] 628 | nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, | ^~~~~~~~~~~~~~ In file included from drivers/gpu/drm/nouveau/dispnv50/wndw.c:22: drivers/gpu/drm/nouveau/dispnv50/wndw.h:39:5: note: previous declaration of 'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, enum nv50_disp_interlock_type, u32, u32, struct nv50_wndw **)' {aka 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const unsigned int *, enum nv50_disp_interlock_type, unsigned int, unsigned int, struct nv50_wndw **)'} 39 | int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *, | ^~~~~~~~~~~~~~ make[1]: Target '__build' not remade because of errors. make: *** [Makefile:1755: drivers] Error 2 make: Target '_all' not remade because of errors.
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Found matching upstream commit but patch is missing proper reference to it ⚠️ Found follow-up fixes in mainline
Found matching upstream commit: 2a03c1314506557277829562dd2ec5c11a6ea914
WARNING: Author mismatch between patch and found commit: Backport author: Hardik Gohilhgohil@mvista.com Commit author: Chuhong Yuanhslester96@gmail.com
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Present (exact SHA1)
Found fixes commits: d1fd03a35efc dmaengine: ti: edma: Fix error return code in edma_probe()
Note: The patch differs from the upstream commit: --- 1: 2a03c13145065 < -: ------------- dmaengine: ti: edma: add missed operations -: ------------- > 1: 1b01d9c341770 Linux 5.4.292 ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.4.y | Success | Success |
On Thu, Apr 24, 2025 at 06:06:33AM +0000, Hardik Gohil wrote:
From: Chuhong Yuan hslester96@gmail.com
The driver forgets to call pm_runtime_disable and pm_runtime_put_sync in probe failure and remove. Add the calls and modify probe failure handling to fix it.
To simplify the fix, the patch adjusts the calling order and merges checks for devm_kcalloc.
Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/") Signed-off-by: Chuhong Yuan hslester96@gmail.com Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191124052855.6472-1-hslester96@gmail.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com
drivers/dma/ti/edma.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)
No upstream git id?
From: Peter Ujfalusi peter.ujfalusi@ti.com
Like paRAM slots, channels could be used by other cores and in this case we need to make sure that the driver do not alter these channels.
Handle the generic dma-channel-mask property to mark channels in a bitmap which can not be used by Linux and convert the legacy rsv_chans if it is provided by platform_data.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191025073056.25450-4-peter.ujfalusi@ti.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com --- The patch [dmaengine: ti: edma: Add some null pointer checks to the edma_probe] fix for CVE-2024-26771 needs to be backported to v5.4.y kernel.
drivers/dma/ti/edma.c | 59 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c index 01089e5c565f..47423bbd7bc7 100644 --- a/drivers/dma/ti/edma.c +++ b/drivers/dma/ti/edma.c @@ -259,6 +259,13 @@ struct edma_cc { */ unsigned long *slot_inuse;
+ /* + * For tracking reserved channels used by DSP. + * If the bit is cleared, the channel is allocated to be used by DSP + * and Linux must not touch it. + */ + unsigned long *channels_mask; + struct dma_device dma_slave; struct dma_device *dma_memcpy; struct edma_chan *slave_chans; @@ -715,6 +722,12 @@ static int edma_alloc_channel(struct edma_chan *echan, struct edma_cc *ecc = echan->ecc; int channel = EDMA_CHAN_SLOT(echan->ch_num);
+ if (!test_bit(echan->ch_num, ecc->channels_mask)) { + dev_err(ecc->dev, "Channel%d is reserved, can not be used!\n", + echan->ch_num); + return -EINVAL; + } + /* ensure access through shadow region 0 */ edma_or_array2(ecc, EDMA_DRAE, 0, EDMA_REG_ARRAY_INDEX(channel), EDMA_CHANNEL_BIT(channel)); @@ -2249,7 +2262,7 @@ static int edma_probe(struct platform_device *pdev) struct edma_soc_info *info = pdev->dev.platform_data; s8 (*queue_priority_mapping)[2]; int i, off; - const s16 (*rsv_slots)[2]; + const s16 (*reserved)[2]; const s16 (*xbar_chans)[2]; int irq; char *irq_name; @@ -2330,15 +2343,32 @@ static int edma_probe(struct platform_device *pdev) if (!ecc->slot_inuse) return -ENOMEM;
+ ecc->channels_mask = devm_kcalloc(dev, + BITS_TO_LONGS(ecc->num_channels), + sizeof(unsigned long), GFP_KERNEL); + if (!ecc->channels_mask) + return -ENOMEM; + + /* Mark all channels available initially */ + bitmap_fill(ecc->channels_mask, ecc->num_channels); + ecc->default_queue = info->default_queue;
if (info->rsv) { /* Set the reserved slots in inuse list */ - rsv_slots = info->rsv->rsv_slots; - if (rsv_slots) { - for (i = 0; rsv_slots[i][0] != -1; i++) - bitmap_set(ecc->slot_inuse, rsv_slots[i][0], - rsv_slots[i][1]); + reserved = info->rsv->rsv_slots; + if (reserved) { + for (i = 0; reserved[i][0] != -1; i++) + bitmap_set(ecc->slot_inuse, reserved[i][0], + reserved[i][1]); + } + + /* Clear channels not usable for Linux */ + reserved = info->rsv->rsv_chans; + if (reserved) { + for (i = 0; reserved[i][0] != -1; i++) + bitmap_clear(ecc->channels_mask, reserved[i][0], + reserved[i][1]); } }
@@ -2398,6 +2428,7 @@ static int edma_probe(struct platform_device *pdev)
if (!ecc->legacy_mode) { int lowest_priority = 0; + unsigned int array_max; struct of_phandle_args tc_args;
ecc->tc_list = devm_kcalloc(dev, ecc->num_tc, @@ -2421,6 +2452,18 @@ static int edma_probe(struct platform_device *pdev) } of_node_put(tc_args.np); } + + /* See if we have optional dma-channel-mask array */ + array_max = DIV_ROUND_UP(ecc->num_channels, BITS_PER_TYPE(u32)); + ret = of_property_read_variable_u32_array(node, + "dma-channel-mask", + (u32 *)ecc->channels_mask, + 1, array_max); + if (ret > 0 && ret != array_max) + dev_warn(dev, "dma-channel-mask is not complete.\n"); + else if (ret == -EOVERFLOW || ret == -ENODATA) + dev_warn(dev, + "dma-channel-mask is out of range or empty\n"); }
/* Event queue priority mapping */ @@ -2438,6 +2481,10 @@ static int edma_probe(struct platform_device *pdev) edma_dma_init(ecc, legacy_mode);
for (i = 0; i < ecc->num_channels; i++) { + /* Do not touch reserved channels */ + if (!test_bit(i, ecc->channels_mask)) + continue; + /* Assign all channels to the default queue */ edma_assign_channel_eventq(&ecc->slave_chans[i], info->default_queue);
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: 31f4b28f6c41f734957ea66ac84c6abb69e696f0
WARNING: Author mismatch between patch and found commit: Backport author: Hardik Gohilhgohil@mvista.com Commit author: Peter Ujfalusipeter.ujfalusi@ti.com
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Present (exact SHA1)
Note: The patch differs from the upstream commit: --- 1: 31f4b28f6c41f ! 1: f2bbef69f3dbe dmaengine: ti: edma: Add support for handling reserved channels @@ Commit message Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191025073056.25450-4-peter.ujfalusi@ti.com Signed-off-by: Vinod Koul vkoul@kernel.org + Signed-off-by: Hardik Gohil hgohil@mvista.com
## drivers/dma/ti/edma.c ## @@ drivers/dma/ti/edma.c: struct edma_cc { @@ drivers/dma/ti/edma.c: static int edma_alloc_channel(struct edma_chan *echan, edma_or_array2(ecc, EDMA_DRAE, 0, EDMA_REG_ARRAY_INDEX(channel), EDMA_CHANNEL_BIT(channel)); @@ drivers/dma/ti/edma.c: static int edma_probe(struct platform_device *pdev) - { struct edma_soc_info *info = pdev->dev.platform_data; s8 (*queue_priority_mapping)[2]; + int i, off; - const s16 (*rsv_slots)[2]; -+ const s16 (*reserved)[2]; - int i, irq; ++ const s16 (*reserved)[2]; + const s16 (*xbar_chans)[2]; + int irq; char *irq_name; - struct resource *mem; @@ drivers/dma/ti/edma.c: static int edma_probe(struct platform_device *pdev) if (!ecc->slot_inuse) return -ENOMEM; @@ drivers/dma/ti/edma.c: static int edma_probe(struct platform_device *pdev)
ecc->tc_list = devm_kcalloc(dev, ecc->num_tc, @@ drivers/dma/ti/edma.c: static int edma_probe(struct platform_device *pdev) - info->default_queue = i; } + of_node_put(tc_args.np); } + + /* See if we have optional dma-channel-mask array */ ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.4.y | Success | Success |
On Thu, Apr 24, 2025 at 06:08:54AM +0000, Hardik Gohil wrote:
From: Peter Ujfalusi peter.ujfalusi@ti.com
Like paRAM slots, channels could be used by other cores and in this case we need to make sure that the driver do not alter these channels.
Handle the generic dma-channel-mask property to mark channels in a bitmap which can not be used by Linux and convert the legacy rsv_chans if it is provided by platform_data.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191025073056.25450-4-peter.ujfalusi@ti.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com
The patch [dmaengine: ti: edma: Add some null pointer checks to the edma_probe] fix for CVE-2024-26771 needs to be backported to v5.4.y kernel.
No upstream git commit id?
Please fix and resend the whole series as a new versioned set of patches.
thanks,
greg k-h
On Fri, Apr 25, 2025 at 10:10:58AM +0200, Greg KH wrote:
On Thu, Apr 24, 2025 at 06:08:54AM +0000, Hardik Gohil wrote:
From: Peter Ujfalusi peter.ujfalusi@ti.com
Like paRAM slots, channels could be used by other cores and in this case we need to make sure that the driver do not alter these channels.
Handle the generic dma-channel-mask property to mark channels in a bitmap which can not be used by Linux and convert the legacy rsv_chans if it is provided by platform_data.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191025073056.25450-4-peter.ujfalusi@ti.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Hardik Gohil hgohil@mvista.com
The patch [dmaengine: ti: edma: Add some null pointer checks to the edma_probe] fix for CVE-2024-26771 needs to be backported to v5.4.y kernel.
No upstream git commit id?
Please fix and resend the whole series as a new versioned set of patches.
Actually, given all of the recent problems here, I recommend taking some time off, and work with some more experienced kernel developers in your company first, to get the experience needed in order to properly submit kernel changes. I'd like to see at least one other mvista.com developer, who has such experience, sign off on your backports as well to get someone else to catch basic problems like this before the community is forced to do so.
thanks,
greg k-h
On Tue, Apr 22, 2025 at 03:17:08PM +0000, Hardik Gohil wrote:
From: Chuhong Yuan hslester96@gmail.com
The driver forgets to call pm_runtime_disable and pm_runtime_put_sync in probe failure and remove. Add the calls and modify probe failure handling to fix it.
To simplify the fix, the patch adjusts the calling order and merges checks for devm_kcalloc.
Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/") Signed-off-by: Chuhong Yuan hslester96@gmail.com Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com Link: https://lore.kernel.org/r/20191124052855.6472-1-hslester96@gmail.com Signed-off-by: Vinod Koul vkoul@kernel.org
Why did you not sign off on this?
And where is patch 1/3?
Please take a pause, and redo this after testing that you have it correct locally, before sending it out again.
Also, properly version your patch series.
thanks,
greg k-h
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Found matching upstream commit but patch is missing proper reference to it ⚠️ Found follow-up fixes in mainline
Found matching upstream commit: 2a03c1314506557277829562dd2ec5c11a6ea914
WARNING: Author mismatch between patch and found commit: Backport author: Hardik Gohilhgohil@mvista.com Commit author: Chuhong Yuanhslester96@gmail.com
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Present (exact SHA1)
Found fixes commits: d1fd03a35efc dmaengine: ti: edma: Fix error return code in edma_probe()
Note: The patch differs from the upstream commit: --- 1: 2a03c13145065 < -: ------------- dmaengine: ti: edma: add missed operations -: ------------- > 1: 1b01d9c341770 Linux 5.4.292 ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.4.y | Success | Success |
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Hardik Gohil -
Sasha Levin