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

[Felipe Balbi]

Hi,

On Wed, Nov 12, 2014 at 12:11:19PM +0530, Kiran Kumar Raparthy wrote:
> From: Todd Poynor <toddpoynor@xxxxxxxxxx>
> 
> 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@xxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx
> Cc: Android Kernel Team <kernel-team@xxxxxxxxxxx>
> Cc: John Stultz <john.stultz@xxxxxxxxxx>
> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> Cc: Arve Hjïnnevïg <arve@xxxxxxxxxxx>
> Cc: Benoit Goby <benoit@xxxxxxxxxxx>
> Signed-off-by: Todd Poynor <toddpoynor@xxxxxxxxxx>
> [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@xxxxxxxxxx>
> ---
>  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: signature.asc
Description: Digital signature