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

[Marek Vasut]

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
> Address command TRB execution. Then U-Boot pop "USB device not accepting new address
> (error=80000000)".
> 
> 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);
> +
>  	/* set setup command */
>  	setup_packet->requesttype = requesttype;
>  	setup_packet->request = request;

+CC Bin, he might have an idea.

-- 
Best regards,
Marek Vasut