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

[Nikolay Borisov <nik.borisov@suse.com>]

On 17.09.26 г. 16:18 ч., Damien Le Moal wrote:
> 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 ?

That was in V2 however Niklas discovered some issue w.r.t to phy_exit. 
https://lore.kernel.org/all/aqGFjsuIMRIy5xfv@ryzen/
> 
>> +	} else {
>> +		/* Enable SATA Interrupts */
>> +		sata_dwc_enable_interrupts(hsdev);
>> +	}
>>   
>> -	/* Enable SATA Interrupts */
>> -	sata_dwc_enable_interrupts(hsdev);
>>   	return 0;
>>   
>>   error_out:
>> --
>> 2.55.0
> 
>