Re: [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver ignores the return value of `clk_prepare_enable()` for its primary clocks, risking unclocked register accesses.
- [High] The driver improperly handles `-EPROBE_DEFER` from `devm_pinctrl_get()`, allowing the probe to succeed without necessary pins.
- [High] The driver ignores the return value of `pm_runtime_get_sync()` in suspend, resume, and remove callbacks.
--

--- Patch [1]: [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x ---
commit 00932f716bef42c94516ea3abd25e12b0e972f6b
Author: Rosen Penev <rosenp@gmail.com>

ARM: dts: marvell: add SDHCI clock-names to Armada 38x

The marvell,armada-380-sdhci binding requires clock-names, but the
Armada 38x SDHCI node only carries an unnamed clock reference.
Add the missing clock name so the node passes schema validation.

> diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> index 1d616edda322d..0618779090d04 100644
> --- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> @@ -627,6 +627,7 @@ sdhci: sdhci@d8000 {
>  					<0x18454 0x4>;
>  				interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&gateclk 17>;
> +				clock-names = "io";
>  				mrvl,clk-delay-cycles = <0x1F>;
>  				status = "disabled";
>  			};

[Severity: High]
This is a pre-existing issue, but does the host driver ignore the return
value of clk_prepare_enable() for its primary clocks?

Looking at drivers/mmc/host/sdhci-pxav3.c in sdhci_pxav3_probe():

    clk_prepare_enable(pxa->clk_io);
    ...
    clk_prepare_enable(pxa->clk_core);

And in sdhci_pxav3_runtime_resume():

    clk_prepare_enable(pxa->clk_io);
    clk_prepare_enable(pxa->clk_core);

If clock enablement fails, could the hardware remain unclocked and cause
subsequent register accesses to trigger a bus hang?

[Severity: High]
This is a pre-existing issue, but does the driver improperly handle
-EPROBE_DEFER?

Looking at drivers/mmc/host/sdhci-pxav3.c in sdhci_pxav3_probe():

    } else {
        dev_dbg(dev, "could not get pinctrl handle: %ld\n", PTR_ERR(pxa->pinctrl));
    }

If devm_pinctrl_get() returns -EPROBE_DEFER, the driver drops the error code
and allows the probe to succeed without necessary pins. Will this cause
high-speed SD/MMC cards to fail tuning or experience data corruption when
the driver attempts to switch to high-speed UHS modes?

[Severity: High]
This is a pre-existing issue, but are the return values of
pm_runtime_get_sync() ignored during power management transitions?

Looking at drivers/mmc/host/sdhci-pxav3.c in sdhci_pxav3_remove():

    pm_runtime_get_sync(&pdev->dev);

This also happens in sdhci_pxav3_suspend():

    pm_runtime_get_sync(dev);

And in sdhci_pxav3_resume():

    pm_runtime_get_sync(dev);

If pm_runtime_get_sync() fails and returns a negative error code, does the
driver unconditionally proceed to call the host functions which read and
write host controller registers?

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