2020-07-10 15:51:12

by Philippe Schenker

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: usb: ci-hdrc-usb2: add property disable-runtime-pm

Chipidea depends on some hardware signals to be there in order
for runtime-pm to work well. Add the possibility to disable runtime
power management that is necessary for certain boards.

Signed-off-by: Philippe Schenker <[email protected]>
---

Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 51376cbe5f3d..67a31df13e69 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -90,6 +90,7 @@ Optional properties:
case, the "idle" state needs to pull down the data and strobe pin
and the "active" state needs to pull up the strobe pin.
- pinctrl-n: alternate pin modes
+- disable-runtime-pm: This disables the runtime power management.

i.mx specific properties
- fsl,usbmisc: phandler of non-core register device, with one
--
2.27.0


2020-07-10 15:51:25

by Philippe Schenker

[permalink] [raw]
Subject: [PATCH 3/3] ARM: dts: colibri-imx6ull: disable runtime pm

The Colibri iMX6ULL has a somewhat special hardware design due to some
legacy decisions regarding USB OTG. This leads to different issues
when runtime PM is enabled.

Disable runtime power management on colibri-imx6ull USB.

Signed-off-by: Philippe Schenker <[email protected]>

---

arch/arm/boot/dts/imx6ull-colibri.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi b/arch/arm/boot/dts/imx6ull-colibri.dtsi
index 9145c536d71a..1c6692237506 100644
--- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
@@ -195,6 +195,7 @@ &uart5 {
};

&usbotg1 {
+ disable-runtime-pm;
dr_mode = "otg";
srp-disable;
hnp-disable;
@@ -202,6 +203,7 @@ &usbotg1 {
};

&usbotg2 {
+ disable-runtime-pm;
dr_mode = "host";
};

--
2.27.0

2020-07-10 15:51:26

by Philippe Schenker

[permalink] [raw]
Subject: [PATCH 2/3] usb: chipidea: imx: support disabling runtime-pm

Chipidea depends on some hardware signals to be there in order
for runtime-pm to work well. Add the possibility to disable runtime
power management that is necessary for certain boards.

Signed-off-by: Philippe Schenker <[email protected]>
---

drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 5ae16368a0c7..5078d0695eb7 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -434,6 +434,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
usb_phy_init(pdata.usb_phy);
}

+ if (of_property_read_bool(np, "disable-runtime-pm"))
+ pdata.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
+
if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
data->supports_runtime_pm = true;

--
2.27.0

2020-07-10 15:57:18

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: usb: ci-hdrc-usb2: add property disable-runtime-pm

Hi Philippe,

On Fri, Jul 10, 2020 at 12:51 PM Philippe Schenker
<[email protected]> wrote:
>
> Chipidea depends on some hardware signals to be there in order

I think this description is too vague.

Could you please provide more details so that a user can know if their
hardware falls into this category?

It is not clear from seeing this series what is the hardware details
that would require this property to be used.

2020-07-13 02:54:19

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: chipidea: imx: support disabling runtime-pm

On 20-07-10 17:49:33, Philippe Schenker wrote:
> Chipidea depends on some hardware signals to be there in order
> for runtime-pm to work well. Add the possibility to disable runtime
> power management that is necessary for certain boards.
>
> Signed-off-by: Philippe Schenker <[email protected]>
> ---
>
> drivers/usb/chipidea/ci_hdrc_imx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 5ae16368a0c7..5078d0695eb7 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -434,6 +434,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> usb_phy_init(pdata.usb_phy);
> }
>
> + if (of_property_read_bool(np, "disable-runtime-pm"))
> + pdata.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
> +
> if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> data->supports_runtime_pm = true;
>

The changes are OK, you may add some descriptions like Fabio suggested,
eg, your use cases.

--

Thanks,
Peter Chen

2020-07-13 08:23:27

by Philippe Schenker

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: usb: ci-hdrc-usb2: add property disable-runtime-pm

On Fri, 2020-07-10 at 12:56 -0300, Fabio Estevam wrote:
> Hi Philippe,
>
> On Fri, Jul 10, 2020 at 12:51 PM Philippe Schenker
> <[email protected]> wrote:
> > Chipidea depends on some hardware signals to be there in order
>
> I think this description is too vague.
>
> Could you please provide more details so that a user can know if their
> hardware falls into this category?
>
> It is not clear from seeing this series what is the hardware details
> that would require this property to be used.

Hi Fabio, Peter,

I tried to keep the description for this patch somewhat general. But as
Peter suggested I will add Toradex's use-case for this.

The problem with Colibri iMX6ULL is that we are stuck with a legacy
module standard that does only include one USB host/device switching
signal. Plus on that specific module we have no 5V available hence left
the connection of VBUS signal away.
This works normally pretty well but for our "dual-role OTG" port I could
not make it work with runtime-pm running and proper extcon
configuration. The problem was, when plugging in something in device or
host mode gets not enumerated and detected.

That are the reasons why I believe the PHY or something else in Chipidea
needs a signal that it does not have on that module that would wake the
whole thing from runtime-suspend.

I will think about a better description and send a v2

Thanks
Philippe