* [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-16 20:01 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-16 20:01 UTC (permalink / raw) To: stern; +Cc: gregkh, linux-usb, usb-storage, linux-kernel, Alexander Kappner The "G-Drive" (sold by G-Technology) external USB 3.0 drive hangs on write access under UAS: [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584052] Aborting journal on device sdi-8. The proposed patch adds compatibility quirks. Because the drive requires two quirks (one to disable UAS, and another to work with usb-storage), adding this under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS receive the quirk. With the patch, the drive works reliably (tested on NEC Corporation uPD720200 USB 3.0 host controller). Signed-off-by: Alexander Kappner <agk@godking.net> --- drivers/usb/storage/unusual_devs.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index 747d3a9..b8661a1 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, "Micro Mini 1GB", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* "G-DRIVE" external HDD hangs on write without these. + * Reported-by: Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), + /* * Nick Bowler <nbowler@elliptictech.com> * SCSI stack spams (otherwise harmless) error messages. -- 2.1.4 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-16 20:01 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-16 20:01 UTC (permalink / raw) To: stern; +Cc: gregkh, linux-usb, usb-storage, linux-kernel, Alexander Kappner The "G-Drive" (sold by G-Technology) external USB 3.0 drive hangs on write access under UAS: [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584052] Aborting journal on device sdi-8. The proposed patch adds compatibility quirks. Because the drive requires two quirks (one to disable UAS, and another to work with usb-storage), adding this under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS receive the quirk. With the patch, the drive works reliably (tested on NEC Corporation uPD720200 USB 3.0 host controller). Signed-off-by: Alexander Kappner <agk@godking.net> --- drivers/usb/storage/unusual_devs.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index 747d3a9..b8661a1 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, "Micro Mini 1GB", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* "G-DRIVE" external HDD hangs on write without these. + * Reported-by: Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), + /* * Nick Bowler <nbowler@elliptictech.com> * SCSI stack spams (otherwise harmless) error messages. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-16 20:55 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-16 20:55 UTC (permalink / raw) To: Alexander Kappner; +Cc: gregkh, linux-usb, usb-storage, linux-kernel On Wed, 16 May 2018, Alexander Kappner wrote: > The "G-Drive" (sold by G-Technology) external USB 3.0 drive > hangs on write access under UAS: > > [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] > [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb > [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 > [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 > [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write > [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) > [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] > [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb > [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 > [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 > [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 > [ 140.584052] Aborting journal on device sdi-8. That's kind of weird. Does the drive work under Windows in UAS mode? If so, why are the WRITE(16) commands failing under Linux? > The proposed patch adds compatibility quirks. Because the drive requires two > quirks (one to disable UAS, and another to work with usb-storage), adding this > under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS > receive the quirk. That doesn't quite make sense. Since you prevent the uas driver from binding to this device, it will end up using usb-storage no matter how the kernel was built. Therefore the second quirk flag has to go into unusual_devs.h no matter what. > With the patch, the drive works reliably > (tested on NEC Corporation uPD720200 USB 3.0 host controller). You don't describe the second quirk flag at all. Are you sure it is needed? > Signed-off-by: Alexander Kappner <agk@godking.net> > --- > drivers/usb/storage/unusual_devs.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h > index 747d3a9..b8661a1 100644 > --- a/drivers/usb/storage/unusual_devs.h > +++ b/drivers/usb/storage/unusual_devs.h > @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, > "Micro Mini 1GB", > USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), > > +/* "G-DRIVE" external HDD hangs on write without these. > + * Reported-by: Alexander Kappner <agk@godking.net> > + */ > +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, > + "SimpleTech", > + "External HDD", > + USB_SC_DEVICE, USB_PR_DEVICE, NULL, > + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), > + > /* > * Nick Bowler <nbowler@elliptictech.com> > * SCSI stack spams (otherwise harmless) error messages. > ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-16 20:55 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-16 20:55 UTC (permalink / raw) To: Alexander Kappner; +Cc: gregkh, linux-usb, usb-storage, linux-kernel On Wed, 16 May 2018, Alexander Kappner wrote: > The "G-Drive" (sold by G-Technology) external USB 3.0 drive > hangs on write access under UAS: > > [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] > [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb > [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 > [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 > [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write > [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) > [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] > [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb > [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 > [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 > [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 > [ 140.584052] Aborting journal on device sdi-8. That's kind of weird. Does the drive work under Windows in UAS mode? If so, why are the WRITE(16) commands failing under Linux? > The proposed patch adds compatibility quirks. Because the drive requires two > quirks (one to disable UAS, and another to work with usb-storage), adding this > under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS > receive the quirk. That doesn't quite make sense. Since you prevent the uas driver from binding to this device, it will end up using usb-storage no matter how the kernel was built. Therefore the second quirk flag has to go into unusual_devs.h no matter what. > With the patch, the drive works reliably > (tested on NEC Corporation uPD720200 USB 3.0 host controller). You don't describe the second quirk flag at all. Are you sure it is needed? > Signed-off-by: Alexander Kappner <agk@godking.net> > --- > drivers/usb/storage/unusual_devs.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h > index 747d3a9..b8661a1 100644 > --- a/drivers/usb/storage/unusual_devs.h > +++ b/drivers/usb/storage/unusual_devs.h > @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, > "Micro Mini 1GB", > USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), > > +/* "G-DRIVE" external HDD hangs on write without these. > + * Reported-by: Alexander Kappner <agk@godking.net> > + */ > +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, > + "SimpleTech", > + "External HDD", > + USB_SC_DEVICE, USB_PR_DEVICE, NULL, > + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), > + > /* > * Nick Bowler <nbowler@elliptictech.com> > * SCSI stack spams (otherwise harmless) error messages. > --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 8:15 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-17 8:15 UTC (permalink / raw) To: Alan Stern; +Cc: gregkh, linux-usb, usb-storage, linux-kernel Hi Alan, thanks for reviewing. (This is my first contribution that touches usb-storage, so please bear with me.) > That's kind of weird. Does the drive work under Windows in UAS mode? On the Windows 10 VM that I just spun up for testing this, access to the drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the answer is no. > If so, why are the WRITE(16) commands failing under Linux? I don't know exactly why they're failing, but another entry in the unusual_uas.h shows another SimpleTech device (only with a slightly different product ID) needing the same UAS quirk (see https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those drives are just buggy. > That doesn't quite make sense. Since you prevent the uas driver from > binding to this device, it will end up using usb-storage no matter how > the kernel was built. Therefore the second quirk flag has to go into > unusual_devs.h no matter what. Yes that's what I was trying to get at. So even though the UAS flag would conceptually belong into unusual_uas, I'm putting both into unusual_devs to avoid having to create two separate entries for the same device. > You don't describe the second quirk flag at all. Are you sure it is > needed? Yes. Without this flag, the device keeps throwing similar errors on usb-storage. That's the same result I get on a host that doesn't have UAS compiled in. Here's a dmesg: [ 2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, bcdDevice=24.03 [ 2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 2.184980] usb 3-1: Product: G-DRIVE [ 2.185447] usb 3-1: Manufacturer: HGST [ 2.185829] usb 3-1: SerialNumber: AA015010004C [ 2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected [ 2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS [ 2.202981] scsi host2: usb-storage 3-1:1.0 ... [ 3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE 2403 PQ: 0 ANSI: 6 [ 3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0 [ 3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ CAPACITY(16). [ 3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 TB/3.64 TiB) [ 3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks [ 3.241255] sd 2:0:0:0: [sda] Write Protect is off [ 3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08 [ 3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA [ 3.257893] sda: sda1 sda9 [ 3.261402] sd 2:0:0:0: [sda] Attached SCSI disk ... [ 92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 92.437493] print_req_error: critical target error, dev sda, sector 0 [ 92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page write [ 92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null) [ 101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 101.452906] print_req_error: critical target error, dev sda, sector 3905159192 [ 101.453593] print_req_error: critical target error, dev sda, sector 3905159192 [ 101.454889] Aborting journal on device sda-8. [ 101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 Source code comments describe this as a known problem (scsiglue.c:182): /* * Some devices don't like MODE SENSE with page=0x3f, * which is the command used for checking if a device * is write-protected. Now that we tell the sd driver * to do a 192-byte transfer with this command the * majority of devices work fine, but a few still can't * handle it. The sd driver will simply assume those * devices are write-enabled. */ if (us->fflags & US_FL_NO_WP_DETECT) sdev->skip_ms_page_3f = 1; --agk On 05/16/2018 01:55 PM, Alan Stern wrote: > On Wed, 16 May 2018, Alexander Kappner wrote: > >> The "G-Drive" (sold by G-Technology) external USB 3.0 drive >> hangs on write access under UAS: >> >> [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE >> [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] >> [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb >> [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 >> [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 >> [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write >> [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) >> [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE >> [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] >> [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb >> [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 >> [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 >> [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 >> [ 140.584052] Aborting journal on device sdi-8. > > That's kind of weird. Does the drive work under Windows in UAS mode? > If so, why are the WRITE(16) commands failing under Linux? > >> The proposed patch adds compatibility quirks. Because the drive requires two >> quirks (one to disable UAS, and another to work with usb-storage), adding this >> under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS >> receive the quirk. > > That doesn't quite make sense. Since you prevent the uas driver from > binding to this device, it will end up using usb-storage no matter how > the kernel was built. Therefore the second quirk flag has to go into > unusual_devs.h no matter what. > >> With the patch, the drive works reliably >> (tested on NEC Corporation uPD720200 USB 3.0 host controller). > > You don't describe the second quirk flag at all. Are you sure it is > needed? > >> Signed-off-by: Alexander Kappner <agk@godking.net> >> --- >> drivers/usb/storage/unusual_devs.h | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h >> index 747d3a9..b8661a1 100644 >> --- a/drivers/usb/storage/unusual_devs.h >> +++ b/drivers/usb/storage/unusual_devs.h >> @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, >> "Micro Mini 1GB", >> USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), >> >> +/* "G-DRIVE" external HDD hangs on write without these. >> + * Reported-by: Alexander Kappner <agk@godking.net> >> + */ >> +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, >> + "SimpleTech", >> + "External HDD", >> + USB_SC_DEVICE, USB_PR_DEVICE, NULL, >> + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), >> + >> /* >> * Nick Bowler <nbowler@elliptictech.com> >> * SCSI stack spams (otherwise harmless) error messages. >> ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 8:15 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-17 8:15 UTC (permalink / raw) To: Alan Stern; +Cc: gregkh, linux-usb, usb-storage, linux-kernel Hi Alan, thanks for reviewing. (This is my first contribution that touches usb-storage, so please bear with me.) > That's kind of weird. Does the drive work under Windows in UAS mode? On the Windows 10 VM that I just spun up for testing this, access to the drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the answer is no. > If so, why are the WRITE(16) commands failing under Linux? I don't know exactly why they're failing, but another entry in the unusual_uas.h shows another SimpleTech device (only with a slightly different product ID) needing the same UAS quirk (see https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those drives are just buggy. > That doesn't quite make sense. Since you prevent the uas driver from > binding to this device, it will end up using usb-storage no matter how > the kernel was built. Therefore the second quirk flag has to go into > unusual_devs.h no matter what. Yes that's what I was trying to get at. So even though the UAS flag would conceptually belong into unusual_uas, I'm putting both into unusual_devs to avoid having to create two separate entries for the same device. > You don't describe the second quirk flag at all. Are you sure it is > needed? Yes. Without this flag, the device keeps throwing similar errors on usb-storage. That's the same result I get on a host that doesn't have UAS compiled in. Here's a dmesg: [ 2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, bcdDevice=24.03 [ 2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 2.184980] usb 3-1: Product: G-DRIVE [ 2.185447] usb 3-1: Manufacturer: HGST [ 2.185829] usb 3-1: SerialNumber: AA015010004C [ 2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected [ 2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS [ 2.202981] scsi host2: usb-storage 3-1:1.0 ... [ 3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE 2403 PQ: 0 ANSI: 6 [ 3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0 [ 3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ CAPACITY(16). [ 3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 TB/3.64 TiB) [ 3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks [ 3.241255] sd 2:0:0:0: [sda] Write Protect is off [ 3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08 [ 3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA [ 3.257893] sda: sda1 sda9 [ 3.261402] sd 2:0:0:0: [sda] Attached SCSI disk ... [ 92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 92.437493] print_req_error: critical target error, dev sda, sector 0 [ 92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page write [ 92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null) [ 101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 101.452906] print_req_error: critical target error, dev sda, sector 3905159192 [ 101.453593] print_req_error: critical target error, dev sda, sector 3905159192 [ 101.454889] Aborting journal on device sda-8. [ 101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 Source code comments describe this as a known problem (scsiglue.c:182): /* * Some devices don't like MODE SENSE with page=0x3f, * which is the command used for checking if a device * is write-protected. Now that we tell the sd driver * to do a 192-byte transfer with this command the * majority of devices work fine, but a few still can't * handle it. The sd driver will simply assume those * devices are write-enabled. */ if (us->fflags & US_FL_NO_WP_DETECT) sdev->skip_ms_page_3f = 1; --agk On 05/16/2018 01:55 PM, Alan Stern wrote: > On Wed, 16 May 2018, Alexander Kappner wrote: > >> The "G-Drive" (sold by G-Technology) external USB 3.0 drive >> hangs on write access under UAS: >> >> [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE >> [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] >> [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb >> [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 >> [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 >> [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write >> [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) >> [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE >> [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] >> [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb >> [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 >> [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 >> [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 >> [ 140.584052] Aborting journal on device sdi-8. > > That's kind of weird. Does the drive work under Windows in UAS mode? > If so, why are the WRITE(16) commands failing under Linux? > >> The proposed patch adds compatibility quirks. Because the drive requires two >> quirks (one to disable UAS, and another to work with usb-storage), adding this >> under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS >> receive the quirk. > > That doesn't quite make sense. Since you prevent the uas driver from > binding to this device, it will end up using usb-storage no matter how > the kernel was built. Therefore the second quirk flag has to go into > unusual_devs.h no matter what. > >> With the patch, the drive works reliably >> (tested on NEC Corporation uPD720200 USB 3.0 host controller). > > You don't describe the second quirk flag at all. Are you sure it is > needed? > >> Signed-off-by: Alexander Kappner <agk@godking.net> >> --- >> drivers/usb/storage/unusual_devs.h | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h >> index 747d3a9..b8661a1 100644 >> --- a/drivers/usb/storage/unusual_devs.h >> +++ b/drivers/usb/storage/unusual_devs.h >> @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, >> "Micro Mini 1GB", >> USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), >> >> +/* "G-DRIVE" external HDD hangs on write without these. >> + * Reported-by: Alexander Kappner <agk@godking.net> >> + */ >> +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, >> + "SimpleTech", >> + "External HDD", >> + USB_SC_DEVICE, USB_PR_DEVICE, NULL, >> + US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT), >> + >> /* >> * Nick Bowler <nbowler@elliptictech.com> >> * SCSI stack spams (otherwise harmless) error messages. >> --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 12:58 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-17 12:58 UTC (permalink / raw) To: Alexander Kappner, Alan Stern Cc: gregkh, usb-storage, linux-kernel, linux-usb [-- Attachment #1: Type: text/plain, Size: 469 bytes --] Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner: > Yes. Without this flag, the device keeps throwing similar errors on > usb-storage. That's the same result I get on a host that doesn't have UAS > compiled in. Here's a dmesg: Hi, this is suspicious. You do not actually whether US_FL_NO_WP_DETECT by itself would make the device work. Can you please test that? You will need the attached patch for the quirk to be supported. Regards Oliver [-- Attachment #2: 0001-UAS-add-support-for-the-quirk-US_FL_NO_WP_DETECT.patch --] [-- Type: text/x-patch, Size: 951 bytes --] From 1ff6c9c9cc66ddb4cf7ca95bea589d6a30c63623 Mon Sep 17 00:00:00 2001 From: Oliver Neukum <oneukum@suse.com> Date: Thu, 17 May 2018 14:46:47 +0200 Subject: [PATCH] UAS: add support for the quirk US_FL_NO_WP_DETECT The assumption that this quirk can be limited to old storage devices was too optimistic. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/usb/storage/uas.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6034c39b67d1..7ee67e8c1f1e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -836,6 +836,10 @@ static int uas_slave_configure(struct scsi_device *sdev) if (devinfo->flags & US_FL_BROKEN_FUA) sdev->broken_fua = 1; + /* UAS also needs to support FL_NO_WP_DETECT */ + if (devinfo->flags & US_FL_NO_WP_DETECT) + sdev->skip_ms_page_3f = 1; + scsi_change_queue_depth(sdev, devinfo->qdepth - 2); return 0; } -- 2.13.6 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 12:58 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-17 12:58 UTC (permalink / raw) To: Alexander Kappner, Alan Stern Cc: gregkh, usb-storage, linux-kernel, linux-usb Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner: > Yes. Without this flag, the device keeps throwing similar errors on > usb-storage. That's the same result I get on a host that doesn't have UAS > compiled in. Here's a dmesg: Hi, this is suspicious. You do not actually whether US_FL_NO_WP_DETECT by itself would make the device work. Can you please test that? You will need the attached patch for the quirk to be supported. Regards Oliver From 1ff6c9c9cc66ddb4cf7ca95bea589d6a30c63623 Mon Sep 17 00:00:00 2001 From: Oliver Neukum <oneukum@suse.com> Date: Thu, 17 May 2018 14:46:47 +0200 Subject: [PATCH] UAS: add support for the quirk US_FL_NO_WP_DETECT The assumption that this quirk can be limited to old storage devices was too optimistic. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/usb/storage/uas.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6034c39b67d1..7ee67e8c1f1e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -836,6 +836,10 @@ static int uas_slave_configure(struct scsi_device *sdev) if (devinfo->flags & US_FL_BROKEN_FUA) sdev->broken_fua = 1; + /* UAS also needs to support FL_NO_WP_DETECT */ + if (devinfo->flags & US_FL_NO_WP_DETECT) + sdev->skip_ms_page_3f = 1; + scsi_change_queue_depth(sdev, devinfo->qdepth - 2); return 0; } -- 2.13.6 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 18:38 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-17 18:38 UTC (permalink / raw) To: Oliver Neukum, Alan Stern; +Cc: gregkh, usb-storage, linux-kernel, linux-usb Oliver and Alan, thank for investigating. > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > by itself would make the device work. Can you please test that? US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, even with the patch you included applied: [ 44.108417] JBD2: Clearing recovery information on journal [ 44.119593] sd 2:0:0:0: [sda] tag#1 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 44.121605] sd 2:0:0:0: [sda] tag#1 Sense Key : Illegal Request [current] [ 44.123254] sd 2:0:0:0: [sda] tag#1 Add. Sense: Invalid field in cdb [ 44.124798] sd 2:0:0:0: [sda] tag#1 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 [ 44.126847] print_req_error: critical target error, dev sda, sector 3905159168 [ 44.128450] Buffer I/O error on dev sda, logical block 488144896, lost sync page write [ 44.130776] JBD2: Error -5 detected when updating journal superblock for sda-8. [ 44.141059] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 44.141376] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 44.141376] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 44.141376] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 44.141376] print_req_error: critical target error, dev sda, sector 0 [ 44.141376] Buffer I/O error on dev sda, logical block 0, lost sync page write > That's bizarre too. Even though the only difference is a MODE SENSE > command, the command that actually faliled was WRITE(16). It looks to me like the MODE SENSE simply hangs the drive, so anything issued after that will fail. Of course the drive says it's the "current command" that caused the failure, but I wouldn't give too much credence to that. FYI -- this device is a consumer grade rotational drive that you can get for less than $200, so I wouldn't be surprised if the implementations have issues. Also, I noticed that copying onto the drive with dd works fine, whereas trying to mount a filesystem immediately crashes it. I suspect this is because check_disk_change is called on mount (which eventually calls down to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag comes into play). On 05/17/2018 05:58 AM, Oliver Neukum wrote: > Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner: >> Yes. Without this flag, the device keeps throwing similar errors on >> usb-storage. That's the same result I get on a host that doesn't have UAS >> compiled in. Here's a dmesg: > > Hi, > > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > by itself would make the device work. Can you please test that? > You will need the attached patch for the quirk to be supported. > > Regards > Oliver > ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 18:38 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-17 18:38 UTC (permalink / raw) To: Oliver Neukum, Alan Stern; +Cc: gregkh, usb-storage, linux-kernel, linux-usb Oliver and Alan, thank for investigating. > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > by itself would make the device work. Can you please test that? US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, even with the patch you included applied: [ 44.108417] JBD2: Clearing recovery information on journal [ 44.119593] sd 2:0:0:0: [sda] tag#1 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 44.121605] sd 2:0:0:0: [sda] tag#1 Sense Key : Illegal Request [current] [ 44.123254] sd 2:0:0:0: [sda] tag#1 Add. Sense: Invalid field in cdb [ 44.124798] sd 2:0:0:0: [sda] tag#1 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 [ 44.126847] print_req_error: critical target error, dev sda, sector 3905159168 [ 44.128450] Buffer I/O error on dev sda, logical block 488144896, lost sync page write [ 44.130776] JBD2: Error -5 detected when updating journal superblock for sda-8. [ 44.141059] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 44.141376] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] [ 44.141376] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb [ 44.141376] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 44.141376] print_req_error: critical target error, dev sda, sector 0 [ 44.141376] Buffer I/O error on dev sda, logical block 0, lost sync page write > That's bizarre too. Even though the only difference is a MODE SENSE > command, the command that actually faliled was WRITE(16). It looks to me like the MODE SENSE simply hangs the drive, so anything issued after that will fail. Of course the drive says it's the "current command" that caused the failure, but I wouldn't give too much credence to that. FYI -- this device is a consumer grade rotational drive that you can get for less than $200, so I wouldn't be surprised if the implementations have issues. Also, I noticed that copying onto the drive with dd works fine, whereas trying to mount a filesystem immediately crashes it. I suspect this is because check_disk_change is called on mount (which eventually calls down to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag comes into play). On 05/17/2018 05:58 AM, Oliver Neukum wrote: > Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner: >> Yes. Without this flag, the device keeps throwing similar errors on >> usb-storage. That's the same result I get on a host that doesn't have UAS >> compiled in. Here's a dmesg: > > Hi, > > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > by itself would make the device work. Can you please test that? > You will need the attached patch for the quirk to be supported. > > Regards > Oliver > --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 19:13 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-17 19:13 UTC (permalink / raw) To: Alexander Kappner Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb On Thu, 17 May 2018, Alexander Kappner wrote: > Oliver and Alan, > > thank for investigating. > > > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > > by itself would make the device work. Can you please test that? > > US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, > even with the patch you included applied: Are you certain Oliver's new code was executed? If you put US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then it would not affect the uas driver. > > That's bizarre too. Even though the only difference is a MODE SENSE > > command, the command that actually faliled was WRITE(16). > It looks to me like the MODE SENSE simply hangs the drive, so anything > issued after that will fail. Of course the drive says it's the "current > command" that caused the failure, but I wouldn't give too much credence to > that. FYI -- this device is a consumer grade rotational drive that you can > get for less than $200, so I wouldn't be surprised if the implementations > have issues. > > Also, I noticed that copying onto the drive with dd works fine, whereas > trying to mount a filesystem immediately crashes it. I suspect this is > because check_disk_change is called on mount (which eventually calls down > to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag > comes into play). Was this tested with uas or usb-storage? Alan Stern ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 19:13 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-17 19:13 UTC (permalink / raw) To: Alexander Kappner Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb On Thu, 17 May 2018, Alexander Kappner wrote: > Oliver and Alan, > > thank for investigating. > > > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT > > by itself would make the device work. Can you please test that? > > US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, > even with the patch you included applied: Are you certain Oliver's new code was executed? If you put US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then it would not affect the uas driver. > > That's bizarre too. Even though the only difference is a MODE SENSE > > command, the command that actually faliled was WRITE(16). > It looks to me like the MODE SENSE simply hangs the drive, so anything > issued after that will fail. Of course the drive says it's the "current > command" that caused the failure, but I wouldn't give too much credence to > that. FYI -- this device is a consumer grade rotational drive that you can > get for less than $200, so I wouldn't be surprised if the implementations > have issues. > > Also, I noticed that copying onto the drive with dd works fine, whereas > trying to mount a filesystem immediately crashes it. I suspect this is > because check_disk_change is called on mount (which eventually calls down > to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag > comes into play). Was this tested with uas or usb-storage? Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-18 17:51 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-18 17:51 UTC (permalink / raw) To: Alan Stern; +Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb > Was this tested with uas or usb-storage? On both. dd works either way. > Are you certain Oliver's new code was executed? If you put > US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then > it would not affect the uas driver. Yes, the code ran. Further debugging shows that the code that causes the device to hang is in drivers/scsi/sd.c:2698. So the reason why usb-storage works and UAS does not is because the device setting both skip_ms_page_3f=1 and skip_ms_page_8=1 is required. The US_FL_NO_WP_DETECT flag does the former, and usb-storage (but not UAS) by default does the latter (drivers/usb/storage/scsiglue.c:198): /* * A number of devices have problems with MODE SENSE for * page x08, so we will skip it. */ sdev->skip_ms_page_8 = 1; Forcing both skip_ms_page_3f and skip_ms_page_8 to 1 results in UAS and usb-storage working. Oliver's code only set skip_ms_page_3f. Do we want a patch to introduce a quirk flag for skip_ms_page_8, similar to the US_FL_NO_WP_DETECT quirk bit for skip_ms_page_3f? This may even resolve the issues with other devices in unusual_uas.h that currently have all UAS support disabled. I'd be happy to write the patch, but I'm not sure we want to reserve a quirk bit for what's currently only a single device known to have this issue. Please advise. On 05/17/2018 12:13 PM, Alan Stern wrote: > On Thu, 17 May 2018, Alexander Kappner wrote: > >> Oliver and Alan, >> >> thank for investigating. >> >>> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT >>> by itself would make the device work. Can you please test that? >> >> US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, >> even with the patch you included applied: > > Are you certain Oliver's new code was executed? If you put > US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then > it would not affect the uas driver. > >>> That's bizarre too. Even though the only difference is a MODE SENSE >>> command, the command that actually faliled was WRITE(16). >> It looks to me like the MODE SENSE simply hangs the drive, so anything >> issued after that will fail. Of course the drive says it's the "current >> command" that caused the failure, but I wouldn't give too much credence to >> that. FYI -- this device is a consumer grade rotational drive that you can >> get for less than $200, so I wouldn't be surprised if the implementations >> have issues. >> >> Also, I noticed that copying onto the drive with dd works fine, whereas >> trying to mount a filesystem immediately crashes it. I suspect this is >> because check_disk_change is called on mount (which eventually calls down >> to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag >> comes into play). > > Was this tested with uas or usb-storage? > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-18 17:51 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-18 17:51 UTC (permalink / raw) To: Alan Stern; +Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb > Was this tested with uas or usb-storage? On both. dd works either way. > Are you certain Oliver's new code was executed? If you put > US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then > it would not affect the uas driver. Yes, the code ran. Further debugging shows that the code that causes the device to hang is in drivers/scsi/sd.c:2698. So the reason why usb-storage works and UAS does not is because the device setting both skip_ms_page_3f=1 and skip_ms_page_8=1 is required. The US_FL_NO_WP_DETECT flag does the former, and usb-storage (but not UAS) by default does the latter (drivers/usb/storage/scsiglue.c:198): /* * A number of devices have problems with MODE SENSE for * page x08, so we will skip it. */ sdev->skip_ms_page_8 = 1; Forcing both skip_ms_page_3f and skip_ms_page_8 to 1 results in UAS and usb-storage working. Oliver's code only set skip_ms_page_3f. Do we want a patch to introduce a quirk flag for skip_ms_page_8, similar to the US_FL_NO_WP_DETECT quirk bit for skip_ms_page_3f? This may even resolve the issues with other devices in unusual_uas.h that currently have all UAS support disabled. I'd be happy to write the patch, but I'm not sure we want to reserve a quirk bit for what's currently only a single device known to have this issue. Please advise. On 05/17/2018 12:13 PM, Alan Stern wrote: > On Thu, 17 May 2018, Alexander Kappner wrote: > >> Oliver and Alan, >> >> thank for investigating. >> >>> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT >>> by itself would make the device work. Can you please test that? >> >> US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, >> even with the patch you included applied: > > Are you certain Oliver's new code was executed? If you put > US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then > it would not affect the uas driver. > >>> That's bizarre too. Even though the only difference is a MODE SENSE >>> command, the command that actually faliled was WRITE(16). >> It looks to me like the MODE SENSE simply hangs the drive, so anything >> issued after that will fail. Of course the drive says it's the "current >> command" that caused the failure, but I wouldn't give too much credence to >> that. FYI -- this device is a consumer grade rotational drive that you can >> get for less than $200, so I wouldn't be surprised if the implementations >> have issues. >> >> Also, I noticed that copying onto the drive with dd works fine, whereas >> trying to mount a filesystem immediately crashes it. I suspect this is >> because check_disk_change is called on mount (which eventually calls down >> to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag >> comes into play). > > Was this tested with uas or usb-storage? > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-18 20:35 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-18 20:35 UTC (permalink / raw) To: Alexander Kappner Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb On Fri, 18 May 2018, Alexander Kappner wrote: > Further debugging shows that the code that causes the device to hang is in > drivers/scsi/sd.c:2698. So the reason why usb-storage works and UAS does > not is because the device setting both skip_ms_page_3f=1 and > skip_ms_page_8=1 is required. The US_FL_NO_WP_DETECT flag does the former, > and usb-storage (but not UAS) by default does the latter > (drivers/usb/storage/scsiglue.c:198): > > /* > * A number of devices have problems with MODE SENSE for > * page x08, so we will skip it. > */ > sdev->skip_ms_page_8 = 1; > > > Forcing both skip_ms_page_3f and skip_ms_page_8 to 1 results in UAS and > usb-storage working. Oliver's code only set skip_ms_page_3f. Good detective work! > Do we want a patch to introduce a quirk flag for skip_ms_page_8, similar to > the US_FL_NO_WP_DETECT quirk bit for skip_ms_page_3f? This may even resolve > the issues with other devices in unusual_uas.h that currently have all UAS > support disabled. I'd be happy to write the patch, but I'm not sure we want > to reserve a quirk bit for what's currently only a single device known to > have this issue. Please advise. Yes, I think we want a patch. Unless Oliver disagrees, please go ahead and prepare one. Alan Stern ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-18 20:35 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-18 20:35 UTC (permalink / raw) To: Alexander Kappner Cc: Oliver Neukum, gregkh, usb-storage, linux-kernel, linux-usb On Fri, 18 May 2018, Alexander Kappner wrote: > Further debugging shows that the code that causes the device to hang is in > drivers/scsi/sd.c:2698. So the reason why usb-storage works and UAS does > not is because the device setting both skip_ms_page_3f=1 and > skip_ms_page_8=1 is required. The US_FL_NO_WP_DETECT flag does the former, > and usb-storage (but not UAS) by default does the latter > (drivers/usb/storage/scsiglue.c:198): > > /* > * A number of devices have problems with MODE SENSE for > * page x08, so we will skip it. > */ > sdev->skip_ms_page_8 = 1; > > > Forcing both skip_ms_page_3f and skip_ms_page_8 to 1 results in UAS and > usb-storage working. Oliver's code only set skip_ms_page_3f. Good detective work! > Do we want a patch to introduce a quirk flag for skip_ms_page_8, similar to > the US_FL_NO_WP_DETECT quirk bit for skip_ms_page_3f? This may even resolve > the issues with other devices in unusual_uas.h that currently have all UAS > support disabled. I'd be happy to write the patch, but I'm not sure we want > to reserve a quirk bit for what's currently only a single device known to > have this issue. Please advise. Yes, I think we want a patch. Unless Oliver disagrees, please go ahead and prepare one. Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then adds the flag as quirks for the device at issue. This allows the G-Drive to work under both UAS and usb-storage. Alexander Kappner (2): [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive drivers/usb/storage/uas.c | 6 ++++++ drivers/usb/storage/unusual_devs.h | 9 +++++++++ drivers/usb/storage/unusual_uas.h | 9 +++++++++ 3 files changed, 24 insertions(+) -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then adds the flag as quirks for the device at issue. This allows the G-Drive to work under both UAS and usb-storage. Alexander Kappner (2): [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive drivers/usb/storage/uas.c | 6 ++++++ drivers/usb/storage/unusual_devs.h | 9 +++++++++ drivers/usb/storage/unusual_uas.h | 9 +++++++++ 3 files changed, 24 insertions(+) ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/2] usb-storage: Add support for FL_ALWAYS_SYNC flag in the UAS driver @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not UAS and is required to work around devices that become unstable upon being queried for cache. This code is taken straight from: drivers/usb/storage/scsiglue.c:284 Signed-off-by: Alexander Kappner <agk@godking.net> Acked-by: Alan Stern <stern@rowland.harvard.edu> Cc: stable <stable@vger.kernel.org> Acked-by: Oliver Neukum <oneukum@suse.com> --- drivers/usb/storage/uas.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6034c39..9e9de54 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -836,6 +836,12 @@ static int uas_slave_configure(struct scsi_device *sdev) if (devinfo->flags & US_FL_BROKEN_FUA) sdev->broken_fua = 1; + /* UAS also needs to support FL_ALWAYS_SYNC */ + if (devinfo->flags & US_FL_ALWAYS_SYNC) { + sdev->skip_ms_page_3f = 1; + sdev->skip_ms_page_8 = 1; + sdev->wce_default_on = 1; + } scsi_change_queue_depth(sdev, devinfo->qdepth - 2); return 0; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [v2,1/2,usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not UAS and is required to work around devices that become unstable upon being queried for cache. This code is taken straight from: drivers/usb/storage/scsiglue.c:284 Signed-off-by: Alexander Kappner <agk@godking.net> --- drivers/usb/storage/uas.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6034c39..9e9de54 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -836,6 +836,12 @@ static int uas_slave_configure(struct scsi_device *sdev) if (devinfo->flags & US_FL_BROKEN_FUA) sdev->broken_fua = 1; + /* UAS also needs to support FL_ALWAYS_SYNC */ + if (devinfo->flags & US_FL_ALWAYS_SYNC) { + sdev->skip_ms_page_3f = 1; + sdev->skip_ms_page_8 = 1; + sdev->wce_default_on = 1; + } scsi_change_queue_depth(sdev, devinfo->qdepth - 2); return 0; } ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver @ 2018-05-22 8:58 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-22 8:58 UTC (permalink / raw) To: Alexander Kappner, stern; +Cc: linux-kernel, linux-usb, usb-storage, gregkh Am Freitag, den 18.05.2018, 21:50 -0700 schrieb Alexander Kappner: > The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not UAS > and is required to work around devices that become unstable upon being > queried for cache. This code is taken straight from: > drivers/usb/storage/scsiglue.c:284 > > Signed-off-by: Alexander Kappner <agk@godking.net> Acked-by: Oliver Neukum <oneukum@suse.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [v2,1/2,usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver @ 2018-05-22 8:58 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-22 8:58 UTC (permalink / raw) To: Alexander Kappner, stern; +Cc: linux-kernel, linux-usb, usb-storage, gregkh Am Freitag, den 18.05.2018, 21:50 -0700 schrieb Alexander Kappner: > The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not UAS > and is required to work around devices that become unstable upon being > queried for cache. This code is taken straight from: > drivers/usb/storage/scsiglue.c:284 > > Signed-off-by: Alexander Kappner <agk@godking.net> Acked-by: Oliver Neukum <oneukum@suse.com> --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/2] usb-storage: Add compatibility quirk flags for G-Technologies G-Drive @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner The "G-Drive" (sold by G-Technology) external USB 3.0 drive hangs on write access under UAS and usb-storage: [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584052] Aborting journal on device sdi-8. The proposed patch adds compatibility quirks. Because the drive requires two quirks (one to work with UAS, and another to work with usb-storage), adding this under unusual_devs.h and not just unusual_uas.h so kernels compiled without UAS receive the quirk. With the patch, the drive works reliably on UAS and usb- storage. (tested on NEC Corporation uPD720200 USB 3.0 host controller). Signed-off-by: Alexander Kappner <agk@godking.net> Acked-by: Alan Stern <stern@rowland.harvard.edu> Cc: stable <stable@vger.kernel.org> --- drivers/usb/storage/unusual_devs.h | 9 +++++++++ drivers/usb/storage/unusual_uas.h | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index 747d3a9..22fcfcc 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, "Micro Mini 1GB", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* "G-DRIVE" external HDD hangs on write without these. + * Patch submitted by Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_ALWAYS_SYNC), + /* * Nick Bowler <nbowler@elliptictech.com> * SCSI stack spams (otherwise harmless) error messages. diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 38434d8..d0bdebd 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -107,3 +107,12 @@ UNUSUAL_DEV(0x4971, 0x8017, 0x0000, 0x9999, "External HDD", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_REPORT_OPCODES), + +/* "G-DRIVE" external HDD hangs on write without these. + * Patch submitted by Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_ALWAYS_SYNC), -- 2.1.4 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [v2,2/2,usb-storage] Add compatibility quirk flags for G-Technologies G-Drive @ 2018-05-19 4:50 ` Alexander Kappner 0 siblings, 0 replies; 30+ messages in thread From: Alexander Kappner @ 2018-05-19 4:50 UTC (permalink / raw) To: stern, oneukum Cc: linux-kernel, linux-usb, usb-storage, gregkh, Alexander Kappner The "G-Drive" (sold by G-Technology) external USB 3.0 drive hangs on write access under UAS and usb-storage: [ 136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 [ 136.079180] print_req_error: critical target error, dev sdi, sector 0 [ 136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page write [ 136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: (null) [ 140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current] [ 140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb [ 140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 [ 140.584025] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584044] print_req_error: critical target error, dev sdi, sector 3905159192 [ 140.584052] Aborting journal on device sdi-8. The proposed patch adds compatibility quirks. Because the drive requires two quirks (one to work with UAS, and another to work with usb-storage), adding this under unusual_devs.h and not just unusual_uas.h so kernels compiled without UAS receive the quirk. With the patch, the drive works reliably on UAS and usb- storage. (tested on NEC Corporation uPD720200 USB 3.0 host controller). Signed-off-by: Alexander Kappner <agk@godking.net> --- drivers/usb/storage/unusual_devs.h | 9 +++++++++ drivers/usb/storage/unusual_uas.h | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index 747d3a9..22fcfcc 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2321,6 +2321,15 @@ UNUSUAL_DEV( 0x4146, 0xba01, 0x0100, 0x0100, "Micro Mini 1GB", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* "G-DRIVE" external HDD hangs on write without these. + * Patch submitted by Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_ALWAYS_SYNC), + /* * Nick Bowler <nbowler@elliptictech.com> * SCSI stack spams (otherwise harmless) error messages. diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 38434d8..d0bdebd 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -107,3 +107,12 @@ UNUSUAL_DEV(0x4971, 0x8017, 0x0000, 0x9999, "External HDD", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_REPORT_OPCODES), + +/* "G-DRIVE" external HDD hangs on write without these. + * Patch submitted by Alexander Kappner <agk@godking.net> + */ +UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999, + "SimpleTech", + "External HDD", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_ALWAYS_SYNC), ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-21 17:47 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-21 17:47 UTC (permalink / raw) To: Alexander Kappner; +Cc: oneukum, linux-kernel, linux-usb, usb-storage, gregkh On Fri, 18 May 2018, Alexander Kappner wrote: > v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then > adds the flag as quirks for the device at issue. This allows the G-Drive to work > under both UAS and usb-storage. > > Alexander Kappner (2): > [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver > [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive > > drivers/usb/storage/uas.c | 6 ++++++ > drivers/usb/storage/unusual_devs.h | 9 +++++++++ > drivers/usb/storage/unusual_uas.h | 9 +++++++++ > 3 files changed, 24 insertions(+) Acked-by: Alan Stern <stern@rowland.harvard.edu> This is okay with me. Oliver, what do you think? Alan Stern ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-21 17:47 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-21 17:47 UTC (permalink / raw) To: Alexander Kappner; +Cc: oneukum, linux-kernel, linux-usb, usb-storage, gregkh On Fri, 18 May 2018, Alexander Kappner wrote: > v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then > adds the flag as quirks for the device at issue. This allows the G-Drive to work > under both UAS and usb-storage. > > Alexander Kappner (2): > [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver > [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive > > drivers/usb/storage/uas.c | 6 ++++++ > drivers/usb/storage/unusual_devs.h | 9 +++++++++ > drivers/usb/storage/unusual_uas.h | 9 +++++++++ > 3 files changed, 24 insertions(+) Acked-by: Alan Stern <stern@rowland.harvard.edu> This is okay with me. Oliver, what do you think? Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [usb-storage] Re: [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-22 8:57 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-22 8:57 UTC (permalink / raw) To: Alan Stern, Alexander Kappner Cc: linux-kernel, linux-usb, usb-storage, gregkh Am Montag, den 21.05.2018, 13:47 -0400 schrieb Alan Stern: > On Fri, 18 May 2018, Alexander Kappner wrote: > > > v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then > > adds the flag as quirks for the device at issue. This allows the G-Drive to work > > under both UAS and usb-storage. > > > > Alexander Kappner (2): > > [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver > > [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive > > > > drivers/usb/storage/uas.c | 6 ++++++ > > drivers/usb/storage/unusual_devs.h | 9 +++++++++ > > drivers/usb/storage/unusual_uas.h | 9 +++++++++ > > 3 files changed, 24 insertions(+) > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > This is okay with me. Oliver, what do you think? Perfect. Regards Oliver ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-22 8:57 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2018-05-22 8:57 UTC (permalink / raw) To: Alan Stern, Alexander Kappner Cc: linux-kernel, linux-usb, usb-storage, gregkh Am Montag, den 21.05.2018, 13:47 -0400 schrieb Alan Stern: > On Fri, 18 May 2018, Alexander Kappner wrote: > > > v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then > > adds the flag as quirks for the device at issue. This allows the G-Drive to work > > under both UAS and usb-storage. > > > > Alexander Kappner (2): > > [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver > > [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive > > > > drivers/usb/storage/uas.c | 6 ++++++ > > drivers/usb/storage/unusual_devs.h | 9 +++++++++ > > drivers/usb/storage/unusual_uas.h | 9 +++++++++ > > 3 files changed, 24 insertions(+) > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > This is okay with me. Oliver, what do you think? Perfect. Regards Oliver --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [usb-storage] Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 14:44 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-17 14:44 UTC (permalink / raw) To: Alexander Kappner; +Cc: gregkh, linux-usb, usb-storage, linux-kernel On Thu, 17 May 2018, Alexander Kappner wrote: > Hi Alan, > > thanks for reviewing. (This is my first contribution that touches > usb-storage, so please bear with me.) > > > That's kind of weird. Does the drive work under Windows in UAS mode? > > On the Windows 10 VM that I just spun up for testing this, access to the > drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the > answer is no. > > > If so, why are the WRITE(16) commands failing under Linux? > > I don't know exactly why they're failing, but another entry in the > unusual_uas.h shows another SimpleTech device (only with a slightly > different product ID) needing the same UAS quirk (see > https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those > drives are just buggy. Okay, that's quite possible. > > That doesn't quite make sense. Since you prevent the uas driver from > > binding to this device, it will end up using usb-storage no matter how > > the kernel was built. Therefore the second quirk flag has to go into > > unusual_devs.h no matter what. > > Yes that's what I was trying to get at. So even though the UAS flag would > conceptually belong into unusual_uas, I'm putting both into unusual_devs to > avoid having to create two separate entries for the same device. > > > You don't describe the second quirk flag at all. Are you sure it is > > needed? > > Yes. Without this flag, the device keeps throwing similar errors on > usb-storage. That's the same result I get on a host that doesn't have UAS > compiled in. Here's a dmesg: > > [ 2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, bcdDevice=24.03 > [ 2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 > [ 2.184980] usb 3-1: Product: G-DRIVE > [ 2.185447] usb 3-1: Manufacturer: HGST > [ 2.185829] usb 3-1: SerialNumber: AA015010004C > [ 2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected > [ 2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS > [ 2.202981] scsi host2: usb-storage 3-1:1.0 > ... > [ 3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE 2403 PQ: 0 ANSI: 6 > [ 3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0 > [ 3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ CAPACITY(16). > [ 3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 TB/3.64 TiB) > [ 3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks > [ 3.241255] sd 2:0:0:0: [sda] Write Protect is off > [ 3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08 > [ 3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA > [ 3.257893] sda: sda1 sda9 > [ 3.261402] sd 2:0:0:0: [sda] Attached SCSI disk > ... > [ 92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 > [ 92.437493] print_req_error: critical target error, dev sda, sector 0 > [ 92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page write > [ 92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null) > [ 101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 > [ 101.452906] print_req_error: critical target error, dev sda, sector 3905159192 > [ 101.453593] print_req_error: critical target error, dev sda, sector 3905159192 > [ 101.454889] Aborting journal on device sda-8. > [ 101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 That's bizarre too. Even though the only difference is a MODE SENSE command, the command that actually failed was WRITE(16). > Source code comments describe this as a known problem (scsiglue.c:182): > > /* > * Some devices don't like MODE SENSE with page=0x3f, > * which is the command used for checking if a device > * is write-protected. Now that we tell the sd driver > * to do a 192-byte transfer with this command the > * majority of devices work fine, but a few still can't > * handle it. The sd driver will simply assume those > * devices are write-enabled. > */ > if (us->fflags & US_FL_NO_WP_DETECT) > sdev->skip_ms_page_3f = 1; Yeah, but that comment referred to driver that fail when they receive the MODE SENSE. Have you tried using uas _and_ setting the sdev->skip_ms_page_3f flag? Alan Stern ^ permalink raw reply [flat|nested] 30+ messages in thread
* usb-storage: Add quirks to make G-Technology "G-Drive" work @ 2018-05-17 14:44 ` Alan Stern 0 siblings, 0 replies; 30+ messages in thread From: Alan Stern @ 2018-05-17 14:44 UTC (permalink / raw) To: Alexander Kappner; +Cc: gregkh, linux-usb, usb-storage, linux-kernel On Thu, 17 May 2018, Alexander Kappner wrote: > Hi Alan, > > thanks for reviewing. (This is my first contribution that touches > usb-storage, so please bear with me.) > > > That's kind of weird. Does the drive work under Windows in UAS mode? > > On the Windows 10 VM that I just spun up for testing this, access to the > drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the > answer is no. > > > If so, why are the WRITE(16) commands failing under Linux? > > I don't know exactly why they're failing, but another entry in the > unusual_uas.h shows another SimpleTech device (only with a slightly > different product ID) needing the same UAS quirk (see > https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those > drives are just buggy. Okay, that's quite possible. > > That doesn't quite make sense. Since you prevent the uas driver from > > binding to this device, it will end up using usb-storage no matter how > > the kernel was built. Therefore the second quirk flag has to go into > > unusual_devs.h no matter what. > > Yes that's what I was trying to get at. So even though the UAS flag would > conceptually belong into unusual_uas, I'm putting both into unusual_devs to > avoid having to create two separate entries for the same device. > > > You don't describe the second quirk flag at all. Are you sure it is > > needed? > > Yes. Without this flag, the device keeps throwing similar errors on > usb-storage. That's the same result I get on a host that doesn't have UAS > compiled in. Here's a dmesg: > > [ 2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, bcdDevice=24.03 > [ 2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 > [ 2.184980] usb 3-1: Product: G-DRIVE > [ 2.185447] usb 3-1: Manufacturer: HGST > [ 2.185829] usb 3-1: SerialNumber: AA015010004C > [ 2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected > [ 2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS > [ 2.202981] scsi host2: usb-storage 3-1:1.0 > ... > [ 3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE 2403 PQ: 0 ANSI: 6 > [ 3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0 > [ 3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ CAPACITY(16). > [ 3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 TB/3.64 TiB) > [ 3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks > [ 3.241255] sd 2:0:0:0: [sda] Write Protect is off > [ 3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08 > [ 3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA > [ 3.257893] sda: sda1 sda9 > [ 3.261402] sd 2:0:0:0: [sda] Attached SCSI disk > ... > [ 92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 00 00 00 00 00 08 00 00 > [ 92.437493] print_req_error: critical target error, dev sda, sector 0 > [ 92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page write > [ 92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: (null) > [ 101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 18 00 00 00 08 00 00 > [ 101.452906] print_req_error: critical target error, dev sda, sector 3905159192 > [ 101.453593] print_req_error: critical target error, dev sda, sector 3905159192 > [ 101.454889] Aborting journal on device sda-8. > [ 101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [ 101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] > [ 101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb > [ 101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 00 00 00 00 00 08 00 00 That's bizarre too. Even though the only difference is a MODE SENSE command, the command that actually failed was WRITE(16). > Source code comments describe this as a known problem (scsiglue.c:182): > > /* > * Some devices don't like MODE SENSE with page=0x3f, > * which is the command used for checking if a device > * is write-protected. Now that we tell the sd driver > * to do a 192-byte transfer with this command the > * majority of devices work fine, but a few still can't > * handle it. The sd driver will simply assume those > * devices are write-enabled. > */ > if (us->fflags & US_FL_NO_WP_DETECT) > sdev->skip_ms_page_3f = 1; Yeah, but that comment referred to driver that fail when they receive the MODE SENSE. Have you tried using uas _and_ setting the sdev->skip_ms_page_3f flag? Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2018-05-22 9:05 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-16 20:01 [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work Alexander Kappner 2018-05-16 20:01 ` Alexander Kappner 2018-05-16 20:55 ` [PATCH] " Alan Stern 2018-05-16 20:55 ` Alan Stern 2018-05-17 8:15 ` [PATCH] " Alexander Kappner 2018-05-17 8:15 ` Alexander Kappner 2018-05-17 12:58 ` [PATCH] " Oliver Neukum 2018-05-17 12:58 ` Oliver Neukum 2018-05-17 18:38 ` [PATCH] " Alexander Kappner 2018-05-17 18:38 ` Alexander Kappner 2018-05-17 19:13 ` [PATCH] " Alan Stern 2018-05-17 19:13 ` Alan Stern 2018-05-18 17:51 ` [PATCH] " Alexander Kappner 2018-05-18 17:51 ` Alexander Kappner 2018-05-18 20:35 ` [PATCH] " Alan Stern 2018-05-18 20:35 ` Alan Stern 2018-05-19 4:50 ` [PATCH v2 0/2] " Alexander Kappner 2018-05-19 4:50 ` Alexander Kappner 2018-05-19 4:50 ` [PATCH v2 1/2] usb-storage: Add support for FL_ALWAYS_SYNC flag in the UAS driver Alexander Kappner 2018-05-19 4:50 ` [v2,1/2,usb-storage] " Alexander Kappner 2018-05-22 8:58 ` [PATCH v2 1/2] [usb-storage] " Oliver Neukum 2018-05-22 8:58 ` [v2,1/2,usb-storage] " Oliver Neukum 2018-05-19 4:50 ` [PATCH v2 2/2] usb-storage: Add compatibility quirk flags for G-Technologies G-Drive Alexander Kappner 2018-05-19 4:50 ` [v2,2/2,usb-storage] " Alexander Kappner 2018-05-21 17:47 ` [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work Alan Stern 2018-05-21 17:47 ` Alan Stern 2018-05-22 8:57 ` [usb-storage] Re: [PATCH v2 0/2] " Oliver Neukum 2018-05-22 8:57 ` Oliver Neukum 2018-05-17 14:44 ` [usb-storage] Re: [PATCH] " Alan Stern 2018-05-17 14:44 ` Alan Stern
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.