diff mbox series

[v6,2/3] usb-storage: Add Hiksemi USB3-FW to IGNORE_UAS

Message ID 1663210188-5485-1-git-send-email-zenghongling@kylinos.cn
State Superseded
Headers show
Series [v6,1/3] uas: add no-uas quirk for Hiksemi usb_disk | expand

Commit Message

Hongling Zeng Sept. 15, 2022, 2:49 a.m. UTC
The UAS mode of Hiksemi USB_HDD is reported to fail to work on several
platforms with the following error message, then after re-connecting the
device will be offlined and not working at all.

[  592.518442][ 2] sd 8:0:0:0: [sda] tag#17 uas_eh_abort_handler 0 uas-tag 18
                   inflight: CMD
[  592.527575][ 2] sd 8:0:0:0: [sda] tag#17 CDB: Write(10) 2a 00 03 6f 88 00 00
                   04 00 00
[  592.536330][ 2] sd 8:0:0:0: [sda] tag#0 uas_eh_abort_handler 0 uas-tag 1
                   inflight: CMD
[  592.545266][ 2] sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 07 44 1a 88 00
                   00 08 00

These disks have a broken uas implementation, the tag field of the status
iu-s is not set properly,so we need to fall-back to usb-storage.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change for v1
 - Change the email real name and the code worng place.

Change for v2
 -Change spelling error.

Change for v3
 -Add acked-by

Change for v4
 -Fix version error

Change for v5
 -change version

Change for v6
 -Change the git message for patch 3
---
 drivers/usb/storage/unusual_uas.h | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Alan Stern Sept. 15, 2022, 7:32 p.m. UTC | #1
On Thu, Sep 15, 2022 at 10:49:48AM +0800, Hongling Zeng wrote:
> The UAS mode of Hiksemi USB_HDD is reported to fail to work on several
> platforms with the following error message, then after re-connecting the
> device will be offlined and not working at all.
> 
> [  592.518442][ 2] sd 8:0:0:0: [sda] tag#17 uas_eh_abort_handler 0 uas-tag 18
>                    inflight: CMD
> [  592.527575][ 2] sd 8:0:0:0: [sda] tag#17 CDB: Write(10) 2a 00 03 6f 88 00 00
>                    04 00 00
> [  592.536330][ 2] sd 8:0:0:0: [sda] tag#0 uas_eh_abort_handler 0 uas-tag 1
>                    inflight: CMD
> [  592.545266][ 2] sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 07 44 1a 88 00
>                    00 08 00
> 
> These disks have a broken uas implementation, the tag field of the status
> iu-s is not set properly,so we need to fall-back to usb-storage.
> 
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> Change for v1
>  - Change the email real name and the code worng place.
> 
> Change for v2
>  -Change spelling error.
> 
> Change for v3
>  -Add acked-by
> 
> Change for v4
>  -Fix version error
> 
> Change for v5
>  -change version
> 
> Change for v6
>  -Change the git message for patch 3
> ---

I already sent you an Acked-by: in v3 of this patch, and the patch 
hasn't changed significantly since then so you can keep the Acked-by: in 
this version.

Alan Stern

>  drivers/usb/storage/unusual_uas.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> index a6bf87a..8a18d58 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -149,6 +149,13 @@ UNUSUAL_DEV(0x0bc2, 0xab2a, 0x0000, 0x9999,
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>  		US_FL_NO_ATA_1X),
>  
> +/* Reported-by: Hongling Zeng <zenghongling@kylinos.cn> */
> +UNUSUAL_DEV(0x0bda, 0x9210, 0x0000, 0x9999,
> +		"Hiksemi",
> +		"External HDD",
> +		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> +		US_FL_IGNORE_UAS),
> +
>  /* Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> */
>  UNUSUAL_DEV(0x13fd, 0x3940, 0x0000, 0x9999,
>  		"Initio Corporation",
> -- 
> 2.1.0
Alan Stern Sept. 16, 2022, 5:21 p.m. UTC | #2
On Fri, Sep 16, 2022 at 02:55:55PM +0800, nana wrote:
> Hi:
> 
>     ok,I have upated to v7.
> 
>     Sorry , The information in the previous answers about thinkplus(0x17ef,
> 0x3899) comes from the tester, which is somewhat confused.
> 
>     I just get the device from other department and the detailed test data
> are as follows:
> 
> 
> Test the thinkplus(0x17ef, 0x3899) speed by gnome-disk-utility tool.
> 
> linux(load uas):                 linux(not load uas):
> 
> read: 394.2 MB/s                read:  382.2 MB/s
> 
> 
> write:  377.4 MB/s                 write:  347.8 MB/s
> 
> 
> ---------------------
> 
> win10(lower than linux,but more stable):
> 
> read : 388.1 MB/s
> 
> write: 320   MB/s
> 
> 
> The main performance differences is write speed.but load uas can cause error
> after long run, not good compatible with uas, So two bosses can
> consideration for the third patch is it feasible.

Is there any way for you to tell which protocol Windows uses 
(usb-storage or UAS)?

Alan Stern
zhongling0719@126.com Sept. 18, 2022, 12:25 p.m. UTC | #3
I checked that the device manager was not load the uas driver, And this lower write speed for win10 doesn't seem to use uas protocol for this device.


zhongling0719@126.com
 
From: Alan Stern
Date: 2022-09-17 01:21
To: nana
CC: gregkh; USB mailing list
Subject: Re: [PATCH v6 2/3] usb-storage: Add Hiksemi USB3-FW to IGNORE_UAS
On Fri, Sep 16, 2022 at 02:55:55PM +0800, nana wrote:
> Hi:
>
>     ok,I have upated to v7.
>
>     Sorry , The information in the previous answers about thinkplus(0x17ef,
> 0x3899) comes from the tester, which is somewhat confused.
>
>     I just get the device from other department and the detailed test data
> are as follows:
>
>
> Test the thinkplus(0x17ef, 0x3899) speed by gnome-disk-utility tool.
>
> linux(load uas):                 linux(not load uas):
>
> read: 394.2 MB/s                read:  382.2 MB/s
>
>
> write:  377.4 MB/s                 write:  347.8 MB/s
>
>
> ---------------------
>
> win10(lower than linux,but more stable):
>
> read : 388.1 MB/s
>
> write: 320   MB/s
>
>
> The main performance differences is write speed.but load uas can cause error
> after long run, not good compatible with uas, So two bosses can
> consideration for the third patch is it feasible.
 
Is there any way for you to tell which protocol Windows uses
(usb-storage or UAS)?
 
Alan Stern
zhongling0719@126.com Sept. 22, 2022, 8:17 a.m. UTC | #4
Hi

    >Is there any way for you to tell which protocol Windows uses

(usb-storage or UAS)?

I checked that the device manager was not load the uas driver, And this 
lower write speed for win10 doesn't seem to use uas protocol for this 
device.

    if you have questions for [PATCH v7 3/3], I can continue to 
investigate thinkplus(0x17ef, 0x3899) and first submit patch v7 1/3 and 
patch v7 2/3.
    you have anything unclear ,can ask me or advise me?

Thanks!

在 2022年09月17日 01:21, Alan Stern 写道:
> On Fri, Sep 16, 2022 at 02:55:55PM +0800, nana wrote:
>> Hi:
>>
>>      ok,I have upated to v7.
>>
>>      Sorry , The information in the previous answers about thinkplus(0x17ef,
>> 0x3899) comes from the tester, which is somewhat confused.
>>
>>      I just get the device from other department and the detailed test data
>> are as follows:
>>
>>
>> Test the thinkplus(0x17ef, 0x3899) speed by gnome-disk-utility tool.
>>
>> linux(load uas):                 linux(not load uas):
>>
>> read: 394.2 MB/s                read:  382.2 MB/s
>>
>>
>> write:  377.4 MB/s                 write:  347.8 MB/s
>>
>>
>> ---------------------
>>
>> win10(lower than linux,but more stable):
>>
>> read : 388.1 MB/s
>>
>> write: 320   MB/s
>>
>>
>> The main performance differences is write speed.but load uas can cause error
>> after long run, not good compatible with uas, So two bosses can
>> consideration for the third patch is it feasible.
> Is there any way for you to tell which protocol Windows uses
> (usb-storage or UAS)?
>
> Alan Stern
>
Alan Stern Sept. 22, 2022, 3:16 p.m. UTC | #5
On Thu, Sep 22, 2022 at 04:17:41PM +0800, nana wrote:
> Hi
> 
>    >Is there any way for you to tell which protocol Windows uses
> 
> (usb-storage or UAS)?
> 
> I checked that the device manager was not load the uas driver, And this
> lower write speed for win10 doesn't seem to use uas protocol for this
> device.

Thank you.

>    if you have questions for [PATCH v7 3/3], I can continue to investigate
> thinkplus(0x17ef, 0x3899) and first submit patch v7 1/3 and patch v7 2/3.
>    you have anything unclear ,can ask me or advise me?

I have already sent Acked-by: messages for the 1/3 and 2/3 patches.  You 
can now add my Acked-by: to the 3/3 patch (the Thinkplus one).

Alan Stern
Juhyung Park Oct. 31, 2022, 6:17 a.m. UTC | #6
Hi,

I'm speaking from my own experience but RTL9210 is arguably the most 
reliable NVMe-to-USB converter available.

Compared to solutions from JMicron (multiple revisions) and ASMedia, 
RTL9210 gave the lowest power consumption (from implementing proper 
power management commands) and the least headache.

I personally own multiple enclosures and not one gave a single UAS error 
from multiple platforms (Android, Intel, AMD) for years, but with this 
commit now, it effectively disables UAS for all RTL9210 enclosures.

Would it be possible to apply this quirk only to a specific firmware 
(range)? RTL9210 have a lot of possible firmware combinations: 
https://www.station-drivers.com/index.php/en/component/remository/Drivers/Realtek/NVMe-USB-3.1/lang,en-gb/

RTL9210 was available since 2019 and the fact that this quirk came up 
this late leads me to believe that this is not a widespread issue and 
it'll be a shame if all of RTL9210s are blacklisted from UAS with Linux 
from now on :(

If any additional information is required, please let me know.

Thanks,

On 9/15/22 11:49, Hongling Zeng wrote:
> The UAS mode of Hiksemi USB_HDD is reported to fail to work on several
> platforms with the following error message, then after re-connecting the
> device will be offlined and not working at all.
> 
> [  592.518442][ 2] sd 8:0:0:0: [sda] tag#17 uas_eh_abort_handler 0 uas-tag 18
>                     inflight: CMD
> [  592.527575][ 2] sd 8:0:0:0: [sda] tag#17 CDB: Write(10) 2a 00 03 6f 88 00 00
>                     04 00 00
> [  592.536330][ 2] sd 8:0:0:0: [sda] tag#0 uas_eh_abort_handler 0 uas-tag 1
>                     inflight: CMD
> [  592.545266][ 2] sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 07 44 1a 88 00
>                     00 08 00
> 
> These disks have a broken uas implementation, the tag field of the status
> iu-s is not set properly,so we need to fall-back to usb-storage.
> 
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> Change for v1
>   - Change the email real name and the code worng place.
> 
> Change for v2
>   -Change spelling error.
> 
> Change for v3
>   -Add acked-by
> 
> Change for v4
>   -Fix version error
> 
> Change for v5
>   -change version
> 
> Change for v6
>   -Change the git message for patch 3
> ---
>   drivers/usb/storage/unusual_uas.h | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> index a6bf87a..8a18d58 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -149,6 +149,13 @@ UNUSUAL_DEV(0x0bc2, 0xab2a, 0x0000, 0x9999,
>   		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>   		US_FL_NO_ATA_1X),
>   
> +/* Reported-by: Hongling Zeng <zenghongling@kylinos.cn> */
> +UNUSUAL_DEV(0x0bda, 0x9210, 0x0000, 0x9999,
> +		"Hiksemi",
> +		"External HDD",
> +		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> +		US_FL_IGNORE_UAS),
> +
>   /* Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> */
>   UNUSUAL_DEV(0x13fd, 0x3940, 0x0000, 0x9999,
>   		"Initio Corporation",
Greg Kroah-Hartman Nov. 9, 2022, 10:40 a.m. UTC | #7
On Mon, Oct 31, 2022 at 03:17:48PM +0900, Juhyung Park wrote:
> Hi,
> 
> I'm speaking from my own experience but RTL9210 is arguably the most
> reliable NVMe-to-USB converter available.
> 
> Compared to solutions from JMicron (multiple revisions) and ASMedia, RTL9210
> gave the lowest power consumption (from implementing proper power management
> commands) and the least headache.
> 
> I personally own multiple enclosures and not one gave a single UAS error
> from multiple platforms (Android, Intel, AMD) for years, but with this
> commit now, it effectively disables UAS for all RTL9210 enclosures.
> 
> Would it be possible to apply this quirk only to a specific firmware
> (range)? RTL9210 have a lot of possible firmware combinations: https://www.station-drivers.com/index.php/en/component/remository/Drivers/Realtek/NVMe-USB-3.1/lang,en-gb/
> 
> RTL9210 was available since 2019 and the fact that this quirk came up this
> late leads me to believe that this is not a widespread issue and it'll be a
> shame if all of RTL9210s are blacklisted from UAS with Linux from now on :(
> 
> If any additional information is required, please let me know.

Can you send a revert of this commit so that we can fix this up?

thanks,

greg k-h
zhongling0719@126.com Nov. 12, 2022, 7:45 a.m. UTC | #8
Hi:
  This error not happend on all RTL9210,The uas blacklist only for on sale device(0x0bda, 0x9210). 

Thanks!

zhongling0719@126.com
 
From: Greg KH
Date: 2022-11-09 18:40
To: Juhyung Park
CC: Hongling Zeng; stern; linux-usb; usb-storage; zhongling0719
Subject: Re: [PATCH v6 2/3] usb-storage: Add Hiksemi USB3-FW to IGNORE_UAS
On Mon, Oct 31, 2022 at 03:17:48PM +0900, Juhyung Park wrote:
> Hi,
>
> I'm speaking from my own experience but RTL9210 is arguably the most
> reliable NVMe-to-USB converter available.
>
> Compared to solutions from JMicron (multiple revisions) and ASMedia, RTL9210
> gave the lowest power consumption (from implementing proper power management
> commands) and the least headache.
>
> I personally own multiple enclosures and not one gave a single UAS error
> from multiple platforms (Android, Intel, AMD) for years, but with this
> commit now, it effectively disables UAS for all RTL9210 enclosures.
>
> Would it be possible to apply this quirk only to a specific firmware
> (range)? RTL9210 have a lot of possible firmware combinations: https://www.station-drivers.com/index.php/en/component/remository/Drivers/Realtek/NVMe-USB-3.1/lang,en-gb/
>
> RTL9210 was available since 2019 and the fact that this quirk came up this
> late leads me to believe that this is not a widespread issue and it'll be a
> shame if all of RTL9210s are blacklisted from UAS with Linux from now on :(
>
> If any additional information is required, please let me know.
 
Can you send a revert of this commit so that we can fix this up?
 
thanks,
 
greg k-h
Juhyung Park Jan. 9, 2023, noon UTC | #9
Hi Greg,

I've posted a revert commit as there were no attempts to fix this to
apply just to the said product (Hiksemi USB3-FW):
https://lore.kernel.org/all/20230109115550.71688-1-qkrwngud825@gmail.com/T/#u

Thanks. Regards

On Sat, Nov 12, 2022 at 4:56 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Nov 12, 2022 at 03:45:36PM +0800, zhongling0719@126.com wrote:
> > Hi:
> >   This error not happend on all RTL9210,The uas blacklist only for on sale device(0x0bda, 0x9210).
>
> What about using the version of the device as the only one to blacklist?
> Any more specific information you have about that device?
>
> thanks,
>
> greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
index a6bf87a..8a18d58 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -149,6 +149,13 @@  UNUSUAL_DEV(0x0bc2, 0xab2a, 0x0000, 0x9999,
 		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
 		US_FL_NO_ATA_1X),
 
+/* Reported-by: Hongling Zeng <zenghongling@kylinos.cn> */
+UNUSUAL_DEV(0x0bda, 0x9210, 0x0000, 0x9999,
+		"Hiksemi",
+		"External HDD",
+		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+		US_FL_IGNORE_UAS),
+
 /* Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> */
 UNUSUAL_DEV(0x13fd, 0x3940, 0x0000, 0x9999,
 		"Initio Corporation",