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 2a882929de4aa3cfcce6b46de459ee7f7360a075..8193b38a1cae7a8d738fb4c29828654dd02f0572 100644 (file)
--- 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)