Linux ATA/IDE development
 help / color / mirror / Atom feed
* [PATCH] ata: sata_mv: accept 1 or 2 resources in platform probe
@ 2026-07-12 22:31 Rosen Penev
  2026-07-12 22:38 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2026-07-12 22:31 UTC (permalink / raw)
  To: linux-ide; +Cc: Damien Le Moal, Niklas Cassel, Andrew Lunn, open list

Board files in arch/arm/plat-orion, arch/arm/mach-dove,
arch/arm/mach-mv78xx0 and arch/arm/mach-orion5x still register the
"sata_mv" device with two resources (IORESOURCE_MEM plus IORESOURCE_IRQ).
Those devices are rejected with -EINVAL, so SATA no longer probes on
legacy Marvell Orion/Kirkwood-style boards.

Accept both 1 resource (DT, IRQ fetched via platform_get_irq()) and 2
resources (legacy, IRQ supplied as a second resource) so both probing
paths work.

Fixes: b3b2bec9646e ("ata: sata_mv: Fixes expected number of resources now IRQs are gone")
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/sata_mv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 0de4f9b8e811..d260f04e6384 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4027,7 +4027,7 @@ static int mv_platform_probe(struct platform_device *pdev)
 	/*
 	 * Simple resource validation ..
 	 */
-	if (unlikely(pdev->num_resources != 1)) {
+	if (unlikely(pdev->num_resources != 1 && pdev->num_resources != 2)) {
 		dev_err(&pdev->dev, "invalid number of resources\n");
 		return -EINVAL;
 	}
-- 
2.55.0


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

* Re: [PATCH] ata: sata_mv: accept 1 or 2 resources in platform probe
  2026-07-12 22:31 [PATCH] ata: sata_mv: accept 1 or 2 resources in platform probe Rosen Penev
@ 2026-07-12 22:38 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-12 22:38 UTC (permalink / raw)
  To: Rosen Penev; +Cc: linux-ide, cassel, dlemoal

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

Pre-existing issues:
- [High] Missing error check for port clock clk_prepare_enable() leads to unbalanced disable and potential hardware faults.
--

--- Patch [1]: [PATCH] ata: sata_mv: accept 1 or 2 resources in platform probe ---
commit 079963d555bbd8a5581f9c80568deb335571afa8
Author: Rosen Penev <rosenp@gmail.com>

ata: sata_mv: accept 1 or 2 resources in platform probe

This commit modifies mv_platform_probe() to accept either 1 or 2 resources,
allowing the SATA driver to probe successfully on legacy Marvell
Orion/Kirkwood-style boards.

> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 41647a56a9f4e..365454390d7e0 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4027,7 +4027,7 @@ static int mv_platform_probe(struct platform_device *pdev)
>  	/*
>  	 * Simple resource validation ..
>  	 */
> -	if (unlikely(pdev->num_resources != 1)) {
> +	if (unlikely(pdev->num_resources != 1 && pdev->num_resources != 2)) {
>  		dev_err(&pdev->dev, "invalid number of resources\n");
>  		return -EINVAL;
>  	}

[Severity: High]
This is a pre-existing issue, but while reviewing this function I noticed
the return value of clk_prepare_enable() for port clocks is ignored further
down in mv_platform_probe():

drivers/ata/sata_mv.c:mv_platform_probe() {
...
        hpriv->port_clks[port] = clk_get(&pdev->dev, port_number);
        if (!IS_ERR(hpriv->port_clks[port]))
            clk_prepare_enable(hpriv->port_clks[port]);
...
}

Does this code risk causing a synchronous external abort on ARM systems if
the port clock fails to enable? If the driver proceeds with hardware
initialization while the IP block is unclocked, it could lead to a kernel
panic upon register access.

Additionally, if clk_prepare_enable() fails or another error occurs during
probe, the cleanup loop unconditionally attempts to disable and unprepare
the clock:

drivers/ata/sata_mv.c:mv_platform_probe() {
...
err:
...
    for (port = 0; port < hpriv->n_ports; port++) {
        if (!IS_ERR(hpriv->port_clks[port])) {
            clk_disable_unprepare(hpriv->port_clks[port]);
            clk_put(hpriv->port_clks[port]);
        }
...
}

Could this unbalanced disable cause a WARN_ON during the error path since
the clock was never successfully enabled?

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

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

end of thread, other threads:[~2026-07-12 22:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12 22:31 [PATCH] ata: sata_mv: accept 1 or 2 resources in platform probe Rosen Penev
2026-07-12 22:38 ` sashiko-bot

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