On Mon, Mar 22, 2021 at 11:46 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Mon, Mar 22, 2021 at 11:10 AM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> >
> > Hello Andy
> >
> > On Fri, Mar 12, 2021 at 09:33:59PM +0200, Andy Shevchenko wrote:
> > > Hi!
> > >
> > > The fix in the commit
> > > e5f4ca3fce90 ("usb: dwc3: ulpi: Fix USB2.0 HS/FS/LS PHY suspend regression")
> > > effectively broke host mode on Intel Edison.
> > >
> > > When device in a gadget mode and I attach the USB Mass Storage device
> > >
> > > Before: automatically switches to host mode and detects storage and
> > > everything okay
> > > After: automatically switches to host mode and that's it.
> > >
> > > Revert, as obviously from above, helps.
> > >
> > > So, please fix this or I will send revert near to rc5,
> >
> > As I've explained in the patchlog the commit was required because the
> > original functionality submitted by Felipe three years ago aside with
> > questionable fix introduced the suspend mode regression. So an access to
> > any PHY register over the ULPI interface just disabled "Suspend USB 2.0
> > HS/FS/LS PHY" mode, which made pointless enabling it in the main part of
> > the driver and consequently increased the USB bus power consumption. See
> > the commit e0082698b689 ("usb: dwc3: ulpi: conditionally resume ULPI PHY")
> > for details. It clears the SusPHY flag and never gets it back. Instead it
> > was enough to wait a little longer in the ULPI RegAccess busy-loop to let
> > PHY to get out of the suspend mode and restore the link. So to speak
> > reverting my patch would mean to get the regression back for all
> > IP-core versions, which isn't what any of us would want.
> >
> > What I can suggest to you is to investigate the problem more thorough of why
> > you having the mass storage device undetected after the mode switching.
> > At least you need to look at the system log and find out whether it's ULPI
> > Registers commands timeout to blame. If so then I would take a look at the
> > PHY hardware manual whether it supports the Low-power mode. In any case
> > most likely setting "snps,dis_u2_susphy_quirk" property in the DWC USB3 DT
> > node will help you to work around the problem. Declaring that DT-prop will
> > effectively disable using the LPM for any mode (Host/gadget/DRD), but will
> > permit PHY suspension on the system suspend procedure (please see
> > snps,dis_u2_susphy_quirk implementation for details).
>
> Before the patch there was no regression on the board in question.
> After the patch
> I got a clear regression that I may reproduce 100%. It's the
> responsibility of the author of the patch to try to figure out what
> broke other's cases. It's as simple as that.
>
> As I told you, I am happy to test any suggestion how to fix, otherwise
> I will send a revert and I have a good justification to do that.
>
> > > Happy to test any proposed patches! (Maybe Ferry also can help with that)
With regards to the second part of the second paragraph, I will look
at that quirk if it helps.
Thanks.
--
With Best Regards,
Andy Shevchenko
