mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: Rosen Penev <rosenp@gmail.com>
Cc: dmaengine@vger.kernel.org, Vinod Koul <vkoul@kernel.org>,
	Frank Li <Frank.Li@kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
Date: Thu, 16 Jul 2026 13:02:57 -0400	[thread overview]
Message-ID: <alkOwebrTsoMTX7l@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <20260714233855.870797-3-rosenp@gmail.com>

On Tue, Jul 14, 2026 at 04:38:54PM -0700, Rosen Penev wrote:
> The fsl_re_ctrl and fsl_re_chan_cfg structures describe memory-mapped
> RAID Engine registers accessed only via ioread32be()/iowrite32be(), yet
> the pointers to them (re_regs in struct fsl_re_drv_private, and jrregs
> in struct fsl_re_chan) were not __iomem-qualified, so sparse emitted
> "different address spaces" warnings for every register access.
>
> Store both MMIO bases as a plain void __iomem * and derive jrregs with
> void __iomem * arithmetic from re_regs, rather than carrying typed
> register struct pointers through the driver. Each function that touches
> the registers introduces a local typed pointer (struct fsl_re_ctrl
> __iomem *ctrl / struct fsl_re_chan_cfg __iomem *jr) and uses ->field,
> which is the idiomatic kernel pattern and keeps the registers' __iomem
> qualification intact.

Any document or reference show this is idiomatic kernel pattern?

Frank

>
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: https://lore.kernel.org/oe-kbuild-all/202008111749.yy85rFMD%25lkp@intel.com/
> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsl_raid.c | 66 ++++++++++++++++++++----------------------
>  drivers/dma/fsl_raid.h |  4 +--
>  2 files changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 888f55b672a5..47ebdf274331 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -106,15 +106,17 @@ static dma_cookie_t fsl_re_tx_submit(struct dma_async_tx_descriptor *tx)
>  static void fsl_re_issue_pending(struct dma_chan *chan)
>  {
>  	struct fsl_re_chan *re_chan;
> +	struct fsl_re_chan_cfg __iomem *jr;
>  	int avail;
>  	struct fsl_re_desc *desc, *_desc;
>  	unsigned long flags;
>
>  	re_chan = container_of(chan, struct fsl_re_chan, chan);
> +	jr = re_chan->jrregs;
>
>  	spin_lock_irqsave(&re_chan->desc_lock, flags);
>  	avail = FSL_RE_SLOT_AVAIL(
> -		in_be32(&re_chan->jrregs->inbring_slot_avail));
> +		in_be32(&jr->inbring_slot_avail));
>
>  	list_for_each_entry_safe(desc, _desc, &re_chan->submit_q, node) {
>  		if (!avail)
> @@ -127,7 +129,7 @@ static void fsl_re_issue_pending(struct dma_chan *chan)
>
>  		re_chan->inb_count = (re_chan->inb_count + 1) &
>  						FSL_RE_RING_SIZE_MASK;
> -		out_be32(&re_chan->jrregs->inbring_add_job, FSL_RE_ADD_JOB(1));
> +		out_be32(&jr->inbring_add_job, FSL_RE_ADD_JOB(1));
>  		avail--;
>  	}
>  	spin_unlock_irqrestore(&re_chan->desc_lock, flags);
> @@ -158,6 +160,7 @@ static void fsl_re_cleanup_descs(struct fsl_re_chan *re_chan)
>  static void fsl_re_dequeue(struct tasklet_struct *t)
>  {
>  	struct fsl_re_chan *re_chan = from_tasklet(re_chan, t, irqtask);
> +	struct fsl_re_chan_cfg __iomem *jr = re_chan->jrregs;
>  	struct fsl_re_desc *desc, *_desc;
>  	struct fsl_re_hw_desc *hwdesc;
>  	unsigned long flags;
> @@ -167,7 +170,7 @@ static void fsl_re_dequeue(struct tasklet_struct *t)
>  	fsl_re_cleanup_descs(re_chan);
>
>  	spin_lock_irqsave(&re_chan->desc_lock, flags);
> -	count =	FSL_RE_SLOT_FULL(in_be32(&re_chan->jrregs->oubring_slot_full));
> +	count =	FSL_RE_SLOT_FULL(in_be32(&jr->oubring_slot_full));
>  	while (count--) {
>  		found = 0;
>  		hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
> @@ -192,8 +195,7 @@ static void fsl_re_dequeue(struct tasklet_struct *t)
>  		oub_count = (re_chan->oub_count + 1) & FSL_RE_RING_SIZE_MASK;
>  		re_chan->oub_count = oub_count;
>
> -		out_be32(&re_chan->jrregs->oubring_job_rmvd,
> -			 FSL_RE_RMVD_JOB(1));
> +		out_be32(&jr->oubring_job_rmvd, FSL_RE_RMVD_JOB(1));
>  	}
>  	spin_unlock_irqrestore(&re_chan->desc_lock, flags);
>  }
> @@ -201,12 +203,12 @@ static void fsl_re_dequeue(struct tasklet_struct *t)
>  /* Per Job Ring interrupt handler */
>  static irqreturn_t fsl_re_isr(int irq, void *data)
>  {
> -	struct fsl_re_chan *re_chan;
> +	struct device *dev = data;
> +	struct fsl_re_chan *re_chan = dev_get_drvdata(dev);
> +	struct fsl_re_chan_cfg __iomem *jr = re_chan->jrregs;
>  	u32 irqstate, status;
>
> -	re_chan = dev_get_drvdata((struct device *)data);
> -
> -	irqstate = in_be32(&re_chan->jrregs->jr_interrupt_status);
> +	irqstate = in_be32(&jr->jr_interrupt_status);
>  	if (!irqstate)
>  		return IRQ_NONE;
>
> @@ -216,13 +218,13 @@ static irqreturn_t fsl_re_isr(int irq, void *data)
>  	 * need to do something more than just crashing
>  	 */
>  	if (irqstate & FSL_RE_ERROR) {
> -		status = in_be32(&re_chan->jrregs->jr_status);
> +		status = in_be32(&jr->jr_status);
>  		dev_err(re_chan->dev, "chan error irqstate: %x, status: %x\n",
>  			irqstate, status);
>  	}
>
>  	/* Clear interrupt */
> -	out_be32(&re_chan->jrregs->jr_interrupt_status, FSL_RE_CLR_INTR);
> +	out_be32(&jr->jr_interrupt_status, FSL_RE_CLR_INTR);
>
>  	tasklet_schedule(&re_chan->irqtask);
>
> @@ -627,6 +629,7 @@ static int fsl_re_chan_probe(struct platform_device *ofdev,
>  	struct device *dev, *chandev;
>  	struct fsl_re_drv_private *re_priv;
>  	struct fsl_re_chan *chan;
> +	struct fsl_re_chan_cfg __iomem *jr;
>  	struct dma_device *dma_dev;
>  	u32 ptr;
>  	u32 status;
> @@ -657,8 +660,8 @@ static int fsl_re_chan_probe(struct platform_device *ofdev,
>  		goto err_free;
>  	}
>
> -	chan->jrregs = (struct fsl_re_chan_cfg *)((u8 *)re_priv->re_regs +
> -			off + ptr);
> +	jr = re_priv->re_regs + off + ptr;
> +	chan->jrregs = jr;
>
>  	/* read irq property from dts */
>  	chan->irq = irq_of_parse_and_map(np, 0);
> @@ -709,30 +712,23 @@ static int fsl_re_chan_probe(struct platform_device *ofdev,
>  	}
>
>  	/* Program the Inbound/Outbound ring base addresses and size */
> -	out_be32(&chan->jrregs->inbring_base_h,
> -		 chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK);
> -	out_be32(&chan->jrregs->oubring_base_h,
> -		 chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK);
> -	out_be32(&chan->jrregs->inbring_base_l,
> -		 chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
> -	out_be32(&chan->jrregs->oubring_base_l,
> -		 chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
> -	out_be32(&chan->jrregs->inbring_size,
> -		 FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT);
> -	out_be32(&chan->jrregs->oubring_size,
> -		 FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT);
> +	out_be32(&jr->inbring_base_h, chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK);
> +	out_be32(&jr->oubring_base_h, chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK);
> +	out_be32(&jr->inbring_base_l, chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
> +	out_be32(&jr->oubring_base_l, chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
> +	out_be32(&jr->inbring_size, FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT);
> +	out_be32(&jr->oubring_size, FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT);
>
>  	/* Read LIODN value from u-boot */
> -	status = in_be32(&chan->jrregs->jr_config_1) & FSL_RE_REG_LIODN_MASK;
> +	status = in_be32(&jr->jr_config_1) & FSL_RE_REG_LIODN_MASK;
>
>  	/* Program the CFG reg */
> -	out_be32(&chan->jrregs->jr_config_1,
> -		 FSL_RE_CFG1_CBSI | FSL_RE_CFG1_CBS0 | status);
> +	out_be32(&jr->jr_config_1, FSL_RE_CFG1_CBSI | FSL_RE_CFG1_CBS0 | status);
>
>  	dev_set_drvdata(chandev, chan);
>
>  	/* Enable RE/CHAN */
> -	out_be32(&chan->jrregs->jr_command, FSL_RE_ENABLE);
> +	out_be32(&jr->jr_command, FSL_RE_ENABLE);
>
>  	return 0;
>
> @@ -752,6 +748,7 @@ static int fsl_re_probe(struct platform_device *ofdev)
>  	u8 ridx = 0;
>  	struct dma_device *dma_dev;
>  	struct resource *res;
> +	struct fsl_re_ctrl __iomem *re_regs;
>  	int rc;
>  	struct device *dev = &ofdev->dev;
>
> @@ -767,17 +764,18 @@ static int fsl_re_probe(struct platform_device *ofdev)
>  	re_priv->re_regs = devm_ioremap(dev, res->start, resource_size(res));
>  	if (!re_priv->re_regs)
>  		return -EBUSY;
> +	re_regs = re_priv->re_regs;
>
>  	/* Program the RE mode */
> -	out_be32(&re_priv->re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> +	out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
>
>  	/* Program Galois Field polynomial */
> -	out_be32(&re_priv->re_regs->galois_field_config, FSL_RE_GFM_POLY);
> +	out_be32(&re_regs->galois_field_config, FSL_RE_GFM_POLY);
>
>  	dev_info(dev, "version %x, mode %x, gfp %x\n",
> -		 in_be32(&re_priv->re_regs->re_version_id),
> -		 in_be32(&re_priv->re_regs->global_config),
> -		 in_be32(&re_priv->re_regs->galois_field_config));
> +		 in_be32(&re_regs->re_version_id),
> +		 in_be32(&re_regs->global_config),
> +		 in_be32(&re_regs->galois_field_config));
>
>  	dma_dev = &re_priv->dma_dev;
>  	dma_dev->dev = dev;
> diff --git a/drivers/dma/fsl_raid.h b/drivers/dma/fsl_raid.h
> index 69d743c04973..6069615e2e1e 100644
> --- a/drivers/dma/fsl_raid.h
> +++ b/drivers/dma/fsl_raid.h
> @@ -256,7 +256,7 @@ struct fsl_re_hw_desc {
>  struct fsl_re_drv_private {
>  	u8 total_chans;
>  	struct dma_device dma_dev;
> -	struct fsl_re_ctrl *re_regs;
> +	void __iomem *re_regs;
>  	struct fsl_re_chan *re_jrs[FSL_RE_MAX_CHANS];
>  	struct dma_pool *cf_desc_pool;
>  	struct dma_pool *hw_desc_pool;
> @@ -273,7 +273,7 @@ struct fsl_re_chan {
>  	struct device *dev;
>  	struct fsl_re_drv_private *re_dev;
>  	struct dma_chan chan;
> -	struct fsl_re_chan_cfg *jrregs;
> +	void __iomem *jrregs;
>  	int irq;
>  	struct tasklet_struct irqtask;
>  	u32 alloc_count;
> --
> 2.55.0
>

  reply	other threads:[~2026-07-16 17:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 23:38 [PATCH 0/3] dma: fsl_raid: fix sparse warnings and simplify probing Rosen Penev
2026-07-14 23:38 ` [PATCH 1/3] dma: fsl_raid: fix endianness of in-memory descriptor stores Rosen Penev
2026-07-16 16:51   ` Frank Li
2026-07-14 23:38 ` [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access Rosen Penev
2026-07-16 17:02   ` Frank Li [this message]
2026-07-16 18:35     ` Rosen Penev
2026-07-16 18:58       ` Frank Li
2026-07-14 23:38 ` [PATCH 3/3] dma: fsl_raid: use devm_platform_ioremap_resource Rosen Penev
2026-07-16 16:53   ` Frank Li

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=alkOwebrTsoMTX7l@lizhi-Precision-Tower-5810 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rosenp@gmail.com \
    --cc=vkoul@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®