[U-Boot] [PATCH] usb: fix usb start problem with SMSC USB hub and Toshiba USB stick

Remy Bohmer
2010-10-29T13:57:54Z

Hi,

2010/10/27 Anatolij Gustschin <agust@denx.de>:
> Hi Remy,
>
> On Wed, 1 Sep 2010 10:49:22 +0200
> Remy Bohmer <linux@bohmer.net> wrote:
> ...
>> 2010/8/31 Anatolij Gustschin <agust@denx.de>:
>> > Checking the status field of the qTD token in the current code
>> > do not take into acount cases where endpoint stall (halted) bit
>> > is set together with some other status bits. As a result clearing
>> > stall on an endpoint won't be done if other status bits were set.
>>
>> Reading this description and this code:
>>
>> > ? ? ? ?/* special handling of STALL in DATA phase */
>> > - ? ? ? if ((result < 0) && (us->pusb_dev->status &
>> > USB_ST_STALLED)) {
>> > + ? ? ? if ((result < 0) &&
>> > + ? ? ? ? ? (us->pusb_dev->status & (USB_ST_STALLED |
>> > USB_ST_CRC_ERR))) { USB_STOR_PRINTF("DATA:stall\n");
>>
>> Description and code do not seem to match. The old implementation
>> explicitly checked if the STALLED bit was set, and if so the endpoint
>> would be cleared. Now, it seems you are clearing the endpoint _also_
>> when the CRC_ERR bit is set.
>
> Yes. This was intentional but unfortunately not directly mentioned
> in the patch description. The reason is the following:
>
> in all cases where transaction error (XactErr) was reported with
> additionally STALLED bit set, the handling of the STALL condition
> successfully recovered from the error case and the device was
> recognized.
>
> in cases where only transaction error was reported (STALLED bit
> not set) the recovery from the error case didn't succeed and
> "Device NOT ready" (Request Sense returned 06 28 00) status was
> finally reported. I didn't find the information in the EHCI spec
> how the recovery from the XactErr should be done properly. Do
> you know how to handle it?
>
>> There are a lot more reasons, at least on other host controllers that
>> set the status USB_ST_CRC_ERR, does this change not break the other
>> code? (A simple grep will show you the situations where it is
>> returned.)
>
> This change will probably break other code. I'll drop it here and
> additionally set USB_ST_STALLED flag in the ehci driver instead.

OK. I will ignore this version of this patch. I assume you will post a
new revision of this patch?

Kind regards,

Remy