[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 at denx.de>
> Cc: Ran Wang <ran.wang_1 at nxp.com>; Marek Vasut
> <marek.vasut at gmail.com>; 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 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?

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