i.MX8MM USB autosuspend broken with power domain support
Jun Li
jun.li at nxp.com
Thu Apr 28 20:14:41 PDT 2022
> -----Original Message-----
> From: Frieder Schrempf <frieder.schrempf at kontron.de>
> Sent: Thursday, April 28, 2022 5:58 PM
> To: Jun Li <jun.li at nxp.com>; Jacky Bai <ping.bai at nxp.com>; Lucas Stach
> <l.stach at pengutronix.de>; Fabio Estevam <festevam at gmail.com>
> Cc: Peter Chen <peter.chen at kernel.org>; Peng Fan <peng.fan at nxp.com>;
> linux-arm-kernel at lists.infradead.org; dl-linux-imx <linux-imx at nxp.com>;
> marek.vasut at gmail.com; tharvey at gateworks.com; Adam Ford
> <aford173 at gmail.com>; Breno Matheus Lima <breno.lima at nxp.com>; Xu Yang
> <xu.yang_2 at nxp.com>
> Subject: Re: i.MX8MM USB autosuspend broken with power domain support
>
> Am 15.04.22 um 10:12 schrieb Jun Li:
> >
> >
> >> -----Original Message-----
> >> From: Jacky Bai <ping.bai at nxp.com>
> >> Sent: Friday, April 15, 2022 10:21 AM
> >> To: Jun Li <jun.li at nxp.com>; Lucas Stach <l.stach at pengutronix.de>;
> >> Fabio Estevam <festevam at gmail.com>; Frieder Schrempf
> >> <frieder.schrempf at kontron.de>
> >> Cc: Peter Chen <peter.chen at kernel.org>; Peng Fan <peng.fan at nxp.com>;
> >> linux-arm-kernel at lists.infradead.org; dl-linux-imx
> >> <linux-imx at nxp.com>; marek.vasut at gmail.com; tharvey at gateworks.com;
> >> Adam Ford <aford173 at gmail.com>; Breno Matheus Lima
> >> <breno.lima at nxp.com>; Xu Yang <xu.yang_2 at nxp.com>
> >> Subject: RE: i.MX8MM USB autosuspend broken with power domain support
> >>
> >>> Subject: RE: i.MX8MM USB autosuspend broken with power domain
> >>> support
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Lucas Stach <l.stach at pengutronix.de>
> >>>> Sent: Wednesday, April 13, 2022 11:03 PM
> >>>> To: Fabio Estevam <festevam at gmail.com>; Frieder Schrempf
> >>>> <frieder.schrempf at kontron.de>; Jun Li <jun.li at nxp.com>
> >>>> Cc: Peter Chen <peter.chen at kernel.org>; Peng Fan
> >>>> <peng.fan at nxp.com>; linux-arm-kernel at lists.infradead.org;
> >>>> dl-linux-imx <linux-imx at nxp.com>; marek.vasut at gmail.com;
> >>>> tharvey at gateworks.com; Adam Ford <aford173 at gmail.com>; Breno
> >>>> Matheus Lima <breno.lima at nxp.com>
> >>>> Subject: Re: i.MX8MM USB autosuspend broken with power domain
> >>>> support
> >>>>
> >>>> Am Mittwoch, dem 13.04.2022 um 11:40 -0300 schrieb Fabio Estevam:
> >>>>> [Adding Jun Li]
> >>>>>
> >>>>> On Wed, Apr 13, 2022 at 11:35 AM Frieder Schrempf
> >>>>> <frieder.schrempf at kontron.de> wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> when power domain support was added for i.MX8MM, it seems like
> >>>>>> this broke the USB autosuspend feature.
> >>>>>>
> >>>>>> I reported this previously when testing the gpcv2 patches before
> >>>>>> they were merged [1] and the issue can also be reproduced on v5.18-rc2.
> >>>>>>
> >>>>>> Did anyone else encounter such a problem? Can anyone help with
> >>>>>> debugging or proposing a fix?
> >>>>>>
> >>>>>> Do the USB power domains need to stay enabled for autosuspend to
> >>> work?
> >>>>>> If yes how can this be achieved?
> >>>>>>
> >>>>>> Below is some more information on how to reproduce the issue
> >>>>>> including some debug output.
> >>>>>>
> >>>>>> Thanks a lot and best regards
> >>>>>> Frieder
> >>>>>>
> >>>>>> 1. Plug in USB device on host port, device is not enumerated, no
> >>>>>> debug output
> >>>>>>
> >>>>>> 2. Disable autosuspend, device gets enumerated
> >>>>>>
> >>>>>> ~# echo on > /sys/bus/usb/devices/usb1/power/control
> >>>>>> [ 2986.582786] imx_usb 32e40000.usb: genpd_runtime_resume() [
> >>>>>> 2986.588155] imx-pgc imx-pgc-domain.2: genpd_runtime_resume() [
> >>>>>> 2986.593876] imx-pgc imx-pgc-domain.2: resume latency exceeded,
> >>>>>> 1125
> >>>> ns
> >>>>>> [ 2986.600446] PM: usb-otg1: Power-on latency exceeded, new value
> >>>>>> 12295000 ns
> >>>>>> [ 2986.607342] imx_usb 32e40000.usb: at imx_controller_resume [
> >>>>>> 2986.612850] ci_hdrc ci_hdrc.0: genpd_runtime_resume() [
> >>>>>> 2986.617919] ci_hdrc ci_hdrc.0: at ci_controller_resume [
> >>>>>> 2986.858565] usb 1-1: new full-speed USB device number 10 using
> >>>>>> ci_hdrc
> >>>>>>
> >>>>>> [1]
> >>>>
> >>
> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Feur01
> .safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Flkml&a
> mp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7C09e76aed6ea746047b580
> 8da1eb7ba52%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C63785607170235
> 8709%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=3STwzi1U9uAYHwa17k7cl2EfAEou6
> qi7vlHtCa%2FiG%2Bs%3D&reserved=0.
> >>>>
> >>> org%2Flkml%2F2021%2F5%2F19%2F883&data=04%7C01%7Cjun.li%40n
> >>> xp.com%7
> >>>> C
> >>>>
> >>> 95df1db516454246d2ef08da1d5eb444%7C686ea1d3bc2b4c6fa92cd99c5c30
> >>> 1635%7C
> >>>> 0
> >>>> %7C0%7C637854589844488598%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> >>> MC4wLjAwMDAi
> >>>> L
> >>>>
> >>> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Zz4
> >>> 47nG
> >>>> g
> >>>> 9Rr%2Bvn96v4KJelygmuqFWaScqPIFDWMBdWg%3D&reserved=0
> >>>>
> >>>> Now that I think about it again, it seems putting the USB
> >>>> controllers into the OTG1/2 power domains is wrong. I guess the
> >>>> controllers are actually located in the HSIOMIX domain, which
> >>>> probably needs to stay enabled even if there is no device
> >>>> connected, so that the wakeup logic works properly. It's the USB
> >>>> PHYs that should be placed in the OTG domains and which I expect
> >>>> can be powered down as long as no device is connected.
> >>>
> >>> Per my current understanding, USB remote wakeup(wakeup host via USB
> >>> data
> >>> line) need PHY power on, but controller can be off, I will Check
> >>> internally
> >> how
> >>> those power domains map to each physical part in SoC, and reproduce
> >>> the
> >> issue
> >>> Frieder is reporting.
> >>>
> >>
> >> On imx8mm, the USB OTG1/2 power domain are actually for PHY only, and
> >> it is just handling the PHY isolation.
> >> If these power domain are put into OFF mode in HW, PHY's output
> >> signal will be isolated, then leads to USB enumeration can NOT work.
> >> HSIOMIX PD only has ADB400 handshake.
> >> Just think again,
> >> the parent/child relationship between OTG1/2 and HSIOMIX is not
> >> really necessary. To simplify things, we can decouple the dependency
> >> between OTG1/2 and HSIOMIX, change them to sibling power domains,
> >> then controller just attaches the HSIOMIX pd, and USB PHY attaches
> >> OTG1/2 correspondingly.
> >
> > Yes, this can work.
> >
> > Hi Frieder,
> >
> > Could you please try below change?
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > index 1ee05677c2dd..3ff71ca122e4 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > @@ -275,6 +275,7 @@ usbphynop1: usbphynop1 {
> > clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>;
> > + power-domains = <&pgc_otg1>;
> > clock-names = "main_clk";
> > };
> >
> > @@ -284,6 +285,7 @@ usbphynop2: usbphynop2 {
> > clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>;
> > + power-domains = <&pgc_otg2>;
> > clock-names = "main_clk";
> > };
> >
> > @@ -669,13 +671,11 @@ pgc_pcie: power-domain at 1 {
> > pgc_otg1: power-domain at 2 {
> > #power-domain-cells = <0>;
> > reg =
> <IMX8MM_POWER_DOMAIN_OTG1>;
> > - power-domains =
> <&pgc_hsiomix>;
> > };
> >
> > pgc_otg2: power-domain at 3 {
> > #power-domain-cells = <0>;
> > reg =
> <IMX8MM_POWER_DOMAIN_OTG2>;
> > - power-domains =
> <&pgc_hsiomix>;
> > };
> >
> > pgc_gpumix: power-domain at 4 {
> > @@ -1180,7 +1180,7 @@ usbotg1: usb at 32e40000 {
> > assigned-clock-parents = <&clk
> IMX8MM_SYS_PLL2_500M>;
> > phys = <&usbphynop1>;
> > fsl,usbmisc = <&usbmisc1 0>;
> > - power-domains = <&pgc_otg1>;
> > + power-domains = <&pgc_hsiomix>;
> > status = "disabled";
> > };
> >
> > @@ -1200,7 +1200,7 @@ usbotg2: usb at 32e50000 {
> > assigned-clock-parents = <&clk
> IMX8MM_SYS_PLL2_500M>;
> > phys = <&usbphynop2>;
> > fsl,usbmisc = <&usbmisc2 0>;
> > - power-domains = <&pgc_otg2>;
> > + power-domains = <&pgc_hsiomix>;
> > status = "disabled";
> > };
> >
>
> Thanks Jacky, Jun and Lucas for the comments and suggestions and sorry for
> the delay!
>
> If Jacky is correct, we can power down the HSIOMIX and keep the OTG1/2 domains
> enabled so the PHY is still in a state where it can detect devices. So it's
> the other way round than what Lucas assumed first.
>
> Jun's patch above does seem to fix the issue for me. The decoupling of the
> OTG1/2 domains from the HSIO domain suggested by Jacky is still missing,
> though. IIUC this should enable us to power down the HSIOMIX if the controller
> is idle and save some power. Removing the "power-domains = <&pgc_hsiomix>"
> from the pgc_otg1/2 nodes should do the trick and seems to work fine for
> me, too.
>
> Jun, do you want to send a formal patch for this, or would you like me to
> do it?
OK, I will send a formal patch.
Li Jun
More information about the linux-arm-kernel
mailing list