diff mbox series

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

Message ID 20200710154935.697190-1-philippe.schenker@toradex.com
State Superseded
Headers show
Series [1/3] dt-bindings: usb: ci-hdrc-usb2: add property disable-runtime-pm | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Philippe Schenker July 10, 2020, 3:49 p.m. UTC
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(+)

Comments

Fabio Estevam July 10, 2020, 3:56 p.m. UTC | #1
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.
Peter Chen July 13, 2020, 2:49 a.m. UTC | #2
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.
Philippe Schenker July 13, 2020, 8:20 a.m. UTC | #3
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
diff mbox series

Patch

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