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

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/09/17 18:35, Nikolay Borisov wrote:
> Recent 4bbc16a353a9 commit changed the timing when interrupts are
> enabled. However it didn't account for the libATA registration step. So
> it's possible that interrupts are enabled when the device is not
> registered with libATA core.
> 
> Resolve the issue by ensuring that interrupts are enabled only if the rest of the
> initialization is successful.
> 
> Fixes: 4bbc16a353a9 ("ata: sata_dwc_460ex: enable SATA interrupts only after IRQ handler is registered")
> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
> ---
> 
> 
> Changes since v1:
> 	* Added proper goto label
> Changes since v2:
>     * Rework logic so that interrupts are enabled if everything is successful
> 	and don't call phy_exit otherwise, as the driver is quite broken w.r.t to it.
> 
>  drivers/ata/sata_dwc_460ex.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 8e3fc713891a..e74335dec42a 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -1185,11 +1185,13 @@ 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");

Why not simply "goto err_out;" here ?

> +	} else {
> +		/* Enable SATA Interrupts */
> +		sata_dwc_enable_interrupts(hsdev);
> +	}
>  
> -	/* Enable SATA Interrupts */
> -	sata_dwc_enable_interrupts(hsdev);
>  	return 0;
>  
>  error_out:
> --
> 2.55.0


-- 
Damien Le Moal
Western Digital Research