On 1/20/23 03:46, marius@xxxxxxxxxxxxxx wrote:
> January 19, 2023 2:29 AM, "Damien Le Moal" <damien.lemoal@xxxxxxxxxxxxxxxxxx> wrote:
>
>> On 2023/01/19 4:43, marius@xxxxxxxxxxxxxx wrote:
>>
>>> [ 73.075849][ T112] hardreset, Online=>Offline
>>> [ 73.075856][ T112] sata_set_spd_needed, scontrol=0x300
>>> [ 73.080328][ T112] __sata_set_spd_needed, initial limit=0xFFFFFFFF
>>> [ 73.085601][ T112] __sata_set_spd_needed, corrected limit=0xFFFFFFFF
>>> [ 73.091900][ T112] __sata_set_spd_needed, target=0x0
>>> [ 73.098386][ T112] __sata_set_spd_needed, spd=0x0
>>> [ 73.103475][ T112] __sata_set_spd_needed, final *scontrol=0x300
>>
>> Can you share the patch/diff printing these ? Just to be sure I look at the
>> right place :)
>
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -427,6 +430,7 @@ static int __sata_set_spd_needed(struct ata_link *link, u32 *scontrol)
> u32 limit, target, spd;
>
> limit = link->sata_spd_limit;
> + printk(KERN_DEBUG "__sata_set_spd_needed, initial limit=0x%X",limit);
>
> /* Don't configure downstream link faster than upstream link.
> * It doesn't speed up anything and some PMPs choke on such
> @@ -435,14 +439,18 @@ static int __sata_set_spd_needed(struct ata_link *link, u32 *scontrol)
> if (!ata_is_host_link(link) && host_link->sata_spd)
> limit &= (1 << host_link->sata_spd) - 1;
>
> + printk(KERN_DEBUG "__sata_set_spd_needed, corrected limit=0x%X",limit);
> if (limit == UINT_MAX)
> target = 0;
> else
> target = fls(limit);
>
> + printk(KERN_DEBUG "__sata_set_spd_needed, target=0x%X",target);
> spd = (*scontrol >> 4) & 0xf;
> *scontrol = (*scontrol & ~0xf0) | ((target & 0xf) << 4);
>
> + printk(KERN_DEBUG "__sata_set_spd_needed, spd=0x%X",spd);
> + printk(KERN_DEBUG "__sata_set_spd_needed, final *scontrol=0x%X",*scontrol);
> return spd != target;
> }
>
> @@ -467,6 +475,7 @@ static int sata_set_spd_needed(struct ata_link *link)
>
> if (sata_scr_read(link, SCR_CONTROL, &scontrol))
> return 1;
> + printk(KERN_DEBUG "sata_set_spd_needed, scontrol=0x%X",scontrol);
>
> return __sata_set_spd_needed(link, &scontrol);
> }
Thanks for that. Unfortunately, my PMP box is currently disconnected from
my test machine in the lab. Need to get there to reconnect it :)
But I think I got an idea of what is wrong here. Can you try this patch:
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 18ef14e749a0..cb12054c733f 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -436,7 +436,8 @@ static int __sata_set_spd_needed(struct ata_link
*link, u32 *scontrol)
limit &= (1 << host_link->sata_spd) - 1;
if (limit == UINT_MAX)
- target = 0;
+ /* Try highest gen 3 limit */
+ target = 3;
else
target = fls(limit);
Note that with further digging, it seems that even for a working system
with all drives detected, we always end-up with something like:
cat
/sys/devices/pci0000:00/0000:00:17.0/ata6/link6/ata_link/link6/sata_spd_limit
<unknown>
And same for hw_sata_spd_limit.
So the UINT_MAX blind initialization of the speed limit before probing
seems to never be corrected and set to the actual limit of the
link/device. That needs further correction. But for your case, the above
should fix the issue I think.
--
Damien Le Moal
Western Digital Research
