| Message ID | 1459426971-11927-1-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 31 Mar 2016, Hans de Goede wrote: > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > an usb-id of: 0bc2:331a, as these will fail to respond to a > REPORT_LUNS command. > @@ -532,6 +532,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) > case 'i': > f |= US_FL_IGNORE_DEVICE; > break; > + case 'j': > + f |= US_FL_NO_REPORT_LUNS; > + break; > case 'l': > f |= US_FL_NOT_LOCKABLE; > break; You forgot to document this new module parameter flag in Documentation/kernel-parameters.txt. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > an usb-id of: 0bc2:331a, as these will fail to respond to a > REPORT_LUNS command. Actually, if we're sending them a report luns command, they must be reporting in at SCSI-3 SPC or higher. Should we be quirking them down to SCSI-2 instead because it reduces the risk of running into something else they're not doing from the SPC command set? James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 31-03-16 16:48, James Bottomley wrote: > On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: >> Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with >> an usb-id of: 0bc2:331a, as these will fail to respond to a >> REPORT_LUNS command. > > Actually, if we're sending them a report luns command, they must be > reporting in at SCSI-3 SPC or higher. Should we be quirking them down > to SCSI-2 instead because it reduces the risk of running into something > else they're not doing from the SPC command set? These are fairly new devices, so they should really be scsi3, but the usb <-> sata bridge (presumably) used does not seem to like report_luns. Note that usb-storage simple sets no_report_luns conditionally for all usb-storage devices. The scsi people have repeatedly asked me to not do this kinda blanket blacklisting for uas devices, because they hope that uas will allow them to more or less do proper scsi over usb, so we end up with blacklisting specific commands every now and then to get devices to work. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2016-03-31 at 17:03 +0200, Hans de Goede wrote: > Hi, > > On 31-03-16 16:48, James Bottomley wrote: > > On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: > > > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > > > an usb-id of: 0bc2:331a, as these will fail to respond to a > > > REPORT_LUNS command. > > > > Actually, if we're sending them a report luns command, they must be > > reporting in at SCSI-3 SPC or higher. Should we be quirking them > > down to SCSI-2 instead because it reduces the risk of running into > > something else they're not doing from the SPC command set? > > These are fairly new devices, so they should really be scsi3, but the > usb <-> sata bridge (presumably) used does not seem to like > report_luns. That's what I'm questioning: REPORT LUNS is one of the big SCSI-3 changes, if they don't support that, it really looks like someone picked up an old engine and just fuzzed the inquiry data to return SCSI -3. In which case we should put it back to SCSI-2 where it belongs. Also, if it's USB<->SCSI bridge, that isn't really UAS, is it? > Note that usb-storage simple sets no_report_luns conditionally for > all usb-storage devices. The scsi people have repeatedly asked me to > not do this kinda blanket blacklisting for uas devices, because they > hope that uas will allow them to more or less do proper scsi over > usb, so we end up with blacklisting specific commands every now and > then to get devices to work. Well, we were hoping that with UAS the USB device creators would actually learn what a standard was when it bit them, yes. The fact that Seagate can release a SCSI-3 UAS device that doesn't do REPORT LUNS kind of dashes that hope. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 31-03-16 17:11, James Bottomley wrote: > On Thu, 2016-03-31 at 17:03 +0200, Hans de Goede wrote: >> Hi, >> >> On 31-03-16 16:48, James Bottomley wrote: >>> On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: >>>> Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with >>>> an usb-id of: 0bc2:331a, as these will fail to respond to a >>>> REPORT_LUNS command. >>> >>> Actually, if we're sending them a report luns command, they must be >>> reporting in at SCSI-3 SPC or higher. Should we be quirking them >>> down to SCSI-2 instead because it reduces the risk of running into >>> something else they're not doing from the SPC command set? >> >> These are fairly new devices, so they should really be scsi3, but the >> usb <-> sata bridge (presumably) used does not seem to like >> report_luns. > > That's what I'm questioning: REPORT LUNS is one of the big SCSI-3 > changes, if they don't support that, it really looks like someone > picked up an old engine and just fuzzed the inquiry data to return SCSI > -3. In which case we should put it back to SCSI-2 where it belongs. Actually it does support REPORT LUNS, some of the time. When you first boot the computer with uas blacklisted for this device, so initialize it once with usb-storage, and then reboot with out the blacklist (and without removing power to the drive) uas will work with REPORT LUNS bit cold-booting directly into uas mode and then doing a REPORT LUNS upsets the drive / disk enclosure (this has all been observed by David Webb, I do not own such a drive). > Also, if it's USB<->SCSI bridge, that isn't really UAS, is it? I assume you mean that a USB<->sata bridge is not really a SCSI device but more of a scsi emulating device. I'm not going to argue that, but all devices talking the uas protocol I've seen sofar are USB<->sata bridges. >> Note that usb-storage simple sets no_report_luns conditionally for >> all usb-storage devices. The scsi people have repeatedly asked me to >> not do this kinda blanket blacklisting for uas devices, because they >> hope that uas will allow them to more or less do proper scsi over >> usb, so we end up with blacklisting specific commands every now and >> then to get devices to work. > > Well, we were hoping that with UAS the USB device creators would > actually learn what a standard was when it bit them, yes. The fact > that Seagate can release a SCSI-3 UAS device that doesn't do REPORT > LUNS kind of dashes that hope. See above it does sortof do REPORT LUNS just not reliable (and thus not usable). Also for some reason Seagate seems to be particularly bad in their uas implementation, we have a ton of quirks for Seagate uas devices in drivers/usb/storage/unusual_uas.h, so far all of them stop responding until reset after ATA_12 or ATA_16 CDBs so we filter those out. This REPORT LUNS issue is new. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > Actually it does support REPORT LUNS, some of the time. When you first > boot the computer with uas blacklisted for this device, so initialize > it once with usb-storage, and then reboot with out the blacklist > (and without removing power to the drive) uas will work with REPORT LUNS > bit cold-booting directly into uas mode and then doing a REPORT LUNS > upsets the drive / disk enclosure (this has all been observed by > David Webb, I do not own such a drive). Just to confirm what Hans has reported. After power has been removed the Seagate Expansion usb disk always produces faults unless it has been blacklisted in some way. Once the disk is working the computer can be powered off and restarted without the blacklist and, as long as its power has not been removed, the disk can be reconnected many times without any error. With Hans's changes the disk mounts correctly with the uas module every time. My guess is that one of the interface registers is not or cannot be initialized correctly. If after a failure I try unplugging and reinserting the usb connector many times it has sometimes connected correctly - which to me means that some random bit eventually has the right value. Regards, David Webb. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index c68e724147..5e15ed03 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -2,7 +2,7 @@ * USB Attached SCSI * Note that this is not the same as the USB Mass Storage driver * - * Copyright Hans de Goede <hdegoede@redhat.com> for Red Hat, Inc. 2013 - 2014 + * Copyright Hans de Goede <hdegoede@redhat.com> for Red Hat, Inc. 2013 - 2016 * Copyright Matthew Wilcox for Intel Corp, 2010 * Copyright Sarah Sharp for Intel Corp, 2010 * @@ -781,6 +781,17 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) return SUCCESS; } +static int uas_target_alloc(struct scsi_target *starget) +{ + struct uas_dev_info *devinfo = (struct uas_dev_info *) + dev_to_shost(starget->dev.parent)->hostdata; + + if (devinfo->flags & US_FL_NO_REPORT_LUNS) + starget->no_report_luns = 1; + + return 0; +} + static int uas_slave_alloc(struct scsi_device *sdev) { struct uas_dev_info *devinfo = @@ -831,6 +842,7 @@ static struct scsi_host_template uas_host_template = { .module = THIS_MODULE, .name = "uas", .queuecommand = uas_queuecommand, + .target_alloc = uas_target_alloc, .slave_alloc = uas_slave_alloc, .slave_configure = uas_slave_configure, .eh_abort_handler = uas_eh_abort_handler, diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index ccc113e..53341a7 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -64,6 +64,13 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x0000, 0x9999, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: David Webb <djw@noc.ac.uk> */ +UNUSUAL_DEV(0x0bc2, 0x331a, 0x0000, 0x9999, + "Seagate", + "Expansion Desk", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_REPORT_LUNS), + /* Reported-by: Hans de Goede <hdegoede@redhat.com> */ UNUSUAL_DEV(0x0bc2, 0x3320, 0x0000, 0x9999, "Seagate", diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 43576ed..9de988a 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -482,7 +482,7 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 | US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE | US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES | - US_FL_MAX_SECTORS_240); + US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS); p = quirks; while (*p) { @@ -532,6 +532,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) case 'i': f |= US_FL_IGNORE_DEVICE; break; + case 'j': + f |= US_FL_NO_REPORT_LUNS; + break; case 'l': f |= US_FL_NOT_LOCKABLE; break; diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h index 7f5f78b..245f57d 100644 --- a/include/linux/usb_usual.h +++ b/include/linux/usb_usual.h @@ -79,6 +79,8 @@ /* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */ \ US_FLAG(MAX_SECTORS_240, 0x08000000) \ /* Sets max_sectors to 240 */ \ + US_FLAG(NO_REPORT_LUNS, 0x10000000) \ + /* Cannot handle REPORT_LUNS */ \ #define US_FLAG(name, value) US_FL_##name = value , enum { US_DO_ALL_FLAGS };
Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with an usb-id of: 0bc2:331a, as these will fail to respond to a REPORT_LUNS command. Cc: stable@vger.kernel.org Reported-and-tested-by: David Webb <djw@noc.ac.uk> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/usb/storage/uas.c | 14 +++++++++++++- drivers/usb/storage/unusual_uas.h | 7 +++++++ drivers/usb/storage/usb.c | 5 ++++- include/linux/usb_usual.h | 2 ++ 4 files changed, 26 insertions(+), 2 deletions(-)