[RFC v6 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[RFC v6 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[Kiran Kumar Raparthy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3983 bytes --]

From: Todd Poynor <toddpoynor@google.com>

usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

Some systems require a mechanism to prevent system to enter into suspend
state when USB is connected and enumerated in peripheral mode.

This patch provides an interface to hold a wakeupsource to prevent suspend.
PHY drivers can use this interface when USB is connected and enumerated in
peripheral mode.

A timed wakeupsource is temporarily held on USB disconnect events, to allow
the rest of the system to react to the USB disconnection (dropping host
sessions, updating charger status, etc.) prior to re-allowing suspend.

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>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Benoit Goby <benoit@android.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
[kiran: Added context to commit message, squished build fixes
from Benoit Goby and Arve Hjønnevåg, changed wakelocks usage
to wakeupsource, merged Todd's refactoring logic and simplified
the structures and code and addressed community feedback]
Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
---
 drivers/usb/phy/phy.c   | 29 +++++++++++++++++++++++++++--
 include/linux/usb/phy.h |  5 +++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 2b1039e..b5c5fd3 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -329,6 +329,7 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
 	int		ret = 0;
 	unsigned long	flags;
 	struct usb_phy	*phy;
+	char wsource_name[40];
 
 	if (x->type != USB_PHY_TYPE_UNDEFINED) {
 		dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
@@ -351,6 +352,10 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
 	x->type = type;
 	list_add_tail(&x->head, &phy_list);
 
+	snprintf(wsource_name, sizeof(wsource_name), "vbus-%s",
+		dev_name(x->dev));
+	wakeup_source_init(&x->wsource, wsource_name);
+
 out:
 	spin_unlock_irqrestore(&phy_lock, flags);
 	return ret;
@@ -402,6 +407,7 @@ void usb_remove_phy(struct usb_phy *x)
 
 	spin_lock_irqsave(&phy_lock, flags);
 	if (x) {
+		wakeup_source_trash(&x->wsource);
 		list_for_each_entry(phy_bind, &phy_bind_list, list)
 			if (phy_bind->phy == x)
 				phy_bind->phy = NULL;
@@ -446,13 +452,32 @@ int usb_bind_phy(const char *dev_name, u8 index,
 EXPORT_SYMBOL_GPL(usb_bind_phy);
 
 /**
- * usb_phy_set_event - set event to phy event
+ * usb_phy_set_event - set event to phy event and
+ * hold/temporarily hold wakeupsource
  * @x: the phy returned by usb_get_phy();
  *
- * This sets event to phy event
+ * This holds per-PHY wakeupsource/timed wakeupsource
  */
 void usb_phy_set_event(struct usb_phy *x, unsigned long event)
 {
+
 	x->last_event = event;
+
+	switch (event) {
+	case USB_EVENT_ENUMERATED:
+		__pm_stay_awake(&x->wsource);
+		break;
+
+	case USB_EVENT_NONE:
+	case USB_EVENT_ID:
+	case USB_EVENT_VBUS:
+	case USB_EVENT_CHARGER:
+		__pm_wakeup_event(&x->wsource,
+				msecs_to_jiffies(TEMPORARY_HOLD_TIME));
+		break;
+
+	default:
+		break;
+	}
 }
 EXPORT_SYMBOL_GPL(usb_phy_set_event);
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 3c713ff..bc58fb8 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -12,6 +12,8 @@
 #include <linux/notifier.h>
 #include <linux/usb.h>
 
+#define TEMPORARY_HOLD_TIME    2000
+
 enum usb_phy_interface {
 	USBPHY_INTERFACE_MODE_UNKNOWN,
 	USBPHY_INTERFACE_MODE_UTMI,
@@ -89,6 +91,9 @@ struct usb_phy {
 	/* for notification of usb_phy_events */
 	struct atomic_notifier_head	notifier;
 
+	/* wakeup source */
+	struct wakeup_source    wsource;
+
 	/* to pass extra port status to the root hub */
 	u16			port_status;
 	u16			port_change;
-- 
1.8.2.1

Re: [RFC v6 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4169 bytes --]

Hi,

On Wed, Nov 12, 2014 at 12:11:19PM +0530, Kiran Kumar Raparthy wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
> 
> Some systems require a mechanism to prevent system to enter into suspend
> state when USB is connected and enumerated in peripheral mode.
> 
> This patch provides an interface to hold a wakeupsource to prevent suspend.
> PHY drivers can use this interface when USB is connected and enumerated in
> peripheral mode.
> 
> A timed wakeupsource is temporarily held on USB disconnect events, to allow
> the rest of the system to react to the USB disconnection (dropping host
> sessions, updating charger status, etc.) prior to re-allowing suspend.
> 
> 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>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Arve Hj�nnev�g <arve@android.com>
> Cc: Benoit Goby <benoit@android.com>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> [kiran: Added context to commit message, squished build fixes
> from Benoit Goby and Arve Hjønnevåg, changed wakelocks usage
> to wakeupsource, merged Todd's refactoring logic and simplified
> the structures and code and addressed community feedback]
> Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
> ---
>  drivers/usb/phy/phy.c   | 29 +++++++++++++++++++++++++++--
>  include/linux/usb/phy.h |  5 +++++
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> index 2b1039e..b5c5fd3 100644
> --- a/drivers/usb/phy/phy.c
> +++ b/drivers/usb/phy/phy.c
> @@ -329,6 +329,7 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>  	int		ret = 0;
>  	unsigned long	flags;
>  	struct usb_phy	*phy;
> +	char wsource_name[40];
>  
>  	if (x->type != USB_PHY_TYPE_UNDEFINED) {
>  		dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
> @@ -351,6 +352,10 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>  	x->type = type;
>  	list_add_tail(&x->head, &phy_list);
>  
> +	snprintf(wsource_name, sizeof(wsource_name), "vbus-%s",
> +		dev_name(x->dev));
> +	wakeup_source_init(&x->wsource, wsource_name);

do you really need the device name on the wakeup source or would
usb-vbus suffice ? Hmm, I guess we could have several of these in the
same system, so maybe there's no other way.

> @@ -446,13 +452,32 @@ int usb_bind_phy(const char *dev_name, u8 index,
>  EXPORT_SYMBOL_GPL(usb_bind_phy);
>  
>  /**
> - * usb_phy_set_event - set event to phy event
> + * usb_phy_set_event - set event to phy event and
> + * hold/temporarily hold wakeupsource
>   * @x: the phy returned by usb_get_phy();
>   *
> - * This sets event to phy event
> + * This holds per-PHY wakeupsource/timed wakeupsource
>   */
>  void usb_phy_set_event(struct usb_phy *x, unsigned long event)
>  {
> +
>  	x->last_event = event;
> +
> +	switch (event) {
> +	case USB_EVENT_ENUMERATED:
> +		__pm_stay_awake(&x->wsource);
> +		break;
> +
> +	case USB_EVENT_NONE:
> +	case USB_EVENT_ID:
> +	case USB_EVENT_VBUS:
> +	case USB_EVENT_CHARGER:
> +		__pm_wakeup_event(&x->wsource,
> +				msecs_to_jiffies(TEMPORARY_HOLD_TIME));
> +		break;
> +
> +	default:
> +		break;
> +	}

much, much better. Completely hidden from PHY drivers.

>  }
>  EXPORT_SYMBOL_GPL(usb_phy_set_event);
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index 3c713ff..bc58fb8 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -12,6 +12,8 @@
>  #include <linux/notifier.h>
>  #include <linux/usb.h>
>  
> +#define TEMPORARY_HOLD_TIME    2000

minor nit:

Let's call this:

#define USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT	2000 /* ms */

When resending, please drop that "RFC" from subject and make sure your
series is sent as a single thread, instead of three separate patches.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC v6 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[Kiran Raparthy]

On 18 November 2014 21:25, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Wed, Nov 12, 2014 at 12:11:19PM +0530, Kiran Kumar Raparthy wrote:
>> From: Todd Poynor <toddpoynor@google.com>
>>
>> usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
>>
>> Some systems require a mechanism to prevent system to enter into suspend
>> state when USB is connected and enumerated in peripheral mode.
>>
>> This patch provides an interface to hold a wakeupsource to prevent suspend.
>> PHY drivers can use this interface when USB is connected and enumerated in
>> peripheral mode.
>>
>> A timed wakeupsource is temporarily held on USB disconnect events, to allow
>> the rest of the system to react to the USB disconnection (dropping host
>> sessions, updating charger status, etc.) prior to re-allowing suspend.
>>
>> 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>
>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>> Cc: Arve Hj�nnev�g <arve@android.com>
>> Cc: Benoit Goby <benoit@android.com>
>> Signed-off-by: Todd Poynor <toddpoynor@google.com>
>> [kiran: Added context to commit message, squished build fixes
>> from Benoit Goby and Arve Hjønnevåg, changed wakelocks usage
>> to wakeupsource, merged Todd's refactoring logic and simplified
>> the structures and code and addressed community feedback]
>> Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
>> ---
>>  drivers/usb/phy/phy.c   | 29 +++++++++++++++++++++++++++--
>>  include/linux/usb/phy.h |  5 +++++
>>  2 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
>> index 2b1039e..b5c5fd3 100644
>> --- a/drivers/usb/phy/phy.c
>> +++ b/drivers/usb/phy/phy.c
>> @@ -329,6 +329,7 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>>       int             ret = 0;
>>       unsigned long   flags;
>>       struct usb_phy  *phy;
>> +     char wsource_name[40];
>>
>>       if (x->type != USB_PHY_TYPE_UNDEFINED) {
>>               dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
>> @@ -351,6 +352,10 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>>       x->type = type;
>>       list_add_tail(&x->head, &phy_list);
>>
>> +     snprintf(wsource_name, sizeof(wsource_name), "vbus-%s",
>> +             dev_name(x->dev));
>> +     wakeup_source_init(&x->wsource, wsource_name);
>
> do you really need the device name on the wakeup source or would
> usb-vbus suffice ? Hmm, I guess we could have several of these in the
> same system, so maybe there's no other way.
yeah true.
>
>> @@ -446,13 +452,32 @@ int usb_bind_phy(const char *dev_name, u8 index,
>>  EXPORT_SYMBOL_GPL(usb_bind_phy);
>>
>>  /**
>> - * usb_phy_set_event - set event to phy event
>> + * usb_phy_set_event - set event to phy event and
>> + * hold/temporarily hold wakeupsource
>>   * @x: the phy returned by usb_get_phy();
>>   *
>> - * This sets event to phy event
>> + * This holds per-PHY wakeupsource/timed wakeupsource
>>   */
>>  void usb_phy_set_event(struct usb_phy *x, unsigned long event)
>>  {
>> +
>>       x->last_event = event;
>> +
>> +     switch (event) {
>> +     case USB_EVENT_ENUMERATED:
>> +             __pm_stay_awake(&x->wsource);
>> +             break;
>> +
>> +     case USB_EVENT_NONE:
>> +     case USB_EVENT_ID:
>> +     case USB_EVENT_VBUS:
>> +     case USB_EVENT_CHARGER:
>> +             __pm_wakeup_event(&x->wsource,
>> +                             msecs_to_jiffies(TEMPORARY_HOLD_TIME));
>> +             break;
>> +
>> +     default:
>> +             break;
>> +     }
>
> much, much better. Completely hidden from PHY drivers.
>
>>  }
>>  EXPORT_SYMBOL_GPL(usb_phy_set_event);
>> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
>> index 3c713ff..bc58fb8 100644
>> --- a/include/linux/usb/phy.h
>> +++ b/include/linux/usb/phy.h
>> @@ -12,6 +12,8 @@
>>  #include <linux/notifier.h>
>>  #include <linux/usb.h>
>>
>> +#define TEMPORARY_HOLD_TIME    2000
>
> minor nit:
>
> Let's call this:
>
> #define USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT      2000 /* ms */
>
> When resending, please drop that "RFC" from subject and make sure your
> series is sent as a single thread, instead of three separate patches.
Sure Felipe,Thanks for your time.
>
> cheers
>
> --
> balbi