[0/4] musb fixes for v4.9-rc cycle
diff mbox

Message ID 20161108220530.GO2428@atomide.com
State New
Headers show

Commit Message

Tony Lindgren Nov. 8, 2016, 10:05 p.m. UTC
* Ladislav Michl <ladis@linux-mips.org> [161108 10:20]:
> On Tue, Nov 08, 2016 at 07:55:50AM -0700, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [161108 07:50]:
> > > Weird, I have not figured out what happens there..
> > > 
> > > The only thing I'm still wondering about is if phy-twl4030-usb.c
> > > is losing ID pin interrupts occasionally but have not been
> > > able to verify that. Maybe try the following additional patch
> > > and see if it makes any difference?
> > 
> > And eventually the glue layers only should need to do this on
> > init:
> > 
> > pm_runtime_enable(glue->dev);
> > 
> > And this on the exit path:
> > 
> > pm_runtime_disable(glue->dev);
> > 
> > And musb core as the child device should keep things awake
> > when needed.
> 
> Tested your patch, does not work, does not trigger "USB link status err"
> both without and with pm_runtime_enable/disable.

OK. The patch below still works for me with musb_core.c
autosuspend_delay set to 100. Also works with it set to 10.

Note that we had two timeouts without this.. Can you try
playing with the timeout in musb_core.c and see if that
helps?

Regards,

Tony

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

Comments

Ladislav Michl Nov. 8, 2016, 10:52 p.m. UTC | #1
On Tue, Nov 08, 2016 at 03:05:30PM -0700, Tony Lindgren wrote:
> OK. The patch below still works for me with musb_core.c
> autosuspend_delay set to 100. Also works with it set to 10.
> 
> Note that we had two timeouts without this.. Can you try
> playing with the timeout in musb_core.c and see if that
> helps?


This patch works for me, also with autosuspend_delay set to 10.

Tested-by: Ladislav Michl <ladis@linux-mips.org>

Thank you,
	ladis

> Regards,
> 
> Tony
> 
> 8< ------------------
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1898,8 +1898,7 @@ static void musb_pm_runtime_check_session(struct musb *musb)
>  		musb->quirk_retries = 3;
>  	} else {
>  		musb_dbg(musb, "Allow PM with no session: %02x", devctl);
> -		pm_runtime_mark_last_busy(musb->controller);
> -		pm_runtime_put_autosuspend(musb->controller);
> +		pm_runtime_put_sync_suspend(musb->controller);
>  	}
>  
>  	musb->session = s;
> @@ -2268,7 +2267,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>  	 * 500 ms for some margin.
>  	 */
>  	pm_runtime_use_autosuspend(musb->controller);
> -	pm_runtime_set_autosuspend_delay(musb->controller, 500);
> +	pm_runtime_set_autosuspend_delay(musb->controller, 100);
>  	pm_runtime_enable(musb->controller);
>  	pm_runtime_get_sync(musb->controller);
>  
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -513,17 +513,18 @@ static int omap2430_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_enable(glue->dev);
> -	pm_runtime_use_autosuspend(glue->dev);
> -	pm_runtime_set_autosuspend_delay(glue->dev, 100);
>  
>  	ret = platform_device_add(musb);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to register musb device\n");
> -		goto err2;
> +		goto err3;
>  	}
>  
>  	return 0;
>  
> +err3:
> +	pm_runtime_disable(glue->dev);
> +
>  err2:
>  	platform_device_put(musb);
>  
> @@ -535,10 +536,7 @@ static int omap2430_remove(struct platform_device *pdev)
>  {
>  	struct omap2430_glue *glue = platform_get_drvdata(pdev);
>  
> -	pm_runtime_get_sync(glue->dev);
>  	platform_device_unregister(glue->musb);
> -	pm_runtime_put_sync(glue->dev);
> -	pm_runtime_dont_use_autosuspend(glue->dev);
>  	pm_runtime_disable(glue->dev);
>  
>  	return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Nov. 8, 2016, 11:16 p.m. UTC | #2
* Ladislav Michl <ladis@linux-mips.org> [161108 15:52]:
> On Tue, Nov 08, 2016 at 03:05:30PM -0700, Tony Lindgren wrote:
> > OK. The patch below still works for me with musb_core.c
> > autosuspend_delay set to 100. Also works with it set to 10.
> > 
> > Note that we had two timeouts without this.. Can you try
> > playing with the timeout in musb_core.c and see if that
> > helps?
> 
> 
> This patch works for me, also with autosuspend_delay set to 10.

Oh so to confirm, the $subject series patches + this one only fixes
your issue? So no more revert needed?

I think the musb_core timeout had to be 500 to idle the 2430 glue
layer phy also, but I need to recheck that. That should be a
separate patch and can be done later.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ladislav Michl Nov. 8, 2016, 11:39 p.m. UTC | #3
On Tue, Nov 08, 2016 at 04:16:37PM -0700, Tony Lindgren wrote:
> * Ladislav Michl <ladis@linux-mips.org> [161108 15:52]:
> > On Tue, Nov 08, 2016 at 03:05:30PM -0700, Tony Lindgren wrote:
> > > OK. The patch below still works for me with musb_core.c
> > > autosuspend_delay set to 100. Also works with it set to 10.
> > > 
> > > Note that we had two timeouts without this.. Can you try
> > > playing with the timeout in musb_core.c and see if that
> > > helps?
> > 
> > 
> > This patch works for me, also with autosuspend_delay set to 10.
> 
> Oh so to confirm, the $subject series patches + this one only fixes
> your issue? So no more revert needed?

Right. Revert is no more needed.

> I think the musb_core timeout had to be 500 to idle the 2430 glue
> layer phy also, but I need to recheck that. That should be a
> separate patch and can be done later.
> 
> Regards,
> 
> Tony

Regards,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Nov. 8, 2016, 11:47 p.m. UTC | #4
* Ladislav Michl <ladis@linux-mips.org> [161108 16:40]:
> On Tue, Nov 08, 2016 at 04:16:37PM -0700, Tony Lindgren wrote:
> > * Ladislav Michl <ladis@linux-mips.org> [161108 15:52]:
> > > On Tue, Nov 08, 2016 at 03:05:30PM -0700, Tony Lindgren wrote:
> > > > OK. The patch below still works for me with musb_core.c
> > > > autosuspend_delay set to 100. Also works with it set to 10.
> > > > 
> > > > Note that we had two timeouts without this.. Can you try
> > > > playing with the timeout in musb_core.c and see if that
> > > > helps?
> > > 
> > > 
> > > This patch works for me, also with autosuspend_delay set to 10.
> > 
> > Oh so to confirm, the $subject series patches + this one only fixes
> > your issue? So no more revert needed?
> 
> Right. Revert is no more needed.

OK good to hear!

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

Patch
diff mbox

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1898,8 +1898,7 @@  static void musb_pm_runtime_check_session(struct musb *musb)
 		musb->quirk_retries = 3;
 	} else {
 		musb_dbg(musb, "Allow PM with no session: %02x", devctl);
-		pm_runtime_mark_last_busy(musb->controller);
-		pm_runtime_put_autosuspend(musb->controller);
+		pm_runtime_put_sync_suspend(musb->controller);
 	}
 
 	musb->session = s;
@@ -2268,7 +2267,7 @@  musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	 * 500 ms for some margin.
 	 */
 	pm_runtime_use_autosuspend(musb->controller);
-	pm_runtime_set_autosuspend_delay(musb->controller, 500);
+	pm_runtime_set_autosuspend_delay(musb->controller, 100);
 	pm_runtime_enable(musb->controller);
 	pm_runtime_get_sync(musb->controller);
 
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -513,17 +513,18 @@  static int omap2430_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(glue->dev);
-	pm_runtime_use_autosuspend(glue->dev);
-	pm_runtime_set_autosuspend_delay(glue->dev, 100);
 
 	ret = platform_device_add(musb);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register musb device\n");
-		goto err2;
+		goto err3;
 	}
 
 	return 0;
 
+err3:
+	pm_runtime_disable(glue->dev);
+
 err2:
 	platform_device_put(musb);
 
@@ -535,10 +536,7 @@  static int omap2430_remove(struct platform_device *pdev)
 {
 	struct omap2430_glue *glue = platform_get_drvdata(pdev);
 
-	pm_runtime_get_sync(glue->dev);
 	platform_device_unregister(glue->musb);
-	pm_runtime_put_sync(glue->dev);
-	pm_runtime_dont_use_autosuspend(glue->dev);
 	pm_runtime_disable(glue->dev);
 
 	return 0;