[PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Tony Lindgren]

We can't use static variable first for checking when musb is
initialized when we have multiple musb instances like on am335x.

Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Reviewed-by: Johan Hovold <johan-UKKVnA4tBMC5azolltMz9laTQe2KTcn/@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 9 +++++----
 drivers/usb/musb/musb_core.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

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
@@ -2291,6 +2291,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	if (status)
 		goto fail5;
 
+	musb->is_initialized = 1;
 	pm_runtime_mark_last_busy(musb->controller);
 	pm_runtime_put_autosuspend(musb->controller);
 
@@ -2629,7 +2630,6 @@ static int musb_runtime_suspend(struct device *dev)
 static int musb_runtime_resume(struct device *dev)
 {
 	struct musb	*musb = dev_to_musb(dev);
-	static int	first = 1;
 
 	/*
 	 * When pm_runtime_get_sync called for the first time in driver
@@ -2640,9 +2640,10 @@ static int musb_runtime_resume(struct device *dev)
 	 * Also context restore without save does not make
 	 * any sense
 	 */
-	if (!first)
-		musb_restore_context(musb);
-	first = 0;
+	if (!musb->is_initialized)
+		return 0;
+
+	musb_restore_context(musb);
 
 	if (musb->need_finish_resume) {
 		musb->need_finish_resume = 0;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -385,6 +385,8 @@ struct musb {
 	int			a_wait_bcon;	/* VBUS timeout in msecs */
 	unsigned long		idle_timeout;	/* Next timeout in jiffies */
 
+	unsigned		is_initialized:1;
+
 	/* active means connected and not suspended */
 	unsigned		is_active:1;
 
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Bin Liu]

Hi,

On Fri, Nov 11, 2016 at 10:42:57AM -0800, Tony Lindgren wrote:
> We can't use static variable first for checking when musb is
> initialized when we have multiple musb instances like on am335x.
> 
> Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Reviewed-by: Johan Hovold <johan-UKKVnA4tBMC5azolltMz9laTQe2KTcn/@public.gmane.org>
> Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/musb/musb_core.c | 9 +++++----
>  drivers/usb/musb/musb_core.h | 2 ++
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> 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
> @@ -2291,6 +2291,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>  	if (status)
>  		goto fail5;
>  
> +	musb->is_initialized = 1;
>  	pm_runtime_mark_last_busy(musb->controller);
>  	pm_runtime_put_autosuspend(musb->controller);
>  
> @@ -2629,7 +2630,6 @@ static int musb_runtime_suspend(struct device *dev)
>  static int musb_runtime_resume(struct device *dev)
>  {
>  	struct musb	*musb = dev_to_musb(dev);
> -	static int	first = 1;
>  
>  	/*
>  	 * When pm_runtime_get_sync called for the first time in driver
> @@ -2640,9 +2640,10 @@ static int musb_runtime_resume(struct device *dev)
>  	 * Also context restore without save does not make
>  	 * any sense
>  	 */
> -	if (!first)
> -		musb_restore_context(musb);
> -	first = 0;
> +	if (!musb->is_initialized)
> +		return 0;
> +
> +	musb_restore_context(musb);
>  
>  	if (musb->need_finish_resume) {
>  		musb->need_finish_resume = 0;
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -385,6 +385,8 @@ struct musb {
>  	int			a_wait_bcon;	/* VBUS timeout in msecs */
>  	unsigned long		idle_timeout;	/* Next timeout in jiffies */
>  
> +	unsigned		is_initialized:1;
> +

The musb strcut is getting bigger and bigger. Is it possible to not add
a new variable but use one initialized in musb_init_controller() for
this flag purpose? Readability shouldn't be an issue, since it is only
used once in musb_runtime_resume().

Sorry I missed if this patch has been sent before.

Regards,
-Bin.

>  	/* active means connected and not suspended */
>  	unsigned		is_active:1;
>  
> -- 
> 2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [161111 11:15]:
> On Fri, Nov 11, 2016 at 10:42:57AM -0800, Tony Lindgren wrote:
> > --- a/drivers/usb/musb/musb_core.h
> > +++ b/drivers/usb/musb/musb_core.h
> > @@ -385,6 +385,8 @@ struct musb {
> >  	int			a_wait_bcon;	/* VBUS timeout in msecs */
> >  	unsigned long		idle_timeout;	/* Next timeout in jiffies */
> >  
> > +	unsigned		is_initialized:1;
> > +
> 
> The musb strcut is getting bigger and bigger. Is it possible to not add
> a new variable but use one initialized in musb_init_controller() for
> this flag purpose? Readability shouldn't be an issue, since it is only
> used once in musb_runtime_resume().

Well I was initially checking for delayed work being initialized,
but Johan did not like the idea of using it. Looking at
musb_init_controller(), we only initialize musb->nIrq after
pm_runtime_get_sync(), but that's no better either.. Do you have
any better ideas for something to use?

I agree in the long run we should split struct musb into smaller
structs though.

Meanwhile the new flags added in this series are bitfields, so it's
probably better to use that and overloading something that's not
strictly related.

Regards,

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

Re: [PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Bin Liu]

On Fri, Nov 11, 2016 at 12:05:19PM -0800, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [161111 11:15]:
> > On Fri, Nov 11, 2016 at 10:42:57AM -0800, Tony Lindgren wrote:
> > > --- a/drivers/usb/musb/musb_core.h
> > > +++ b/drivers/usb/musb/musb_core.h
> > > @@ -385,6 +385,8 @@ struct musb {
> > >  	int			a_wait_bcon;	/* VBUS timeout in msecs */
> > >  	unsigned long		idle_timeout;	/* Next timeout in jiffies */
> > >  
> > > +	unsigned		is_initialized:1;
> > > +
> > 
> > The musb strcut is getting bigger and bigger. Is it possible to not add
> > a new variable but use one initialized in musb_init_controller() for
> > this flag purpose? Readability shouldn't be an issue, since it is only
> > used once in musb_runtime_resume().
> 
> Well I was initially checking for delayed work being initialized,
> but Johan did not like the idea of using it. Looking at
> musb_init_controller(), we only initialize musb->nIrq after
> pm_runtime_get_sync(), but that's no better either.. Do you have
> any better ideas for something to use?

I only spent a few seconds on musb_init_controller(), and musb->nIrq is
what I found.

> 
> I agree in the long run we should split struct musb into smaller
> structs though.
> 
> Meanwhile the new flags added in this series are bitfields, so it's
> probably better to use that and overloading something that's not
> strictly related.

Ok, just leave it as is. We will clean up musb struct later. *cough* not
sure when it will ever happen...

> 
> Regards,
> 
> Tony

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

[PATCHv3 0/6] musb fixes for v4.9-rc cycle

[Tony Lindgren]

Hi all,

Here's v3 of musb fixes for issues that I've been able to track down.
The PM changes merged for v4.9 popped up various issues reported by
people that I had not seen earlier with my tests.

As many people depend on this driver, I'd like to have these merged
for v4.9-rc cycle.

Regards,

Tony


Changes since v2:

- Collect acks, update patch descriptions and remove one comment
  line based on comments from Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Changes since v1:

- Numerous changes to patch 2/4 for the sleeping function fix
  based on comments from Johan

  - Two new patches to fix issues with devices not always
    enumerating by removing pointless PM runtime code

Tony Lindgren (6):
  usb: musb: Fix broken use of static variable for multiple instances
  usb: musb: Fix sleeping function called from invalid context for hdrc
    glue
  usb: musb: Fix PM for hub disconnect
  usb: musb: Add missing pm_runtime_disable and drop 2430 PM timeout
  usb: musb: Drop pointless PM runtime code for dsps glue
  phy: twl4030-usb: Fix for musb session bit based PM

 drivers/phy/phy-twl4030-usb.c  |   4 +-
 drivers/usb/musb/musb_core.c   | 147 ++++++++++++++++++++++++++++++++++++-----
 drivers/usb/musb/musb_core.h   |  13 +++-
 drivers/usb/musb/musb_dsps.c   |  58 ++++++++--------
 drivers/usb/musb/musb_gadget.c |  39 +++++++++--
 drivers/usb/musb/omap2430.c    |  10 ++-
 drivers/usb/musb/tusb6010.c    |   6 +-
 7 files changed, 209 insertions(+), 68 deletions(-)

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

[PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Tony Lindgren]

We can't use static variable first for checking when musb is
initialized when we have multiple musb instances like on am335x.

Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Reviewed-by: Johan Hovold <johan-UKKVnA4tBMC5azolltMz9laTQe2KTcn/@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 9 +++++----
 drivers/usb/musb/musb_core.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

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
@@ -2291,6 +2291,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	if (status)
 		goto fail5;
 
+	musb->is_initialized = 1;
 	pm_runtime_mark_last_busy(musb->controller);
 	pm_runtime_put_autosuspend(musb->controller);
 
@@ -2629,7 +2630,6 @@ static int musb_runtime_suspend(struct device *dev)
 static int musb_runtime_resume(struct device *dev)
 {
 	struct musb	*musb = dev_to_musb(dev);
-	static int	first = 1;
 
 	/*
 	 * When pm_runtime_get_sync called for the first time in driver
@@ -2640,9 +2640,10 @@ static int musb_runtime_resume(struct device *dev)
 	 * Also context restore without save does not make
 	 * any sense
 	 */
-	if (!first)
-		musb_restore_context(musb);
-	first = 0;
+	if (!musb->is_initialized)
+		return 0;
+
+	musb_restore_context(musb);
 
 	if (musb->need_finish_resume) {
 		musb->need_finish_resume = 0;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -385,6 +385,8 @@ struct musb {
 	int			a_wait_bcon;	/* VBUS timeout in msecs */
 	unsigned long		idle_timeout;	/* Next timeout in jiffies */
 
+	unsigned		is_initialized:1;
+
 	/* active means connected and not suspended */
 	unsigned		is_active:1;
 
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] usb: musb: Fix sleeping function called from invalid context for hdrc glue

[Tony Lindgren]

Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer
that runs in softirq context. That causes a "BUG: sleeping function called
from invalid context" every time when polling the cable status:

[<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0)
[<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254)
[<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c)
[<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210)
[<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc)
[<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594)

I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled.
And looks like also musb_gadget_queue() suffers from the same problem.

Let's fix the issue by using a list of delayed work then call it on
resume. Note that we want to do this only when musb core and it's
parent devices are awake, and we need to make sure the DSPS glue
timer is stopped as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>.
Note that we already are re-enabling the timer with mod_timer() in
dsps_musb_enable().

Later on we may be able to remove other delayed work in the musb driver
and just do it from pending_resume_work. But this should be done only
for delayed work that does not have other timing requirements beyond
just being run on resume.

Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer")
Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c   | 109 +++++++++++++++++++++++++++++++++++++++--
 drivers/usb/musb/musb_core.h   |   7 +++
 drivers/usb/musb/musb_dsps.c   |  36 ++++++++++----
 drivers/usb/musb/musb_gadget.c |  33 +++++++++++--
 4 files changed, 167 insertions(+), 18 deletions(-)

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
@@ -1969,6 +1969,7 @@ static struct musb *allocate_instance(struct device *dev,
 	INIT_LIST_HEAD(&musb->control);
 	INIT_LIST_HEAD(&musb->in_bulk);
 	INIT_LIST_HEAD(&musb->out_bulk);
+	INIT_LIST_HEAD(&musb->pending_list);
 
 	musb->vbuserr_retry = VBUSERR_RETRY_COUNT;
 	musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON;
@@ -2018,6 +2019,84 @@ static void musb_free(struct musb *musb)
 	musb_host_free(musb);
 }
 
+struct musb_pending_work {
+	int (*callback)(struct musb *musb, void *data);
+	void *data;
+	struct list_head node;
+};
+
+/*
+ * Called from musb_runtime_resume(), musb_resume(), and
+ * musb_queue_resume_work(). Callers must take musb->lock.
+ */
+static int musb_run_resume_work(struct musb *musb)
+{
+	struct musb_pending_work *w, *_w;
+	unsigned long flags;
+	int error = 0;
+
+	spin_lock_irqsave(&musb->list_lock, flags);
+	list_for_each_entry_safe(w, _w, &musb->pending_list, node) {
+		if (w->callback) {
+			error = w->callback(musb, w->data);
+			if (error < 0) {
+				dev_err(musb->controller,
+					"resume callback %p failed: %i\n",
+					w->callback, error);
+			}
+		}
+		list_del(&w->node);
+		devm_kfree(musb->controller, w);
+	}
+	spin_unlock_irqrestore(&musb->list_lock, flags);
+
+	return error;
+}
+
+/*
+ * Called to run work if device is active or else queue the work to happen
+ * on resume. Caller must take musb->lock and must hold an RPM reference.
+ *
+ * Note that we cowardly refuse queuing work after musb PM runtime
+ * resume is done calling musb_run_resume_work() and return -EINPROGRESS
+ * instead.
+ */
+int musb_queue_resume_work(struct musb *musb,
+			   int (*callback)(struct musb *musb, void *data),
+			   void *data)
+{
+	struct musb_pending_work *w;
+	unsigned long flags;
+	int error;
+
+	if (WARN_ON(!callback))
+		return -EINVAL;
+
+	if (pm_runtime_active(musb->controller))
+		return callback(musb, data);
+
+	w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC);
+	if (!w)
+		return -ENOMEM;
+
+	w->callback = callback;
+	w->data = data;
+	spin_lock_irqsave(&musb->list_lock, flags);
+	if (musb->is_runtime_suspended) {
+		list_add_tail(&w->node, &musb->pending_list);
+		error = 0;
+	} else {
+		dev_err(musb->controller, "could not add resume work %p\n",
+			callback);
+		devm_kfree(musb->controller, w);
+		error = -EINPROGRESS;
+	}
+	spin_unlock_irqrestore(&musb->list_lock, flags);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(musb_queue_resume_work);
+
 static void musb_deassert_reset(struct work_struct *work)
 {
 	struct musb *musb;
@@ -2065,6 +2144,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	}
 
 	spin_lock_init(&musb->lock);
+	spin_lock_init(&musb->list_lock);
 	musb->board_set_power = plat->set_power;
 	musb->min_power = plat->min_power;
 	musb->ops = plat->platform_ops;
@@ -2558,6 +2638,7 @@ static int musb_suspend(struct device *dev)
 
 	musb_platform_disable(musb);
 	musb_generic_disable(musb);
+	WARN_ON(!list_empty(&musb->pending_list));
 
 	spin_lock_irqsave(&musb->lock, flags);
 
@@ -2579,9 +2660,11 @@ static int musb_suspend(struct device *dev)
 
 static int musb_resume(struct device *dev)
 {
-	struct musb	*musb = dev_to_musb(dev);
-	u8		devctl;
-	u8		mask;
+	struct musb *musb = dev_to_musb(dev);
+	unsigned long flags;
+	int error;
+	u8 devctl;
+	u8 mask;
 
 	/*
 	 * For static cmos like DaVinci, register values were preserved
@@ -2615,6 +2698,13 @@ static int musb_resume(struct device *dev)
 
 	musb_start(musb);
 
+	spin_lock_irqsave(&musb->lock, flags);
+	error = musb_run_resume_work(musb);
+	if (error)
+		dev_err(musb->controller, "resume work failed with %i\n",
+			error);
+	spin_unlock_irqrestore(&musb->lock, flags);
+
 	return 0;
 }
 
@@ -2623,13 +2713,16 @@ static int musb_runtime_suspend(struct device *dev)
 	struct musb	*musb = dev_to_musb(dev);
 
 	musb_save_context(musb);
+	musb->is_runtime_suspended = 1;
 
 	return 0;
 }
 
 static int musb_runtime_resume(struct device *dev)
 {
-	struct musb	*musb = dev_to_musb(dev);
+	struct musb *musb = dev_to_musb(dev);
+	unsigned long flags;
+	int error;
 
 	/*
 	 * When pm_runtime_get_sync called for the first time in driver
@@ -2651,6 +2744,14 @@ static int musb_runtime_resume(struct device *dev)
 				msecs_to_jiffies(USB_RESUME_TIMEOUT));
 	}
 
+	spin_lock_irqsave(&musb->lock, flags);
+	error = musb_run_resume_work(musb);
+	if (error)
+		dev_err(musb->controller, "resume work failed with %i\n",
+			error);
+	musb->is_runtime_suspended = 0;
+	spin_unlock_irqrestore(&musb->lock, flags);
+
 	return 0;
 }
 
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -303,6 +303,7 @@ struct musb_context_registers {
 struct musb {
 	/* device lock */
 	spinlock_t		lock;
+	spinlock_t		list_lock;	/* resume work list lock */
 
 	struct musb_io		io;
 	const struct musb_platform_ops *ops;
@@ -337,6 +338,7 @@ struct musb {
 	struct list_head	control;	/* of musb_qh */
 	struct list_head	in_bulk;	/* of musb_qh */
 	struct list_head	out_bulk;	/* of musb_qh */
+	struct list_head	pending_list;	/* pending work list */
 
 	struct timer_list	otg_timer;
 	struct notifier_block	nb;
@@ -386,6 +388,7 @@ struct musb {
 	unsigned long		idle_timeout;	/* Next timeout in jiffies */
 
 	unsigned		is_initialized:1;
+	unsigned		is_runtime_suspended:1;
 
 	/* active means connected and not suspended */
 	unsigned		is_active:1;
@@ -542,6 +545,10 @@ extern irqreturn_t musb_interrupt(struct musb *);
 
 extern void musb_hnp_stop(struct musb *musb);
 
+int musb_queue_resume_work(struct musb *musb,
+			   int (*callback)(struct musb *musb, void *data),
+			   void *data);
+
 static inline void musb_platform_set_vbus(struct musb *musb, int is_on)
 {
 	if (musb->ops->set_vbus)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -185,24 +185,19 @@ static void dsps_musb_disable(struct musb *musb)
 	musb_writel(reg_base, wrp->coreintr_clear, wrp->usb_bitmap);
 	musb_writel(reg_base, wrp->epintr_clear,
 			 wrp->txep_bitmap | wrp->rxep_bitmap);
+	del_timer_sync(&glue->timer);
 	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 }
 
-static void otg_timer(unsigned long _musb)
+/* Caller must take musb->lock */
+static int dsps_check_status(struct musb *musb, void *unused)
 {
-	struct musb *musb = (void *)_musb;
 	void __iomem *mregs = musb->mregs;
 	struct device *dev = musb->controller;
 	struct dsps_glue *glue = dev_get_drvdata(dev->parent);
 	const struct dsps_musb_wrapper *wrp = glue->wrp;
 	u8 devctl;
-	unsigned long flags;
 	int skip_session = 0;
-	int err;
-
-	err = pm_runtime_get_sync(dev);
-	if (err < 0)
-		dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
 
 	/*
 	 * We poll because DSPS IP's won't expose several OTG-critical
@@ -212,7 +207,6 @@ static void otg_timer(unsigned long _musb)
 	dev_dbg(musb->controller, "Poll devctl %02x (%s)\n", devctl,
 				usb_otg_state_string(musb->xceiv->otg->state));
 
-	spin_lock_irqsave(&musb->lock, flags);
 	switch (musb->xceiv->otg->state) {
 	case OTG_STATE_A_WAIT_VRISE:
 		mod_timer(&glue->timer, jiffies +
@@ -245,8 +239,30 @@ static void otg_timer(unsigned long _musb)
 	default:
 		break;
 	}
-	spin_unlock_irqrestore(&musb->lock, flags);
 
+	return 0;
+}
+
+static void otg_timer(unsigned long _musb)
+{
+	struct musb *musb = (void *)_musb;
+	struct device *dev = musb->controller;
+	unsigned long flags;
+	int err;
+
+	err = pm_runtime_get(dev);
+	if ((err != -EINPROGRESS) && err < 0) {
+		dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
+		pm_runtime_put_noidle(dev);
+
+		return;
+	}
+
+	spin_lock_irqsave(&musb->lock, flags);
+	err = musb_queue_resume_work(musb, dsps_check_status, NULL);
+	if (err < 0)
+		dev_err(dev, "%s resume work: %i\n", __func__, err);
+	spin_unlock_irqrestore(&musb->lock, flags);
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 }
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1222,13 +1222,22 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req)
 		rxstate(musb, req);
 }
 
+static int musb_ep_restart_resume_work(struct musb *musb, void *data)
+{
+	struct musb_request *req = data;
+
+	musb_ep_restart(musb, req);
+
+	return 0;
+}
+
 static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
 			gfp_t gfp_flags)
 {
 	struct musb_ep		*musb_ep;
 	struct musb_request	*request;
 	struct musb		*musb;
-	int			status = 0;
+	int			status;
 	unsigned long		lockflags;
 
 	if (!ep || !req)
@@ -1245,6 +1254,17 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
 	if (request->ep != musb_ep)
 		return -EINVAL;
 
+	status = pm_runtime_get(musb->controller);
+	if ((status != -EINPROGRESS) && status < 0) {
+		dev_err(musb->controller,
+			"pm runtime get failed in %s\n",
+			__func__);
+		pm_runtime_put_noidle(musb->controller);
+
+		return status;
+	}
+	status = 0;
+
 	trace_musb_req_enq(request);
 
 	/* request is mine now... */
@@ -1255,7 +1275,6 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
 
 	map_dma_buffer(request, musb, musb_ep);
 
-	pm_runtime_get_sync(musb->controller);
 	spin_lock_irqsave(&musb->lock, lockflags);
 
 	/* don't queue if the ep is down */
@@ -1271,8 +1290,14 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
 	list_add_tail(&request->list, &musb_ep->req_list);
 
 	/* it this is the head of the queue, start i/o ... */
-	if (!musb_ep->busy && &request->list == musb_ep->req_list.next)
-		musb_ep_restart(musb, request);
+	if (!musb_ep->busy && &request->list == musb_ep->req_list.next) {
+		status = musb_queue_resume_work(musb,
+						musb_ep_restart_resume_work,
+						request);
+		if (status < 0)
+			dev_err(musb->controller, "%s resume work: %i\n",
+				__func__, status);
+	}
 
 unlock:
 	spin_unlock_irqrestore(&musb->lock, lockflags);
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] usb: musb: Fix PM for hub disconnect

[Tony Lindgren]

With a USB hub disconnected, devctl can be 0x19 for about a second
on am335x and will stay forever on at least omap3. And we get no
further interrupts when devctl session bit clears. This keeps
PM runtime active.

Let's fix the issue by polling devctl until the session bit clears
or times out. We can do this by making musb->irq_work into
delayed_work.

And with the polling implemented, we can now also have the quirk
for invalid VBUS it to avoid disconnecting too early while VBUS
is ramping up.

Fixes: 467d5c980709 ("usb: musb: Implement session bit based runtime
PM for musb-core")
Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c   | 29 +++++++++++++++++++----------
 drivers/usb/musb/musb_core.h   |  4 ++--
 drivers/usb/musb/musb_gadget.c |  6 +++---
 drivers/usb/musb/tusb6010.c    |  6 +++---
 4 files changed, 27 insertions(+), 18 deletions(-)

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
@@ -986,7 +986,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 	}
 #endif
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	return handled;
 }
@@ -1855,14 +1855,23 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 		MUSB_DEVCTL_HR;
 	switch (devctl & ~s) {
 	case MUSB_QUIRK_B_INVALID_VBUS_91:
-		if (!musb->session && !musb->quirk_invalid_vbus) {
-			musb->quirk_invalid_vbus = true;
+		if (musb->quirk_retries--) {
 			musb_dbg(musb,
-				 "First invalid vbus, assume no session");
+				 "Poll devctl on invalid vbus, assume no session");
+			schedule_delayed_work(&musb->irq_work,
+					      msecs_to_jiffies(1000));
+
 			return;
 		}
-		break;
 	case MUSB_QUIRK_A_DISCONNECT_19:
+		if (musb->quirk_retries--) {
+			musb_dbg(musb,
+				 "Poll devctl on possible host mode disconnect");
+			schedule_delayed_work(&musb->irq_work,
+					      msecs_to_jiffies(1000));
+
+			return;
+		}
 		if (!musb->session)
 			break;
 		musb_dbg(musb, "Allow PM on possible host mode disconnect");
@@ -1886,9 +1895,9 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 		if (error < 0)
 			dev_err(musb->controller, "Could not enable: %i\n",
 				error);
+		musb->quirk_retries = 3;
 	} else {
 		musb_dbg(musb, "Allow PM with no session: %02x", devctl);
-		musb->quirk_invalid_vbus = false;
 		pm_runtime_mark_last_busy(musb->controller);
 		pm_runtime_put_autosuspend(musb->controller);
 	}
@@ -1899,7 +1908,7 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 /* Only used to provide driver mode change events */
 static void musb_irq_work(struct work_struct *data)
 {
-	struct musb *musb = container_of(data, struct musb, irq_work);
+	struct musb *musb = container_of(data, struct musb, irq_work.work);
 
 	musb_pm_runtime_check_session(musb);
 
@@ -2288,7 +2297,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_generic_disable(musb);
 
 	/* Init IRQ workqueue before request_irq */
-	INIT_WORK(&musb->irq_work, musb_irq_work);
+	INIT_DELAYED_WORK(&musb->irq_work, musb_irq_work);
 	INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);
 	INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume);
 
@@ -2385,7 +2394,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_host_cleanup(musb);
 
 fail3:
-	cancel_work_sync(&musb->irq_work);
+	cancel_delayed_work_sync(&musb->irq_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	if (musb->dma_controller)
@@ -2452,7 +2461,7 @@ static int musb_remove(struct platform_device *pdev)
 	 */
 	musb_exit_debugfs(musb);
 
-	cancel_work_sync(&musb->irq_work);
+	cancel_delayed_work_sync(&musb->irq_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	pm_runtime_get_sync(musb->controller);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -310,7 +310,7 @@ struct musb {
 	struct musb_context_registers context;
 
 	irqreturn_t		(*isr)(int, void *);
-	struct work_struct	irq_work;
+	struct delayed_work	irq_work;
 	struct delayed_work	deassert_reset_work;
 	struct delayed_work	finish_resume_work;
 	struct delayed_work	gadget_work;
@@ -381,7 +381,7 @@ struct musb {
 
 	int			port_mode;	/* MUSB_PORT_MODE_* */
 	bool			session;
-	bool			quirk_invalid_vbus;
+	unsigned long		quirk_retries;
 	bool			is_host;
 
 	int			a_wait_bcon;	/* VBUS timeout in msecs */
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1114,7 +1114,7 @@ static int musb_gadget_enable(struct usb_ep *ep,
 			musb_ep->dma ? "dma, " : "",
 			musb_ep->packet_sz);
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 fail:
 	spin_unlock_irqrestore(&musb->lock, flags);
@@ -1158,7 +1158,7 @@ static int musb_gadget_disable(struct usb_ep *ep)
 	musb_ep->desc = NULL;
 	musb_ep->end_point.desc = NULL;
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	spin_unlock_irqrestore(&(musb->lock), flags);
 
@@ -1994,7 +1994,7 @@ static int musb_gadget_stop(struct usb_gadget *g)
 	 */
 
 	/* Force check of devctl register for PM runtime */
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	pm_runtime_mark_last_busy(musb->controller);
 	pm_runtime_put_autosuspend(musb->controller);
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -724,7 +724,7 @@ tusb_otg_ints(struct musb *musb, u32 int_src, void __iomem *tbase)
 			dev_dbg(musb->controller, "vbus change, %s, otg %03x\n",
 				usb_otg_state_string(musb->xceiv->otg->state), otg_stat);
 			idle_timeout = jiffies + (1 * HZ);
-			schedule_work(&musb->irq_work);
+			schedule_delayed_work(&musb->irq_work, 0);
 
 		} else /* A-dev state machine */ {
 			dev_dbg(musb->controller, "vbus change, %s, otg %03x\n",
@@ -814,7 +814,7 @@ tusb_otg_ints(struct musb *musb, u32 int_src, void __iomem *tbase)
 			break;
 		}
 	}
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	return idle_timeout;
 }
@@ -864,7 +864,7 @@ static irqreturn_t tusb_musb_interrupt(int irq, void *__hci)
 		musb_writel(tbase, TUSB_PRCM_WAKEUP_CLEAR, reg);
 		if (reg & ~TUSB_PRCM_WNORCS) {
 			musb->is_active = 1;
-			schedule_work(&musb->irq_work);
+			schedule_delayed_work(&musb->irq_work, 0);
 		}
 		dev_dbg(musb->controller, "wake %sactive %02x\n",
 				musb->is_active ? "" : "in", reg);
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] usb: musb: Add missing pm_runtime_disable and drop 2430 PM timeout

[Tony Lindgren]

We are missing pm_runtime_disable() in 2430 glue layer. Further,
we only need to enable PM runtime and disable it on exit. With
musb_core.c doing PM, the glue layer as a parent will always be
active when musb_core.c is active.

This fixes host enumeration issues with some devices as reported
by Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>.

And holding an RPM reference while deregistering the child would
lead to a crash in omap2430_runtime_suspend() which dereferences
the now freed child's driver data on put as pointed out by
Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:

Unable to handle kernel paging request at virtual address 6b6b6f17
...
[<c05453d4>] (omap2430_runtime_suspend) from [<c0481410>]
 (pm_generic_runtime_suspend+0x3c/0x48)
[<c0481410>] (pm_generic_runtime_suspend) from [<c0121028>]
 (_od_runtime_suspend+0x1c/0x30)
[<c0121028>] (_od_runtime_suspend) from [<c04833b0>] (__rpm_callback+0x3c/0x70)
[<c04833b0>] (__rpm_callback) from [<c0483414>] (rpm_callback+0x30/0x90)
[<c0483414>] (rpm_callback) from [<c0483984>] (rpm_suspend+0x118/0x6b4)
[<c0483984>] (rpm_suspend) from [<c04840f4>] (rpm_idle+0x104/0x440)
[<c04840f4>] (rpm_idle) from [<c04844ac>] (__pm_runtime_idle+0x7c/0xb0)
[<c04844ac>] (__pm_runtime_idle) from [<c0545458>] (omap2430_remove+0x38/0x58)
[<c0545458>] (omap2430_remove) from [<c047b2bc>] (platform_drv_remove+0x34/0x4c)

Note that if changes are needed to the autosuspend timeout, it should
be done in musb_core.c.

Reported-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Fixes: 87326e858448 ("usb: musb: Remove extra PM runtime calls from
2430 glue layer")
Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Reviewed-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

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;
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] usb: musb: Drop pointless PM runtime code for dsps glue

[Tony Lindgren]

This already gets done automatically by PM runtime and we have
a separate autosuspend timeout in musb_core.c.

Reviewed-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_dsps.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -783,28 +783,13 @@ static int dsps_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, glue);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_autosuspend_delay(&pdev->dev, 200);
-
-	ret = pm_runtime_get_sync(&pdev->dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "pm_runtime_get_sync FAILED");
-		goto err2;
-	}
-
 	ret = dsps_create_musb_pdev(glue, pdev);
 	if (ret)
-		goto err3;
-
-	pm_runtime_mark_last_busy(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
+		goto err;
 
 	return 0;
 
-err3:
-	pm_runtime_put_sync(&pdev->dev);
-err2:
-	pm_runtime_dont_use_autosuspend(&pdev->dev);
+err:
 	pm_runtime_disable(&pdev->dev);
 	return ret;
 }
@@ -815,9 +800,6 @@ static int dsps_remove(struct platform_device *pdev)
 
 	platform_device_unregister(glue->musb);
 
-	/* disable usbss clocks */
-	pm_runtime_dont_use_autosuspend(&pdev->dev);
-	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] phy: twl4030-usb: Fix for musb session bit based PM

[Tony Lindgren]

Now with musb driver implementing generic session bit based
PM, we need to have the USB PHYs behaving in a sane way for
platforms implementing PM.

Currently twl4030-usb enables PM in twl4030_phy_power_on()
and then disables it in twl4030_phy_power_off(). This will
block PM runtime for the SoC when no cable is connected.

Fix the issue by moving PM runtime autosuspend call to
happen where it gets called in twl4030_phy_power_on().

Note that this patch should not be backported to anything
before commit 467d5c980709 ("usb: musb: Implement session bit
based runtime PM for musb-core") as before that all the
glue layers implemented their own PM.

Fixes: 467d5c980709 ("usb: musb: Implement session bit based
runtime PM for musb-core")
Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/phy/phy-twl4030-usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -459,8 +459,6 @@ static int twl4030_phy_power_off(struct phy *phy)
 	struct twl4030_usb *twl = phy_get_drvdata(phy);
 
 	dev_dbg(twl->dev, "%s\n", __func__);
-	pm_runtime_mark_last_busy(twl->dev);
-	pm_runtime_put_autosuspend(twl->dev);
 
 	return 0;
 }
@@ -472,6 +470,8 @@ static int twl4030_phy_power_on(struct phy *phy)
 	dev_dbg(twl->dev, "%s\n", __func__);
 	pm_runtime_get_sync(twl->dev);
 	schedule_delayed_work(&twl->id_workaround_work, HZ);
+	pm_runtime_mark_last_busy(twl->dev);
+	pm_runtime_put_autosuspend(twl->dev);
 
 	return 0;
 }
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] phy: twl4030-usb: Fix for musb session bit based PM

[Bin Liu]

On Tue, Nov 15, 2016 at 01:37:55PM -0800, Tony Lindgren wrote:
> Now with musb driver implementing generic session bit based
> PM, we need to have the USB PHYs behaving in a sane way for
> platforms implementing PM.
> 
> Currently twl4030-usb enables PM in twl4030_phy_power_on()
> and then disables it in twl4030_phy_power_off(). This will
> block PM runtime for the SoC when no cable is connected.
> 
> Fix the issue by moving PM runtime autosuspend call to
> happen where it gets called in twl4030_phy_power_on().
> 
> Note that this patch should not be backported to anything
> before commit 467d5c980709 ("usb: musb: Implement session bit
> based runtime PM for musb-core") as before that all the
> glue layers implemented their own PM.
> 
> Fixes: 467d5c980709 ("usb: musb: Implement session bit based
> runtime PM for musb-core")
> Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

I would need Kishon's Acked-by, or he picks it to his tree.

Thanks,
-Bin.

> ---
>  drivers/phy/phy-twl4030-usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -459,8 +459,6 @@ static int twl4030_phy_power_off(struct phy *phy)
>  	struct twl4030_usb *twl = phy_get_drvdata(phy);
>  
>  	dev_dbg(twl->dev, "%s\n", __func__);
> -	pm_runtime_mark_last_busy(twl->dev);
> -	pm_runtime_put_autosuspend(twl->dev);
>  
>  	return 0;
>  }
> @@ -472,6 +470,8 @@ static int twl4030_phy_power_on(struct phy *phy)
>  	dev_dbg(twl->dev, "%s\n", __func__);
>  	pm_runtime_get_sync(twl->dev);
>  	schedule_delayed_work(&twl->id_workaround_work, HZ);
> +	pm_runtime_mark_last_busy(twl->dev);
> +	pm_runtime_put_autosuspend(twl->dev);
>  
>  	return 0;
>  }
> -- 
> 2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] phy: twl4030-usb: Fix for musb session bit based PM

[Kishon Vijay Abraham I]

On Wednesday 16 November 2016 03:29 AM, Bin Liu wrote:
> On Tue, Nov 15, 2016 at 01:37:55PM -0800, Tony Lindgren wrote:
>> Now with musb driver implementing generic session bit based
>> PM, we need to have the USB PHYs behaving in a sane way for
>> platforms implementing PM.
>>
>> Currently twl4030-usb enables PM in twl4030_phy_power_on()
>> and then disables it in twl4030_phy_power_off(). This will
>> block PM runtime for the SoC when no cable is connected.
>>
>> Fix the issue by moving PM runtime autosuspend call to
>> happen where it gets called in twl4030_phy_power_on().
>>
>> Note that this patch should not be backported to anything
>> before commit 467d5c980709 ("usb: musb: Implement session bit
>> based runtime PM for musb-core") as before that all the
>> glue layers implemented their own PM.
>>
>> Fixes: 467d5c980709 ("usb: musb: Implement session bit based
>> runtime PM for musb-core")
>> Tested-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
>> Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> 
> I would need Kishon's Acked-by, or he picks it to his tree.

Acked-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> 
> Thanks,
> -Bin.
> 
>> ---
>>  drivers/phy/phy-twl4030-usb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
>> --- a/drivers/phy/phy-twl4030-usb.c
>> +++ b/drivers/phy/phy-twl4030-usb.c
>> @@ -459,8 +459,6 @@ static int twl4030_phy_power_off(struct phy *phy)
>>  	struct twl4030_usb *twl = phy_get_drvdata(phy);
>>  
>>  	dev_dbg(twl->dev, "%s\n", __func__);
>> -	pm_runtime_mark_last_busy(twl->dev);
>> -	pm_runtime_put_autosuspend(twl->dev);
>>  
>>  	return 0;
>>  }
>> @@ -472,6 +470,8 @@ static int twl4030_phy_power_on(struct phy *phy)
>>  	dev_dbg(twl->dev, "%s\n", __func__);
>>  	pm_runtime_get_sync(twl->dev);
>>  	schedule_delayed_work(&twl->id_workaround_work, HZ);
>> +	pm_runtime_mark_last_busy(twl->dev);
>> +	pm_runtime_put_autosuspend(twl->dev);
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 0/6] musb fixes for v4.9-rc cycle

[Bin Liu]

Hi,

On Tue, Nov 15, 2016 at 01:37:49PM -0800, Tony Lindgren wrote:
> Hi all,
> 
> Here's v3 of musb fixes for issues that I've been able to track down.
> The PM changes merged for v4.9 popped up various issues reported by
> people that I had not seen earlier with my tests.
> 
> As many people depend on this driver, I'd like to have these merged
> for v4.9-rc cycle.
> 
> Regards,
> 
> Tony
> 
> 
> Changes since v2:
> 
> - Collect acks, update patch descriptions and remove one comment
>   line based on comments from Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> Changes since v1:
> 
> - Numerous changes to patch 2/4 for the sleeping function fix
>   based on comments from Johan
> 
>   - Two new patches to fix issues with devices not always
>     enumerating by removing pointless PM runtime code
> 
> Tony Lindgren (6):
>   usb: musb: Fix broken use of static variable for multiple instances
>   usb: musb: Fix sleeping function called from invalid context for hdrc
>     glue
>   usb: musb: Fix PM for hub disconnect
>   usb: musb: Add missing pm_runtime_disable and drop 2430 PM timeout
>   usb: musb: Drop pointless PM runtime code for dsps glue
>   phy: twl4030-usb: Fix for musb session bit based PM
> 
>  drivers/phy/phy-twl4030-usb.c  |   4 +-
>  drivers/usb/musb/musb_core.c   | 147 ++++++++++++++++++++++++++++++++++++-----
>  drivers/usb/musb/musb_core.h   |  13 +++-
>  drivers/usb/musb/musb_dsps.c   |  58 ++++++++--------
>  drivers/usb/musb/musb_gadget.c |  39 +++++++++--
>  drivers/usb/musb/omap2430.c    |  10 ++-
>  drivers/usb/musb/tusb6010.c    |   6 +-
>  7 files changed, 209 insertions(+), 68 deletions(-)
> 
Applied and pushed out.

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