USB runtime PM issues on i.MX6ULL

Commit Message

Francesco Dolcini Oct. 28, 2022, 12:31 p.m. UTC

Hello all,

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).

Focusing on the second interface, USB Host only, what I do experience is the
following:

 - if there is no USB HUB connected and no device connected at boot, any USB
   device hotplugged is not working, ci_runtime_suspend is called and it never
   resume.
 - if there is a HUB in between it somehow works, however I have a continuos
   runtime powermanagement reset loop every 2 seconds
       [ 1026.146360] ci_hdrc ci_hdrc.1: at ci_runtime_suspend
       [ 1026.164725] ci_hdrc ci_hdrc.1: at ci_controller_resume
       [ 1026.487844] usb 1-1: reset high-speed USB device number 2 using ci_hdrc
       [ 1028.326789] ci_hdrc ci_hdrc.1: at ci_runtime_suspend
       [ 1028.335378] ci_hdrc ci_hdrc.1: at ci_controller_resume
       [ 1028.657853] usb 1-1: reset high-speed USB device number 2 using ci_hdrc


I was able to have it fully working disabling the runtime power management in
the chipidea driver or using sysfs (`echo on > /sys/.../power/control`).

https://github.com/nxp-imx/linux-imx/commit/89ec73836a9b1347743e406d62dd446dc4365db3
however it builds on other commits that allow to communicate the actual mode to
the USB PHY driver and prevent mxs_phy_disconnect_line() to be called for the
USB host case.

With that the situation is way better, but while without
`CI_HDRC_SUPPORTS_RUNTIME_PM` it works perfectly, without
`MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS` it does not really work smootly
in case there is a hub in between (~20% of the time the device is not
enumerated after plugging it in).

When it does not work I see that after plugging in a device runtime resume
function is called, but after that the device is not enumerated on the USB bus.
It looks like something else is missing.

It seems like having a pure USB Host interface without having a way to re-read
the VBUS is not really supported in SW at the moment, am I wrong?
Any idea?

One last comment, even the USB dual role port is not working smoothly, however
I have not investigated this specific case that much because disabling runtime pm
solves everything also in that case.

Francesco

Comments

Jun Li (OSS) Oct. 31, 2022, 1:40 p.m. UTC | #1

Hi Francesco,

> -----Original Message-----
> From: Francesco Dolcini <[e-mail]>
> Sent: Friday, October 28, 2022 8:32 PM
> To: Peter Chen <[e-mail]>; [e-mail]; Jun Li
> <[e-mail]>
> Cc: Greg Kroah-Hartman <[e-mail]>; Shawn Guo
> <[e-mail]>; Sascha Hauer <[e-mail]>; Pengutronix
> Kernel Team <[e-mail]>; Fabio Estevam <[e-mail]>;
> dl-linux-imx <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: USB runtime PM issues on i.MX6ULL
> 
> Hello all,
> 
> 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?

Li Jun

> 
> Focusing on the second interface, USB Host only, what I do experience is
> the
> following:
> 
>  - if there is no USB HUB connected and no device connected at boot, any
> USB
>    device hotplugged is not working, ci_runtime_suspend is called and it
> never
>    resume.
>  - if there is a HUB in between it somehow works, however I have a continuos
>    runtime powermanagement reset loop every 2 seconds
>        [ 1026.146360] ci_hdrc ci_hdrc.1: at ci_runtime_suspend
>        [ 1026.164725] ci_hdrc ci_hdrc.1: at ci_controller_resume
>        [ 1026.487844] usb 1-1: reset high-speed USB device number 2 using
> ci_hdrc
>        [ 1028.326789] ci_hdrc ci_hdrc.1: at ci_runtime_suspend
>        [ 1028.335378] ci_hdrc ci_hdrc.1: at ci_controller_resume
>        [ 1028.657853] usb 1-1: reset high-speed USB device number 2 using
> ci_hdrc
> 
> 
> I was able to have it fully working disabling the runtime power management
> in
> the chipidea driver or using sysfs (`echo on > /sys/.../power/control`).
> 
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -56,7 +56,7 @@ static const struct ci_hdrc_imx_platform_flag
> imx6sx_usb_data = {
>  };
> 
>  static const struct ci_hdrc_imx_platform_flag imx6ul_usb_data = {
> -       .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
> +       .flags =
> 
> I was digging even more into the topic and I found out that what is happening
> is that mxs_phy_disconnect_line() is called. I than tried to remove the
> MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS flag.
> 
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -178,7 +178,7 @@ static const struct mxs_phy_data imx6sx_phy_data = {
>  };
> 
>  static const struct mxs_phy_data imx6ul_phy_data = {
> -       .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,
> +       /*.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,*/
>  };
> 
> This commit from NXP downstream kernel seems somehow related
> https://github.com/nxp-imx/linux-imx/commit/89ec73836a9b1347743e406d62d
> d446dc4365db3
> however it builds on other commits that allow to communicate the actual mode
> to
> the USB PHY driver and prevent mxs_phy_disconnect_line() to be called for
> the
> USB host case.
> 
> With that the situation is way better, but while without
> `CI_HDRC_SUPPORTS_RUNTIME_PM` it works perfectly, without
> `MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS` it does not really work smootly
> in case there is a hub in between (~20% of the time the device is not
> enumerated after plugging it in).
> 
> When it does not work I see that after plugging in a device runtime resume
> function is called, but after that the device is not enumerated on the USB
> bus.
> It looks like something else is missing.
> 
> It seems like having a pure USB Host interface without having a way to re-read
> the VBUS is not really supported in SW at the moment, am I wrong?
> Any idea?
> 
> One last comment, even the USB dual role port is not working smoothly, however
> I have not investigated this specific case that much because disabling runtime
> pm
> solves everything also in that case.
> 
> Francesco

Francesco Dolcini Oct. 31, 2022, 1:53 p.m. UTC | #2

Hello Li Jun,

On Mon, Oct 31, 2022 at 01:40:39PM +0000, Jun Li (OSS) wrote:
> > -----Original Message-----
> > From: Francesco Dolcini <[e-mail]>
> > Sent: Friday, October 28, 2022 8:32 PM
> > To: Peter Chen <[e-mail]>; [e-mail]; Jun Li
> > <[e-mail]>
> > Cc: Greg Kroah-Hartman <[e-mail]>; Shawn Guo
> > <[e-mail]>; Sascha Hauer <[e-mail]>; Pengutronix
> > Kernel Team <[e-mail]>; Fabio Estevam <[e-mail]>;
> > dl-linux-imx <[e-mail]>; Felipe Balbi <[e-mail]>;
> > [e-mail]; Francesco Dolcini
> > <[e-mail]>
> > Subject: USB runtime PM issues on i.MX6ULL
> > 
> > Hello all,
> > 
> > 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.

Francesco

Jun Li Nov. 1, 2022, 3:10 a.m. UTC | #3

> -----Original Message-----
> From: Francesco Dolcini <[e-mail]>
> Sent: Monday, October 31, 2022 9:54 PM
> To: Jun Li (OSS) <[e-mail]>
> Cc: Francesco Dolcini <[e-mail]>; Peter Chen
> <[e-mail]>; [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: Re: USB runtime PM issues on i.MX6ULL
> 
> Hello Li Jun,
> 
> On Mon, Oct 31, 2022 at 01:40:39PM +0000, Jun Li (OSS) wrote:
> > > -----Original Message-----
> > > From: Francesco Dolcini <[e-mail]>
> > > Sent: Friday, October 28, 2022 8:32 PM
> > > To: Peter Chen <[e-mail]>; [e-mail];
> > > Jun Li <[e-mail]>
> > > Cc: Greg Kroah-Hartman <[e-mail]>; Shawn Guo
> > > <[e-mail]>; Sascha Hauer <[e-mail]>;
> > > Pengutronix Kernel Team <[e-mail]>; Fabio Estevam
> > > <[e-mail]>; dl-linux-imx <[e-mail]>; Felipe Balbi
> > > <[e-mail]>; [e-mail]; Francesco Dolcini
> > > <[e-mail]>
> > > Subject: USB runtime PM issues on i.MX6ULL
> > >
> > > Hello all,
> > >
> > > 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, so you need change
your HW to meet this: for Host only port, you have to connect USB_OTG2_VBUS
to a valid VBUS and make it always present for simple. You can do some
quick HW change to prove this.

Li Jun
> 
> Francesco

Francesco Dolcini Nov. 1, 2022, 7:51 p.m. UTC | #4

Hello Jun Li,

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
 - downstream NXP kernel seems to work fine ("seems" since we do have
   some patches there, so I could be wrong)
 - disabling runtime pm on upstream Linux kernel make it works
   perfectly, so there is a way in SW to have this HW configuration
   working.

> your HW to meet this: for Host only port, you have to connect USB_OTG2_VBUS
> to a valid VBUS and make it always present for simple.
> You can do some quick HW change to prove this.
We have no way to change the HW in reality, therefore doing a one off
test would be pretty much irrelevant.

Said all of that, given what you wrote, I feel like having a specific
dts property in the chipidea driver to disable runtime pm might be the
way to go.

Something like `ci,disable-runtime-pm`? I know the DTS is supposed to
describe the HW, so maybe a different property name would be required.

What do you think about this?

Francesco

Jun Li Nov. 2, 2022, 9:45 a.m. UTC | #5

> -----Original Message-----
> From: Francesco Dolcini <[e-mail]>
> Sent: Wednesday, November 2, 2022 3:51 AM
> To: Jun Li <[e-mail]>
> Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> <[e-mail]>; Peter Chen <[e-mail]>;
> [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: Re: USB runtime PM issues on i.MX6ULL
> 
> Hello Jun Li,
> 
> 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?

>  - 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.

> 
> > your HW to meet this: for Host only port, you have to connect
> > USB_OTG2_VBUS to a valid VBUS and make it always present for simple.
> > You can do some quick HW change to prove this.
> We have no way to change the HW in reality, therefore doing a one off test
> would be pretty much irrelevant.

My intention of doing this HW rework is just for debug.

> 
> Said all of that, given what you wrote, I feel like having a specific dts
> property in the chipidea driver to disable runtime pm might be the way to
> go.
> 
> Something like `ci,disable-runtime-pm`? I know the DTS is supposed to
> describe the HW, so maybe a different property name would be required.
> 
> What do you think about this?

This is the last step to consider, we cannot go this way before root cause
identified.

Thanks
Li Jun
 
> 
> Francesco

Jun Li Nov. 2, 2022, 10:12 a.m. UTC | #6

> -----Original Message-----
> From: Jun Li <[e-mail]>
> Sent: Wednesday, November 2, 2022 5:45 PM
> To: Francesco Dolcini <[e-mail]>
> Cc: Jun Li (OSS) <[e-mail]>; Peter Chen <[e-mail]>;
> [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: RE: USB runtime PM issues on i.MX6ULL
> 
> 
> 
> > -----Original Message-----
> > From: Francesco Dolcini <[e-mail]>
> > Sent: Wednesday, November 2, 2022 3:51 AM
> > To: Jun Li <[e-mail]>
> > Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> > <[e-mail]>; Peter Chen <[e-mail]>;
> > [e-mail]; Greg Kroah-Hartman
> > <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> > Hauer <[e-mail]>; Pengutronix Kernel Team
> > <[e-mail]>; Fabio Estevam <[e-mail]>;
> dl-linux-imx
> > <[e-mail]>; Felipe Balbi <[e-mail]>;
> > [e-mail]; Francesco Dolcini
> > <[e-mail]>
> > Subject: Re: USB runtime PM issues on i.MX6ULL
> >
> > Hello Jun Li,
> >
> > 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?
> 
> >  - 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?

Li Jun

Francesco Dolcini Nov. 2, 2022, 5:59 p.m. UTC | #7

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).

Francesco

Jun Li Nov. 3, 2022, 11:53 a.m. UTC | #8

> -----Original Message-----
> From: Francesco Dolcini <[e-mail]>
> Sent: Thursday, November 3, 2022 1:59 AM
> To: Jun Li <[e-mail]>
> Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> <[e-mail]>; Peter Chen <[e-mail]>;
> [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: Re: USB runtime PM issues on i.MX6ULL
> 
> 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.

Li Jun
> 
> Francesco

Jun Li Dec. 26, 2022, 3:17 a.m. UTC | #9

Hi Francesco,

> -----Original Message-----
> From: Jun Li
> Sent: Thursday, November 3, 2022 7:53 PM
> To: Francesco Dolcini <[e-mail]>
> Cc: Jun Li (OSS) <[e-mail]>; Peter Chen <[e-mail]>;
> [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>
> Subject: RE: USB runtime PM issues on i.MX6ULL
> 
> 
> 
> > -----Original Message-----
> > From: Francesco Dolcini <[e-mail]>
> > Sent: Thursday, November 3, 2022 1:59 AM
> > To: Jun Li <[e-mail]>
> > Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> > <[e-mail]>; Peter Chen <[e-mail]>;
> > [e-mail]; Greg Kroah-Hartman
> > <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> > Hauer <[e-mail]>; Pengutronix Kernel Team
> > <[e-mail]>; Fabio Estevam <[e-mail]>;
> > dl-linux-imx <[e-mail]>; Felipe Balbi <[e-mail]>;
> > [e-mail]; Francesco Dolcini
> > <[e-mail]>
> > Subject: Re: USB runtime PM issues on i.MX6ULL
> >
> > 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);

> 
> Li Jun
> >
> > Francesco

Francesco Dolcini Dec. 26, 2022, 3:44 p.m. UTC | #10

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 <[e-mail]>
> > > 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);

Jun Li Jan. 3, 2023, 3:45 a.m. UTC | #11

> -----Original Message-----
> From: Francesco Dolcini <[e-mail]>
> Sent: Monday, December 26, 2022 11:45 PM
> To: Jun Li <[e-mail]>
> Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> <[e-mail]>; Peter Chen <[e-mail]>;
> [e-mail]; Greg Kroah-Hartman
> <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> Hauer <[e-mail]>; Pengutronix Kernel Team
> <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> <[e-mail]>; Felipe Balbi <[e-mail]>;
> [e-mail]; Francesco Dolcini
> <[e-mail]>; Xu Yang <[e-mail]>
> 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 <[e-mail]>
> > > > 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.

if this cannot work for your HW, I need a similar HW to reproduce the
issue for further debug.

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?

Li Jun

Francesco Dolcini Jan. 4, 2023, 7:33 p.m. UTC | #12

On Tue, Jan 03, 2023 at 03:45:42AM +0000, Jun Li wrote:
> > -----Original Message-----
> > From: Francesco Dolcini <[e-mail]>
> > Sent: Monday, December 26, 2022 11:45 PM
> > To: Jun Li <[e-mail]>
> > Cc: Francesco Dolcini <[e-mail]>; Jun Li (OSS)
> > <[e-mail]>; Peter Chen <[e-mail]>;
> > [e-mail]; Greg Kroah-Hartman
> > <[e-mail]>; Shawn Guo <[e-mail]>; Sascha
> > Hauer <[e-mail]>; Pengutronix Kernel Team
> > <[e-mail]>; Fabio Estevam <[e-mail]>; dl-linux-imx
> > <[e-mail]>; Felipe Balbi <[e-mail]>;
> > [e-mail]; Francesco Dolcini
> > <[e-mail]>; Xu Yang <[e-mail]>
> > 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 <[e-mail]>
> > > > > 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

Fabio Estevam Jan. 5, 2023, 2:18 a.m. UTC | #13

On Fri, Oct 28, 2022 at 9:31 AM Francesco Dolcini <[e-mail]> wrote:

>  static const struct ci_hdrc_imx_platform_flag imx6ul_usb_data = {
> -       .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
> +       .flags =

Not sure if this is related to a problem I was having on imx7d.

I was also disabling runtime pm as you showed above to workaround a
USB modem detection problem.

Li Jun suggested I check for the over-current polarity description of
the devicetree.

Passing 'disable-over-current' or ''over-current-active-low' made the
issue go away.

Patch

--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -56,7 +56,7 @@  static const struct ci_hdrc_imx_platform_flag imx6sx_usb_data = {
 };
 
 static const struct ci_hdrc_imx_platform_flag imx6ul_usb_data = {
-       .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
+       .flags =

I was digging even more into the topic and I found out that what is happening
is that mxs_phy_disconnect_line() is called. I than tried to remove the MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS flag.

--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -178,7 +178,7 @@  static const struct mxs_phy_data imx6sx_phy_data = {
 };
 
 static const struct mxs_phy_data imx6ul_phy_data = {
-       .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,
+       /*.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,*/
 };

This commit from NXP downstream kernel seems somehow related