mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] dma: fsl_raid: fix sparse warnings and simplify probing
@ 2026-07-14 23:38 Rosen Penev
  2026-07-14 23:38 ` [PATCH 1/3] dma: fsl_raid: fix endianness of in-memory descriptor stores Rosen Penev
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rosen Penev @ 2026-07-14 23:38 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, Frank Li, open list

This series cleans up the fsl_raid driver to address issues reported by
sparse and to simplify its MMIO handling.

Patch 1 fixes the endianness of in-memory descriptor stores. The
descriptor structs are handed to the device as big-endian, but the driver
stored CPU-endian values directly, which is both incorrect and flagged by
sparse as a base-type mismatch. The stores are wrapped in cpu_to_be32()
and the final-frame bit is now passed as an argument rather than
read-modify-written.

Patch 2 keeps the MMIO bases (re_regs and jrregs) as void __iomem *
instead of typed register struct pointers, eliminating "different address
spaces" sparse warnings on every register access. Each accessor derives a
local __iomem-qualified typed pointer.

Patch 3 replaces the open-coded platform_get_resource() +
devm_ioremap() sequence with devm_platform_ioremap_resource().

Rosen Penev (3):
  dma: fsl_raid: fix endianness of in-memory descriptor stores
  dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
  dma: fsl_raid: use devm_platform_ioremap_resource

 drivers/dma/fsl_raid.c | 109 ++++++++++++++++++++---------------------
 drivers/dma/fsl_raid.h |   4 +-
 2 files changed, 54 insertions(+), 59 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] dma: fsl_raid: fix endianness of in-memory descriptor stores
  2026-07-14 23:38 [PATCH 0/3] dma: fsl_raid: fix sparse warnings and simplify probing Rosen Penev
@ 2026-07-14 23:38 ` 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-14 23:38 ` [PATCH 3/3] dma: fsl_raid: use devm_platform_ioremap_resource Rosen Penev
  2 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2026-07-14 23:38 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, Frank Li, open list

The descriptor structs (fsl_re_cmpnd_frame / fsl_re_hw_desc) are
in-memory but their fields are __be32, because the structures are handed
to the device as big-endian. The driver stored CPU-endian u32 values
into them directly, which is both wrong (the engine would see
byte-swapped lengths/addresses) and flagged by sparse as a base-type
mismatch.

Wrap those stores in cpu_to_be32() so the values are little->big
converted. The final-frame bit is now passed as the "final" argument of
fill_cfd_frame() (as fsl_re_prep_dma_memcpy already did) and set in CPU
order before the single cpu_to_be32() store, replacing the previous
read-modify-write of the __be32 efrl32 field.

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 | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index 99945845d8b5..888f55b672a5 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -242,9 +242,9 @@ static void fill_cfd_frame(struct fsl_re_cmpnd_frame *cf, u8 index,
 	u32 efrl = length & FSL_RE_CF_LENGTH_MASK;
 
 	efrl |= final << FSL_RE_CF_FINAL_SHIFT;
-	cf[index].efrl32 = efrl;
-	cf[index].addr_high = upper_32_bits(addr);
-	cf[index].addr_low = lower_32_bits(addr);
+	cf[index].efrl32 = cpu_to_be32(efrl);
+	cf[index].addr_high = cpu_to_be32(upper_32_bits(addr));
+	cf[index].addr_low = cpu_to_be32(lower_32_bits(addr));
 }
 
 static struct fsl_re_desc *fsl_re_init_desc(struct fsl_re_chan *re_chan,
@@ -256,9 +256,10 @@ static struct fsl_re_desc *fsl_re_init_desc(struct fsl_re_chan *re_chan,
 	dma_async_tx_descriptor_init(&desc->async_tx, &re_chan->chan);
 	INIT_LIST_HEAD(&desc->node);
 
-	desc->hwdesc.fmt32 = FSL_RE_FRAME_FORMAT << FSL_RE_HWDESC_FMT_SHIFT;
-	desc->hwdesc.lbea32 = upper_32_bits(paddr);
-	desc->hwdesc.addr_low = lower_32_bits(paddr);
+	desc->hwdesc.fmt32 = cpu_to_be32(FSL_RE_FRAME_FORMAT <<
+					  FSL_RE_HWDESC_FMT_SHIFT);
+	desc->hwdesc.lbea32 = cpu_to_be32(upper_32_bits(paddr));
+	desc->hwdesc.addr_low = cpu_to_be32(lower_32_bits(paddr));
 	desc->cf_addr = cf;
 	desc->cf_paddr = paddr;
 
@@ -374,11 +375,11 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
 	for (i = 2, j = 0; j < save_src_cnt; i++, j++)
 		fill_cfd_frame(cf, i, len, src[j], 0);
 
+	/* Fill the last frame and mark it final */
 	if (cont_q)
-		fill_cfd_frame(cf, i++, len, dest, 0);
-
-	/* Setting the final bit in the last source buffer frame in CFD */
-	cf[i - 1].efrl32 |= 1 << FSL_RE_CF_FINAL_SHIFT;
+		fill_cfd_frame(cf, i, len, dest, 1);
+	else
+		fill_cfd_frame(cf, i - 1, len, src[j - 1], 1);
 
 	return &desc->async_tx;
 }
@@ -504,16 +505,16 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_pq(
 			p[save_src_cnt + 2] = 1;
 			fill_cfd_frame(cf, i++, len, dest[0], 0);
 			fill_cfd_frame(cf, i++, len, dest[1], 0);
-			fill_cfd_frame(cf, i++, len, dest[1], 0);
+			fill_cfd_frame(cf, i++, len, dest[1], 1);
 		} else {
 			dev_err(re_chan->dev, "PQ tx continuation error!\n");
 			return NULL;
 		}
+	} else {
+		/* Mark the last source buffer frame final */
+		fill_cfd_frame(cf, i - 1, len, src[j - 1], 1);
 	}
 
-	/* Setting the final bit in the last source buffer frame in CFD */
-	cf[i - 1].efrl32 |= 1 << FSL_RE_CF_FINAL_SHIFT;
-
 	return &desc->async_tx;
 }
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
  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-14 23:38 ` Rosen Penev
  2026-07-16 17:02   ` Frank Li
  2026-07-14 23:38 ` [PATCH 3/3] dma: fsl_raid: use devm_platform_ioremap_resource Rosen Penev
  2 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2026-07-14 23:38 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, Frank Li, open list

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.

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] dma: fsl_raid: use devm_platform_ioremap_resource
  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-14 23:38 ` [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access Rosen Penev
@ 2026-07-14 23:38 ` Rosen Penev
  2026-07-16 16:53   ` Frank Li
  2 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2026-07-14 23:38 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, Frank Li, open list

Replace the open-coded platform_get_resource() plus devm_ioremap()
sequence with devm_platform_ioremap_resource(), which fetches the
resource, requests the region and maps it in one call. Switch the error
check to IS_ERR()/PTR_ERR() and drop the now-unused struct resource
pointer.

The raideng node has a single reg region (0x320000, 0x10000); the
job-queue/ring children are separate OF devices probed independently, so
the region reservation added by devm_ioremap_resource() is exclusive and
does not introduce overlap failures.

Assisted-by: opencode:hy3-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsl_raid.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index 47ebdf274331..aadb3dcb1b03 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -747,24 +747,20 @@ static int fsl_re_probe(struct platform_device *ofdev)
 	u32 off;
 	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;
 
+	/* IOMAP the entire RAID Engine region */
+	re_regs = devm_platform_ioremap_resource(ofdev, 0);
+	if (IS_ERR(re_regs))
+		return PTR_ERR(re_regs);
+
 	re_priv = devm_kzalloc(dev, sizeof(*re_priv), GFP_KERNEL);
 	if (!re_priv)
 		return -ENOMEM;
 
-	res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
-
-	/* IOMAP the entire RAID Engine region */
-	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;
+	re_priv->re_regs = re_regs;
 
 	/* Program the RE mode */
 	out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dma: fsl_raid: fix endianness of in-memory descriptor stores
  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
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-07-16 16:51 UTC (permalink / raw)
  To: Rosen Penev; +Cc: dmaengine, Vinod Koul, Frank Li, open list

On Tue, Jul 14, 2026 at 04:38:53PM -0700, Rosen Penev wrote:
> The descriptor structs (fsl_re_cmpnd_frame / fsl_re_hw_desc) are
> in-memory but their fields are __be32, because the structures are handed
> to the device as big-endian. The driver stored CPU-endian u32 values
> into them directly, which is both wrong (the engine would see
> byte-swapped lengths/addresses) and flagged by sparse as a base-type
> mismatch.
>
> Wrap those stores in cpu_to_be32() so the values are little->big
> converted. The final-frame bit is now passed as the "final" argument of
> fill_cfd_frame() (as fsl_re_prep_dma_memcpy already did) and set in CPU
> order before the single cpu_to_be32() store, replacing the previous
> read-modify-write of the __be32 efrl32 field.

Can you split two patches, one add cpu_le_be32(), other change to call
fill_cfd_frame()

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 | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 99945845d8b5..888f55b672a5 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -242,9 +242,9 @@ static void fill_cfd_frame(struct fsl_re_cmpnd_frame *cf, u8 index,
>  	u32 efrl = length & FSL_RE_CF_LENGTH_MASK;
>
>  	efrl |= final << FSL_RE_CF_FINAL_SHIFT;
> -	cf[index].efrl32 = efrl;
> -	cf[index].addr_high = upper_32_bits(addr);
> -	cf[index].addr_low = lower_32_bits(addr);
> +	cf[index].efrl32 = cpu_to_be32(efrl);
> +	cf[index].addr_high = cpu_to_be32(upper_32_bits(addr));
> +	cf[index].addr_low = cpu_to_be32(lower_32_bits(addr));
>  }
>
>  static struct fsl_re_desc *fsl_re_init_desc(struct fsl_re_chan *re_chan,
> @@ -256,9 +256,10 @@ static struct fsl_re_desc *fsl_re_init_desc(struct fsl_re_chan *re_chan,
>  	dma_async_tx_descriptor_init(&desc->async_tx, &re_chan->chan);
>  	INIT_LIST_HEAD(&desc->node);
>
> -	desc->hwdesc.fmt32 = FSL_RE_FRAME_FORMAT << FSL_RE_HWDESC_FMT_SHIFT;
> -	desc->hwdesc.lbea32 = upper_32_bits(paddr);
> -	desc->hwdesc.addr_low = lower_32_bits(paddr);
> +	desc->hwdesc.fmt32 = cpu_to_be32(FSL_RE_FRAME_FORMAT <<
> +					  FSL_RE_HWDESC_FMT_SHIFT);
> +	desc->hwdesc.lbea32 = cpu_to_be32(upper_32_bits(paddr));
> +	desc->hwdesc.addr_low = cpu_to_be32(lower_32_bits(paddr));
>  	desc->cf_addr = cf;
>  	desc->cf_paddr = paddr;
>
> @@ -374,11 +375,11 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
>  	for (i = 2, j = 0; j < save_src_cnt; i++, j++)
>  		fill_cfd_frame(cf, i, len, src[j], 0);
>
> +	/* Fill the last frame and mark it final */
>  	if (cont_q)
> -		fill_cfd_frame(cf, i++, len, dest, 0);
> -
> -	/* Setting the final bit in the last source buffer frame in CFD */
> -	cf[i - 1].efrl32 |= 1 << FSL_RE_CF_FINAL_SHIFT;
> +		fill_cfd_frame(cf, i, len, dest, 1);
> +	else
> +		fill_cfd_frame(cf, i - 1, len, src[j - 1], 1);
>
>  	return &desc->async_tx;
>  }
> @@ -504,16 +505,16 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_pq(
>  			p[save_src_cnt + 2] = 1;
>  			fill_cfd_frame(cf, i++, len, dest[0], 0);
>  			fill_cfd_frame(cf, i++, len, dest[1], 0);
> -			fill_cfd_frame(cf, i++, len, dest[1], 0);
> +			fill_cfd_frame(cf, i++, len, dest[1], 1);
>  		} else {
>  			dev_err(re_chan->dev, "PQ tx continuation error!\n");
>  			return NULL;
>  		}
> +	} else {
> +		/* Mark the last source buffer frame final */
> +		fill_cfd_frame(cf, i - 1, len, src[j - 1], 1);
>  	}
>
> -	/* Setting the final bit in the last source buffer frame in CFD */
> -	cf[i - 1].efrl32 |= 1 << FSL_RE_CF_FINAL_SHIFT;
> -
>  	return &desc->async_tx;
>  }
>
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] dma: fsl_raid: use devm_platform_ioremap_resource
  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
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-07-16 16:53 UTC (permalink / raw)
  To: Rosen Penev; +Cc: dmaengine, Vinod Koul, Frank Li, open list

On Tue, Jul 14, 2026 at 04:38:55PM -0700, Rosen Penev wrote:
> Replace the open-coded platform_get_resource() plus devm_ioremap()
> sequence with devm_platform_ioremap_resource(), which fetches the
> resource, requests the region and maps it in one call. Switch the error
> check to IS_ERR()/PTR_ERR() and drop the now-unused struct resource
> pointer.
>
> The raideng node has a single reg region (0x320000, 0x10000); the
> job-queue/ring children are separate OF devices probed independently, so
> the region reservation added by devm_ioremap_resource() is exclusive and
> does not introduce overlap failures.
>
> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/fsl_raid.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 47ebdf274331..aadb3dcb1b03 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -747,24 +747,20 @@ static int fsl_re_probe(struct platform_device *ofdev)
>  	u32 off;
>  	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;
>
> +	/* IOMAP the entire RAID Engine region */
> +	re_regs = devm_platform_ioremap_resource(ofdev, 0);
> +	if (IS_ERR(re_regs))
> +		return PTR_ERR(re_regs);
> +
>  	re_priv = devm_kzalloc(dev, sizeof(*re_priv), GFP_KERNEL);
>  	if (!re_priv)
>  		return -ENOMEM;
>
> -	res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> -	if (!res)
> -		return -ENODEV;
> -
> -	/* IOMAP the entire RAID Engine region */
> -	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;
> +	re_priv->re_regs = re_regs;
>
>  	/* Program the RE mode */
>  	out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
  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
  2026-07-16 18:35     ` Rosen Penev
  0 siblings, 1 reply; 9+ messages in thread
From: Frank Li @ 2026-07-16 17:02 UTC (permalink / raw)
  To: Rosen Penev; +Cc: dmaengine, Vinod Koul, Frank Li, open list

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
  2026-07-16 17:02   ` Frank Li
@ 2026-07-16 18:35     ` Rosen Penev
  2026-07-16 18:58       ` Frank Li
  0 siblings, 1 reply; 9+ messages in thread
From: Rosen Penev @ 2026-07-16 18:35 UTC (permalink / raw)
  To: Frank Li; +Cc: dmaengine, Vinod Koul, Frank Li, open list

On Thu, Jul 16, 2026 at 10:03 AM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> 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?
Closest thing I can find is

https://lkml.indiana.edu/hypermail/linux/kernel/0409.1/2592.html

As the link mentions, it's real nice to avoid the unnecessary casting in

chan->jrregs = (struct fsl_re_chan_cfg *)((u8 *)re_priv->re_regs +
               off + ptr);

sparse would flag that anyway. __iomem would need to be added to both pointers.
>
> 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
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] dma: fsl_raid: keep MMIO bases as void __iomem and cast at access
  2026-07-16 18:35     ` Rosen Penev
@ 2026-07-16 18:58       ` Frank Li
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Li @ 2026-07-16 18:58 UTC (permalink / raw)
  To: Rosen Penev; +Cc: dmaengine, Vinod Koul, Frank Li, open list

On Thu, Jul 16, 2026 at 11:35:31AM -0700, Rosen Penev wrote:
> On Thu, Jul 16, 2026 at 10:03 AM Frank Li <Frank.li@oss.nxp.com> wrote:
> >
> > 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?
> Closest thing I can find is
>
> https://lkml.indiana.edu/hypermail/linux/kernel/0409.1/2592.html
>
> As the link mentions, it's real nice to avoid the unnecessary casting in
>
> chan->jrregs = (struct fsl_re_chan_cfg *)((u8 *)re_priv->re_regs +
>                off + ptr);
>
> sparse would flag that anyway. __iomem would need to be added to both pointers.

I see.
...
> > >               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;

Suggest rename 're_regs' to 'base' to disinguish easily typed or void pointer.


> > >       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;

struct fsl_re_chan_cfg __iomem *jregs;

reduce many change. still can use
	chan->jrregs = re_priv->base + off + ptr

Frank

> > >       int irq;
> > >       struct tasklet_struct irqtask;
> > >       u32 alloc_count;
> > > --
> > > 2.55.0
> > >

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-16 18:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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®