LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work 
@ 2018-05-16 20:01 Alexander Kappner
  2018-05-16 20:55 ` Alan Stern
  0 siblings, 1 reply; 15+ 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	[flat|nested] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-16 20:01 [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work Alexander Kappner
@ 2018-05-16 20:55 ` Alan Stern
  2018-05-17  8:15   ` Alexander Kappner
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-16 20:55 ` Alan Stern
@ 2018-05-17  8:15   ` Alexander Kappner
  2018-05-17 12:58     ` Oliver Neukum
  2018-05-17 14:44     ` [usb-storage] Re: [PATCH] " Alan Stern
  0 siblings, 2 replies; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-17  8:15   ` Alexander Kappner
@ 2018-05-17 12:58     ` Oliver Neukum
  2018-05-17 18:38       ` Alexander Kappner
  2018-05-17 14:44     ` [usb-storage] Re: [PATCH] " Alan Stern
  1 sibling, 1 reply; 15+ 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	[flat|nested] 15+ messages in thread

* Re: [usb-storage] Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-17  8:15   ` Alexander Kappner
  2018-05-17 12:58     ` Oliver Neukum
@ 2018-05-17 14:44     ` Alan Stern
  1 sibling, 0 replies; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-17 12:58     ` Oliver Neukum
@ 2018-05-17 18:38       ` Alexander Kappner
  2018-05-17 19:13         ` Alan Stern
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-17 18:38       ` Alexander Kappner
@ 2018-05-17 19:13         ` Alan Stern
  2018-05-18 17:51           ` Alexander Kappner
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-17 19:13         ` Alan Stern
@ 2018-05-18 17:51           ` Alexander Kappner
  2018-05-18 20:35             ` Alan Stern
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-18 17:51           ` Alexander Kappner
@ 2018-05-18 20:35             ` Alan Stern
  2018-05-19  4:50               ` [PATCH v2 0/2] " Alexander Kappner
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-18 20:35             ` Alan Stern
@ 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
                                   ` (2 more replies)
  0 siblings, 3 replies; 15+ 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] 15+ 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               ` [PATCH v2 0/2] " Alexander Kappner
@ 2018-05-19  4:50                 ` Alexander Kappner
  2018-05-22  8:58                   ` [PATCH 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-21 17:47                 ` [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work Alan Stern
  2 siblings, 1 reply; 15+ 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	[flat|nested] 15+ messages in thread

* [PATCH v2 2/2] usb-storage: Add compatibility quirk flags for G-Technologies G-Drive
  2018-05-19  4:50               ` [PATCH v2 0/2] " 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                 ` 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
  2 siblings, 0 replies; 15+ 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	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work
  2018-05-19  4:50               ` [PATCH v2 0/2] " 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                 ` [PATCH v2 2/2] usb-storage: Add compatibility quirk flags for G-Technologies G-Drive Alexander Kappner
@ 2018-05-21 17:47                 ` Alan Stern
  2018-05-22  8:57                   ` [usb-storage] " Oliver Neukum
  2 siblings, 1 reply; 15+ 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] 15+ 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-21 17:47                 ` [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work Alan Stern
@ 2018-05-22  8:57                   ` Oliver Neukum
  0 siblings, 0 replies; 15+ 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] 15+ messages in thread

* Re: [PATCH v2 1/2] [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver
  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-22  8:58                   ` Oliver Neukum
  0 siblings, 0 replies; 15+ 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] 15+ messages in thread

end of thread, other threads:[~2018-05-22  9:05 UTC | newest]

Thread overview: 15+ 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:55 ` Alan Stern
2018-05-17  8:15   ` Alexander Kappner
2018-05-17 12:58     ` Oliver Neukum
2018-05-17 18:38       ` Alexander Kappner
2018-05-17 19:13         ` Alan Stern
2018-05-18 17:51           ` Alexander Kappner
2018-05-18 20:35             ` Alan Stern
2018-05-19  4:50               ` [PATCH v2 0/2] " 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-22  8:58                   ` [PATCH 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-21 17:47                 ` [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work Alan Stern
2018-05-22  8:57                   ` [usb-storage] " Oliver Neukum
2018-05-17 14:44     ` [usb-storage] Re: [PATCH] " Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).