All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Rosen Penev" <rosenp@gmail.com>
Cc: vkoul@kernel.org, Frank.Li@kernel.org, dmaengine@vger.kernel.org
Subject: Re: [PATCHv8] dmaengine: fsl_raid: check fsl_re_chan_probe() return value
Date: Mon, 21 Sep 2026 22:36:48 +0000	[thread overview]
Message-ID: <20260921223648.D7DF31F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260921222544.52655-1-rosenp@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: fsl_raid: unconditional total_chans increment contradicts commit
--

commit 208ae247c55a1b6bf4d94121e4104ab723b71bae
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsl_raid: check fsl_re_chan_probe() return value

The commit adds a check for fsl_re_chan_probe() failure in fsl_re_probe()
to prevent leaving uninitialized array elements in re_jrs[]. It also adds
a cleanup path to correctly tear down channels if an error occurs.

> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 2d86f61105e52..352e8cc9e8c71 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
[ ... ]
> @@ -824,33 +839,46 @@ static int fsl_re_probe(struct platform_device *ofdev)
[ ... ]
>  		/* Find out the Job Rings present under each JQ */
>  		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 (!of_device_is_compatible(child, "fsl,raideng-v1.0-job-ring"))
> +				continue;
> +
> +			if (ridx >= FSL_RE_MAX_CHANS) {
> +				dev_warn(dev, "too many job rings, max %d\n", FSL_RE_MAX_CHANS);
> +				of_node_put(child);
> +				break;
>  			}
> +
> +			rc = fsl_re_chan_probe(ofdev, child, ridx, off);
> +			if (rc)
> +				dev_err(dev, "job ring %d probe failed: %d\n", ridx, rc);
> +			re_priv->total_chans++;
> +			ridx++;
>  		}

[Severity: Medium]
The commit message states "only count successfully probed channels", but does
this code still unconditionally increment the counters?

If fsl_re_chan_probe() fails and returns a non-zero rc, it logs an error but 
then falls through to increment re_priv->total_chans and ridx since there is 
no continue statement. This advances the counters even for failed probes,
wasting slots in the fixed-size re_jrs array. Should this include a continue
when rc is non-zero?

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

      reply	other threads:[~2026-09-21 22:36 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21 22:25 [PATCHv8] dmaengine: fsl_raid: check fsl_re_chan_probe() return value Rosen Penev
2026-09-21 22:36 ` sashiko-bot [this message]

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=20260921223648.D7DF31F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=rosenp@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --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 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.