From: Frank Li <Frank.li@oss.nxp.com>
To: Jia Wang <wangjia@ultrarisc.com>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>,
Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
Pandith N <pandith.n@intel.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Sia Jee Heng <jee.heng.sia@intel.com>,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
Frank Li <Frank.Li@nxp.com>
Subject: Re: [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers
Date: Wed, 2 Sep 2026 11:08:53 -0500 [thread overview]
Message-ID: <aphKFW_klyNwOU21@SMW015318> (raw)
In-Reply-To: <20260902-dma-fix-v3-4-414c6449fbcc@ultrarisc.com>
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
>
next prev parent reply other threads:[~2026-09-02 16:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
2026-09-10 14:10 ` [PATCH v3 0/4] dmaengine: dw-axi-dmac: Fix burst encoding, LLI dump and priority Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aphKFW_klyNwOU21@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Eugeniy.Paltsev@synopsys.com \
--cc=Frank.Li@kernel.org \
--cc=Frank.Li@nxp.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=dmaengine@vger.kernel.org \
--cc=jee.heng.sia@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pandith.n@intel.com \
--cc=vkoul@kernel.org \
--cc=wangjia@ultrarisc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.