Re: Bug report for ahci-mvebu driver

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 2023/01/19 4:43, marius@psihoexpert.ro 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 :)

...

> [ 76.203475][ T112] debounce, SCR=0x100
> [ 76.263475][ T112] debounce, SCR=0x100
> [ 76.323475][ T112] debounce, SCR=0x100
> [ 76.327332][ T112] resume, return at end of function
> [ 76.331189][ T112] hardreset, ata_phys_link_offline check failed
> [ 76.336287][ T112] ata2: SATA link down (SStatus 100 SControl 300)
> [ 76.348747][ T112] sata_down_spd_limit: limit 0x1, cur spd 0x0, saved limit 0xffffffff, hw limit
> 0xffffffff

OK. And here is the problem: cur spd is 0x0, so unknown, and that leads to an
einval exit of sata_down_spd_limit() instead of trying something sensible like
starting with 6Gbps. That leads sata_set_spd to also have cur spd = 0 and
setting of a bad scontrol which prevents detecting the drive. See the difference
with forcing 1.5gbps:

> [ 19.819298][ T112] hardreset, Online=>Offline
> [ 19.819306][ T112] sata_set_spd_needed, scontrol=0x310
> [ 19.823798][ T112] __sata_set_spd_needed, initial limit=0x1
> [ 19.829052][ T112] __sata_set_spd_needed, corrected limit=0x1
> [ 19.834752][ T112] __sata_set_spd_needed, target=0x1
> [ 19.840614][ T112] __sata_set_spd_needed, spd=0x1
> [ 19.845703][ T112] __sata_set_spd_needed, final *scontrol=0x310
> [ 19.851592][ T112] resume, do loop
> [ 20.083568][ T112] resume, after do loop
> [ 20.143567][ T112] debounce, SCR=0x113

We have a good SCR thanks to spd being set to 1 = 1.5 gbps.

So patching sata_down_spd_limit() is the right solution I think. Let me cook
something for you to test.

-- 
Damien Le Moal
Western Digital Research