Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-26 Thread Roger Quadros 
On 25/05/17 19:13, Tony Lindgren wrote:
> This is needed in preparation of adding support for omap3 and
> later OHCI. The runtime PM will only do something on platforms
> that implement it.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Hans de Goede 
> Cc: Rob Herring 
> Cc: Roger Quadros 
> Cc: Sebastian Reichel 
> Cc: Yoshihiro Shimoda 
> Signed-off-by: Tony Lindgren 

Acked-by: Roger Quadros 

> ---
>  drivers/usb/host/ohci-platform.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-platform.c 
> b/drivers/usb/host/ohci-platform.c
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   }
>  #endif
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
>   if (pdata->power_on) {
>   err = pdata->power_on(dev);
>   if (err < 0)
> @@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   if (pdata->power_off)
>   pdata->power_off(dev);
>  err_reset:
> + pm_runtime_disable(>dev);
>   while (--rst >= 0)
>   reset_control_assert(priv->resets[rst]);
>  err_put_clks:
> @@ -292,6 +296,7 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>   struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
>   int clk, rst;
>  
> + pm_runtime_get_sync(>dev);
>   usb_remove_hcd(hcd);
>  
>   if (pdata->power_off)
> @@ -305,6 +310,9 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>  
>   usb_put_hcd(hcd);
>  
> + pm_runtime_put_sync(>dev);
> + pm_runtime_disable(>dev);
> +
>   if (pdata == _platform_defaults)
>   dev->dev.platform_data = NULL;
>  
> 

-- 
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-25 Thread Alan Stern 
On Thu, 25 May 2017, Tony Lindgren wrote:

> This is needed in preparation of adding support for omap3 and
> later OHCI. The runtime PM will only do something on platforms
> that implement it.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Hans de Goede 
> Cc: Rob Herring 
> Cc: Roger Quadros 
> Cc: Sebastian Reichel 
> Cc: Yoshihiro Shimoda 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/usb/host/ohci-platform.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-platform.c 
> b/drivers/usb/host/ohci-platform.c
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   }
>  #endif
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
>   if (pdata->power_on) {
>   err = pdata->power_on(dev);
>   if (err < 0)
> @@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   if (pdata->power_off)
>   pdata->power_off(dev);
>  err_reset:
> + pm_runtime_disable(>dev);
>   while (--rst >= 0)
>   reset_control_assert(priv->resets[rst]);
>  err_put_clks:
> @@ -292,6 +296,7 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>   struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
>   int clk, rst;
>  
> + pm_runtime_get_sync(>dev);
>   usb_remove_hcd(hcd);
>  
>   if (pdata->power_off)
> @@ -305,6 +310,9 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>  
>   usb_put_hcd(hcd);
>  
> + pm_runtime_put_sync(>dev);
> + pm_runtime_disable(>dev);
> +
>   if (pdata == _platform_defaults)
>   dev->dev.platform_data = NULL;

Acked-by: Alan Stern 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-25 Thread Tony Lindgren 
This is needed in preparation of adding support for omap3 and
later OHCI. The runtime PM will only do something on platforms
that implement it.

Cc: devicet...@vger.kernel.org
Cc: Hans de Goede 
Cc: Rob Herring 
Cc: Roger Quadros 
Cc: Sebastian Reichel 
Cc: Yoshihiro Shimoda 
Signed-off-by: Tony Lindgren 
---
 drivers/usb/host/ohci-platform.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device *dev)
}
 #endif
 
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
if (pdata->power_on) {
err = pdata->power_on(dev);
if (err < 0)
@@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device *dev)
if (pdata->power_off)
pdata->power_off(dev);
 err_reset:
+   pm_runtime_disable(>dev);
while (--rst >= 0)
reset_control_assert(priv->resets[rst]);
 err_put_clks:
@@ -292,6 +296,7 @@ static int ohci_platform_remove(struct platform_device *dev)
struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
int clk, rst;
 
+   pm_runtime_get_sync(>dev);
usb_remove_hcd(hcd);
 
if (pdata->power_off)
@@ -305,6 +310,9 @@ static int ohci_platform_remove(struct platform_device *dev)
 
usb_put_hcd(hcd);
 
+   pm_runtime_put_sync(>dev);
+   pm_runtime_disable(>dev);
+
if (pdata == _platform_defaults)
dev->dev.platform_data = NULL;
 
-- 
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-24 Thread Roger Quadros 
On 23/05/17 17:08, Tony Lindgren wrote:
> * Roger Quadros  [170523 00:14]:
>> On 22/05/17 19:00, Tony Lindgren wrote:
>>> --- a/drivers/usb/host/ohci-platform.c
>>> +++ b/drivers/usb/host/ohci-platform.c
>>> @@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
>>> *dev)
>>> struct usb_hcd *hcd = platform_get_drvdata(dev);
>>> struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
>>> struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
>>> -   int clk, rst;
>>> +   int clk, rst, enabled;
>>> +
>>> +   enabled = pm_runtime_get_sync(>dev);
>>
>> Why do we need to pm_runtime_get_sync() here?
> 
> ohci_platform_remove()
>   usb_remove_hcd()
>   ohci_stop()
> 
> Will cause "external abort on non-linefetch" otherwise.

Got it, thanks.

> 
> Regards,
> 
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-23 Thread Tony Lindgren 
* Alan Stern  [170523 09:57]:
> On Mon, 22 May 2017, Tony Lindgren wrote:
> > Alan, do you have some better ideas for the ohci_platform_remove()
> > path below?
...
> > --- a/drivers/usb/host/ohci-platform.c
> > +++ b/drivers/usb/host/ohci-platform.c
> > @@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
> > *dev)
> > struct usb_hcd *hcd = platform_get_drvdata(dev);
> > struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
> > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> > -   int clk, rst;
> > +   int clk, rst, enabled;
> > +
> > +   enabled = pm_runtime_get_sync(>dev);
> > +   if (enabled < 0) {
> > +   dev_warn(>dev, "pm_runtime_get failed: %i\n",
> > +enabled);
> > +   pm_runtime_put_noidle(>dev);
> > +   }
> 
> I wouldn't worry about checking the return value.  There's no 
> particular reason for this call to fail, is there?

OK yeah if it was to fail it would already fail on probe
already :)

> > usb_remove_hcd(hcd);
> >  
> > @@ -305,6 +316,10 @@ static int ohci_platform_remove(struct platform_device 
> > *dev)
> >  
> > usb_put_hcd(hcd);
> >  
> > +   if (enabled >= 0)
> > +   pm_runtime_put_sync(>dev);
> > +   pm_runtime_disable(>dev);
> 
> And here you could just do the put_sync, with no test.  If the get_sync
> failed then this will probably fail too, but you won't be any worse off
> for the attempt.

OK fine with me.

Thanks,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-23 Thread Alan Stern 
On Mon, 22 May 2017, Tony Lindgren wrote:

> This is needed in preparation of adding support for omap3 and
> later OHCI. The runtime PM will only do something on platforms
> that implement it.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Hans de Goede 
> Cc: Rob Herring 
> Cc: Roger Quadros 
> Cc: Sebastian Reichel 
> Cc: Yoshihiro Shimoda 
> Signed-off-by: Tony Lindgren 
> ---
> 
> Alan, do you have some better ideas for the ohci_platform_remove()
> path below?
> 
> ---
>  drivers/usb/host/ohci-platform.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-platform.c 
> b/drivers/usb/host/ohci-platform.c
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   }
>  #endif
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
>   if (pdata->power_on) {
>   err = pdata->power_on(dev);
>   if (err < 0)
> @@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   if (pdata->power_off)
>   pdata->power_off(dev);
>  err_reset:
> + pm_runtime_disable(>dev);
>   while (--rst >= 0)
>   reset_control_assert(priv->resets[rst]);
>  err_put_clks:
> @@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>   struct usb_hcd *hcd = platform_get_drvdata(dev);
>   struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
>   struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> - int clk, rst;
> + int clk, rst, enabled;
> +
> + enabled = pm_runtime_get_sync(>dev);
> + if (enabled < 0) {
> + dev_warn(>dev, "pm_runtime_get failed: %i\n",
> +  enabled);
> + pm_runtime_put_noidle(>dev);
> + }

I wouldn't worry about checking the return value.  There's no 
particular reason for this call to fail, is there?

>  
>   usb_remove_hcd(hcd);
>  
> @@ -305,6 +316,10 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>  
>   usb_put_hcd(hcd);
>  
> + if (enabled >= 0)
> + pm_runtime_put_sync(>dev);
> + pm_runtime_disable(>dev);

And here you could just do the put_sync, with no test.  If the get_sync
failed then this will probably fail too, but you won't be any worse off
for the attempt.

Alan Stern

> +
>   if (pdata == _platform_defaults)
>   dev->dev.platform_data = NULL;

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-23 Thread Tony Lindgren 
* Roger Quadros  [170523 00:14]:
> On 22/05/17 19:00, Tony Lindgren wrote:
> > --- a/drivers/usb/host/ohci-platform.c
> > +++ b/drivers/usb/host/ohci-platform.c
> > @@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
> > *dev)
> > struct usb_hcd *hcd = platform_get_drvdata(dev);
> > struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
> > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> > -   int clk, rst;
> > +   int clk, rst, enabled;
> > +
> > +   enabled = pm_runtime_get_sync(>dev);
> 
> Why do we need to pm_runtime_get_sync() here?

ohci_platform_remove()
usb_remove_hcd()
ohci_stop()

Will cause "external abort on non-linefetch" otherwise.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-23 Thread Roger Quadros 
Hi,

On 22/05/17 19:00, Tony Lindgren wrote:
> This is needed in preparation of adding support for omap3 and
> later OHCI. The runtime PM will only do something on platforms
> that implement it.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Hans de Goede 
> Cc: Rob Herring 
> Cc: Roger Quadros 
> Cc: Sebastian Reichel 
> Cc: Yoshihiro Shimoda 
> Signed-off-by: Tony Lindgren 
> ---
> 
> Alan, do you have some better ideas for the ohci_platform_remove()
> path below?
> 
> ---
>  drivers/usb/host/ohci-platform.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-platform.c 
> b/drivers/usb/host/ohci-platform.c
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   }
>  #endif
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
>   if (pdata->power_on) {
>   err = pdata->power_on(dev);
>   if (err < 0)
> @@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device 
> *dev)
>   if (pdata->power_off)
>   pdata->power_off(dev);
>  err_reset:
> + pm_runtime_disable(>dev);
>   while (--rst >= 0)
>   reset_control_assert(priv->resets[rst]);
>  err_put_clks:
> @@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>   struct usb_hcd *hcd = platform_get_drvdata(dev);
>   struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
>   struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> - int clk, rst;
> + int clk, rst, enabled;
> +
> + enabled = pm_runtime_get_sync(>dev);

Why do we need to pm_runtime_get_sync() here?

> + if (enabled < 0) {
> + dev_warn(>dev, "pm_runtime_get failed: %i\n",
> +  enabled);
> + pm_runtime_put_noidle(>dev);
> + }
>  
>   usb_remove_hcd(hcd);
>  
> @@ -305,6 +316,10 @@ static int ohci_platform_remove(struct platform_device 
> *dev)
>  
>   usb_put_hcd(hcd);
>  
> + if (enabled >= 0)
> + pm_runtime_put_sync(>dev);
> + pm_runtime_disable(>dev);
> +
>   if (pdata == _platform_defaults)
>   dev->dev.platform_data = NULL;
>  
> 

-- 
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] usb: host: ohci-platform: Add basic runtime PM support

2017-05-22 Thread Tony Lindgren 
This is needed in preparation of adding support for omap3 and
later OHCI. The runtime PM will only do something on platforms
that implement it.

Cc: devicet...@vger.kernel.org
Cc: Hans de Goede 
Cc: Rob Herring 
Cc: Roger Quadros 
Cc: Sebastian Reichel 
Cc: Yoshihiro Shimoda 
Signed-off-by: Tony Lindgren 
---

Alan, do you have some better ideas for the ohci_platform_remove()
path below?

---
 drivers/usb/host/ohci-platform.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -242,6 +243,8 @@ static int ohci_platform_probe(struct platform_device *dev)
}
 #endif
 
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
if (pdata->power_on) {
err = pdata->power_on(dev);
if (err < 0)
@@ -271,6 +274,7 @@ static int ohci_platform_probe(struct platform_device *dev)
if (pdata->power_off)
pdata->power_off(dev);
 err_reset:
+   pm_runtime_disable(>dev);
while (--rst >= 0)
reset_control_assert(priv->resets[rst]);
 err_put_clks:
@@ -290,7 +294,14 @@ static int ohci_platform_remove(struct platform_device 
*dev)
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct usb_ohci_pdata *pdata = dev_get_platdata(>dev);
struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
-   int clk, rst;
+   int clk, rst, enabled;
+
+   enabled = pm_runtime_get_sync(>dev);
+   if (enabled < 0) {
+   dev_warn(>dev, "pm_runtime_get failed: %i\n",
+enabled);
+   pm_runtime_put_noidle(>dev);
+   }
 
usb_remove_hcd(hcd);
 
@@ -305,6 +316,10 @@ static int ohci_platform_remove(struct platform_device 
*dev)
 
usb_put_hcd(hcd);
 
+   if (enabled >= 0)
+   pm_runtime_put_sync(>dev);
+   pm_runtime_disable(>dev);
+
if (pdata == _platform_defaults)
dev->dev.platform_data = NULL;
 
-- 
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html