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

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

[Philippe Schenker]

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 <philippe.schenker@toradex.com>
---

 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

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

[Philippe Schenker]

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 <philippe.schenker@toradex.com>
---

 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

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

[Peter Chen]

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 <philippe.schenker@toradex.com>
> ---
> 
>  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

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

[Philippe Schenker]

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 <philippe.schenker@toradex.com>

---

 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

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

[Fabio Estevam]

Hi Philippe,

On Fri, Jul 10, 2020 at 12:51 PM Philippe Schenker
<philippe.schenker@toradex.com> 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.

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

[Philippe Schenker]

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
> <philippe.schenker@toradex.com> 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