diff mbox

[U-Boot,5/7] usb: Assure Get Descriptor request is in separate microframe

Message ID 1462308680-9366-5-git-send-email-marex@denx.de
State Accepted
Commit ef71290be9b70d8cfa63b506c7d93c5069f63c42
Delegated to: Marek Vasut
Headers show

Commit Message

Marek Vasut May 3, 2016, 8:51 p.m. UTC
The Kingston DT Ultimate USB 3.0 stick is sensitive to this first
Get Descriptor request and if the request is not in a separate
microframe, the stick refuses to operate. Add slight delay, which
is enough for one microframe to pass on any USB spec revision.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Stephen Warren <swarren@nvidia.com>
---
 common/usb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Stefan Roese May 4, 2016, 8:03 a.m. UTC | #1
On 03.05.2016 22:51, Marek Vasut wrote:
> The Kingston DT Ultimate USB 3.0 stick is sensitive to this first
> Get Descriptor request and if the request is not in a separate
> microframe, the stick refuses to operate. Add slight delay, which
> is enough for one microframe to pass on any USB spec revision.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Stephen Warren <swarren@nvidia.com>
> ---
>   common/usb.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 205041b..8d9efe5 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1077,6 +1077,14 @@ int usb_select_config(struct usb_device *dev)
>   	le16_to_cpus(&dev->descriptor.idProduct);
>   	le16_to_cpus(&dev->descriptor.bcdDevice);
>
> +	/*
> +	 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
> +	 * about this first Get Descriptor request. If there are any other
> +	 * requests in the first microframe, the stick crashes. Wait about
> +	 * one microframe duration here (1mS for USB 1.x , 125uS for USB 2.0).
> +	 */
> +	mdelay(1);
> +
>   	/* only support for one config for now */
>   	err = usb_get_configuration_len(dev, 0);
>   	if (err >= 0) {
>

Again my question, if this problem also occurs on other platforms
with this USB key. A 1ms delay is not really a big deal, but its
my general feeling that we should manifest such changes by testing
on different platforms.

Thanks,
Stefan
Marek Vasut May 4, 2016, 10:25 a.m. UTC | #2
On 05/04/2016 10:03 AM, Stefan Roese wrote:
> On 03.05.2016 22:51, Marek Vasut wrote:
>> The Kingston DT Ultimate USB 3.0 stick is sensitive to this first
>> Get Descriptor request and if the request is not in a separate
>> microframe, the stick refuses to operate. Add slight delay, which
>> is enough for one microframe to pass on any USB spec revision.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Chin Liang See <clsee@altera.com>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> ---
>>   common/usb.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index 205041b..8d9efe5 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1077,6 +1077,14 @@ int usb_select_config(struct usb_device *dev)
>>       le16_to_cpus(&dev->descriptor.idProduct);
>>       le16_to_cpus(&dev->descriptor.bcdDevice);
>>
>> +    /*
>> +     * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
>> +     * about this first Get Descriptor request. If there are any other
>> +     * requests in the first microframe, the stick crashes. Wait about
>> +     * one microframe duration here (1mS for USB 1.x , 125uS for USB
>> 2.0).
>> +     */
>> +    mdelay(1);
>> +
>>       /* only support for one config for now */
>>       err = usb_get_configuration_len(dev, 0);
>>       if (err >= 0) {
>>
> 
> Again my question, if this problem also occurs on other platforms
> with this USB key. A 1ms delay is not really a big deal, but its
> my general feeling that we should manifest such changes by testing
> on different platforms.

I tested it by connecting the bus analyzer between the stick and socfpga
and between the stick and x86 host. I wouldn't be able to
come up with this solution. btw. any ehci ends up inserting these
control requests into separate microframes, but we need the mdelay
to also cater for ohci/uhci, which has longer frames (1ms).
Stephen Warren May 4, 2016, 5:10 p.m. UTC | #3
On 05/03/2016 02:51 PM, Marek Vasut wrote:
> The Kingston DT Ultimate USB 3.0 stick is sensitive to this first
> Get Descriptor request and if the request is not in a separate
> microframe, the stick refuses to operate. Add slight delay, which
> is enough for one microframe to pass on any USB spec revision.

> diff --git a/common/usb.c b/common/usb.c

> +	/*
> +	 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
> +	 * about this first Get Descriptor request. If there are any other
> +	 * requests in the first microframe, the stick crashes. Wait about
> +	 * one microframe duration here (1mS for USB 1.x , 125uS for USB 2.0).
> +	 */
> +	mdelay(1);

Do we know the connection speed here? If so, we could sleep 1ms for 
USB1.x and 125us for USB2.x, thus reducing any performance impact. 
Still, this is a short delay that I think only happens once per actual 
device so perhaps it isn't worth it.
diff mbox

Patch

diff --git a/common/usb.c b/common/usb.c
index 205041b..8d9efe5 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1077,6 +1077,14 @@  int usb_select_config(struct usb_device *dev)
 	le16_to_cpus(&dev->descriptor.idProduct);
 	le16_to_cpus(&dev->descriptor.bcdDevice);
 
+	/*
+	 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
+	 * about this first Get Descriptor request. If there are any other
+	 * requests in the first microframe, the stick crashes. Wait about
+	 * one microframe duration here (1mS for USB 1.x , 125uS for USB 2.0).
+	 */
+	mdelay(1);
+
 	/* only support for one config for now */
 	err = usb_get_configuration_len(dev, 0);
 	if (err >= 0) {