[U-Boot] About the way to fix platform specific issue in source file xhci.c (U-Boot)

[Bin Meng]

Hi Ran,

On Wed, Nov 1, 2017 at 4:19 PM, Marek Vasut <marex at denx.de> wrote:
> On 11/01/2017 07:22 AM, Ran Wang wrote:
>> Hi Marek,
>>
>>> -----Original Message-----
>>> From: Ran Wang
>>> Sent: Wednesday, November 01, 2017 9:05 AM
>>> To: Marek Vasut <marex at denx.de>; Marek Vasut <marek.vasut at gmail.com>
>>> Cc: open list <u-boot at lists.denx.de>
>>> Subject: RE: [U-Boot] About the way to fix platform specific issue in source
>>> file xhci.c (U-Boot)
>>>
>>> Hi Marek,
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Tuesday, October 31, 2017 6:24 PM
>>>> To: Ran Wang <ran.wang_1 at nxp.com>; Marek Vasut
>>> <marek.vasut at gmail.com>
>>>> Cc: open list <u-boot at lists.denx.de>; Bin Meng <bmeng.cn at gmail.com>
>>>> Subject: Re: [U-Boot] About the way to fix platform specific issue in
>>>> source file xhci.c (U-Boot)
>>>>
>>>> On 10/31/2017 10:43 AM, Ran Wang wrote:
>>>>> Hi
>>>>>> -----Original Message-----
>>>>>> From: Marek Vasut [mailto:marek.vasut at gmail.com]
>>>>>> Sent: Tuesday, October 31, 2017 5:31 PM
>>>>>> To: Ran Wang <ran.wang_1 at nxp.com>; Marek Vasut <marex at denx.de>
>>>>>> Cc: open list <u-boot at lists.denx.de>
>>>>>> Subject: Re: [U-Boot] About the way to fix platform specific issue
>>>>>> in source file xhci.c (U-Boot)
>>>>>>
>>>>>> On 10/31/2017 10:15 AM, Ran Wang wrote:
>>>>>>> Hi Marek,
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>>>> Sent: Monday, October 30, 2017 6:55 PM
>>>>>>>> To: Ran Wang <ran.wang_1 at nxp.com>
>>>>>>>> Cc: bmeng.cn at gmail.com
>>>>>>>> Subject: Re: About the way to fix platform specific issue in
>>>>>>>> source file xhci.c
>>>>>>>> (U-Boot)
>>>>>>>>
>>>>>>>> On 10/30/2017 09:39 AM, Ran Wang wrote:
>>>>>>>>> Hi Vasut,
>>>>>>>>>    For git://git.denx.de/u-boot-usb.git, I work out a patch to
>>>>>>>>> fix USB issue
>>>>>>>> which will happen on SoC LS2080A only (it's using DWC3).
>>>>>>>>>    Per my understanding, we should not use platform define in
>>>>>>>>> xhci.c to
>>>>>>>> control its effect. However, I am not sure how to do it that can
>>>>>>>> be accepted by upstream, so send you this mail for suggestion
>>>>>>>> before I post the patch to patchwork. Thank you in advance.
>>>>>>>>
>>>>>>>> This should be fixed in common code, not in drivers.
>>>>>>>
>>>>>>> Did you mean it should be fixed in common/usb*.c rather than
>>>>>> drivers/usb/*?
>>>>>>
>>>>>> Yes
>>>>>>
>>>>>>> If yes, is it acceptable that I use 'if
>>>>>>> defined(CONFIG_ARCH_LS2080A)' in
>>>>>> common/usb.c?
>>>>>>
>>>>>> No
>>>>>>
>>>>>>> If answer is no, how should I do? I cannot find an example and not
>>>>>>> sure it's OK to related rule.
>>>>>>
>>>>>> What is the problem exactly ?
>>>>>> I recall there were reports of shitty USB sticks failing, but
>>>>>> without further details, it's hard to tell if this is the same problem.
>>>>>>
>>>>> We observed some USB2.0 drives (Transcend 8GB, 4GB, Samtec) fail to
>>>>> be enumerated by U-Boot, and if we try to add some time interval
>>>>> between control transfers (not in bulk transfers), issue get resolved.
>>>>
>>>> Try disabling cache support, does it still happen ?
>>>> I had such an issue with DWC2 controller and it failed due to
>>>> incorrect cache handling.
>>>>
>>> Could you pls tell me where to disable it on DWC3? May sure I do the right
>>> change.
>>
>> I try to remove function dcache_enable() calling, failure still happen. And I think
>> cache issue might not be able to bypass if I only add delay in control message sends.
>>
>> More debugging shows that xHC will generate TRB event with complete code of TX_ERR for Set

Do you mean xHC "Address Device" command?

>> Address command TRB execution. Then U-Boot pop "USB device not accepting new address
>> (error=80000000)".
>>

What is the completion codes for the TX_ERR you were seeing?

>> Anyway, now I work out a patch fix in common/usb.c as below, is it acceptable?
>> diff --git a/common/usb.c b/common/usb.c
>> index 0904259757..26f2e89ba3 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -223,6 +223,8 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
>>               return -EINVAL;
>>       }
>>
>> +     mdelay(10);
>> +

This to me is a workaround. The root cause is still under water.

>>       /* set setup command */
>>       setup_packet->requesttype = requesttype;
>>       setup_packet->request = request;
>
> +CC Bin, he might have an idea.

Regards,
Bin