diff mbox

[RFC,v2,2/2] usb: phy: Temporarily hold wakeupsource on charger connect and disconnect events

Message ID 1409658558-12318-1-git-send-email-kiran.kumar@linaro.org
State New
Headers show

Commit Message

Kiran Kumar Raparthy Sept. 2, 2014, 11:49 a.m. UTC 
From: Todd Poynor <toddpoynor@google.com>

usb: phy: Temporarily hold wakeupsource on charger connect and disconnect
events

Allow other parts of the system to react to the charger connect/disconnect
event without allowing the system to suspend before the other parts can process
the event. This wakeup_source times out after 2 seconds; if nobody else holds a
wakeup_source by that time then the device can sleep.

Cc: Felipe Balbi <balbi@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: Android Kernel Team <kernel-team@android.com>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
[kiran: Added context to commit message
	wakeupsource handling implemented per PHY in phy core]
Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
---
 drivers/usb/phy/otg-wakeupsource.c |  2 +-
 drivers/usb/phy/phy.c              | 10 ++++++++++
 include/linux/usb/phy.h            |  3 +++
 3 files changed, 14 insertions(+), 1 deletion(-)

Comments

Felipe Balbi Sept. 2, 2014, 2:54 p.m. UTC | #1 
Hi,

On Tue, Sep 02, 2014 at 05:19:18PM +0530, Kiran Kumar Raparthy wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> usb: phy: Temporarily hold wakeupsource on charger connect and disconnect
> events
> 
> Allow other parts of the system to react to the charger connect/disconnect
> event without allowing the system to suspend before the other parts can process
> the event. This wakeup_source times out after 2 seconds; if nobody else holds a
> wakeup_source by that time then the device can sleep.
> 
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> Cc: Android Kernel Team <kernel-team@android.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> [kiran: Added context to commit message
> 	wakeupsource handling implemented per PHY in phy core]
> Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
> ---
>  drivers/usb/phy/otg-wakeupsource.c |  2 +-
>  drivers/usb/phy/phy.c              | 10 ++++++++++
>  include/linux/usb/phy.h            |  3 +++
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c
> index fca2010..70fa05e 100644
> --- a/drivers/usb/phy/otg-wakeupsource.c
> +++ b/drivers/usb/phy/otg-wakeupsource.c
> @@ -48,7 +48,7 @@ static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event)
>  	case USB_EVENT_NONE:
>  	case USB_EVENT_ID:
>  	case USB_EVENT_CHARGER:
> -		usb_drop_wsource(otgws_xceiv);
> +		usb_temporary_hold_wsource(otgws_xceiv);

looks like this won't work. You're holding the lock even on
USB_EVEN_NONE. Why ?
Todd Poynor Sept. 5, 2014, 7:40 a.m. UTC | #2 
On Tue, Sep 2, 2014 at 7:54 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Tue, Sep 02, 2014 at 05:19:18PM +0530, Kiran Kumar Raparthy wrote:
...
>> diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c
>> index fca2010..70fa05e 100644
>> --- a/drivers/usb/phy/otg-wakeupsource.c
>> +++ b/drivers/usb/phy/otg-wakeupsource.c
>> @@ -48,7 +48,7 @@ static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event)
>>       case USB_EVENT_NONE:
>>       case USB_EVENT_ID:
>>       case USB_EVENT_CHARGER:
>> -             usb_drop_wsource(otgws_xceiv);
>> +             usb_temporary_hold_wsource(otgws_xceiv);
>
> looks like this won't work. You're holding the lock even on
> USB_EVEN_NONE. Why ?

It temporarily holds a timed wakeup source on USB disconnect events,
to allow the rest of the system time to react to the USB disconnection
(dropping host sessions, updating charger status, etc.) prior to
re-allowing suspend.


Todd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Felipe Balbi Sept. 5, 2014, 2:19 p.m. UTC | #3 
On Fri, Sep 05, 2014 at 12:40:22AM -0700, Todd Poynor wrote:
> On Tue, Sep 2, 2014 at 7:54 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Tue, Sep 02, 2014 at 05:19:18PM +0530, Kiran Kumar Raparthy wrote:
> ...
> >> diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c
> >> index fca2010..70fa05e 100644
> >> --- a/drivers/usb/phy/otg-wakeupsource.c
> >> +++ b/drivers/usb/phy/otg-wakeupsource.c
> >> @@ -48,7 +48,7 @@ static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event)
> >>       case USB_EVENT_NONE:
> >>       case USB_EVENT_ID:
> >>       case USB_EVENT_CHARGER:
> >> -             usb_drop_wsource(otgws_xceiv);
> >> +             usb_temporary_hold_wsource(otgws_xceiv);
> >
> > looks like this won't work. You're holding the lock even on
> > USB_EVEN_NONE. Why ?
> 
> It temporarily holds a timed wakeup source on USB disconnect events,
> to allow the rest of the system time to react to the USB disconnection
> (dropping host sessions, updating charger status, etc.) prior to
> re-allowing suspend.

alright, please add a note like this to commit log.
diff mbox

Patch

diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c
index fca2010..70fa05e 100644
--- a/drivers/usb/phy/otg-wakeupsource.c
+++ b/drivers/usb/phy/otg-wakeupsource.c
@@ -48,7 +48,7 @@  static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event)
 	case USB_EVENT_NONE:
 	case USB_EVENT_ID:
 	case USB_EVENT_CHARGER:
-		usb_drop_wsource(otgws_xceiv);
+		usb_temporary_hold_wsource(otgws_xceiv);
 		break;
 
 	default:
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 659b1e8..c488716 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -483,3 +483,13 @@  void usb_drop_wsource(struct usb_phy *x)
 	__pm_relax(&x->wsource);
 }
 EXPORT_SYMBOL_GPL(usb_drop_wsource);
+
+/**
+ * usb_temporary_hold_wsource - temporarily hold wakeupsource
+ * @usb_phy: the phy returned by usb_get_phy()
+ */
+void usb_temporary_hold_wsource(struct usb_phy *x)
+{
+	__pm_wakeup_event(&x->wsource, msecs_to_jiffies(TEMPORARY_HOLD_TIME));
+}
+EXPORT_SYMBOL_GPL(usb_temporary_hold_wsource);
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 6c680e6..5f9b70e 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -13,6 +13,8 @@ 
 #include <linux/usb.h>
 #include <linux/pm_wakeup.h>
 
+#define TEMPORARY_HOLD_TIME    2000
+
 enum usb_phy_interface {
 	USBPHY_INTERFACE_MODE_UNKNOWN,
 	USBPHY_INTERFACE_MODE_UTMI,
@@ -218,6 +220,7 @@  void usb_wsource_init(struct usb_phy *x, char *phy_dev_name);
 void usb_wsource_trash(struct usb_phy *x);
 void usb_grab_wsource(struct usb_phy *x);
 void usb_drop_wsource(struct usb_phy *x);
+void usb_temporary_hold_wsource(struct usb_phy *x);
 #else
 static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
 {