Re: [PATCH 2/2] uas: Disable uas on ASM1051 devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 09/09/2014 05:23 PM, Alan Stern wrote:
On Tue, 9 Sep 2014, Hans de Goede wrote:

Even with REPORT SUPPORTED OPERATION CODES blacklisted the ASM1051 chipset
still does not work when combined with some disks, e.g. a Crucial M500 ssd.

When used with a troublesome disk, the chipset throws all kinds of USB errors,
and eventually hangs, where as in BOT mode it works fine.

To make matters worse the ASM1051 chipset and older versions of the ASM1053
chipset use the same usb-id.

When connected over USB-3 the 2 can be told apart by the number of streams
they support. So this patch adds some less then pretty code to disable uas for
the ASM1051. When connected over USB-2, simply disable uas alltogether for
devices with the shared usb-id.

This ends up disabling uas in many cases where it actually works fine, which
is unfortunate, but better then leaving some users with completely non working
external disks. This patch adds a log level explaining how perfomance conscious
users can re-enable uas.

Cc: stable@xxxxxxxxxxxxxxx # 3.16
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
  drivers/usb/storage/uas-detect.h | 34 ++++++++++++++++++++++++++++++----
  1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 503ac5c..3119f3f 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -59,10 +59,6 @@ static int uas_use_uas_driver(struct usb_interface *intf,
  	unsigned long flags = id->driver_info;
  	int r, alt;

-	usb_stor_adjust_quirks(udev, &flags);
-
-	if (flags & US_FL_IGNORE_UAS)
-		return 0;

  	alt = uas_find_uas_alt_setting(intf);
  	if (alt < 0)
@@ -72,6 +68,36 @@ static int uas_use_uas_driver(struct usb_interface *intf,
  	if (r < 0)
  		return 0;

+	/*
+	 * ASM1051 and older ASM1053 devices have the same usb-id, and UAS is
+	 * broken with some disks on the ASM1051, use the number of streams to
+	 * differentiate between the 2. Newer ASM1053 devices also support 32
+	 * streams, but have a different product-id.
+	 */
+	if (udev->descriptor.idVendor == 0x174c &&
+			udev->descriptor.idProduct == 0x55aa) {
+		if (udev->speed < USB_SPEED_SUPER) {
+			/* No streams info, assume ASM1051E */
+			dev_warn(&udev->dev, "Assuming ASM1051E chipset\n");
+			flags |= US_FL_IGNORE_UAS;
+		} else if (usb_ss_max_streams(&eps[1]->ss_ep_comp) == 32) {
+			dev_warn(&udev->dev, "Detected ASM1051E chipset\n");
+			flags |= US_FL_IGNORE_UAS;
+		}
+	}
+
+	usb_stor_adjust_quirks(udev, &flags);

This won't work.  usb_stor_adjust_quirks masks out all the quirks that
it handles before applying the user-specified quirks.  Therefore it
will erase your US_FL_IGNORE_UAS flag.

Only if there is a matching quirk entry in the quirks parameter, and then
masking it out is fine, we want to allow this as we're setting the
US_FL_IGNORE_US flag on bridges where it is known to cause issues when
combined with *some* disks, while uas works fine with other disks, so
users may want to override it.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]