DMA Engine development
 help / color / mirror / Atom feed
* [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be
@ 2026-07-18  3:31 Rosen Penev
  2026-07-18  3:46 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Rosen Penev @ 2026-07-18  3:31 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul, Frank Li, open list

Mechanical conversion of the ppc4xx-specific accessors to the generic
portable helpers.

As a result, enable COMPILE_TEST for extra compile coverage.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/Kconfig    |  2 +-
 drivers/dma/fsl_raid.c | 50 ++++++++++++++++++------------------------
 2 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index cb92687294c3..dbdb35f6610a 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -242,7 +242,7 @@ config FSL_QDMA
 
 config FSL_RAID
 	tristate "Freescale RAID engine Support"
-	depends on FSL_SOC && !ASYNC_TX_ENABLE_CHANNEL_SWITCH
+	depends on (FSL_SOC && !ASYNC_TX_ENABLE_CHANNEL_SWITCH) || COMPILE_TEST
 	select DMA_ENGINE
 	select DMA_ENGINE_RAID
 	help
diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index 2d86f61105e5..e559c636bf55 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -114,7 +114,7 @@ static void fsl_re_issue_pending(struct dma_chan *chan)
 
 	spin_lock_irqsave(&re_chan->desc_lock, flags);
 	avail = FSL_RE_SLOT_AVAIL(
-		in_be32(&re_chan->jrregs->inbring_slot_avail));
+		ioread32be(&re_chan->jrregs->inbring_slot_avail));
 
 	list_for_each_entry_safe(desc, _desc, &re_chan->submit_q, node) {
 		if (!avail)
@@ -127,7 +127,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));
+		iowrite32be(FSL_RE_ADD_JOB(1), &re_chan->jrregs->inbring_add_job);
 		avail--;
 	}
 	spin_unlock_irqrestore(&re_chan->desc_lock, flags);
@@ -167,7 +167,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(ioread32be(&re_chan->jrregs->oubring_slot_full));
 	while (count--) {
 		found = 0;
 		hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
@@ -192,8 +192,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));
+		iowrite32be(FSL_RE_RMVD_JOB(1), &re_chan->jrregs->oubring_job_rmvd);
 	}
 	spin_unlock_irqrestore(&re_chan->desc_lock, flags);
 }
@@ -206,7 +205,7 @@ static irqreturn_t fsl_re_isr(int irq, void *data)
 
 	re_chan = dev_get_drvdata((struct device *)data);
 
-	irqstate = in_be32(&re_chan->jrregs->jr_interrupt_status);
+	irqstate = ioread32be(&re_chan->jrregs->jr_interrupt_status);
 	if (!irqstate)
 		return IRQ_NONE;
 
@@ -216,13 +215,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 = ioread32be(&re_chan->jrregs->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);
+	iowrite32be(FSL_RE_CLR_INTR, &re_chan->jrregs->jr_interrupt_status);
 
 	tasklet_schedule(&re_chan->irqtask);
 
@@ -708,30 +707,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);
+	iowrite32be(chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->inbring_base_h);
+	iowrite32be(chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->oubring_base_h);
+	iowrite32be(chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->inbring_base_l);
+	iowrite32be(chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->oubring_base_l);
+	iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->inbring_size);
+	iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->oubring_size);
 
 	/* Read LIODN value from u-boot */
-	status = in_be32(&chan->jrregs->jr_config_1) & FSL_RE_REG_LIODN_MASK;
+	status = ioread32be(&chan->jrregs->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);
+	iowrite32be(FSL_RE_CFG1_CBSI | FSL_RE_CFG1_CBS0 | status, &chan->jrregs->jr_config_1);
 
 	dev_set_drvdata(chandev, chan);
 
 	/* Enable RE/CHAN */
-	out_be32(&chan->jrregs->jr_command, FSL_RE_ENABLE);
+	iowrite32be(FSL_RE_ENABLE, &chan->jrregs->jr_command);
 
 	return 0;
 
@@ -766,15 +758,15 @@ static int fsl_re_probe(struct platform_device *ofdev)
 	re_priv->base = re_regs;
 
 	/* Program the RE mode */
-	out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
+	iowrite32be(FSL_RE_NON_DPAA_MODE, &re_regs->global_config);
 
 	/* Program Galois Field polynomial */
-	out_be32(&re_regs->galois_field_config, FSL_RE_GFM_POLY);
+	iowrite32be(FSL_RE_GFM_POLY, &re_regs->galois_field_config);
 
 	dev_info(dev, "version %x, mode %x, gfp %x\n",
-		 in_be32(&re_regs->re_version_id),
-		 in_be32(&re_regs->global_config),
-		 in_be32(&re_regs->galois_field_config));
+		 ioread32be(&re_regs->re_version_id),
+		 ioread32be(&re_regs->global_config),
+		 ioread32be(&re_regs->galois_field_config));
 
 	dma_dev = &re_priv->dma_dev;
 	dma_dev->dev = dev;
-- 
2.55.0


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

* Re: [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be
  2026-07-18  3:31 [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be Rosen Penev
@ 2026-07-18  3:46 ` sashiko-bot
  2026-07-18  5:31   ` Rosen Penev
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-18  3:46 UTC (permalink / raw)
  To: Rosen Penev; +Cc: vkoul, dmaengine, Frank.Li

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `request_irq` is used without `devm_` and is never freed on error paths or during device removal, leading to Use-After-Free in the interrupt handler.
- [High] The driver configures a 40-bit DMA mask but incorrectly programs the 64-bit ring base addresses, truncating upper address bits and incorrectly placing the lowest 4 bits into the high address register, causing silent memory corruption for high memory allocations.
- [High] Deadlock in `fsl_re_dequeue` due to invoking DMA client callback with the channel spinlock held.
- [High] Unchecked return value of `fsl_re_chan_probe` causes a NULL pointer dereference during device removal.
--

--- Patch [1]: [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be ---
commit 9a55b4c8c34b72fe5484852d6708a7ea677fc0df
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be

This commit replaces ppc4xx-specific accessors (in_be32/out_be32) with
generic portable helpers (ioread32be/iowrite32be) in the fsl_raid driver.
It also enables COMPILE_TEST for extra compile coverage.

> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 2d86f61105e52..e559c636bf552 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
[ ... ]
> @@ -167,7 +167,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(ioread32be(&re_chan->jrregs->oubring_slot_full));

[Severity: High]
This is a pre-existing issue, but can this lead to a deadlock?

Looking at fsl_re_dequeue(), it acquires the desc_lock spinlock using
spin_lock_irqsave() and holds it while processing completed descriptors:

spin_lock_irqsave(&re_chan->desc_lock, flags);
...
if (found) {
        fsl_re_desc_done(desc);
        list_move_tail(&desc->node, &re_chan->ack_q);
}

The fsl_re_desc_done() function synchronously invokes the client callback
via dmaengine_desc_get_callback_invoke().

If the DMA client callback submits a new request, would it call
fsl_re_tx_submit() and attempt to reacquire the exact same desc_lock,
resulting in a deadlock?

>  	while (count--) {
>  		found = 0;
>  		hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
[ ... ]
> @@ -708,30 +707,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);
> +	iowrite32be(chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->inbring_base_h);
> +	iowrite32be(chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->oubring_base_h);
> +	iowrite32be(chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->inbring_base_l);
> +	iowrite32be(chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->oubring_base_l);

[Severity: High]
This is a pre-existing issue, but does this correctly program the 64-bit
DMA base addresses?

The driver advertises 40-bit DMA address support in fsl_re_probe() by
calling:

dma_set_mask(dev, DMA_BIT_MASK(40));

However, when writing the physical addresses here in fsl_re_chan_probe(), it
shifts the address right by FSL_RE_ADDR_BIT_SHIFT into inbring_base_l, and
extracts the lowest bits via FSL_RE_ADDR_BIT_MASK into inbring_base_h.

If a DMA buffer is allocated above the 64GB boundary, will this drop the
upper bits and extract the lowest bits into the high register, leading to
the hardware reading or writing the wrong physical memory?

[Severity: Critical]
This is a pre-existing issue, but is there a missing IRQ cleanup here?

Earlier in fsl_re_chan_probe(), the driver requests an interrupt without
using the managed devm_ wrapper:

ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);

If fsl_re_chan_probe() fails later, or if the driver is unbound and
fsl_re_remove_chan() is called, free_irq() is never called to unregister
the handler.

Since the chan structure is allocated with devm_kzalloc, will a dangling
interrupt firing after the device is removed cause fsl_re_isr() to
dereference the freed chan pointer and crash?

> +	iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->inbring_size);
> +	iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->oubring_size);
[ ... ]
> @@ -766,15 +758,15 @@ static int fsl_re_probe(struct platform_device *ofdev)
>  	re_priv->base = re_regs;
>  
>  	/* Program the RE mode */
> -	out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> +	iowrite32be(FSL_RE_NON_DPAA_MODE, &re_regs->global_config);

[Severity: High]
This is a pre-existing issue, but are we missing an error check on the
return value of fsl_re_chan_probe()?

Later in fsl_re_probe(), the driver iterates over child nodes and calls
fsl_re_chan_probe(), but ignores the return value and unconditionally
increments total_chans:

for_each_child_of_node(np, child) {
        rc = of_device_is_compatible(child,
                                     "fsl,raideng-v1.0-job-ring");
        if (rc) {
                fsl_re_chan_probe(ofdev, child, ridx++, off);
                re_priv->total_chans++;
        }
}

If fsl_re_chan_probe() fails (for example, failing to map an IRQ), the array
element re_priv->re_jrs[ridx] remains uninitialized.

During module unload, fsl_re_remove_chan() unconditionally dereferences the
pointer when calling tasklet_kill(&chan->irqtask).

Will this result in a NULL pointer dereference on cleanup if any channel
failed to probe?

>  
>  	/* Program Galois Field polynomial */
> -	out_be32(&re_regs->galois_field_config, FSL_RE_GFM_POLY);
> +	iowrite32be(FSL_RE_GFM_POLY, &re_regs->galois_field_config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718033122.2372985-1-rosenp@gmail.com?part=1

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

* Re: [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be
  2026-07-18  3:46 ` sashiko-bot
@ 2026-07-18  5:31   ` Rosen Penev
  0 siblings, 0 replies; 3+ messages in thread
From: Rosen Penev @ 2026-07-18  5:31 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: vkoul, dmaengine, Frank.Li

On Fri, Jul 17, 2026 at 8:47 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> Pre-existing issues:
Based on these. I think this should be defered.
> - [Critical] `request_irq` is used without `devm_` and is never freed on error paths or during device removal, leading to Use-After-Free in the interrupt handler.
> - [High] The driver configures a 40-bit DMA mask but incorrectly programs the 64-bit ring base addresses, truncating upper address bits and incorrectly placing the lowest 4 bits into the high address register, causing silent memory corruption for high memory allocations.
> - [High] Deadlock in `fsl_re_dequeue` due to invoking DMA client callback with the channel spinlock held.
> - [High] Unchecked return value of `fsl_re_chan_probe` causes a NULL pointer dereference during device removal.
> --
>
> --- Patch [1]: [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be ---
> commit 9a55b4c8c34b72fe5484852d6708a7ea677fc0df
> Author: Rosen Penev <rosenp@gmail.com>
>
> dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be
>
> This commit replaces ppc4xx-specific accessors (in_be32/out_be32) with
> generic portable helpers (ioread32be/iowrite32be) in the fsl_raid driver.
> It also enables COMPILE_TEST for extra compile coverage.
>
> > diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> > index 2d86f61105e52..e559c636bf552 100644
> > --- a/drivers/dma/fsl_raid.c
> > +++ b/drivers/dma/fsl_raid.c
> [ ... ]
> > @@ -167,7 +167,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(ioread32be(&re_chan->jrregs->oubring_slot_full));
>
> [Severity: High]
> This is a pre-existing issue, but can this lead to a deadlock?
>
> Looking at fsl_re_dequeue(), it acquires the desc_lock spinlock using
> spin_lock_irqsave() and holds it while processing completed descriptors:
>
> spin_lock_irqsave(&re_chan->desc_lock, flags);
> ...
> if (found) {
>         fsl_re_desc_done(desc);
>         list_move_tail(&desc->node, &re_chan->ack_q);
> }
>
> The fsl_re_desc_done() function synchronously invokes the client callback
> via dmaengine_desc_get_callback_invoke().
>
> If the DMA client callback submits a new request, would it call
> fsl_re_tx_submit() and attempt to reacquire the exact same desc_lock,
> resulting in a deadlock?
>
> >       while (count--) {
> >               found = 0;
> >               hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
> [ ... ]
> > @@ -708,30 +707,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);
> > +     iowrite32be(chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->inbring_base_h);
> > +     iowrite32be(chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK, &chan->jrregs->oubring_base_h);
> > +     iowrite32be(chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->inbring_base_l);
> > +     iowrite32be(chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT, &chan->jrregs->oubring_base_l);
>
> [Severity: High]
> This is a pre-existing issue, but does this correctly program the 64-bit
> DMA base addresses?
>
> The driver advertises 40-bit DMA address support in fsl_re_probe() by
> calling:
>
> dma_set_mask(dev, DMA_BIT_MASK(40));
>
> However, when writing the physical addresses here in fsl_re_chan_probe(), it
> shifts the address right by FSL_RE_ADDR_BIT_SHIFT into inbring_base_l, and
> extracts the lowest bits via FSL_RE_ADDR_BIT_MASK into inbring_base_h.
>
> If a DMA buffer is allocated above the 64GB boundary, will this drop the
> upper bits and extract the lowest bits into the high register, leading to
> the hardware reading or writing the wrong physical memory?
>
> [Severity: Critical]
> This is a pre-existing issue, but is there a missing IRQ cleanup here?
>
> Earlier in fsl_re_chan_probe(), the driver requests an interrupt without
> using the managed devm_ wrapper:
>
> ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);
>
> If fsl_re_chan_probe() fails later, or if the driver is unbound and
> fsl_re_remove_chan() is called, free_irq() is never called to unregister
> the handler.
>
> Since the chan structure is allocated with devm_kzalloc, will a dangling
> interrupt firing after the device is removed cause fsl_re_isr() to
> dereference the freed chan pointer and crash?
>
> > +     iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->inbring_size);
> > +     iowrite32be(FSL_RE_RING_SIZE << FSL_RE_RING_SIZE_SHIFT, &chan->jrregs->oubring_size);
> [ ... ]
> > @@ -766,15 +758,15 @@ static int fsl_re_probe(struct platform_device *ofdev)
> >       re_priv->base = re_regs;
> >
> >       /* Program the RE mode */
> > -     out_be32(&re_regs->global_config, FSL_RE_NON_DPAA_MODE);
> > +     iowrite32be(FSL_RE_NON_DPAA_MODE, &re_regs->global_config);
>
> [Severity: High]
> This is a pre-existing issue, but are we missing an error check on the
> return value of fsl_re_chan_probe()?
>
> Later in fsl_re_probe(), the driver iterates over child nodes and calls
> fsl_re_chan_probe(), but ignores the return value and unconditionally
> increments total_chans:
>
> for_each_child_of_node(np, child) {
>         rc = of_device_is_compatible(child,
>                                      "fsl,raideng-v1.0-job-ring");
>         if (rc) {
>                 fsl_re_chan_probe(ofdev, child, ridx++, off);
>                 re_priv->total_chans++;
>         }
> }
>
> If fsl_re_chan_probe() fails (for example, failing to map an IRQ), the array
> element re_priv->re_jrs[ridx] remains uninitialized.
>
> During module unload, fsl_re_remove_chan() unconditionally dereferences the
> pointer when calling tasklet_kill(&chan->irqtask).
>
> Will this result in a NULL pointer dereference on cleanup if any channel
> failed to probe?
>
> >
> >       /* Program Galois Field polynomial */
> > -     out_be32(&re_regs->galois_field_config, FSL_RE_GFM_POLY);
> > +     iowrite32be(FSL_RE_GFM_POLY, &re_regs->galois_field_config);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718033122.2372985-1-rosenp@gmail.com?part=1

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18  3:31 [PATCH] dmaengine: fsl_raid: replace in_be32/out_be32 with ioread32be/iowrite32be Rosen Penev
2026-07-18  3:46 ` sashiko-bot
2026-07-18  5:31   ` Rosen Penev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox