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