On Mon, Mar 22, 2021 at 02:50:48PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 22, 2021 at 11:49 AM Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
> >
> > 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.
>
> Quirk helped, I will send a patch. Thanks for a hint.
Glad I could help.
-Sergey
>
> --
> With Best Regards,
> Andy Shevchenko
