Re: [chipidea] continuous USB resets on NXP i.MX 6ULL device

[Peter Chen <peter.chen@nxp.com>]

On 19-10-02 19:15:11, Igor Opaniuk wrote:
> + Li Jun
> 
> Hi Peter, Li,
> 
> On Mon, Sep 30, 2019 at 2:35 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> >
> > Hi Peter,
> >
> > On Wed, Sep 25, 2019 at 3:44 AM Peter Chen <peter.chen@nxp.com> wrote:
> > >
> > >
> > > >
> > > > Hi Fabio, Peter, Stefan,
> > > >
> > > > I've incidentally discovered your last year discussion in ML [1] (I hope it rings the
> > > > bell) regarding the issue I'm still observing on the same device with the mainline
> > > > kernel.
> > > >
> > > > The difference between i.MX 6ULL EVK and this particular device, is that usbotg2 is
> > > > used only in host mode with the usb hub integrated on the carrier board [2] [3].
> > > >
> > > > root@colibri-imx6:~# lsusb -s 1:1 --tree
> > > > /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ci_hdrc/1p, 480M
> > > >     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> > > >
> > > > However I can't reproduce the same behavior with i.MX 6ULL EVK with connected
> > > > usb hub to usbotg2. Also this behavior can't be reproduced with NXP downstream
> > > > kernel (Linux version 4.9.144) on my device.
> > > >
> > > > After digging in a bit I found out that this happens only when autosuspend is
> > > > enabled for the usb controller:
> > > > echo auto > /sys/bus/usb/devices/1-1/power/control
> > > >
> > > > It tries to go to suspend mode, but everytime fails and resumes:
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > suspended
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > resuming
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > suspended
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > suspended
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > suspended
> > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status
> > > > resuming
> > > >
> > > > I'm observing ~2 seconds wakeup interrupts handled in ci_irq() in core.c and
> > > > subsequent invocation of imx_controller_resume().
> > > >
> > > > Meantime usboh3 remains enabled all the time (though
> > > > imx_disable_unprepare_clks() should disable it):
> > > > root@colibri-imx6:~# cat /sys/kernel/debug/clk/clk_summary | grep usb
> > > >     usbphy2_gate                      1        1        0           0
> > > >         0     0  50000
> > > >     usbphy1_gate                      1        1        0           0
> > > >         0     0  50000
> > > >           pll7_usb_host               1        1        0   480000000
> > > >         0     0  50000
> > > >              usbphy2                  1        1        0   480000000
> > > >         0     0  50000
> > > >           pll3_usb_otg                2        3        0   480000000
> > > >         0     0  50000
> > > >              usbphy1                  0        0        0   480000000
> > > >         0     0  50000
> > > >                          usboh3       1        1        0    66000000
> > > >         0     0  50000
> > > >
> > > > While I'm trying to localize the root cause, maybe you can give some hints where to
> > > > look into?
> > > >
> > >
> > > Would you please look below two downstream patches see if it helps?
> > >
> > > commit e8e95658fe75f3873e06d5ad72a6bf0bad40f068
> > > Author: Li Jun <jun.li@nxp.com>
> > > Date:   Mon Oct 16 23:13:19 2017 +0800
> > >
> > >     MLK-16576 usb: phy: mxs: set hold_ring_off for USB2 PLL power up
> > >
> > >     USB2 PLL use ring VCO, when the PLL power up, the ring VCO’s supply also
> > >     ramp up. There is a possibility that the ring VCO start oscillation at
> > >     multi nodes in this phase, especially for VCO which has many stages, then
> > >     the multiwave will kept until PLL power down. Hold_ring_off(bit11) can
> > >     force the VCO in one determined state when VCO supply start ramp up, to
> > >     avoid this multiwave issue. Per IC design's suggestion it's better this
> > >     bit can be off from 25us after pll power up to 25us before USB TX/RX.
> > >
> > >
> > > commit aa4680d844a340a5b6b60484f6e04cb9ba613c65
> > > Author: Peter Chen <peter.chen@nxp.com>
> > > Date:   Wed Sep 7 12:16:59 2016 +0800
> > >
> > >     MLK-13125 usb: phy: phy-mxs-usb: enable weak 1p1 regulator for imx6ul during suspend
> > >
> > >     For imx6ul PHY, when the system enters suspend, its 1p1 is off by default,
> > >     that may cause the PHY get inaccurate USB DP/DM value. If the USB wakeup
> > >     is enabled at this time, the unexpected wakeup may occur when the system
> > >     enters suspend.
> > >
> > >     In this patch, when the vbus is there, we enable weak 1p1 during the PHY
> > >     suspend API, in that case, the USB DP/DM will be accurate for USB PHY,
> > >     then unexpected usb wakeup will not be occurred, especially for the USB
> > >     charger is connected scenario. The user needs to enable PHY wakeup for
> > >     USB wakeup function using below setting.
> > >
> > > Peter
> > >
> > > > Thanks for helping me!
> > > >
> > > > [1]
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmarc.info%2
> > > > F%3Fl%3Dlinux-
> > > > usb%26m%3D151844741732751&amp;data=02%7C01%7Cpeter.chen%40nxp.co
> > > > m%7Cb61742d4ff4646c2cb9408d740d735fe%7C686ea1d3bc2b4c6fa92cd99c5c30
> > > > 1635%7C0%7C0%7C637049165376859788&amp;sdata=eZdOVCuyVyA502DgwoN
> > > > 16QmU5WDknrqCK5xmEbIovAE%3D&amp;reserved=0
> > > > [2]
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%
> > > > 2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Farch%2Farm%2Fboot%2Fdts%2Fimx6
> > > > ull-
> > > > colibri.dtsi%23L191&amp;data=02%7C01%7Cpeter.chen%40nxp.com%7Cb61742d
> > > > 4ff4646c2cb9408d740d735fe%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > > > 0%7C637049165376859788&amp;sdata=7%2FVP15v1XGeQzweIu%2FaGhJEt1ov
> > > > Tn4zl%2F3pghKBcGlw%3D&amp;reserved=0
> > > > [3]
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%
> > > > 2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Farch%2Farm%2Fboot%2Fdts%2Fimx6
> > > > ull-colibri-eval-
> > > > v3.dtsi%23L143&amp;data=02%7C01%7Cpeter.chen%40nxp.com%7Cb61742d4ff4
> > > > 646c2cb9408d740d735fe%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > > > C637049165376859788&amp;sdata=M43BXrJedKgszOJkZ9W4h5bqQo9WBaxe7C
> > > > L4tAD2Ouk%3D&amp;reserved=0
> > > >
> > > > --
> > > > Best regards - Freundliche Grüsse - Meilleures salutations
> > > >
> > > > Igor Opaniuk
> > > >
> > > > mailto: igor.opaniuk@gmail.com
> > > > skype: igor.opanyuk
> > > > +380 (93) 836 40 67
> > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fua.linkedin.co
> > > > m%2Fin%2Fiopaniuk&amp;data=02%7C01%7Cpeter.chen%40nxp.com%7Cb61742
> > > > d4ff4646c2cb9408d740d735fe%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> > > > C0%7C637049165376859788&amp;sdata=qzgWZaeraIUjiLG24s9x%2BUQEnRrJ2
> > > > TiRKD%2F6ndna4Ws%3D&amp;reserved=0
> >
> >
> > Unfortunately these two commits didn't help (+ 'MLK-10170 usb: phy:
> > mxs: keep USBPHY2's clk always on',
> > which one of the commits depends on).
> >
> >
> > --
> > Best regards - Freundliche Grüsse - Meilleures salutations
> >
> > Igor Opaniuk
> >
> > mailto: igor.opaniuk@gmail.com
> > skype: igor.opanyuk
> > +380 (93) 836 40 67
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fua.linkedin.com%2Fin%2Fiopaniuk&amp;data=02%7C01%7Cpeter.chen%40nxp.com%7C71a50ceb5d1b48a1030908d74753ba85%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637056297246882064&amp;sdata=Nl9SoZb1m6YJhFcAaMwb2jfupdsZEXmyQDTEW4ceae4%3D&amp;reserved=0
> 
> So finally I found the commit in the downstream NXP kernel
> which fixes this behaviour
> 6bdc038751("MLK-14285-3 usb: phy: mxs: optimize disconnect line condition").
> 
> Seems that no one tried to upstream it yet (I can't find anything in
> the ML archieves),
> so if you don't have any objections I'll apply/test on top 5.4-rc1/
> send to ML (or just let me know if you are planning to do it on your own).
> 

Do it please, thanks.

-- 

Thanks,
Peter Chen