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