* [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority
@ 2026-09-02 9:26 Jia Wang
2026-09-02 9:26 ` [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Jia Wang @ 2026-09-02 9:26 UTC (permalink / raw)
To: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko,
Sia Jee Heng
Cc: dmaengine, linux-kernel, Jia Wang, Frank Li
This series fixes AXI burst length encoding, an out-of-bounds access in
the error-path LLI dump, and the CH_CFG2 channel priority field position.
It also converts register field definitions and programming to GENMASK()
and FIELD_PREP().
The series was tested with dmatest on all eight channels using
snps,axi-max-burst-len = <256>. All channels completed without errors. It
was also build-tested on RISC-V with CONFIG_DW_AXI_DMAC as both a module
and built-in.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
Changes in v3:
- Initialize cfg_lo separately in each channel configuration branch.
- Remove redundant parentheses from register assignments.
- Link to v2: https://patch.msgid.link/20260901-dma-fix-v2-0-d7f0459ebb14@ultrarisc.com
Changes in v2:
- Use a loop-local unsigned iterator in the LLI dump helper.
- Add a patch to fix the CH_CFG2 channel priority field position.
- Add a cleanup patch to use GENMASK() and FIELD_PREP() for register fields.
- Link to v1: https://patch.msgid.link/20260828-dma-fix-v1-0-a6947f487e07@ultrarisc.com
To: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
To: Vinod Koul <vkoul@kernel.org>
To: Frank Li <Frank.Li@kernel.org>
To: Pandith N <pandith.n@intel.com>
To: Sia Jee Heng <jee.heng.sia@intel.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Jia Wang (4):
dmaengine: dw-axi-dmac: Fix AXI burst length encoding
dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access
dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position
dmaengine: dw-axi-dmac: Use bitfield helpers for registers
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 84 +++++++++++++-------------
drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 44 +++++++-------
2 files changed, 64 insertions(+), 64 deletions(-)
---
base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
change-id: 20260827-dma-fix-c2b27795ce12
Best regards,
--
Jia Wang <wangjia@ultrarisc.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding 2026-09-02 9:26 [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang @ 2026-09-02 9:26 ` Jia Wang 2026-09-02 9:39 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access Jia Wang ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Jia Wang @ 2026-09-02 9:26 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko, Sia Jee Heng Cc: dmaengine, linux-kernel, Jia Wang, Frank Li The snps,axi-max-burst-len property describes the number of beats in an AXI burst, while the ARLEN and AWLEN fields encode that value minus one. The driver keeps axi_rw_burst_len as the actual burst length so that dma_device.max_burst reports the correct value. However, it also programs that unencoded value directly into the hardware fields. A value of 256 therefore overflows the 8-bit fields and can cause AXI decode errors. Subtract one only when constructing hardware descriptors, while keeping the actual value for dma_device.max_burst. Fixes: c454d16a7d5a ("dmaengine: dw-axi-dmac: Burst length settings") Signed-off-by: Jia Wang <wangjia@ultrarisc.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> --- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index eebed2474210..742e08cfab43 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -706,7 +706,7 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, ctlhi = CH_CTL_H_LLI_VALID; if (chan->chip->dw->hdata->restrict_axi_burst_len) { - burst_len = chan->chip->dw->hdata->axi_rw_burst_len; + burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; ctlhi |= CH_CTL_H_ARLEN_EN | CH_CTL_H_AWLEN_EN | burst_len << CH_CTL_H_ARLEN_POS | burst_len << CH_CTL_H_AWLEN_POS; @@ -975,7 +975,7 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr, reg = CH_CTL_H_LLI_VALID; if (chan->chip->dw->hdata->restrict_axi_burst_len) { - u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len; + u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; reg |= (CH_CTL_H_ARLEN_EN | burst_len << CH_CTL_H_ARLEN_POS | -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding 2026-09-02 9:26 ` [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang @ 2026-09-02 9:39 ` sashiko-bot 0 siblings, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-09-02 9:39 UTC (permalink / raw) To: Jia Wang; +Cc: dmaengine, Frank.Li, vkoul Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: Pre-existing issues: - [High] Zero-length transfers passed to `dma_chan_prep_dma_memcpy` result in negative array indexing and an underflowing infinite loop, leading to severe out-of-bounds access and arbitrary kernel memory corruption. - [High] Lack of zero-length validation in dw_axi_dma_chan_prep_slave_sg and dw_axi_dma_chan_prep_cyclic can cause divide-by-zero panics. - [High] The error path for hardware descriptor allocation blindly frees the entire array of requested descriptors without checking for NULL, causing a guaranteed NULL pointer dereference. - [High] Silent data truncation and subsequent NULL pointer dereference in scatterlist transfers if lengths are not exact multiples of the computed segment size. - [High] The driver fails to start the next queued descriptor when a non-cyclic transfer completes, leading to stalled DMA channels. - [Medium] dma_chan_issue_pending unconditionally attempts to start the channel even if it is already running, resulting in spurious 'non-idle' errors. -- commit 212e0a527db7f8f42167aa10e801996bc3ac1256 Author: Jia Wang <wangjia@ultrarisc.com> Subject: dmaengine: dw-axi-dmac: Fix AXI burst length encoding This commit fixes AXI decode errors caused by unencoded burst lengths overflowing hardware fields. It subtracts one from the burst length value when configuring the hardware descriptors while preserving the actual value for reporting. > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index eebed2474210d..742e08cfab435 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c [ ... ] > @@ -975,7 +975,7 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr, > > reg = CH_CTL_H_LLI_VALID; > if (chan->chip->dw->hdata->restrict_axi_burst_len) { > - u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len; > + u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; > > reg |= (CH_CTL_H_ARLEN_EN | > burst_len << CH_CTL_H_ARLEN_POS | [Severity: High] This isn't a bug introduced by this patch, but I noticed a potential problem in this function for zero-length transfers. If dma_chan_prep_dma_memcpy() is called with len == 0, num starts at 0 and the while (len) loop is skipped. Does this lead to an out-of-bounds access here: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_prep_dma_memcpy() { ... set_desc_last(&desc->hw_desc[num - 1]); ... } And subsequently, could this cause an underflowing loop that writes to arbitrary memory? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_prep_dma_memcpy() { ... do { hw_desc = &desc->hw_desc[--num]; write_desc_llp(hw_desc, llp | lms); llp = hw_desc->llp; } while (num); ... } [Severity: High] This is a pre-existing issue, but the error path in dma_chan_prep_dma_memcpy() seems like it might dereference a NULL pointer. If axi_desc_get() fails and the code jumps to err_desc_get, we call axi_desc_put(desc): drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_prep_dma_memcpy() { ... err_desc_get: if (desc) axi_desc_put(desc); ... } Since the uninitialized elements of desc->hw_desc will have a NULL lli pointer, does axi_desc_put() crash when it unconditionally calls dma_pool_free() on the entire array? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_desc_put() { ... dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp); ... } [Severity: High] This is a pre-existing issue, but I noticed a potential division by zero elsewhere in this file. If a caller passes a period_len of 0 to dw_axi_dma_chan_prep_cyclic(), could this cause a panic? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_cyclic() { ... num_periods = buf_len / period_len; ... } Similarly, in dw_axi_dma_chan_prep_slave_sg(), if any scatterlist element has a length of 0, num_segments evaluates to 0. Does this macro then cause a divide-by-zero panic? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_slave_sg() { ... segment_len = DIV_ROUND_UP(sg_dma_len(sg), num_segments); ... } [Severity: High] This is a pre-existing issue, but does dw_axi_dma_chan_prep_slave_sg() truncate data if the transfer length is not an exact multiple of the computed segment size? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_slave_sg() { ... segment_len = DIV_ROUND_UP(sg_dma_len(sg), num_segments); ... } while (len >= segment_len); ... } If the loop exits early, the remaining bytes seem to be discarded. Further down in the function, it loops over all allocated segments: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_slave_sg() { ... do { hw_desc = &desc->hw_desc[--num_sgs]; write_desc_llp(hw_desc, llp | lms); ... } Since the earlier loop exited prematurely, the trailing elements in hw_desc have a NULL lli pointer. Does calling write_desc_llp() on them cause a panic? [Severity: High] This is a pre-existing issue, but I have a question about how hardware queueing is managed. In axi_chan_block_xfer_complete(), after completing a transfer: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_chan_block_xfer_complete() { ... list_del(&vd->node); vchan_cookie_complete(vd); } out: ... } Should this function call axi_chan_start_first_queued() or check vchan_next_desc to start the next pending transfer? Without it, does a queued subsequent transfer sit indefinitely without being processed? [Severity: Medium] This is a pre-existing issue, but does dma_chan_issue_pending() unconditionally attempt to start the channel even if it is already actively processing a transfer? drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_issue_pending() { ... if (vchan_issue_pending(&chan->vc)) axi_chan_start_first_queued(chan); ... } If the hardware is already active, this leads to a spurious error printed in axi_chan_block_xfer_start(): drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_chan_block_xfer_start() { ... if (unlikely(axi_chan_is_hw_enable(chan))) { dev_err(chan2dev(chan), "%s is non-idle!\n", ... } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access 2026-09-02 9:26 [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang 2026-09-02 9:26 ` [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang @ 2026-09-02 9:26 ` Jia Wang 2026-09-02 9:40 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position Jia Wang 2026-09-02 9:26 ` [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang 3 siblings, 1 reply; 10+ messages in thread From: Jia Wang @ 2026-09-02 9:26 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko, Sia Jee Heng Cc: dmaengine, linux-kernel, Jia Wang, Frank Li axi_chan_list_dump_lli() uses the channel-wide descs_allocated count to walk the hw_desc[] array of a single transaction. If multiple transactions have allocated LLIs, the channel count can exceed the transaction-local nr_hw_descs and make the DMA error path read past the end of hw_desc[]. Use the descriptor-local nr_hw_descs count when dumping LLIs. Fixes: ef6fb2d6f1ab ("dmaengine: dw-axi-dmac: simplify descriptor management") Signed-off-by: Jia Wang <wangjia@ultrarisc.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> --- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index 742e08cfab43..61230d2b1c56 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -1052,10 +1052,7 @@ static void axi_chan_dump_lli(struct axi_dma_chan *chan, static void axi_chan_list_dump_lli(struct axi_dma_chan *chan, struct axi_dma_desc *desc_head) { - int count = atomic_read(&chan->descs_allocated); - int i; - - for (i = 0; i < count; i++) + for (unsigned int i = 0; i < desc_head->nr_hw_descs; i++) axi_chan_dump_lli(chan, &desc_head->hw_desc[i]); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access 2026-09-02 9:26 ` [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access Jia Wang @ 2026-09-02 9:40 ` sashiko-bot 0 siblings, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-09-02 9:40 UTC (permalink / raw) To: Jia Wang; +Cc: vkoul, dmaengine, Frank.Li Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [Critical] Out-of-bounds array access in `axi_chan_block_xfer_complete()` due to using a channel-wide allocation count instead of a descriptor-specific count. - [Critical] Unbalanced error cleanup leads to fatal NULL pointer dereference in `axi_desc_put()`. -- commit fcec4647e1e2dadb380a57d84fb44bb3b6d294d3 Author: Jia Wang <wangjia@ultrarisc.com> dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access This patch addresses an out-of-bounds array read in axi_chan_list_dump_lli(). It modifies the loop to use the descriptor-local count instead of the channel-wide allocation count. This prevents reading past the end of the array when multiple transactions are present. > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 742e08cfab435..61230d2b1c561 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -1052,10 +1052,7 @@ static void axi_chan_dump_lli(struct axi_dma_chan *chan, > static void axi_chan_list_dump_lli(struct axi_dma_chan *chan, > struct axi_dma_desc *desc_head) > { > - int count = atomic_read(&chan->descs_allocated); > - int i; > - > - for (i = 0; i < count; i++) > + for (unsigned int i = 0; i < desc_head->nr_hw_descs; i++) > axi_chan_dump_lli(chan, &desc_head->hw_desc[i]); > } [Severity: Critical] This isn't a bug introduced by this patch, but the exact same logic flaw appears to exist in axi_chan_block_xfer_complete(): drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_chan_block_xfer_complete() { [ ... ] int count = atomic_read(&chan->descs_allocated); [ ... ] if (chan->cyclic) { desc = vd_to_axi_desc(vd); if (desc) { llp = lo_hi_readq(chan->chan_regs + CH_LLP); for (i = 0; i < count; i++) { hw_desc = &desc->hw_desc[i]; if (hw_desc->llp == llp) { axi_chan_irq_clear(chan, hw_desc->lli->status_lo); hw_desc->lli->ctl_hi |= cpu_to_le32(CH_CTL_H_LLI_VALID); [ ... ] } If multiple transfers are queued or prepared, can the channel-wide count exceed the transaction-local desc->nr_hw_descs count and cause an out-of-bounds access of the desc->hw_desc[] array here as well? Could this out-of-bounds memory be dereferenced as an LLI descriptor pointer and subsequently cause memory corruption when written to? [Severity: Critical] This isn't a bug introduced by this patch, but is there also a regression with unbalanced error cleanup causing a NULL pointer dereference? In dw_axi_dma_chan_prep_cyclic(), if an error occurs midway through setting up the segments: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_chan_prep_cyclic() { [ ... ] for (i = 0; i < total_segments; i++) { hw_desc = &desc->hw_desc[i]; status = dw_axi_dma_set_hw_desc(chan, hw_desc, src_addr, segment_len); if (status < 0) goto err_desc_get; [ ... ] } The error path will call axi_desc_put(), which loops over the entire desc->nr_hw_descs array unconditionally: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_desc_put() { [ ... ] for (descs_put = 0; descs_put < count; descs_put++) { hw_desc = &desc->hw_desc[descs_put]; dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp); } [ ... ] } Since desc->hw_desc is zero-initialized and trailing elements may not have been populated yet, won't hw_desc->lli be NULL for those elements? Since dma_pool_free() does not handle NULL virtual addresses, will this lead to a panic? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position 2026-09-02 9:26 [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang 2026-09-02 9:26 ` [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang 2026-09-02 9:26 ` [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access Jia Wang @ 2026-09-02 9:26 ` Jia Wang 2026-09-02 9:40 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang 3 siblings, 1 reply; 10+ messages in thread From: Jia Wang @ 2026-09-02 9:26 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko, Sia Jee Heng Cc: dmaengine, linux-kernel, Jia Wang, Frank Li The CH_CFG2 channel priority field occupies bits 51:47 of the 64-bit channel configuration register, corresponding to bits 19:15 of CH_CFG2_H. The driver currently shifts the priority value by 20, programming the wrong bits. Use bit 15 as the field position. Fixes: 824351668a41 ("dmaengine: dw-axi-dmac: support DMAX_NUM_CHANNELS > 8") Signed-off-by: Jia Wang <wangjia@ultrarisc.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> --- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h index 67cc199e24d1..97451bb8b16a 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -327,7 +327,7 @@ enum { #define CH_CFG2_H_TT_FC_POS 0 #define CH_CFG2_H_HS_SEL_SRC_POS 3 #define CH_CFG2_H_HS_SEL_DST_POS 4 -#define CH_CFG2_H_PRIORITY_POS 20 +#define CH_CFG2_H_PRIORITY_POS 15 /** * DW AXI DMA channel interrupts -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position 2026-09-02 9:26 ` [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position Jia Wang @ 2026-09-02 9:40 ` sashiko-bot 0 siblings, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-09-02 9:40 UTC (permalink / raw) To: Jia Wang; +Cc: dmaengine, Frank.Li, vkoul Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] The shared interrupt handler unconditionally returns `IRQ_HANDLED`, defeating the kernel's spurious interrupt detection mechanism. -- commit 24abf61259a958020ffb498a11487c8d593eb9a6 Author: Jia Wang <wangjia@ultrarisc.com> dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position The CH_CFG2 channel priority field occupies bits 51:47 of the 64-bit channel configuration register, corresponding to bits 19:15 of CH_CFG2_H. The driver currently shifts the priority value by 20, programming the wrong bits. Use bit 15 as the field position. > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > index 67cc199e24d1f..97451bb8b16a7 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > @@ -327,7 +327,7 @@ enum { > #define CH_CFG2_H_TT_FC_POS 0 > #define CH_CFG2_H_HS_SEL_SRC_POS 3 > #define CH_CFG2_H_HS_SEL_DST_POS 4 > -#define CH_CFG2_H_PRIORITY_POS 20 > +#define CH_CFG2_H_PRIORITY_POS 15 > > /** > * DW AXI DMA channel interrupts [Severity: High] This isn't a bug introduced by this patch, but there is a pre-existing issue in the shared interrupt handler for this driver. Does dw_axi_dma_interrupt() unconditionally return IRQ_HANDLED even if no channel interrupt statuses were set? Looking at drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_interrupt(): static irqreturn_t dw_axi_dma_interrupt(int irq, void *dev_id) { struct axi_dma_chip *chip = dev_id; ... /* Re-enable interrupts */ axi_dma_irq_enable(chip); return IRQ_HANDLED; } If any other device sharing the IRQ line asserts an interrupt that it fails to clear, returning IRQ_HANDLED here unconditionally defeats the kernel's spurious interrupt detection mechanism. Because the IRQ line is registered with IRQF_SHARED, this converts a localized hardware or driver failure into a complete system freeze due to an interrupt storm. Could this function track whether any channel interrupts were actually processed and return IRQ_NONE if none were found? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers 2026-09-02 9:26 [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang ` (2 preceding siblings ...) 2026-09-02 9:26 ` [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position Jia Wang @ 2026-09-02 9:26 ` Jia Wang 2026-09-02 9:39 ` sashiko-bot 2026-09-02 16:08 ` Frank Li 3 siblings, 2 replies; 10+ messages in thread From: Jia Wang @ 2026-09-02 9:26 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko, Sia Jee Heng Cc: dmaengine, linux-kernel, Jia Wang, Frank Li The driver open-codes several channel configuration and descriptor control bitfield writes with left shifts. Define masks for those fields and use FIELD_PREP() when programming the registers. Valid field values keep the same encoding. FIELD_PREP() confines values to their respective fields, but does not validate handshake numbers supplied through DMA specifiers. Suggested-by: Frank Li <Frank.Li@nxp.com> Signed-off-by: Jia Wang <wangjia@ultrarisc.com> --- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 75 +++++++++++++------------- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 44 +++++++-------- 2 files changed, 61 insertions(+), 58 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index 61230d2b1c56..b4aa6461684c 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -7,6 +7,7 @@ * Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> */ +#include <linux/bitfield.h> #include <linux/bitops.h> #include <linux/delay.h> #include <linux/device.h> @@ -101,23 +102,25 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, { u32 cfg_lo, cfg_hi; - cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | - config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); if (chan->chip->dw->hdata->reg_map_8_channels && !chan->chip->dw->hdata->use_cfg2) { - cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS | - config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS | - config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS | - config->src_per << CH_CFG_H_SRC_PER_POS | - config->dst_per << CH_CFG_H_DST_PER_POS | - config->prior << CH_CFG_H_PRIORITY_POS; + cfg_lo = FIELD_PREP(CH_CFG_L_DST_MULTBLK_TYPE, config->dst_multblk_type) | + FIELD_PREP(CH_CFG_L_SRC_MULTBLK_TYPE, config->src_multblk_type); + cfg_hi = FIELD_PREP(CH_CFG_H_TT_FC, config->tt_fc) | + FIELD_PREP(CH_CFG_H_HS_SEL_SRC, config->hs_sel_src) | + FIELD_PREP(CH_CFG_H_HS_SEL_DST, config->hs_sel_dst) | + FIELD_PREP(CH_CFG_H_SRC_PER, config->src_per) | + FIELD_PREP(CH_CFG_H_DST_PER, config->dst_per) | + FIELD_PREP(CH_CFG_H_PRIORITY, config->prior); } else { - cfg_lo |= config->src_per << CH_CFG2_L_SRC_PER_POS | - config->dst_per << CH_CFG2_L_DST_PER_POS; - cfg_hi = config->tt_fc << CH_CFG2_H_TT_FC_POS | - config->hs_sel_src << CH_CFG2_H_HS_SEL_SRC_POS | - config->hs_sel_dst << CH_CFG2_H_HS_SEL_DST_POS | - config->prior << CH_CFG2_H_PRIORITY_POS; + cfg_lo = FIELD_PREP(CH_CFG_L_DST_MULTBLK_TYPE, config->dst_multblk_type) | + FIELD_PREP(CH_CFG_L_SRC_MULTBLK_TYPE, config->src_multblk_type) | + FIELD_PREP(CH_CFG2_L_SRC_PER, config->src_per) | + FIELD_PREP(CH_CFG2_L_DST_PER, config->dst_per); + cfg_hi = FIELD_PREP(CH_CFG2_H_TT_FC, config->tt_fc) | + FIELD_PREP(CH_CFG2_H_HS_SEL_SRC, config->hs_sel_src) | + FIELD_PREP(CH_CFG2_H_HS_SEL_DST, config->hs_sel_dst) | + FIELD_PREP(CH_CFG2_H_PRIORITY, config->prior); } axi_chan_iowrite32(chan, CH_CFG_L, cfg_lo); axi_chan_iowrite32(chan, CH_CFG_H, cfg_hi); @@ -677,19 +680,19 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, case DMA_MEM_TO_DEV: reg_width = __ffs(chan->config.dst_addr_width); device_addr = chan->config.dst_addr; - ctllo = reg_width << CH_CTL_L_DST_WIDTH_POS | - mem_width << CH_CTL_L_SRC_WIDTH_POS | - DWAXIDMAC_CH_CTL_L_NOINC << CH_CTL_L_DST_INC_POS | - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS; + ctllo = FIELD_PREP(CH_CTL_L_DST_WIDTH, reg_width) | + FIELD_PREP(CH_CTL_L_SRC_WIDTH, mem_width) | + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_NOINC) | + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC); block_ts = len >> mem_width; break; case DMA_DEV_TO_MEM: reg_width = __ffs(chan->config.src_addr_width); device_addr = chan->config.src_addr; - ctllo = reg_width << CH_CTL_L_SRC_WIDTH_POS | - mem_width << CH_CTL_L_DST_WIDTH_POS | - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS | - DWAXIDMAC_CH_CTL_L_NOINC << CH_CTL_L_SRC_INC_POS; + ctllo = FIELD_PREP(CH_CTL_L_SRC_WIDTH, reg_width) | + FIELD_PREP(CH_CTL_L_DST_WIDTH, mem_width) | + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) | + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_NOINC); block_ts = len >> reg_width; break; default: @@ -708,8 +711,8 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, if (chan->chip->dw->hdata->restrict_axi_burst_len) { burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; ctlhi |= CH_CTL_H_ARLEN_EN | CH_CTL_H_AWLEN_EN | - burst_len << CH_CTL_H_ARLEN_POS | - burst_len << CH_CTL_H_AWLEN_POS; + FIELD_PREP(CH_CTL_H_ARLEN, burst_len) | + FIELD_PREP(CH_CTL_H_AWLEN, burst_len); } hw_desc->lli->ctl_hi = cpu_to_le32(ctlhi); @@ -724,8 +727,8 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, hw_desc->lli->block_ts_lo = cpu_to_le32(block_ts - 1); - ctllo |= DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS | - DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS; + ctllo |= FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | + FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4); hw_desc->lli->ctl_lo = cpu_to_le32(ctllo); set_desc_src_master(hw_desc); @@ -977,19 +980,19 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr, if (chan->chip->dw->hdata->restrict_axi_burst_len) { u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; - reg |= (CH_CTL_H_ARLEN_EN | - burst_len << CH_CTL_H_ARLEN_POS | - CH_CTL_H_AWLEN_EN | - burst_len << CH_CTL_H_AWLEN_POS); + reg |= CH_CTL_H_ARLEN_EN | + FIELD_PREP(CH_CTL_H_ARLEN, burst_len) | + CH_CTL_H_AWLEN_EN | + FIELD_PREP(CH_CTL_H_AWLEN, burst_len); } hw_desc->lli->ctl_hi = cpu_to_le32(reg); - reg = (DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS | - DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS | - xfer_width << CH_CTL_L_DST_WIDTH_POS | - xfer_width << CH_CTL_L_SRC_WIDTH_POS | - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS | - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS); + reg = FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | + FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | + FIELD_PREP(CH_CTL_L_DST_WIDTH, xfer_width) | + FIELD_PREP(CH_CTL_L_SRC_WIDTH, xfer_width) | + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) | + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC); hw_desc->lli->ctl_lo = cpu_to_le32(reg); set_desc_src_master(hw_desc); diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h index 97451bb8b16a..b4ed241e87e2 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -234,9 +234,9 @@ static inline struct axi_dma_chan *dchan_to_axi_dma_chan(struct dma_chan *dchan) /* CH_CTL_H */ #define CH_CTL_H_ARLEN_EN BIT(6) -#define CH_CTL_H_ARLEN_POS 7 +#define CH_CTL_H_ARLEN GENMASK(14, 7) #define CH_CTL_H_AWLEN_EN BIT(15) -#define CH_CTL_H_AWLEN_POS 16 +#define CH_CTL_H_AWLEN GENMASK(23, 16) enum { DWAXIDMAC_ARWLEN_1 = 0, @@ -258,8 +258,8 @@ enum { /* CH_CTL_L */ #define CH_CTL_L_LAST_WRITE_EN BIT(30) -#define CH_CTL_L_DST_MSIZE_POS 18 -#define CH_CTL_L_SRC_MSIZE_POS 14 +#define CH_CTL_L_DST_MSIZE GENMASK(21, 18) +#define CH_CTL_L_SRC_MSIZE GENMASK(17, 14) enum { DWAXIDMAC_BURST_TRANS_LEN_1 = 0, @@ -274,11 +274,11 @@ enum { DWAXIDMAC_BURST_TRANS_LEN_1024 }; -#define CH_CTL_L_DST_WIDTH_POS 11 -#define CH_CTL_L_SRC_WIDTH_POS 8 +#define CH_CTL_L_DST_WIDTH GENMASK(13, 11) +#define CH_CTL_L_SRC_WIDTH GENMASK(10, 8) -#define CH_CTL_L_DST_INC_POS 6 -#define CH_CTL_L_SRC_INC_POS 4 +#define CH_CTL_L_DST_INC BIT(6) +#define CH_CTL_L_SRC_INC BIT(4) enum { DWAXIDMAC_CH_CTL_L_INC = 0, DWAXIDMAC_CH_CTL_L_NOINC @@ -288,17 +288,17 @@ enum { #define CH_CTL_L_SRC_MAST BIT(0) /* CH_CFG_H */ -#define CH_CFG_H_PRIORITY_POS 17 -#define CH_CFG_H_DST_PER_POS 12 -#define CH_CFG_H_SRC_PER_POS 7 -#define CH_CFG_H_HS_SEL_DST_POS 4 -#define CH_CFG_H_HS_SEL_SRC_POS 3 +#define CH_CFG_H_PRIORITY GENMASK(19, 17) +#define CH_CFG_H_DST_PER GENMASK(15, 12) +#define CH_CFG_H_SRC_PER GENMASK(10, 7) +#define CH_CFG_H_HS_SEL_DST BIT(4) +#define CH_CFG_H_HS_SEL_SRC BIT(3) enum { DWAXIDMAC_HS_SEL_HW = 0, DWAXIDMAC_HS_SEL_SW }; -#define CH_CFG_H_TT_FC_POS 0 +#define CH_CFG_H_TT_FC GENMASK(2, 0) enum { DWAXIDMAC_TT_FC_MEM_TO_MEM_DMAC = 0, DWAXIDMAC_TT_FC_MEM_TO_PER_DMAC, @@ -311,8 +311,8 @@ enum { }; /* CH_CFG_L */ -#define CH_CFG_L_DST_MULTBLK_TYPE_POS 2 -#define CH_CFG_L_SRC_MULTBLK_TYPE_POS 0 +#define CH_CFG_L_DST_MULTBLK_TYPE GENMASK(3, 2) +#define CH_CFG_L_SRC_MULTBLK_TYPE GENMASK(1, 0) enum { DWAXIDMAC_MBLK_TYPE_CONTIGUOUS = 0, DWAXIDMAC_MBLK_TYPE_RELOAD, @@ -321,13 +321,13 @@ enum { }; /* CH_CFG2 */ -#define CH_CFG2_L_SRC_PER_POS 4 -#define CH_CFG2_L_DST_PER_POS 11 +#define CH_CFG2_L_SRC_PER GENMASK(9, 4) +#define CH_CFG2_L_DST_PER GENMASK(16, 11) -#define CH_CFG2_H_TT_FC_POS 0 -#define CH_CFG2_H_HS_SEL_SRC_POS 3 -#define CH_CFG2_H_HS_SEL_DST_POS 4 -#define CH_CFG2_H_PRIORITY_POS 15 +#define CH_CFG2_H_TT_FC GENMASK(2, 0) +#define CH_CFG2_H_HS_SEL_SRC BIT(3) +#define CH_CFG2_H_HS_SEL_DST BIT(4) +#define CH_CFG2_H_PRIORITY GENMASK(19, 15) /** * DW AXI DMA channel interrupts -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers 2026-09-02 9:26 ` [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang @ 2026-09-02 9:39 ` sashiko-bot 2026-09-02 16:08 ` Frank Li 1 sibling, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-09-02 9:39 UTC (permalink / raw) To: Jia Wang; +Cc: vkoul, Frank.Li, dmaengine Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Kernel crash in axi_desc_put due to dma_pool_free() being called with a NULL virtual address when descriptor setup fails. -- commit cfefdb6fe5a6c1faad810fe6247a2229e7ffd900 Author: Jia Wang <wangjia@ultrarisc.com> dmaengine: dw-axi-dmac: Use bitfield helpers for registers The driver open-codes several channel configuration and descriptor control bitfield writes with left shifts. Define masks for those fields and use FIELD_PREP() when programming the registers. Valid field values keep the same encoding. FIELD_PREP() confines values to their respective fields, but does not validate handshake numbers supplied through DMA specifiers. > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 61230d2b1c561..b4aa6461684cb 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c [ ... ] > @@ -977,19 +980,19 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr, > if (chan->chip->dw->hdata->restrict_axi_burst_len) { > u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; > > - reg |= (CH_CTL_H_ARLEN_EN | > - burst_len << CH_CTL_H_ARLEN_POS | > - CH_CTL_H_AWLEN_EN | > - burst_len << CH_CTL_H_AWLEN_POS); > + reg |= CH_CTL_H_ARLEN_EN | > + FIELD_PREP(CH_CTL_H_ARLEN, burst_len) | > + CH_CTL_H_AWLEN_EN | > + FIELD_PREP(CH_CTL_H_AWLEN, burst_len); > } > hw_desc->lli->ctl_hi = cpu_to_le32(reg); > > - reg = (DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS | > - DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS | > - xfer_width << CH_CTL_L_DST_WIDTH_POS | > - xfer_width << CH_CTL_L_SRC_WIDTH_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS); > + reg = FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | > + FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | > + FIELD_PREP(CH_CTL_L_DST_WIDTH, xfer_width) | > + FIELD_PREP(CH_CTL_L_SRC_WIDTH, xfer_width) | > + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) | > + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC); > hw_desc->lli->ctl_lo = cpu_to_le32(reg); > > set_desc_src_master(hw_desc); [Severity: High] This is a pre-existing issue, but can the error handling path in dma_chan_prep_dma_memcpy() cause a kernel crash? If axi_desc_get() fails to allocate a descriptor from the DMA pool, the code jumps to err_desc_get and calls axi_desc_put(desc): drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_prep_dma_memcpy() { ... hw_desc->lli = axi_desc_get(chan, &hw_desc->llp); if (unlikely(!hw_desc->lli)) goto err_desc_get; ... err_desc_get: if (desc) axi_desc_put(desc); return NULL; } In axi_desc_put(), the loop unconditionally iterates over all requested hardware descriptors up to desc->nr_hw_descs and attempts to free them: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_desc_put() { ... for (descs_put = 0; descs_put < count; descs_put++) { hw_desc = &desc->hw_desc[descs_put]; dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp); } ... } Because unallocated descriptors will have a NULL hw_desc->lli, dma_pool_free() is called with a NULL virtual address. Since dma_pool_free() does not check for a NULL vaddr input, will this trigger a NULL pointer dereference in the DMA pool allocator? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers 2026-09-02 9:26 ` [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang 2026-09-02 9:39 ` sashiko-bot @ 2026-09-02 16:08 ` Frank Li 1 sibling, 0 replies; 10+ messages in thread From: Frank Li @ 2026-09-02 16:08 UTC (permalink / raw) To: Jia Wang Cc: Eugeniy Paltsev, Vinod Koul, Frank Li, Pandith N, Andy Shevchenko, Sia Jee Heng, dmaengine, linux-kernel, Frank Li On Wed, Sep 02, 2026 at 05:26:14PM +0800, Jia Wang wrote: > The driver open-codes several channel configuration and descriptor > control bitfield writes with left shifts. Define masks for those fields > and use FIELD_PREP() when programming the registers. > > Valid field values keep the same encoding. FIELD_PREP() confines values > to their respective fields, but does not validate handshake numbers > supplied through DMA specifiers. > > Suggested-by: Frank Li <Frank.Li@nxp.com> > Signed-off-by: Jia Wang <wangjia@ultrarisc.com> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 75 +++++++++++++------------- > drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 44 +++++++-------- > 2 files changed, 61 insertions(+), 58 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 61230d2b1c56..b4aa6461684c 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -7,6 +7,7 @@ > * Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > */ > > +#include <linux/bitfield.h> > #include <linux/bitops.h> > #include <linux/delay.h> > #include <linux/device.h> > @@ -101,23 +102,25 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, > { > u32 cfg_lo, cfg_hi; > > - cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | > - config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); > if (chan->chip->dw->hdata->reg_map_8_channels && > !chan->chip->dw->hdata->use_cfg2) { > - cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS | > - config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS | > - config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS | > - config->src_per << CH_CFG_H_SRC_PER_POS | > - config->dst_per << CH_CFG_H_DST_PER_POS | > - config->prior << CH_CFG_H_PRIORITY_POS; > + cfg_lo = FIELD_PREP(CH_CFG_L_DST_MULTBLK_TYPE, config->dst_multblk_type) | > + FIELD_PREP(CH_CFG_L_SRC_MULTBLK_TYPE, config->src_multblk_type); > + cfg_hi = FIELD_PREP(CH_CFG_H_TT_FC, config->tt_fc) | > + FIELD_PREP(CH_CFG_H_HS_SEL_SRC, config->hs_sel_src) | > + FIELD_PREP(CH_CFG_H_HS_SEL_DST, config->hs_sel_dst) | > + FIELD_PREP(CH_CFG_H_SRC_PER, config->src_per) | > + FIELD_PREP(CH_CFG_H_DST_PER, config->dst_per) | > + FIELD_PREP(CH_CFG_H_PRIORITY, config->prior); > } else { > - cfg_lo |= config->src_per << CH_CFG2_L_SRC_PER_POS | > - config->dst_per << CH_CFG2_L_DST_PER_POS; > - cfg_hi = config->tt_fc << CH_CFG2_H_TT_FC_POS | > - config->hs_sel_src << CH_CFG2_H_HS_SEL_SRC_POS | > - config->hs_sel_dst << CH_CFG2_H_HS_SEL_DST_POS | > - config->prior << CH_CFG2_H_PRIORITY_POS; > + cfg_lo = FIELD_PREP(CH_CFG_L_DST_MULTBLK_TYPE, config->dst_multblk_type) | > + FIELD_PREP(CH_CFG_L_SRC_MULTBLK_TYPE, config->src_multblk_type) | > + FIELD_PREP(CH_CFG2_L_SRC_PER, config->src_per) | > + FIELD_PREP(CH_CFG2_L_DST_PER, config->dst_per); > + cfg_hi = FIELD_PREP(CH_CFG2_H_TT_FC, config->tt_fc) | > + FIELD_PREP(CH_CFG2_H_HS_SEL_SRC, config->hs_sel_src) | > + FIELD_PREP(CH_CFG2_H_HS_SEL_DST, config->hs_sel_dst) | > + FIELD_PREP(CH_CFG2_H_PRIORITY, config->prior); > } > axi_chan_iowrite32(chan, CH_CFG_L, cfg_lo); > axi_chan_iowrite32(chan, CH_CFG_H, cfg_hi); > @@ -677,19 +680,19 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, > case DMA_MEM_TO_DEV: > reg_width = __ffs(chan->config.dst_addr_width); > device_addr = chan->config.dst_addr; > - ctllo = reg_width << CH_CTL_L_DST_WIDTH_POS | > - mem_width << CH_CTL_L_SRC_WIDTH_POS | > - DWAXIDMAC_CH_CTL_L_NOINC << CH_CTL_L_DST_INC_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS; > + ctllo = FIELD_PREP(CH_CTL_L_DST_WIDTH, reg_width) | > + FIELD_PREP(CH_CTL_L_SRC_WIDTH, mem_width) | > + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_NOINC) | > + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC); > block_ts = len >> mem_width; > break; > case DMA_DEV_TO_MEM: > reg_width = __ffs(chan->config.src_addr_width); > device_addr = chan->config.src_addr; > - ctllo = reg_width << CH_CTL_L_SRC_WIDTH_POS | > - mem_width << CH_CTL_L_DST_WIDTH_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS | > - DWAXIDMAC_CH_CTL_L_NOINC << CH_CTL_L_SRC_INC_POS; > + ctllo = FIELD_PREP(CH_CTL_L_SRC_WIDTH, reg_width) | > + FIELD_PREP(CH_CTL_L_DST_WIDTH, mem_width) | > + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) | > + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_NOINC); > block_ts = len >> reg_width; > break; > default: > @@ -708,8 +711,8 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, > if (chan->chip->dw->hdata->restrict_axi_burst_len) { > burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; > ctlhi |= CH_CTL_H_ARLEN_EN | CH_CTL_H_AWLEN_EN | > - burst_len << CH_CTL_H_ARLEN_POS | > - burst_len << CH_CTL_H_AWLEN_POS; > + FIELD_PREP(CH_CTL_H_ARLEN, burst_len) | > + FIELD_PREP(CH_CTL_H_AWLEN, burst_len); > } > > hw_desc->lli->ctl_hi = cpu_to_le32(ctlhi); > @@ -724,8 +727,8 @@ static int dw_axi_dma_set_hw_desc(struct axi_dma_chan *chan, > > hw_desc->lli->block_ts_lo = cpu_to_le32(block_ts - 1); > > - ctllo |= DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS | > - DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS; > + ctllo |= FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | > + FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4); > hw_desc->lli->ctl_lo = cpu_to_le32(ctllo); > > set_desc_src_master(hw_desc); > @@ -977,19 +980,19 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr, > if (chan->chip->dw->hdata->restrict_axi_burst_len) { > u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1; > > - reg |= (CH_CTL_H_ARLEN_EN | > - burst_len << CH_CTL_H_ARLEN_POS | > - CH_CTL_H_AWLEN_EN | > - burst_len << CH_CTL_H_AWLEN_POS); > + reg |= CH_CTL_H_ARLEN_EN | > + FIELD_PREP(CH_CTL_H_ARLEN, burst_len) | > + CH_CTL_H_AWLEN_EN | > + FIELD_PREP(CH_CTL_H_AWLEN, burst_len); > } > hw_desc->lli->ctl_hi = cpu_to_le32(reg); > > - reg = (DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS | > - DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS | > - xfer_width << CH_CTL_L_DST_WIDTH_POS | > - xfer_width << CH_CTL_L_SRC_WIDTH_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS | > - DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS); > + reg = FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | > + FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) | > + FIELD_PREP(CH_CTL_L_DST_WIDTH, xfer_width) | > + FIELD_PREP(CH_CTL_L_SRC_WIDTH, xfer_width) | > + FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) | > + FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC); > hw_desc->lli->ctl_lo = cpu_to_le32(reg); > > set_desc_src_master(hw_desc); > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > index 97451bb8b16a..b4ed241e87e2 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > @@ -234,9 +234,9 @@ static inline struct axi_dma_chan *dchan_to_axi_dma_chan(struct dma_chan *dchan) > > /* CH_CTL_H */ > #define CH_CTL_H_ARLEN_EN BIT(6) > -#define CH_CTL_H_ARLEN_POS 7 > +#define CH_CTL_H_ARLEN GENMASK(14, 7) > #define CH_CTL_H_AWLEN_EN BIT(15) > -#define CH_CTL_H_AWLEN_POS 16 > +#define CH_CTL_H_AWLEN GENMASK(23, 16) > > enum { > DWAXIDMAC_ARWLEN_1 = 0, > @@ -258,8 +258,8 @@ enum { > /* CH_CTL_L */ > #define CH_CTL_L_LAST_WRITE_EN BIT(30) > > -#define CH_CTL_L_DST_MSIZE_POS 18 > -#define CH_CTL_L_SRC_MSIZE_POS 14 > +#define CH_CTL_L_DST_MSIZE GENMASK(21, 18) > +#define CH_CTL_L_SRC_MSIZE GENMASK(17, 14) > > enum { > DWAXIDMAC_BURST_TRANS_LEN_1 = 0, > @@ -274,11 +274,11 @@ enum { > DWAXIDMAC_BURST_TRANS_LEN_1024 > }; > > -#define CH_CTL_L_DST_WIDTH_POS 11 > -#define CH_CTL_L_SRC_WIDTH_POS 8 > +#define CH_CTL_L_DST_WIDTH GENMASK(13, 11) > +#define CH_CTL_L_SRC_WIDTH GENMASK(10, 8) > > -#define CH_CTL_L_DST_INC_POS 6 > -#define CH_CTL_L_SRC_INC_POS 4 > +#define CH_CTL_L_DST_INC BIT(6) > +#define CH_CTL_L_SRC_INC BIT(4) > enum { > DWAXIDMAC_CH_CTL_L_INC = 0, > DWAXIDMAC_CH_CTL_L_NOINC > @@ -288,17 +288,17 @@ enum { > #define CH_CTL_L_SRC_MAST BIT(0) > > /* CH_CFG_H */ > -#define CH_CFG_H_PRIORITY_POS 17 > -#define CH_CFG_H_DST_PER_POS 12 > -#define CH_CFG_H_SRC_PER_POS 7 > -#define CH_CFG_H_HS_SEL_DST_POS 4 > -#define CH_CFG_H_HS_SEL_SRC_POS 3 > +#define CH_CFG_H_PRIORITY GENMASK(19, 17) > +#define CH_CFG_H_DST_PER GENMASK(15, 12) > +#define CH_CFG_H_SRC_PER GENMASK(10, 7) > +#define CH_CFG_H_HS_SEL_DST BIT(4) > +#define CH_CFG_H_HS_SEL_SRC BIT(3) > enum { > DWAXIDMAC_HS_SEL_HW = 0, > DWAXIDMAC_HS_SEL_SW > }; > > -#define CH_CFG_H_TT_FC_POS 0 > +#define CH_CFG_H_TT_FC GENMASK(2, 0) > enum { > DWAXIDMAC_TT_FC_MEM_TO_MEM_DMAC = 0, > DWAXIDMAC_TT_FC_MEM_TO_PER_DMAC, > @@ -311,8 +311,8 @@ enum { > }; > > /* CH_CFG_L */ > -#define CH_CFG_L_DST_MULTBLK_TYPE_POS 2 > -#define CH_CFG_L_SRC_MULTBLK_TYPE_POS 0 > +#define CH_CFG_L_DST_MULTBLK_TYPE GENMASK(3, 2) > +#define CH_CFG_L_SRC_MULTBLK_TYPE GENMASK(1, 0) > enum { > DWAXIDMAC_MBLK_TYPE_CONTIGUOUS = 0, > DWAXIDMAC_MBLK_TYPE_RELOAD, > @@ -321,13 +321,13 @@ enum { > }; > > /* CH_CFG2 */ > -#define CH_CFG2_L_SRC_PER_POS 4 > -#define CH_CFG2_L_DST_PER_POS 11 > +#define CH_CFG2_L_SRC_PER GENMASK(9, 4) > +#define CH_CFG2_L_DST_PER GENMASK(16, 11) > > -#define CH_CFG2_H_TT_FC_POS 0 > -#define CH_CFG2_H_HS_SEL_SRC_POS 3 > -#define CH_CFG2_H_HS_SEL_DST_POS 4 > -#define CH_CFG2_H_PRIORITY_POS 15 > +#define CH_CFG2_H_TT_FC GENMASK(2, 0) > +#define CH_CFG2_H_HS_SEL_SRC BIT(3) > +#define CH_CFG2_H_HS_SEL_DST BIT(4) > +#define CH_CFG2_H_PRIORITY GENMASK(19, 15) > > /** > * DW AXI DMA channel interrupts > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-02 16:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-02 9:26 [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Jia Wang 2026-09-02 9:26 ` [PATCH v3 1/4] dmaengine: dw-axi-dmac: Fix AXI burst length encoding Jia Wang 2026-09-02 9:39 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 2/4] dmaengine: dw-axi-dmac: Fix LLI dump out-of-bounds access Jia Wang 2026-09-02 9:40 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 3/4] dmaengine: dw-axi-dmac: Fix CH_CFG2 channel priority position Jia Wang 2026-09-02 9:40 ` sashiko-bot 2026-09-02 9:26 ` [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers Jia Wang 2026-09-02 9:39 ` sashiko-bot 2026-09-02 16:08 ` Frank Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox