Re: Bug report for ahci-mvebu driver

Damien Le Moal
2022-12-06T06:08:39Z

On 12/4/22 10:41, marius@psihoexpert.ro wrote:
> November 24, 2022 4:40 AM, "Damien Le Moal" <damien.lemoal@opensource.wdc.com> wrote:
>
>> So the hardware is failing to detect the device presence. No clue why.
>> Only an ATA bus trace could tell us more here (do you have an ATA bus
>> analyzer around ?).
>
> No. :-)
>
>> So options are:
>> 1) Involve the router vendor and/or SoC vendor to see if they can help.
>> 2) Use UDB connection between your router & drive box ? (if you can)
>>
>> Another thing is: Did you try earlier kernel versions ? Any earlier kernel
>> working ? (I forgot if that is the case). If yes, then we should be able
>> to figure out the change that broke the driver (which may be something
>> changed in the SoC support rather than the mvebu driver itself).
>> gi-bisect will likely not be of any help given that you are not getting a
>> crash. So you will need to try out different kernel versions, starting
>> with LTS kernels, and refine (use Linus tags, including RCs, to find the
>> last working kernel). That may take some time, but will likely narrow down
>> the search for fixing this (if that is possible given the hardware setup
>> you have).
>
> Sorry for the late reply, but I really really hate bisecting, especially manually, and especially if the test must be run on another machine.
> You are correct. It initially worked, but it was broken by this commit:
>
>
> commit 2dc0b46b5ea30f169b0b272253ea846a5a281731
> Author: David Milburn <dmilburn@redhat.com>
> Date: Tue Nov 14 16:17:25 2017 -0600
>
> libata: sata_down_spd_limit should return if driver has not recorded sstatus speed
>
> During hotplug, it is possible for 6Gbps link speed to be limited all
> the way down to 1.5 Gbps which may lead to a slower link speed when
> drive is re-connected.
>
> This behavior has been seen on a Intel Lewisburg SATA controller
> (8086:a1d2) with HGST HUH728080ALE600 drive where SATA link speed was
> limited to 1.5 Gbps and when re-connected the link came up 3.0 Gbps.
>
> This patch was retested on above configuration and showed the
> hotplugged link to come back online at max speed (6Gbps). I did not
> see the downgrade when testing on Intel C600/X79, but retested patched
> linux-4.14-rc5 kernel and didn't see any side effects from this
> change. Also, successfully retested hotplug on port multiplier 3Gbps
> link.
>
> tj: Minor comment updates.
>
> Signed-off-by: David Milburn <dmilburn@redhat.com>
> Signed-off-by: Tejun Heo <tj@kernel.org>
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 2a882929de4a..8193b38a1cae 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -3082,13 +3082,19 @@ int sata_down_spd_limit(struct ata_link *link, u32 spd_limit)
> bit = fls(mask) - 1;
> mask &= ~(1 << bit);
>
> - /* Mask off all speeds higher than or equal to the current
> - * one. Force 1.5Gbps if current SPD is not available.
> + /*
> + * Mask off all speeds higher than or equal to the current one. At
> + * this point, if current SPD is not available and we previously
> + * recorded the link speed from SStatus, the driver has already
> + * masked off the highest bit so mask should already be 1 or 0.
> + * Otherwise, we should not force 1.5Gbps on a link where we have
> + * not previously recorded speed from SStatus. Just return in this
> + * case.
> */
> if (spd > 1)
> mask &= (1 << (spd - 1)) - 1;
> else
> - mask &= 1;
> + return -EINVAL;
>
> /* were we already at the bottom? */
> if (!mask)

Without reverting the above patch, could you please try the following patch ?

diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
index 22ecc4f3ae79..396aacc3fbd1 100644
--- a/drivers/ata/ahci_mvebu.c
+++ b/drivers/ata/ahci_mvebu.c
@@ -203,8 +203,6 @@ static int ahci_mvebu_probe(struct platform_device *pdev)
if (rc)
return rc;

- hpriv->stop_engine = ahci_mvebu_stop_engine;
-
rc = pdata->plat_config(hpriv);
if (rc)
goto disable_resources;

This essentially disables the use of ahci_mvebu_stop_engine() function for
your adapter, that is, it assumes that your newer adapter is not subject
to errata Ref#226. If this helps, then we can have a cleaner fix using the
compatible string for the adapter/SoC.

Otherwise, we will need to add a quirk for this adapter to have a behavior
for it that is the same as if patch 2dc0b46b5ea3 is reverted.

By the way, what is the maximum speed of you pmp box ? Is it really 1.5
Gbps (sata gen 1) ?