Re: [PATCH] ata: libata: preserve sata_spd link cache across ata_dev_init

[Damien Le Moal]

On 2026/10/15 7:30, Damien Le Moal wrote:
> On 2026/10/10 9:54, tongzhiwei wrote:
>> a hot-unplug goes through
>> ata_eh_schedule_probe() -> ata_dev_init() -> sata_spd = 0.
>> After commit 69f2c9346313 ("ata: libata: Fix sata_down_spd_limit() when no
>> link speed is reported"), when
>> the device is unplugged, every probe lowers sata_spd_limit, which breaks
>> commit 2dc0b46b5ea3 ("libata: sata_down_spd_limit should return if
>> driver has not recorded sstatus speed"). The harm is, when the device
>> is plugged back, it is limited to low speed which will never be recovered.
>>
>> This patch stops zeroing sata_spd in ata_dev_init (sata_spd was
>> introduced in commit 008a78961ec7 ("libata: improve SATA PHY speed down
>> logic")).  sata_spd is then written only on a successful reset.  At
>> struct ata_port's init time the sata_spd field is zeroed by kzalloc.
>>
>> Fixes: 69f2c9346313 ("ata: libata: Fix sata_down_spd_limit() when no link speed is reported")
>> Assisted-by: LLM
>> Signed-off-by: tongzhiwei <tongzhiwei@xxxxxxxxxxxxxxxxxxxx>
>> ---
>>  drivers/ata/libata-core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index f482c0a6d7e9..2ddad2e6ebd2 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -5659,7 +5659,7 @@ void ata_dev_init(struct ata_device *dev)
>>  
>>  	/* SATA spd limit is bound to the attached device, reset together */
>>  	link->sata_spd_limit = link->hw_sata_spd_limit;
>> -	link->sata_spd = 0;
>> +	/* link->sata_spd caches last negotiated link speed; not zeroed here */
> 
> This contradicts the previous comment, so let's make this consistent. E.g.
> something like:
> 
> 	/*
> 	 * The SATA speed limit is bound to the attached device, so reset it
> 	 * but keep the last negotiated speed indicated by link->sata_spd.
> 	 */
> 
> to replace the previous comment.
> 
>>  
>>  	/* High bits of dev->flags are used to record warm plug
>>  	 * requests which occur asynchronously.  Synchronize using

Looking at the Sashiko comment on this patch, I think it is valid and should be
addressed as well.



-- 
Damien Le Moal
Western Digital Research