Re: [PATCH v2] ata: sata_dwc_460ex: Don't enable interrupt if libATA core registration fails

[Niklas Cassel <cassel@kernel.org>]

On Wed, Sep 09, 2026 at 11:38:21AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Inverted teardown sequence for the PHY on probe error path, calling phy_power_off() after phy_exit().

Looking at ahci_platform.c:ahci_probe()

ahci_probe() calls
ahci_platform_enable_resources() which calls ahci_platform_enable_phys()
which calls
phy_init() and phy_power_on().

ahci_probe() later calls
ahci_platform_init_host() which calls ahci_host_activate()

if ahci_platform_init_host() fails, ahci_probe() calls

calls ahci_platform_disable_resources(), which calls
phy_power_off() and and phy_exit().


So the problem is that sata_dwc_460ex calls phy_power_on() in
->port_stop.
(And that it calls phy_power_on() in ->port_start()).


These calls should just be in sata_dwc_probe().
I don't see any other driver doing phy_power_on()/phy_power_off()
in ->port_start()/->port_stop().

It seems very wrong... Because sata_dwc_460ex will call phy_power_off()
for the same phy, when having multiple ports. phy_power_off() should
not be called multiple times on the same phy.


This driver is quite broken.

You could just send a patch that does:

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 8e3fc713891a..0690e31719d2 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1185,11 +1185,12 @@ static int sata_dwc_probe(struct platform_device *ofdev)
         * error_handler() to execute a dummy Softreset EH session
         */
        err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
-       if (err)
+       if (err) {
                dev_err(dev, "failed to activate host");
-
-       /* Enable SATA Interrupts */
-       sata_dwc_enable_interrupts(hsdev);
+       } else {
+               /* Enable SATA Interrupts */
+               sata_dwc_enable_interrupts(hsdev);
+       }
        return 0;
 
 error_out:


Or if you want to clean this up, create a patch that moves
the phy_power_on() from ->port_start() to sata_dwc_probe()
and phy_power_off() from ->port_stop() to sata_dwc_probe().

Then you will be able to call goto error_out; on ata_host_activate() failure.


Kind regards,
Niklas