On Tue, Jan 03, 2023 at 03:45:42AM +0000, Jun Li wrote:
> > -----Original Message-----
> > From: Francesco Dolcini <francesco@xxxxxxxxxx>
> > Sent: Monday, December 26, 2022 11:45 PM
> > To: Jun Li <jun.li@xxxxxxx>
> > Cc: Francesco Dolcini <francesco@xxxxxxxxxx>; Jun Li (OSS)
> > <jun.li@xxxxxxxxxxx>; Peter Chen <peter.chen@xxxxxxxxxx>;
> > linux-usb@xxxxxxxxxxxxxxx; Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha
> > Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> > <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; dl-linux-imx
> > <linux-imx@xxxxxxx>; Felipe Balbi <balbi@xxxxxxxxxx>;
> > philippe.schenker@xxxxxxxxxxx; Francesco Dolcini
> > <francesco.dolcini@xxxxxxxxxxx>; Xu Yang <xu.yang_2@xxxxxxx>
> > Subject: Re: USB runtime PM issues on i.MX6ULL
> >
> > On Mon, Dec 26, 2022 at 03:17:12AM +0000, Jun Li wrote:
> > > > From: Jun Li
> > > > Sent: Thursday, November 3, 2022 7:53 PM
> > > > > From: Francesco Dolcini <francesco@xxxxxxxxxx>
> > > > > Sent: Thursday, November 3, 2022 1:59 AM On Wed, Nov 02, 2022 at
> > > > > 10:12:42AM +0000, Jun Li wrote:
> > > > > > > > On Tue, Nov 01, 2022 at 03:10:46AM +0000, Jun Li wrote:
> > > > > > > > > > On Mon, Oct 31, 2022 at 01:40:39PM +0000, Jun Li (OSS) wrote:
> > > > > > > > > > > > I am debugging some unexpected USB behavior on a
> > > > > > > > > > > > i.MX6ULL SOC, chipidea controller ("fsl,imx6ul-usb")
> > > > > > > > > > > > and a fsl mxs usbphy ("fsl,imx6ul-usbphy").
> > > > > > > > > > > >
> > > > > > > > > > > > The HW design has 2 USB interface, the first one is
> > > > > > > > > > > > dual-role, while the second one is a host port with
> > > > > > > > > > > > NO way to re-read
> > > > > the
> > > > > > > > > > > > VBUS (USB_OTG2_VBUS is not really connected, there
> > > > > > > > > > > > is just
> > > > > a
> > > > > > > > > > > > capacitor to GND).
> > > > > > > > > > >
> > > > > > > > > > > How is your USB_OTG1_VBUS status? Can you try to make
> > > > > > > > > > > your USB_OTG1_VBUS pad has a valid VBUS voltage, then
> > > > > > > > > > > run your Host only port test with runtime PM enabled?
> > > > > > > > > >
> > > > > > > > > > USB_OTG1_VBUS is tied to GND the same way as
> > > > > > > > > > USB_OTG2_VBUS, not really straightforward to do such a test.
> > > > > > > > >
> > > > > > > > > iMX6ULL need at least one valid VBUS(either from OTG1 or
> > > > > > > > > OTG2) as input to power the internal USB LDO if I understand
> > correctly.
> > > > > > > > This surprise me a little bit, since
> > > > > > > > - the i.MX6ULL datasheet prescribe to keep the VBUS
> > > > > > > > disconnected
> > > > if
> > > > > > > > unused
> > > > > > >
> > > > > > > I think "unused" here means you do not need/enable the port at
> > all.
> > > > > > >
> > > > > > > > - downstream NXP kernel seems to work fine ("seems" since
> > > > > > > > we do
> > > > have
> > > > > > > > some patches there, so I could be wrong)
> > > > > > >
> > > > > > > What do you mean by " downstream NXP kernel seems to work fine"?
> > > > > > > The downstream kernel can work on your HW? But upstream kernel
> > > > > > > driver does not?
> > > > >
> > > > > Correct, NXP downstream kernel is working fine, upstream kernel
> > > > > requires runtime PM disabled to work correctly.
> > > > >
> > > > > > > > - disabling runtime pm on upstream Linux kernel make it works
> > > > > > > > perfectly, so there is a way in SW to have this HW configuration
> > > > > > > > working.
> > > > > > >
> > > > > > > Again I want to make sure the both VBUS pads(OTG1 and OTG2)
> > > > > > > voltage are always at 0v on your HW, can you double check and confirm?
> > > > > > > I ask this again because such situation should cause the USB
> > > > > > > port Cannot work at any cases, but your current status is:
> > > > > > > only low power wakeup cannot work.
> > > > > >
> > > > > > Could you please check the voltage of pad of VDD_USB_CAP on your
> > HW?
> > > > >
> > > > > I was about to clarify you this point, it's important and I forgot
> > > > > about it, sorry about that!
> > > > >
> > > > > VDD_USB_CAP in our design is connected to a 3.0V external LDO,
> > > > > voltage on both VBUS pads is 0V (FYI: this specific hardware
> > > > > design was validated by NXP hardware engineers).
> > > >
> > > > Then the HW design should be fine.
> > > > I need find time to try the upstream kernel on my iMX6ULL board to
> > > > check this.
> > >
> > > My iMX6ULL EVK board cannot reproduce this issue with upstream kernel.
> > >
> > > Could you try to set the bits [7,3] of 0x020c8200(for 2nd USB
> > > controller OTG2) to be value like 0x1000FC? This may be done at your
> > > bootloader(uboot), so make sure those targets bits are set before
> > > doing your test, or doing this with below change(not compiled or tested):
> > >
> > > diff --git a/drivers/usb/phy/phy-mxs-usb.c
> > > b/drivers/usb/phy/phy-mxs-usb.c index d2836ef5d15c..e390ef534a7c
> > > 100644
> > > --- a/drivers/usb/phy/phy-mxs-usb.c
> > > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > > @@ -89,6 +89,9 @@
> > > #define ANADIG_USB1_CHRG_DET_STAT_CHRG_DETECTED BIT(1)
> > > #define ANADIG_USB1_CHRG_DET_STAT_PLUG_CONTACT BIT(0)
> > >
> > > +#define ANADIG_USB2_VBUS_DET_SET 0x204
> > > +#define ANADIG_USB2_VBUS_DET_VBUS_OVERRIDE GENMASK(7, 3)
> > > +
> > > #define ANADIG_USB2_VBUS_DET_STAT 0x220
> > >
> > > #define ANADIG_USB1_LOOPBACK_SET 0x1e4
> > > @@ -288,6 +291,11 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> > > if (ret)
> > > goto disable_pll;
> > >
> > > + if (mxs_phy->regmap_anatop && (mxs_phy->port_id == 1))
> > > + regmap_write(mxs_phy->regmap_anatop,
> > > + ANADIG_USB2_VBUS_DET_SET,
> > > + ANADIG_USB2_VBUS_DET_VBUS_OVERRIDE);
> > > +
> > > /* Power up the PHY */
> > > writel(0, base + HW_USBPHY_PWD);
> >
> > Hello,
> > I tested your patch and it does not work. I therefore tested a slightly
> > improved version that really ensure the right register value is written.
> >
> > [ 8.408564] port=0 reg=0x200 val=0x1000fc
> > [ 8.440235] port=1 reg=0x204 val=0x1000fc
> >
> > but it does not work never the less. Unfortunately bits 7-3 are not documented,
> > so I was not able to do much more.
> >
> >
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c
> > b/drivers/usb/phy/phy-mxs-usb.c index d2836ef5d15c..3ff5489d679e 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -89,6 +89,10 @@
> > #define ANADIG_USB1_CHRG_DET_STAT_CHRG_DETECTED BIT(1)
> > #define ANADIG_USB1_CHRG_DET_STAT_PLUG_CONTACT BIT(0)
> >
> > +#define ANADIG_USB1_VBUS_DET_SET 0x200
> > +#define ANADIG_USB2_VBUS_DET_SET 0x204
> > +#define ANADIG_USB1_VBUS_DET_VBUS_OVERRIDE GENMASK(7, 3)
> > +
> > #define ANADIG_USB2_VBUS_DET_STAT 0x220
> >
> > #define ANADIG_USB1_LOOPBACK_SET 0x1e4
> > @@ -309,6 +313,7 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> > writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
> >
> > if (mxs_phy->regmap_anatop) {
> > + unsigned int val;
> > unsigned int reg = mxs_phy->port_id ?
> > ANADIG_USB1_CHRG_DETECT_SET :
> > ANADIG_USB2_CHRG_DETECT_SET; @@ -319,6 +324,15 @@
> > static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> > regmap_write(mxs_phy->regmap_anatop, reg,
> > ANADIG_USB1_CHRG_DETECT_EN_B |
> > ANADIG_USB1_CHRG_DETECT_CHK_CHRG_B);
> > +
> > + reg = mxs_phy->port_id ?
> > + ANADIG_USB2_VBUS_DET_SET :
> > + ANADIG_USB1_VBUS_DET_SET;
> > +
> > + regmap_set_bits(mxs_phy->regmap_anatop, reg,
> > + ANADIG_USB1_VBUS_DET_VBUS_OVERRIDE);
> > + regmap_read(mxs_phy->regmap_anatop, reg, &val);
> > + printk("port=%d reg=0x%x val=0x%x\n", mxs_phy->port_id,
> > + reg, val);
> > }
> >
> > mxs_phy_tx_init(mxs_phy);
> >
>
> Thanks for try those, those bits are for vbus valid override,
> You can verify if the override real works by checking the OTGSC
> Registers(a few bits to show the BSV/ASV/AVV) via the debugfs
> /sys/kernel/debug/usb/ci_hdrc.1/registers.
>
> echo on > /sys/bus/platform/devices/ci_hdrc.1/power/control
> cat /sys/kernel/debug/usb/ci_hdrc.1/registers
>
> if the BSV/ASV/AVV are all set to be 1, means it works.
>
> Enable runtime PM back:
> echo auto > /sys/bus/platform/devices/ci_hdrc.1/power/control
> then do your issue test.
== Running Linux 6.0.16, WITH the patch shared on this email thread.
[ 8.318987] port=0 reg=0x200 val=0x1000fc
[ 8.350386] port=1 reg=0x204 val=0x1000fc
# echo on > /sys/bus/platform/devices/ci_hdrc.1/power/control
# cat /sys/kernel/debug/usb/ci_hdrc.1/registers
USBINTR reg: 00000037
USBSTS reg: 000c0088
USBMODE reg: 00005003
USBCMD reg: 00010005
PORTSC reg: 18601a85
OTGSC reg: 00201720
And these the various valid bits - correct me if I'm wrong.
- AVV is set
- ASV is set
- BSP is NOT set
[ 268.421848] usb 1-1: reset high-speed USB device number 2 using ci_hdrc
[ 270.931732] usb 1-1: reset high-speed USB device number 2 using ci_hdrc
...
and it's not working as already stated.
== WITHOUT any patch, plain v6.0.16 kernel:
root@colibri-imx6ull-emmc-06906487:~# echo on > /sys/bus/platform/devices/ci_hdrc.1/power/control
root@colibri-imx6ull-emmc-06906487:~# cat /sys/kernel/debug/usb/ci_hdrc.1/registers
USBINTR reg: 00000037
USBSTS reg: 00040088
USBMODE reg: 00005003
USBCMD reg: 00010005
PORTSC reg: 18601a85
OTGSC reg: 00200520
And these the various valid bits - correct me if I'm wrong.
- AVV is NOT set
- ASV is set
- BSP is NOT set
> BTW, how is __mxs_phy_disconnect_line() impacting this issue
> result on your HW? If not called, then the direct plug usb device
> to the OTG2 port can work, but adding an external hub between still
> cannot work?
== Running Linux 6.0.16, WITH the patch shared on this email thread.
With NO external HUB:
- it does not work and it does not recover in any way
With external HUB:
- `reset high-speed USB` every 2 seconds, it stops as soon as I connect something to the USB HUB.
== Running Linux 6.0.16, WITH the patch shared on this email thread
with the following patch in addition
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 3e190638bcae..1a4ec039005b 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -424,7 +424,7 @@ static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
if (!mxs_phy->regmap_anatop)
return;
- vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);
+ vbus_is_on = true; // mxs_phy_get_vbus_status(mxs_phy);
if (on && !vbus_is_on && !mxs_phy_is_otg_host(mxs_phy))
__mxs_phy_disconnect_line(mxs_phy, true);
the behavior is greatly improved. It just works fine 95% of the time,
but sometime, after doing multiple plug in/out it fails to recognize
the device and it does not really recover. I never saw this behavior
just disabling runtimepm that is our current workaround.
Francesco
