Re: Bug report for ahci-mvebu driver

Damien Le Moal
2023-01-30T02:38:54Z

On 1/29/23 19:24, marius@psihoexpert.ro wrote:
> January 29, 2023 4:25 AM, "Damien Le Moal" <damien.lemoal@opensource.wdc.com> wrote:
>>
>> OK. That is because the limit is not being changed with the added "return 0".
>> What about this version:
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 884ae73b11ea..f404e2c2869e 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -3085,10 +3085,31 @@ int sata_down_spd_limit(struct ata_link *link, u32
>> spd_limit)
>> * If not, use cached value in link->sata_spd.
>> */
>> rc = sata_scr_read(link, SCR_STATUS, &sstatus);
>> - if (rc == 0 && ata_sstatus_online(sstatus))
>> + if (rc == 0 && ata_sstatus_online(sstatus)) {
>> spd = (sstatus >> 4) & 0xf;
>> - else
>> + } else {
>> + /*
>> + * If we reach here, the device is still offline or not
>> + * reporting a current speed yet. If we do not have a recorded
>> + * speed either, it means that probing the drive is failing,
>> + * most likely because problem with link speed autonegotiation
>> + * (e.g. reported case with mvebu adapter with a port multiplier
>> + * device). In this case, assume that probing was done at Gen3
>> + * speed and set the speed limit to the lower Gen2 speed.
>> + * With this, the next probe retry will be done after
>> + * sata_set_spd() explicitly lowers the link speed. If the
>> + * device then responds, the actual maximum speed will be set
>> + * once libata-eh finishes the device revalidation.
>> + */
>> + if (!link->sata_spd && link->sata_spd_limit == UINT_MAX) {
>> + link->sata_spd_limit = 2;
>> + ata_link_warn(link,
>> + "No reported SATA link speed. Limiting to
>> %s\n",
>> + sata_spd_string(link->sata_spd_limit));
>> + return 0;
>> + }
>> spd = link->sata_spd;
>> + }
>>
>> mask = link->sata_spd_limit;
>> if (mask <= 1)
>>
>> Does this work ?
>>
>> --
>> Damien Le Moal
>> Western Digital Research
>
>
> No, it doesn't work.

Apologies for asking you to test again, but could you try this simpler
version of the initial tweak:

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 884ae73b11ea..2ea572628b1c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3109,7 +3109,7 @@ int sata_down_spd_limit(struct ata_link *link, u32
spd_limit)
*/
if (spd > 1)
mask &= (1 << (spd - 1)) - 1;
- else
+ else if (link->sata_spd)
return -EINVAL;

/* were we already at the bottom? */

This is a partial revert of the commit that created the issue in the
first place. But better check this is OK.