Re: [PATCH v6 3/5] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changing from `devm_clk_get_enabled` to `devm_clk_bulk_get_optional` silently ignores missing DMAMUX clocks, which were previously mandatory and can lead to synchronous bus errors.
--

commit 7885b2482d67d70ba7ccdebb1cd210d1f8dc8ba2
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API

Convert the DMAMUX clock management from individual clock operations to the
bulk clock API to simplify the code. Prepare to add edma engine runtime pm
support.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
[ ... ]
> @@ -751,23 +750,37 @@ static int fsl_edma_probe(struct platform_device *pdev)
[ ... ]
> -		fsl_edma->muxclk[i] = devm_clk_get_enabled(&pdev->dev, clkname);
> -		if (IS_ERR(fsl_edma->muxclk[i]))
> -			return dev_err_probe(&pdev->dev,
> -					     PTR_ERR(fsl_edma->muxclk[i]),
> -					     "Missing DMAMUX block clock.\n");
> +		ret = devm_clk_bulk_get_optional(&pdev->dev, fsl_edma->drvdata->dmamuxs,
> +						 fsl_edma->muxclk);

[Severity: High]
Does this change to an optional clock fetch risk a synchronous bus error?

Previously, devm_clk_get_enabled() was used, which would fail probe if the
clock was missing. By switching to devm_clk_bulk_get_optional(), a missing
DMAMUX clock in the device tree will be silently ignored.

If the clocks are missing, devm_clk_bulk_get_optional() returns 0 and sets
the clock pointers to NULL. The subsequent clk_bulk_prepare_enable() call
will succeed safely. However, the driver will later unconditionally access the
unclocked DMAMUX registers:

drivers/dma/fsl-edma-common.c:fsl_edma_chan_mux() {
    ...
    if (fsl_chan->edma->drvdata->flags & FSL_EDMA_DRV_CONFIG32)
        mux_configure32(fsl_chan, muxaddr, ch_off, slot, enable);
    else
        mux_configure8(fsl_chan, muxaddr, ch_off, slot, enable);
    ...
}

Could this lead to a kernel panic when accessing the unclocked registers?
Should this use devm_clk_bulk_get() instead so probe correctly fails for
missing mandatory clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=3