Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Probe error paths leave hardware interrupts unmasked, potentially causing an interrupt storm if a subsequent step fails or defers probe.
- [High] `sata_dwc_probe()` silently ignores `ata_host_activate()` failures, falsely reporting success and skipping cleanup.
- [High] Probe error path calls `phy_exit()` when `phy_init()` fails, corrupting the PHY's internal state.
- [High] Missing cleanup of old DMA resources on probe error paths.
--

commit 38e92fff8b9d315da09f885e72d12d704d5b7b50
Author: Rosen Penev <rosenp@gmail.com>

ata: sata_dwc_460ex: use platform_get_irq()

Replace irq_of_parse_and_map() with platform_get_irq() in both
sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
way to obtain IRQs for platform devices and provides better error
reporting.

> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 4fc22ce4bd9aa..35aa7f9acdf75 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
[ ... ]
> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
>  	sata_dwc_enable_interrupts(hsdev);
>  
>  	/* Get SATA interrupt number */
> -	irq = irq_of_parse_and_map(np, 0);
> -	if (!irq) {
> -		dev_err(dev, "no SATA DMA irq\n");
> -		return -ENODEV;
> -	}
> +	irq = platform_get_irq(ofdev, 0);
> +	if (irq < 0)
> +		return irq;

[Severity: High]
This is a pre-existing issue, but does returning directly here leave hardware
interrupts unmasked?

Because sata_dwc_enable_interrupts() is called right before obtaining the
IRQ, if platform_get_irq() returns -EPROBE_DEFER, probe will fail while
interrupts remain enabled on the hardware. Could this lead to an unhandled
interrupt storm?

>  
>  #ifdef CONFIG_SATA_DWC_OLD_DMA
>  	if (!of_property_present(np, "dmas")) {

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing cleanup
for the old DMA resources on the error paths that follow this block?

If sata_dwc_dma_init_old() succeeds, but a subsequent operation like
phy_init() fails later in sata_dwc_probe(), does the driver leave the DMA
controller active and registered without calling a corresponding cleanup
function?

[Severity: High]
This is a pre-existing issue, but does jumping to the error_out label when
phy_init() fails corrupt the PHY's internal state?

Later in sata_dwc_probe(), when phy_init() fails, it jumps to error_out
which unconditionally calls phy_exit(). Calling phy_exit() will decrement
the internal init_count to -1, which can cause subsequent probe retries to
falsely assume the PHY is already initialized.

[Severity: High]
This isn't a bug introduced by this patch, but does the driver silently
ignore failures from ata_host_activate() at the end of sata_dwc_probe()?

If ata_host_activate() fails, it logs an error but still executes return 0;
instead of jumping to error_out. Does this falsely signal success to the
driver core and skip necessary cleanup, potentially leading to a crash on
unbind?

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