i.MX8MM USB autosuspend broken with power domain support
Frieder Schrempf
frieder.schrempf at kontron.de
Thu Apr 28 02:57:53 PDT 2022
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&data=04%7C01%7Cfrieder.schrempf%40kontron.de%7C09e76aed6ea746047b5808da1eb7ba52%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637856071702358709%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=3STwzi1U9uAYHwa17k7cl2EfAEou6qi7vlHtCa%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?
More information about the linux-arm-kernel
mailing list