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

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

[Ran Wang]

Hi Marek,

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Monday, October 30, 2017 6:55 PM
> To: Ran Wang <ran.wang_1@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/*?
If yes, is it acceptable that I use 'if defined(CONFIG_ARCH_LS2080A)' in common/usb.c?
If answer is no, how should I do? I cannot find an example and not sure it's OK 
to related rule.

Another question: what's the rule to put fix in common/usb* or drivers/usb/*?
>Also, such questions should be sent to the ML, not privately.
> 
Got it.

BR
Ran
> >
> 
> 
> --
> Best regards,
> Marek Vasut
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-usb-Add-delay-to-fix-USB-2.0-stick-enumeration-failu.patch
Type: application/octet-stream
Size: 1005 bytes
Desc: 0001-usb-Add-delay-to-fix-USB-2.0-stick-enumeration-failu.patch
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171031/9fc3dee1/attachment.obj>

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

[Marek Vasut]

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@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.

[...]

-- 
Best regards,
Marek Vasut

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

[Ran Wang]

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@nxp.com>; Marek Vasut <marex@denx.de>
> Cc: open list <u-boot@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@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.

BR
Ran
> [...]
> 
> --
> Best regards,
> Marek Vasut

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

[Marek Vasut]

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@nxp.com>; Marek Vasut <marex@denx.de>
>> Cc: open list <u-boot@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@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.

-- 
Best regards,
Marek Vasut

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

[Ran Wang]

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@nxp.com>; Marek Vasut
> <marek.vasut@gmail.com>
> Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut <marex@denx.de>
> >> Cc: open list <u-boot@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@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.

Best regards,
Ran

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

[Ran Wang]

Hi Marek,

> -----Original Message-----
> From: Ran Wang
> Sent: Wednesday, November 01, 2017 9:05 AM
> To: Marek Vasut <marex@denx.de>; Marek Vasut <marek.vasut@gmail.com>
> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
> <marek.vasut@gmail.com>
> > Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut <marex@denx.de>
> > >> Cc: open list <u-boot@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@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;

 Best regards,
 Ran
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jetFlash.diff
Type: application/octet-stream
Size: 343 bytes
Desc: jetFlash.diff
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171101/5eebd78a/attachment.obj>

[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@denx.de>; Marek Vasut <marek.vasut@gmail.com>
>> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
>> <marek.vasut@gmail.com>
>>> Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut <marex@denx.de>
>>>>> Cc: open list <u-boot@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@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

[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@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@denx.de>; Marek Vasut <marek.vasut@gmail.com>
>>> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
>>> <marek.vasut@gmail.com>
>>>> Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut <marex@denx.de>
>>>>>> Cc: open list <u-boot@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@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

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

[Ran Wang]

Hi Bin,

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: Wednesday, November 01, 2017 6:08 PM
> To: Marek Vasut <marex@denx.de>
> Cc: Ran Wang <ran.wang_1@nxp.com>; Marek Vasut
> <marek.vasut@gmail.com>; open list <u-boot@lists.denx.de>
> Subject: Re: [U-Boot] About the way to fix platform specific issue in source
> file xhci.c (U-Boot)
> 
> Hi Ran,
> 
> On Wed, Nov 1, 2017 at 4:19 PM, Marek Vasut <marex@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@denx.de>; Marek Vasut
> <marek.vasut@gmail.com>
> >>> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
> >>> <marek.vasut@gmail.com>
> >>>> Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut
> <marex@denx.de>
> >>>>>> Cc: open list <u-boot@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@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?

Yes
> 
> >> 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?

It's 0x04: USB Transaction Error
> 
> >> 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.

Yes, for now I have to do the remote debug for ENV limitation and don't have
USB protocol analyzer to catch the trace log. Actually I occasionally found this workaround
When opening verbose debug log of USB. 

So my plan is to submit this simple workaround firstly to cover more shitty
devices and then submit another one in the future when I found the root
cause and confirm it can be fixed by SW.

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

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

[Bin Meng]

Hi Ran,

On Thu, Nov 2, 2017 at 10:29 AM, Ran Wang <ran.wang_1@nxp.com> wrote:
> Hi Bin,
>
>> -----Original Message-----
>> From: Bin Meng [mailto:bmeng.cn at gmail.com]
>> Sent: Wednesday, November 01, 2017 6:08 PM
>> To: Marek Vasut <marex@denx.de>
>> Cc: Ran Wang <ran.wang_1@nxp.com>; Marek Vasut
>> <marek.vasut@gmail.com>; open list <u-boot@lists.denx.de>
>> Subject: Re: [U-Boot] About the way to fix platform specific issue in source
>> file xhci.c (U-Boot)
>>
>> Hi Ran,
>>
>> On Wed, Nov 1, 2017 at 4:19 PM, Marek Vasut <marex@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@denx.de>; Marek Vasut
>> <marek.vasut@gmail.com>
>> >>> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
>> >>> <marek.vasut@gmail.com>
>> >>>> Cc: open list <u-boot@lists.denx.de>; Bin Meng <bmeng.cn@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@nxp.com>; Marek Vasut
>> <marex@denx.de>
>> >>>>>> Cc: open list <u-boot@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@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?
>
> Yes
>>
>> >> 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?
>
> It's 0x04: USB Transaction Error
>>

OK, I see.

>> >> 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);
>> >> +

So even if we need add a workaround, I suggest it should be added to
xHCI driver where it issues the "Address Device" command, not here.

>>
>> This to me is a workaround. The root cause is still under water.
>
> Yes, for now I have to do the remote debug for ENV limitation and don't have
> USB protocol analyzer to catch the trace log. Actually I occasionally found this workaround
> When opening verbose debug log of USB.
>
> So my plan is to submit this simple workaround firstly to cover more shitty
> devices and then submit another one in the future when I found the root
> cause and confirm it can be fixed by SW.
>

Yes, can you please check why xHC reports transaction error? I suspect
your USB device respond with a STALL.

Regards,
Bin

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

[Ran Wang]

Hi Bin,

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: Thursday, November 02, 2017 4:37 PM
> To: Ran Wang <ran.wang_1@nxp.com>
> Cc: Marek Vasut <marex@denx.de>; Marek Vasut
> <marek.vasut@gmail.com>; open list <u-boot@lists.denx.de>
> Subject: Re: [U-Boot] About the way to fix platform specific issue in source
> file xhci.c (U-Boot)
> 
> Hi Ran,
> 
> On Thu, Nov 2, 2017 at 10:29 AM, Ran Wang <ran.wang_1@nxp.com> wrote:
> > Hi Bin,
> >
> >> -----Original Message-----
> >> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> >> Sent: Wednesday, November 01, 2017 6:08 PM
> >> To: Marek Vasut <marex@denx.de>
> >> Cc: Ran Wang <ran.wang_1@nxp.com>; Marek Vasut
> >> <marek.vasut@gmail.com>; open list <u-boot@lists.denx.de>
> >> Subject: Re: [U-Boot] About the way to fix platform specific issue in
> >> source file xhci.c (U-Boot)
> >>
> >> Hi Ran,
> >>
> >> On Wed, Nov 1, 2017 at 4:19 PM, Marek Vasut <marex@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@denx.de>; Marek Vasut
> >> <marek.vasut@gmail.com>
> >> >>> Cc: open list <u-boot@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@nxp.com>; Marek Vasut
> >> >>> <marek.vasut@gmail.com>
> >> >>>> Cc: open list <u-boot@lists.denx.de>; Bin Meng
> >> >>>> <bmeng.cn@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@nxp.com>; Marek Vasut
> >> <marex@denx.de>
> >> >>>>>> Cc: open list <u-boot@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@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?
> >
> > Yes
> >>
> >> >> 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?
> >
> > It's 0x04: USB Transaction Error
> >>
> 
> OK, I see.
> 
> >> >> 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);
> >> >> +
> 
> So even if we need add a workaround, I suggest it should be added to xHCI
> driver where it issues the "Address Device" command, not here.
> 
Well, this issue still happen if I only added delay before issue "Address Device" command once.
It can only be fixed when I add delay to most control transfer on root-hub enumeration.
My original patch was to add to file xHCI driver, but Marek suggest me move to common/.

> >>
> >> This to me is a workaround. The root cause is still under water.
> >
> > Yes, for now I have to do the remote debug for ENV limitation and
> > don't have USB protocol analyzer to catch the trace log. Actually I
> > occasionally found this workaround When opening verbose debug log of
> USB.
> >
> > So my plan is to submit this simple workaround firstly to cover more
> > shitty devices and then submit another one in the future when I found
> > the root cause and confirm it can be fixed by SW.
> >
> 
> Yes, can you please check why xHC reports transaction error? I suspect your
> USB device respond with a STALL.
So far I don't have the equipment to catch transactions on USB bus for the root cause.
Do you know are there the SW way to confirm STALL happen?

Best regards
Ran
> 
> Regards,
> Bin