Devicetree
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x
@ 2026-08-23 23:56 Rosen Penev
  2026-08-24  0:08 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2026-08-23 23:56 UTC (permalink / raw)
  To: devicetree
  Cc: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Thomas Petazzoni, Jason Cooper,
	moderated list:ARM/Marvell Kirkwood and Armada 370, 375, 38x,...,
	open list

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.

Fixes: 6eccc52b448e ("ARM: mvebu: enable the SDHCI interface on Armada 385")
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 arch/arm/boot/dts/marvell/armada-38x.dtsi | 1 +
 1 file changed, 1 insertion(+)

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


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

* Re: [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x
  2026-08-23 23:56 [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x Rosen Penev
@ 2026-08-24  0:08 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-24  0:08 UTC (permalink / raw)
  To: Rosen Penev; +Cc: devicetree, robh, conor+dt

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

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

end of thread, other threads:[~2026-08-24  0:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-23 23:56 [PATCH] ARM: dts: marvell: add SDHCI clock-names to Armada 38x Rosen Penev
2026-08-24  0:08 ` sashiko-bot

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