[U-Boot] [PATCH] usb_storage: skip all unknown devices when probing

[U-Boot] [PATCH] usb_storage: skip all unknown devices when probing

[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 <smoch@web.de>
--
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
 common/usb_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index eb7706c..0ac7b48 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -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;
 	}
-- 
1.9.1

[U-Boot] [PATCH] usb_storage: skip all unknown devices when probing

[Marek Vasut]

On Thursday, November 06, 2014 at 01:51:51 PM, Soeren Moch wrote:
> 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 <smoch@web.de>
> --
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>  common/usb_storage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index eb7706c..0ac7b48 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -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 ?

>  		/* skip unknown devices */
>  		return 0;
>  	}

Best regards,
Marek Vasut

[U-Boot] [PATCH] usb_storage: skip all unknown devices when probing

[Soeren Moch]

On 11/06/14 21:41, Marek Vasut wrote:
> On Thursday, November 06, 2014 at 01:51:51 PM, Soeren Moch wrote:
>> 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 <smoch@web.de>
>> --
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Tom Rini <trini@ti.com>
>> ---
>>   common/usb_storage.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/usb_storage.c b/common/usb_storage.c
>> index eb7706c..0ac7b48 100644
>> --- a/common/usb_storage.c
>> +++ b/common/usb_storage.c
>> @@ -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 ?

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

>
>>   		/* skip unknown devices */
>>   		return 0;
>>   	}
>
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH] usb_storage: skip all unknown devices when probing

[Marek Vasut]

On Friday, November 07, 2014 at 10:28:18 AM, Soeren Moch wrote:
> On 11/06/14 21:41, Marek Vasut wrote:
> > On Thursday, November 06, 2014 at 01:51:51 PM, Soeren Moch wrote:
> >> 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 <smoch@web.de>
> >> --
> >> Cc: Marek Vasut <marex@denx.de>
> >> Cc: Tom Rini <trini@ti.com>
> >> ---
> >> 
> >>   common/usb_storage.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/common/usb_storage.c b/common/usb_storage.c
> >> index eb7706c..0ac7b48 100644
> >> --- a/common/usb_storage.c
> >> +++ b/common/usb_storage.c
> >> @@ -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 ?
> 
> 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

[U-Boot] [PATCH] usb_storage: blacklist Enclosure Service Devices

[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 <smoch@web.de>
--

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 <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
 common/usb_storage.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index eb7706c..9198f73 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -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) {
-- 
1.9.1

[U-Boot] [PATCH] usb_storage: blacklist Enclosure Service Devices

[Nikolay Dimitrov]

Hi Soeren,

On 11/08/2014 08:02 AM, Soeren Moch wrote:
> 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 <smoch@web.de>
> --
>
> 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 <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>   common/usb_storage.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index eb7706c..9198f73 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -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) {
>

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

Regards,
Nikolay

[U-Boot] [PATCH] usb_storage: blacklist Enclosure Service Devices

[Marek Vasut]

On Saturday, November 08, 2014 at 07:02:14 AM, Soeren Moch wrote:
> 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 <smoch@web.de>

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