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 Best Regards,
Andy Shevchenko
