usb_storage: skip all unknown devices when probing

PATCH New Soeren Moch 3 participants 7 messages Nov 6, 2014 – Nov 8, 2014

Share & tools

Soeren Moch

author

2014-11-06 12:51 UTC #0 lore raw

PATCH 1/1 usb_storage: skip all unknown devices when probing

Not only skip storage devices with DEV_TYPE_UNKNOWN, but also all devices which are unknown to u-boot (e.g., are not HARDDISK, TAPE, CDROM, OPDISK).

This especially avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".

Signed-off-by Soeren Moch <[email protected]>

--
Cc: Marek Vasut <[email protected]>
Cc: Tom Rini <[email protected]>

---
 common/usb_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

common/usb_storage.c +1 -1

index eb7706c..0ac7b48 100644
@@ -1351,7 +1351,7 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 	perq = usb_stor_buf[0];
 	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
+	if ((perq & 0x1f) > DEV_TYPE_OPDISK) {
 		/* skip unknown devices */
 		return 0;
 	}

Signature · 1 line

1.9.1

Marek Vasut

2014-11-06 20:41 UTC #1 lore raw

PATCH 1/1 usb_storage: skip all unknown devices when probing

Quoted 19 lines · Soeren Moch

Not only skip storage devices with DEV_TYPE_UNKNOWN, but also all devices which are unknown to u-boot (e.g., are not HARDDISK, TAPE, CDROM, OPDISK).
This especially avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".
Signed-off-by Soeren Moch <[email protected]>
--
Cc: Marek Vasut <[email protected]>
Cc: Tom Rini <[email protected]>
---
 common/usb_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/usb_storage.c +0 -0
index eb7706c..0ac7b48 100644

common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, struct · line 1354

Quoted 5 lines

common/usb_storage.c +1 -1

@@ -1351,7 +1351,7 @@ int usb_stor_get_info(struct usb_device *dev, struct
 us_data *ss, perq = usb_stor_buf[0];
 	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
+	if ((perq & 0x1f) > DEV_TYPE_OPDISK) {

Why can't you just blacklist 0xd instead ? I mean, this patch would do a bulk blacklisting of all the obscure devices with peripheral ID above 0x7, but might still work with this layer (like 0xc ... the RAID controller ; or 0xe ... the reduced block device).

Won't it make sense to just selectively blacklist the 0xd ?

common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, · line 1357

Quoted 3 lines

common/usb_storage.c +0 -0

 		/* skip unknown devices */
 		return 0;
 	}

Best regards,
Marek Vasut

Soeren Moch

author

2014-11-07 09:28 UTC #2 lore raw

PATCH 1/1 usb_storage: skip all unknown devices when probing

Quoted 31 lines · Marek Vasut

Quoted 19 lines · Soeren Moch
Not only skip storage devices with DEV_TYPE_UNKNOWN, but also all devices which are unknown to u-boot (e.g., are not HARDDISK, TAPE, CDROM, OPDISK).
This especially avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".
Signed-off-by Soeren Moch <[email protected]>
--
Cc: Marek Vasut <[email protected]>
Cc: Tom Rini <[email protected]>
---
  common/usb_storage.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
common/usb_storage.c +0 -0
index eb7706c..0ac7b48 100644
common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, struct · line 1354
Quoted 5 lines
common/usb_storage.c +1 -1
@@ -1351,7 +1351,7 @@ int usb_stor_get_info(struct usb_device *dev, struct
 us_data *ss, perq = usb_stor_buf[0];
  	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
+	if ((perq & 0x1f) > DEV_TYPE_OPDISK) {
Why can't you just blacklist 0xd instead ? I mean, this patch would do a bulk blacklisting of all the obscure devices with peripheral ID above 0x7, but might still work with this layer (like 0xc ... the RAID controller ; or 0xe ... the reduced block device).

Quoted 1 line

Won't it make sense to just selectively blacklist the 0xd ?

If you think it makes more sense to blacklist 0xd only, that is fine with me. I will prepare a new patch for this.

Best regards,
Soeren Moch

Quoted 8 lines

common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, · line 1357
Quoted 4 lines
common/usb_storage.c +0 -0
  		/* skip unknown devices */
  		return 0;
  	}
Best regards,
Marek Vasut

Marek Vasut

2014-11-07 09:29 UTC #3 lore raw

PATCH 1/1 usb_storage: skip all unknown devices when probing

Quoted 35 lines · Soeren Moch

Quoted 34 lines · Marek Vasut
Quoted 21 lines · Soeren Moch
Not only skip storage devices with DEV_TYPE_UNKNOWN, but also all devices which are unknown to u-boot (e.g., are not HARDDISK, TAPE, CDROM, OPDISK).
This especially avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".
Signed-off-by Soeren Moch <[email protected]>
--
Cc: Marek Vasut <[email protected]>
Cc: Tom Rini <[email protected]>
---
  common/usb_storage.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
common/usb_storage.c +0 -0
index eb7706c..0ac7b48 100644
common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, · line 1355
Quoted 6 lines
common/usb_storage.c +1 -1
@@ -1351,7 +1351,7 @@ int usb_stor_get_info(struct usb_device *dev,
 struct us_data *ss, perq = usb_stor_buf[0];
 
  	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
+	if ((perq & 0x1f) > DEV_TYPE_OPDISK) {
Why can't you just blacklist 0xd instead ? I mean, this patch would do a bulk blacklisting of all the obscure devices with peripheral ID above 0x7, but might still work with this layer (like 0xc ... the RAID controller ; or 0xe ... the reduced block device).

Quoted 4 lines

Quoted 1 line
Won't it make sense to just selectively blacklist the 0xd ?
If you think it makes more sense to blacklist 0xd only, that is fine with me. I will prepare a new patch for this.

Please also make sure to document why this change is in place. (because the Enclosure Services cause trouble). A simple comment in the code would be nice.

Thank you!

Best regards,
Marek Vasut

Soeren Moch

author

2014-11-08 06:02 UTC #4 lore raw

PATCH 1/1 usb_storage: blacklist Enclosure Service Devices

In reply to Soeren Moch

Skip enclosure service devices when probing for usb storage devices.

This avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".

Signed-off-by Soeren Moch <[email protected]>

This is a new version of the patch "usb_storage: skip all unknown devices when probing"
http://http://lists.denx.de/pipermail/u-boot/2014-November/194622.html

Cc Marek Vasut <[email protected]>

Cc Tom Rini <[email protected]>

---
 common/usb_storage.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

common/usb_storage.c +3 -2

index eb7706c..9198f73 100644
@@ -1351,8 +1351,9 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 	perq = usb_stor_buf[0];
 	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
-		/* skip unknown devices */
+	if (((perq & 0x1f) == 0x1f) || ((perq & 0x1f) == 0x0d)) {
+		/* skip unknown devices and enclosure service devices, */
+		/* they would not respond to test_unit_ready           */
 		return 0;
 	}
 	if ((modi&0x80) == 0x80) {

Signature · 1 line

1.9.1

Nikolay Dimitrov

2014-11-08 06:28 UTC #5 lore raw

PATCH 1/1 usb_storage: blacklist Enclosure Service Devices

Hi Soeren,

Quoted 25 lines · Soeren Moch

Skip enclosure service devices when probing for usb storage devices.
This avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".
Signed-off-by Soeren Moch <[email protected]>
--
This is a new version of the patch "usb_storage: skip all unknown devices when probing"
http://http://lists.denx.de/pipermail/u-boot/2014-November/194622.html
Cc Marek Vasut <[email protected]>
Cc Tom Rini <[email protected]>
---
  common/usb_storage.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
common/usb_storage.c +0 -0
index eb7706c..9198f73 100644
@@ -1351,8 +1351,9 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
  	perq = usb_stor_buf[0];

common/usb_storage.c int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, · line 1359

Quoted 10 lines

common/usb_storage.c +3 -2

  	modi = usb_stor_buf[1];
 
-	if ((perq & 0x1f) == 0x1f) {
-		/* skip unknown devices */
+	if (((perq & 0x1f) == 0x1f) || ((perq & 0x1f) == 0x0d)) {
+		/* skip unknown devices and enclosure service devices, */
+		/* they would not respond to test_unit_ready           */
  		return 0;
  	}
  	if ((modi&0x80) == 0x80) {

Is it possible to provide a configuration option and some default behavior, instead of just disabling it?

Regards,
Nikolay

Marek Vasut

2014-11-08 11:04 UTC #6 lore raw

PATCH 1/1 usb_storage: blacklist Enclosure Service Devices

In reply to Soeren Moch

Quoted 6 lines · Soeren Moch

Skip enclosure service devices when probing for usb storage devices.
This avoids long timeouts when probing for external usb harddisks which provide "Enclosure Services".
Signed-off-by Soeren Moch <[email protected]>

I tweaked the comment so it matches the patch guidelines [1] and applied. Thanks!

[1] http://www.denx.de/wiki/U-Boot/Patches

Best regards,
Marek Vasut