[PATCH v3 1/3] usb: phy: introduce usb_phy_set_event interface

[PATCH v3 1/3] usb: phy: introduce usb_phy_set_event interface

[Kiran Raparthy]

PHY drivers require a generic interface to handle per-PHY events.

usb_phy_set_event interface sets event to phy event.
PHY drivers call this interface for each phy event.

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>
[Original patch in Android from Todd]
Cc: Todd Poynor <toddpoynor@google.com>
Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/phy/phy.c   | 12 ++++++++++++
 include/linux/usb/phy.h |  5 +++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index e6bf801..b4066a0 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -446,3 +446,15 @@ int usb_bind_phy(const char *dev_name, u8 index,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usb_bind_phy);
+
+/**
+ * usb_phy_set_event - set event to phy event
+ * @x: the phy returned by usb_get_phy();
+ *
+ * This sets event to phy event
+ */
+void usb_phy_set_event(struct usb_phy *x, unsigned long event)
+{
+	x->last_event = event;
+}
+EXPORT_SYMBOL_GPL(usb_phy_set_event);
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index ac7d791..f499c23 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -209,6 +209,7 @@ extern void usb_put_phy(struct usb_phy *);
 extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
 extern int usb_bind_phy(const char *dev_name, u8 index,
 				const char *phy_dev_name);
+extern void usb_phy_set_event(struct usb_phy *x, unsigned long event);
 #else
 static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
 {
@@ -250,6 +251,10 @@ static inline int usb_bind_phy(const char *dev_name, u8 index,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline void usb_phy_set_event(struct usb_phy *x, unsigned long event)
+{
+}
 #endif
 
 static inline int
-- 
1.9.1

[PATCH v3 2/3] usb: phy: Handle per-PHY event for connect and disconnect events

[Kiran Raparthy]

usb: phy: Handle per-PHY event for connnect and disconnect events

When usb is connected and enumerated in device mode or when usb is
disconnected,call usb_phy_set_event from phy drivers to handle per-PHY event.

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>
[Original patch in Android from Todd]
Cc: Todd Poynor <toddpoynor@google.com>
Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
---
 drivers/usb/phy/phy-ab8500-usb.c    | 15 +++++++++++++++
 drivers/usb/phy/phy-gpio-vbus-usb.c |  2 ++
 drivers/usb/phy/phy-mv-usb.c        |  2 ++
 drivers/usb/phy/phy-tahvo.c         |  2 ++
 4 files changed, 21 insertions(+)

diff --git a/drivers/usb/phy/phy-ab8500-usb.c b/drivers/usb/phy/phy-ab8500-usb.c
index 30c96f0..8cd7d19 100644
--- a/drivers/usb/phy/phy-ab8500-usb.c
+++ b/drivers/usb/phy/phy-ab8500-usb.c
@@ -445,6 +445,7 @@ static int ab9540_usb_link_status_update(struct ab8500_usb *ab,
 			event = UX500_MUSB_NONE;
 		/* Fallback to default B_IDLE as nothing is connected. */
 		ab->phy.otg->state = OTG_STATE_B_IDLE;
+		usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 		break;
 
 	case USB_LINK_ACA_RID_C_NM_9540:
@@ -459,12 +460,14 @@ static int ab9540_usb_link_status_update(struct ab8500_usb *ab,
 			ab8500_usb_peri_phy_en(ab);
 			atomic_notifier_call_chain(&ab->phy.notifier,
 					UX500_MUSB_PREPARE, &ab->vbus_draw);
+			usb_phy_set_event(&ab->phy, USB_EVENT_ENUMERATED);
 		}
 		if (ab->mode == USB_IDLE) {
 			ab->mode = USB_PERIPHERAL;
 			ab8500_usb_peri_phy_en(ab);
 			atomic_notifier_call_chain(&ab->phy.notifier,
 					UX500_MUSB_PREPARE, &ab->vbus_draw);
+			usb_phy_set_event(&ab->phy, USB_EVENT_ENUMERATED);
 		}
 		if (event != UX500_MUSB_RIDC)
 			event = UX500_MUSB_VBUS;
@@ -500,6 +503,7 @@ static int ab9540_usb_link_status_update(struct ab8500_usb *ab,
 		event = UX500_MUSB_CHARGER;
 		atomic_notifier_call_chain(&ab->phy.notifier,
 				event, &ab->vbus_draw);
+		usb_phy_set_event(&ab->phy, USB_EVENT_CHARGER);
 		break;
 
 	case USB_LINK_PHYEN_NO_VBUS_NO_IDGND_9540:
@@ -524,6 +528,7 @@ static int ab9540_usb_link_status_update(struct ab8500_usb *ab,
 				ab->mode = USB_IDLE;
 				ab->phy.otg->default_a = false;
 				ab->vbus_draw = 0;
+				usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 			}
 		}
 		break;
@@ -583,6 +588,7 @@ static int ab8540_usb_link_status_update(struct ab8500_usb *ab,
 		 * is connected
 		 */
 		ab->phy.otg->state = OTG_STATE_B_IDLE;
+		usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 		break;
 
 	case USB_LINK_ACA_RID_C_NM_8540:
@@ -596,6 +602,7 @@ static int ab8540_usb_link_status_update(struct ab8500_usb *ab,
 			ab8500_usb_peri_phy_en(ab);
 			atomic_notifier_call_chain(&ab->phy.notifier,
 					UX500_MUSB_PREPARE, &ab->vbus_draw);
+			usb_phy_set_event(&ab->phy, USB_EVENT_ENUMERATED);
 		}
 		if (event != UX500_MUSB_RIDC)
 			event = UX500_MUSB_VBUS;
@@ -624,6 +631,7 @@ static int ab8540_usb_link_status_update(struct ab8500_usb *ab,
 		event = UX500_MUSB_CHARGER;
 		atomic_notifier_call_chain(&ab->phy.notifier,
 				event, &ab->vbus_draw);
+		usb_phy_set_event(&ab->phy, USB_EVENT_CHARGER);
 		break;
 
 	case USB_LINK_PHYEN_NO_VBUS_NO_IDGND_8540:
@@ -646,6 +654,7 @@ static int ab8540_usb_link_status_update(struct ab8500_usb *ab,
 			ab->mode = USB_IDLE;
 			ab->phy.otg->default_a = false;
 			ab->vbus_draw = 0;
+		usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 		}
 		break;
 
@@ -692,6 +701,7 @@ static int ab8505_usb_link_status_update(struct ab8500_usb *ab,
 		 * is connected
 		 */
 		ab->phy.otg->state = OTG_STATE_B_IDLE;
+		usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 		break;
 
 	case USB_LINK_ACA_RID_C_NM_8505:
@@ -705,6 +715,7 @@ static int ab8505_usb_link_status_update(struct ab8500_usb *ab,
 			ab8500_usb_peri_phy_en(ab);
 			atomic_notifier_call_chain(&ab->phy.notifier,
 					UX500_MUSB_PREPARE, &ab->vbus_draw);
+			usb_phy_set_event(&ab->phy, USB_EVENT_ENUMERATED);
 		}
 		if (event != UX500_MUSB_RIDC)
 			event = UX500_MUSB_VBUS;
@@ -732,6 +743,7 @@ static int ab8505_usb_link_status_update(struct ab8500_usb *ab,
 		event = UX500_MUSB_CHARGER;
 		atomic_notifier_call_chain(&ab->phy.notifier,
 				event, &ab->vbus_draw);
+		usb_phy_set_event(&ab->phy, USB_EVENT_CHARGER);
 		break;
 
 	default:
@@ -775,6 +787,7 @@ static int ab8500_usb_link_status_update(struct ab8500_usb *ab,
 			event = UX500_MUSB_NONE;
 		/* Fallback to default B_IDLE as nothing is connected */
 		ab->phy.otg->state = OTG_STATE_B_IDLE;
+		usb_phy_set_event(&ab->phy, USB_EVENT_NONE);
 		break;
 
 	case USB_LINK_ACA_RID_C_NM_8500:
@@ -792,6 +805,7 @@ static int ab8500_usb_link_status_update(struct ab8500_usb *ab,
 			ab8500_usb_peri_phy_en(ab);
 			atomic_notifier_call_chain(&ab->phy.notifier,
 					UX500_MUSB_PREPARE, &ab->vbus_draw);
+			usb_phy_set_event(&ab->phy, USB_EVENT_ENUMERATED);
 		}
 		if (event != UX500_MUSB_RIDC)
 			event = UX500_MUSB_VBUS;
@@ -818,6 +832,7 @@ static int ab8500_usb_link_status_update(struct ab8500_usb *ab,
 		event = UX500_MUSB_CHARGER;
 		atomic_notifier_call_chain(&ab->phy.notifier,
 				event, &ab->vbus_draw);
+		usb_phy_set_event(&ab->phy, USB_EVENT_CHARGER);
 		break;
 
 	case USB_LINK_RESERVED_8500:
diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index 9fcf19b..5451172 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -134,6 +134,7 @@ static void gpio_vbus_work(struct work_struct *work)
 
 		atomic_notifier_call_chain(&gpio_vbus->phy.notifier,
 					   status, gpio_vbus->phy.otg->gadget);
+		usb_phy_set_event(&gpio_vbus->phy, USB_EVENT_ENUMERATED);
 	} else {
 		/* optionally disable D+ pullup */
 		if (gpio_is_valid(gpio))
@@ -148,6 +149,7 @@ static void gpio_vbus_work(struct work_struct *work)
 
 		atomic_notifier_call_chain(&gpio_vbus->phy.notifier,
 					   status, gpio_vbus->phy.otg->gadget);
+		usb_phy_set_event(&gpio_vbus->phy, USB_EVENT_NONE);
 	}
 }
 
diff --git a/drivers/usb/phy/phy-mv-usb.c b/drivers/usb/phy/phy-mv-usb.c
index 81d934f..b234d46 100644
--- a/drivers/usb/phy/phy-mv-usb.c
+++ b/drivers/usb/phy/phy-mv-usb.c
@@ -441,10 +441,12 @@ run:
 				mv_otg_start_periphrals(mvotg, 0);
 			mv_otg_reset(mvotg);
 			mv_otg_disable(mvotg);
+			usb_phy_set_event(&mvotg->phy, USB_EVENT_NONE);
 			break;
 		case OTG_STATE_B_PERIPHERAL:
 			mv_otg_enable(mvotg);
 			mv_otg_start_periphrals(mvotg, 1);
+			usb_phy_set_event(&mvotg->phy, USB_EVENT_ENUMERATED);
 			break;
 		case OTG_STATE_A_IDLE:
 			otg->default_a = 1;
diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
index 9cabc1a..d391c79 100644
--- a/drivers/usb/phy/phy-tahvo.c
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -87,6 +87,7 @@ static void check_vbus_state(struct tahvo_usb *tu)
 			if (tu->phy.otg->gadget)
 				usb_gadget_vbus_connect(tu->phy.otg->gadget);
 			tu->phy.otg->state = OTG_STATE_B_PERIPHERAL;
+			usb_phy_set_event(&tu->phy, USB_EVENT_ENUMERATED);
 			break;
 		case OTG_STATE_A_IDLE:
 			/*
@@ -105,6 +106,7 @@ static void check_vbus_state(struct tahvo_usb *tu)
 			if (tu->phy.otg->gadget)
 				usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
 			tu->phy.otg->state = OTG_STATE_B_IDLE;
+			usb_phy_set_event(&tu->phy, USB_EVENT_NONE);
 			break;
 		case OTG_STATE_A_HOST:
 			tu->phy.otg->state = OTG_STATE_A_IDLE;
-- 
1.9.1

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

[Kiran Raparthy]

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>
[Original patch in Android from Todd]
Cc: Todd Poynor <toddpoynor@google.com>
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 b4066a0..8c87bc3 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -331,6 +331,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);
@@ -353,6 +354,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;
@@ -404,6 +409,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;
@@ -448,13 +454,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,
+				  USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT);
+		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 f499c23..63a56bf 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 USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT	msecs_to_jiffies(2000)
+
 enum usb_phy_interface {
 	USBPHY_INTERFACE_MODE_UNKNOWN,
 	USBPHY_INTERFACE_MODE_UTMI,
@@ -88,6 +90,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.9.1

RE: [PATCH v3 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[Peter Chen]

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

 
> 
> 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.
> 

Hi Kiran & Felipe,

Just two questions for this series

- Will it be the default behavior for all peripheral drivers?
- If the peripheral driver's PHY driver does not vbus event, how to support it?
For example, chipidea udc driver has its vbus interface at its controller register.

Peter

> 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>
> [Original patch in Android from Todd]
> Cc: Todd Poynor <toddpoynor@google.com>
> 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
> b4066a0..8c87bc3 100644
> --- a/drivers/usb/phy/phy.c
> +++ b/drivers/usb/phy/phy.c
> @@ -331,6 +331,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);
> @@ -353,6 +354,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;
> @@ -404,6 +409,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;
> @@ -448,13 +454,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,
> +				  USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT);
> +		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
> f499c23..63a56bf 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 USB_PHY_DEFAULT_WAKEUP_SRC_TIMEOUT
> 	msecs_to_jiffies(2000)
> +
>  enum usb_phy_interface {
>  	USBPHY_INTERFACE_MODE_UNKNOWN,
>  	USBPHY_INTERFACE_MODE_UTMI,
> @@ -88,6 +90,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.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Felipe Balbi]

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

On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>  
> > 
> > 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.
> > 
> 
> Hi Kiran & Felipe,
> 
> Just two questions for this series
> 
> - Will it be the default behavior for all peripheral drivers?
> - If the peripheral driver's PHY driver does not vbus event, how to
> support it?
> For example, chipidea udc driver has its vbus interface at its
> controller register.

hmm, good point. Since it's so late, I'll just go ahead and drop
$subject from v3.20. Let's delay only $subject to v3.20 merge window so
we have some more time to discuss these details.

cheers

-- 
balbi

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

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

[Kiran Raparthy]

Hi Felipe,

On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>>
>> >
>> > 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.
>> >
>>
>> Hi Kiran & Felipe,
>>
>> Just two questions for this series
>>
>> - Will it be the default behavior for all peripheral drivers?
>> - If the peripheral driver's PHY driver does not vbus event, how to
>> support it?
>> For example, chipidea udc driver has its vbus interface at its
>> controller register.
>
> hmm, good point. Since it's so late, I'll just go ahead and drop
> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
> we have some more time to discuss these details.

I am just curious to know/understand why this feature needs to be
default behavior for all peripheral drivers?
If this needs to be default behavior, could you please suggest any
alternate plan/design for this feature so that i can incorporate them
in new patch.
Regards,
Kiran

>
> cheers
>
> --
> balbi

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

[Kiran Raparthy]

Hi Felipe,

On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> Hi Felipe,
>
> On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
>> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>>>
>>> >
>>> > 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.
>>> >
>>>
>>> Hi Kiran & Felipe,
>>>
>>> Just two questions for this series
>>>
>>> - Will it be the default behavior for all peripheral drivers?
>>> - If the peripheral driver's PHY driver does not vbus event, how to
>>> support it?
>>> For example, chipidea udc driver has its vbus interface at its
>>> controller register.
>>
>> hmm, good point. Since it's so late, I'll just go ahead and drop
>> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
>> we have some more time to discuss these details.
>
> I am just curious to know/understand why this feature needs to be
> default behavior for all peripheral drivers?
> If this needs to be default behavior, could you please suggest any
> alternate plan/design for this feature so that i can incorporate them
> in new patch.
> Regards,
> Kiran

Just a gentle reminder to consider the patch.

>
>>
>> cheers
>>
>> --
>> balbi

Regards,
Kiran

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

[Felipe Balbi]

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

On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
> Hi Felipe,
> 
> On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> > Hi Felipe,
> >
> > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
> >>>
> >>> >
> >>> > 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.
> >>> >
> >>>
> >>> Hi Kiran & Felipe,
> >>>
> >>> Just two questions for this series
> >>>
> >>> - Will it be the default behavior for all peripheral drivers?
> >>> - If the peripheral driver's PHY driver does not vbus event, how to
> >>> support it?
> >>> For example, chipidea udc driver has its vbus interface at its
> >>> controller register.
> >>
> >> hmm, good point. Since it's so late, I'll just go ahead and drop
> >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
> >> we have some more time to discuss these details.
> >
> > I am just curious to know/understand why this feature needs to be
> > default behavior for all peripheral drivers?
> > If this needs to be default behavior, could you please suggest any
> > alternate plan/design for this feature so that i can incorporate them
> > in new patch.
> > Regards,
> > Kiran
> 
> Just a gentle reminder to consider the patch.

sorry, for the delay... man this release round has been hectic. We still
haven't sorted out what should be the default behavior here, considering
some PHYs will not be able to report VBUS levels, I'm not sure what we
should do here.

If anybody has suggestions, I'm all ears

-- 
balbi

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

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

[Peter Chen]

On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
> > Hi Felipe,
> > 
> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> > > Hi Felipe,
> > >
> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
> > >>>
> > >>> >
> > >>> > 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.
> > >>> >
> > >>>
> > >>> Hi Kiran & Felipe,
> > >>>
> > >>> Just two questions for this series
> > >>>
> > >>> - Will it be the default behavior for all peripheral drivers?
> > >>> - If the peripheral driver's PHY driver does not vbus event, how to
> > >>> support it?
> > >>> For example, chipidea udc driver has its vbus interface at its
> > >>> controller register.
> > >>
> > >> hmm, good point. Since it's so late, I'll just go ahead and drop
> > >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
> > >> we have some more time to discuss these details.
> > >
> > > I am just curious to know/understand why this feature needs to be
> > > default behavior for all peripheral drivers?
> > > If this needs to be default behavior, could you please suggest any
> > > alternate plan/design for this feature so that i can incorporate them
> > > in new patch.
> > > Regards,
> > > Kiran
> > 
> > Just a gentle reminder to consider the patch.
> 
> sorry, for the delay... man this release round has been hectic. We still
> haven't sorted out what should be the default behavior here, considering
> some PHYs will not be able to report VBUS levels, I'm not sure what we
> should do here.
> 

My opinion is: if we need this feature, we should depend on gadget's
state, eg 'configured', not vbus level. The system suspend should be
allowed if the gadget state is 'suspended' which vbus is still there.

-- 

Best Regards,
Peter Chen

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

[Sumit Semwal]

H Peter, Felipe,

On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
> On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
>> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
>> > Hi Felipe,
>> >
>> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
>> > > Hi Felipe,
>> > >
>> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
>> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>> > >>>
>> > >>> >
>> > >>> > 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.
>> > >>> >
>> > >>>
>> > >>> Hi Kiran & Felipe,
>> > >>>
>> > >>> Just two questions for this series
>> > >>>
>> > >>> - Will it be the default behavior for all peripheral drivers?
>> > >>> - If the peripheral driver's PHY driver does not vbus event, how to
>> > >>> support it?
>> > >>> For example, chipidea udc driver has its vbus interface at its
>> > >>> controller register.
>> > >>
>> > >> hmm, good point. Since it's so late, I'll just go ahead and drop
>> > >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
>> > >> we have some more time to discuss these details.
>> > >
>> > > I am just curious to know/understand why this feature needs to be
>> > > default behavior for all peripheral drivers?
>> > > If this needs to be default behavior, could you please suggest any
>> > > alternate plan/design for this feature so that i can incorporate them
>> > > in new patch.
>> > > Regards,
>> > > Kiran
>> >
>> > Just a gentle reminder to consider the patch.
>>
>> sorry, for the delay... man this release round has been hectic. We still
>> haven't sorted out what should be the default behavior here, considering
>> some PHYs will not be able to report VBUS levels, I'm not sure what we
>> should do here.
>>
>
> My opinion is: if we need this feature, we should depend on gadget's
> state, eg 'configured', not vbus level. The system suspend should be
> allowed if the gadget state is 'suspended' which vbus is still there.
>

Kiran is unable to continue on this patch anymore, so despite my very
limited usb experience, I would like to take this over and with your
help, take it to closure.

>From what I understand, it might be possible to do this either at the
usb controller driver (where it detects if it has enumerated in host
mode or device mode) or in the extcon driver (where it detects VBUS
etc..) -  which would be a cleaner / more logical way to do it?

Thanks for all your help!

> --
>
> Best Regards,
> Peter Chen
> --
> 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/



-- 
Thanks and regards,
Sumit Semwal

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

[Peter Chen]

On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
> H Peter, Felipe,
> 
> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
> >> > Hi Felipe,
> >> >
> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> >> > > Hi Felipe,
> >> > >
> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
> >> > >>>
> >> > >>> >
> >> > >>> > 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.
> >> > >>> >
> >> > >>>
> >> > >>> Hi Kiran & Felipe,
> >> > >>>
> >> > >>> Just two questions for this series
> >> > >>>
> >> > >>> - Will it be the default behavior for all peripheral drivers?
> >> > >>> - If the peripheral driver's PHY driver does not vbus event, how to
> >> > >>> support it?
> >> > >>> For example, chipidea udc driver has its vbus interface at its
> >> > >>> controller register.
> >> > >>
> >> > >> hmm, good point. Since it's so late, I'll just go ahead and drop
> >> > >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so
> >> > >> we have some more time to discuss these details.
> >> > >
> >> > > I am just curious to know/understand why this feature needs to be
> >> > > default behavior for all peripheral drivers?
> >> > > If this needs to be default behavior, could you please suggest any
> >> > > alternate plan/design for this feature so that i can incorporate them
> >> > > in new patch.
> >> > > Regards,
> >> > > Kiran
> >> >
> >> > Just a gentle reminder to consider the patch.
> >>
> >> sorry, for the delay... man this release round has been hectic. We still
> >> haven't sorted out what should be the default behavior here, considering
> >> some PHYs will not be able to report VBUS levels, I'm not sure what we
> >> should do here.
> >>
> >
> > My opinion is: if we need this feature, we should depend on gadget's
> > state, eg 'configured', not vbus level. The system suspend should be
> > allowed if the gadget state is 'suspended' which vbus is still there.
> >
> 
> Kiran is unable to continue on this patch anymore, so despite my very
> limited usb experience, I would like to take this over and with your
> help, take it to closure.
> 
> From what I understand, it might be possible to do this either at the
> usb controller driver (where it detects if it has enumerated in host
> mode or device mode) or in the extcon driver (where it detects VBUS
> etc..) -  which would be a cleaner / more logical way to do it?
> 
> Thanks for all your help!

I added my suggestions below:

My opinion is: if we need this feature, we should depend on gadget's
state, eg 'configured', not vbus level. The system suspend should be
allowed if the gadget state is 'suspended' even vbus is still there.

And when the gadget goes to suspend or reset or disconnect state, we can
go to disable this wakeup source.

At drivers/usb/gadget/composite.c, we can know the gadget's state.

-- 

Best Regards,
Peter Chen

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

[Sumit Semwal]

Hi Peter,

On 8 June 2015 at 14:02, Peter Chen <peter.chen@freescale.com> wrote:
> On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
>> H Peter, Felipe,
>>
>> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
>> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
>> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
>> >> > Hi Felipe,
>> >> >
>> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
>> >> > > Hi Felipe,
>> >> > >
>> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
>> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>> >> > >>>
>> >> > >>> >
>> >> > >>> > 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.
>> >> > >>> >
>> >> > >>>
<snip>
>>
>> Kiran is unable to continue on this patch anymore, so despite my very
>> limited usb experience, I would like to take this over and with your
>> help, take it to closure.
>>
>> From what I understand, it might be possible to do this either at the
>> usb controller driver (where it detects if it has enumerated in host
>> mode or device mode) or in the extcon driver (where it detects VBUS
>> etc..) -  which would be a cleaner / more logical way to do it?
>>
>> Thanks for all your help!
>
> I added my suggestions below:
>
> My opinion is: if we need this feature, we should depend on gadget's
> state, eg 'configured', not vbus level. The system suspend should be
> allowed if the gadget state is 'suspended' even vbus is still there.
>
> And when the gadget goes to suspend or reset or disconnect state, we can
> go to disable this wakeup source.
>
> At drivers/usb/gadget/composite.c, we can know the gadget's state.
>
Thanks very much for your suggestion, and apologies in the delay in response.

I tried to study your suggestion and the code a little bit more, and
while I liked your idea quite a bit, I also realised that for the case
of letting the USB charger use cases, in gadget framework, there
doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
would likely not be a gadget.

In this case, how'd we be able to enable / disable wakeup_source for
charger events?

If indeed we can't handle charger events in the way suggested by you,
may I propose the following:

- implement this in USB-phy as proposed in Kiran's original patch,
- keep it disabled by default, and add a module param to enable/disable it
- for no vbus case, use another event (USB_EVENT_ENUMERATED /
USB_EVENT_NONE? ) to identify connect / disconnect?
     (OR, maybe don't do anything for such cases, since most likely
the chipidea kind of implementations would handle it themselves?)

Of course these are based on my limited reading of the USB code, so
I'd be grateful if you'd let me know of any obvious gaps in my
understanding!

> --
>
> Best Regards,
> Peter Chen


Best regards,
Sumit.

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

[Peter Chen]

On Mon, Jul 13, 2015 at 01:26:23PM +0530, Sumit Semwal wrote:
> Hi Peter,
> 
> On 8 June 2015 at 14:02, Peter Chen <peter.chen@freescale.com> wrote:
> > On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
> >> H Peter, Felipe,
> >>
> >> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
> >> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
> >> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
> >> >> > Hi Felipe,
> >> >> >
> >> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> >> >> > > Hi Felipe,
> >> >> > >
> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> >> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
> >> >> > >>>
> >> >> > >>> >
> >> >> > >>> > 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.
> >> >> > >>> >
> >> >> > >>>
> <snip>
> >>
> >> Kiran is unable to continue on this patch anymore, so despite my very
> >> limited usb experience, I would like to take this over and with your
> >> help, take it to closure.
> >>
> >> From what I understand, it might be possible to do this either at the
> >> usb controller driver (where it detects if it has enumerated in host
> >> mode or device mode) or in the extcon driver (where it detects VBUS
> >> etc..) -  which would be a cleaner / more logical way to do it?
> >>
> >> Thanks for all your help!
> >
> > I added my suggestions below:
> >
> > My opinion is: if we need this feature, we should depend on gadget's
> > state, eg 'configured', not vbus level. The system suspend should be
> > allowed if the gadget state is 'suspended' even vbus is still there.
> >
> > And when the gadget goes to suspend or reset or disconnect state, we can
> > go to disable this wakeup source.
> >
> > At drivers/usb/gadget/composite.c, we can know the gadget's state.
> >
> Thanks very much for your suggestion, and apologies in the delay in response.
> 
> I tried to study your suggestion and the code a little bit more, and
> while I liked your idea quite a bit, I also realised that for the case
> of letting the USB charger use cases, in gadget framework, there
> doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
> would likely not be a gadget.
> 
> In this case, how'd we be able to enable / disable wakeup_source for
> charger events?

We may not prevent the system entering system if it is a charger, isn't
it, so don't need to consider USB charger.
 
> 
> If indeed we can't handle charger events in the way suggested by you,
> may I propose the following:
> 
> - implement this in USB-phy as proposed in Kiran's original patch,
> - keep it disabled by default, and add a module param to enable/disable it
> - for no vbus case, use another event (USB_EVENT_ENUMERATED /
> USB_EVENT_NONE? ) to identify connect / disconnect?
>      (OR, maybe don't do anything for such cases, since most likely
> the chipidea kind of implementations would handle it themselves?)
> 

In my opinion, we can do noop for wakeup source if it is a USB charger.

> Of course these are based on my limited reading of the USB code, so
> I'd be grateful if you'd let me know of any obvious gaps in my
> understanding!
> 
> > --
> >
> > Best Regards,
> > Peter Chen
> 
> 
> Best regards,
> Sumit.

-- 

Best Regards,
Peter Chen

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

[Sumit Semwal]

On 13 July 2015 at 12:30, Peter Chen <peter.chen@freescale.com> wrote:
> On Mon, Jul 13, 2015 at 01:26:23PM +0530, Sumit Semwal wrote:
>> Hi Peter,
>>
>> On 8 June 2015 at 14:02, Peter Chen <peter.chen@freescale.com> wrote:
>> > On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
>> >> H Peter, Felipe,
>> >>
>> >> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
>> >> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
>> >> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
>> >> >> > Hi Felipe,
>> >> >> >
>> >> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
>> >> >> > > Hi Felipe,
>> >> >> > >
>> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
>> >> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>> >> >> > >>>
>> >> >> > >>> >
>> >> >> > >>> > 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.
>> >> >> > >>> >
>> >> >> > >>>
>> <snip>
>> >>
>> >> Kiran is unable to continue on this patch anymore, so despite my very
>> >> limited usb experience, I would like to take this over and with your
>> >> help, take it to closure.
>> >>
>> >> From what I understand, it might be possible to do this either at the
>> >> usb controller driver (where it detects if it has enumerated in host
>> >> mode or device mode) or in the extcon driver (where it detects VBUS
>> >> etc..) -  which would be a cleaner / more logical way to do it?
>> >>
>> >> Thanks for all your help!
>> >
>> > I added my suggestions below:
>> >
>> > My opinion is: if we need this feature, we should depend on gadget's
>> > state, eg 'configured', not vbus level. The system suspend should be
>> > allowed if the gadget state is 'suspended' even vbus is still there.
>> >
>> > And when the gadget goes to suspend or reset or disconnect state, we can
>> > go to disable this wakeup source.
>> >
>> > At drivers/usb/gadget/composite.c, we can know the gadget's state.
>> >
>> Thanks very much for your suggestion, and apologies in the delay in response.
>>
>> I tried to study your suggestion and the code a little bit more, and
>> while I liked your idea quite a bit, I also realised that for the case
>> of letting the USB charger use cases, in gadget framework, there
>> doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
>> would likely not be a gadget.
>>
>> In this case, how'd we be able to enable / disable wakeup_source for
>> charger events?
>
> We may not prevent the system entering system if it is a charger, isn't
> it, so don't need to consider USB charger.
>
Thanks for your comments, Peter!
>From the Android code that I see, it seems that for a charger
connect/disconnect, some systems might require some time to react to
the change in power, and hence there's a timed-out wakeup event for
USB charger cases. That;s where the consideration for USB chargers
came from.
>>
>> If indeed we can't handle charger events in the way suggested by you,
>> may I propose the following:
>>
>> - implement this in USB-phy as proposed in Kiran's original patch,
>> - keep it disabled by default, and add a module param to enable/disable it
>> - for no vbus case, use another event (USB_EVENT_ENUMERATED /
>> USB_EVENT_NONE? ) to identify connect / disconnect?
>>      (OR, maybe don't do anything for such cases, since most likely
>> the chipidea kind of implementations would handle it themselves?)
>>
>
> In my opinion, we can do noop for wakeup source if it is a USB charger.
>
I think I misunderstood your comment here - do you mean for 'no vbus
case (like chipidea)' do a noop for wakeup source? In the gadget
framework, there doesn't seem to be a way to get to know if a USB
charger is connected.

>> Of course these are based on my limited reading of the USB code, so
>> I'd be grateful if you'd let me know of any obvious gaps in my
>> understanding!
>>
>> > --
>> >
>> > Best Regards,
>> > Peter Chen
>>
>>
>> Best regards,
>> Sumit.
>
> --
>
> Best Regards,
> Peter Chen

Best regards,
Sumit.

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

[Peter Chen]

On Mon, Jul 20, 2015 at 01:18:35PM +0530, Sumit Semwal wrote:
> On 13 July 2015 at 12:30, Peter Chen <peter.chen@freescale.com> wrote:
> > On Mon, Jul 13, 2015 at 01:26:23PM +0530, Sumit Semwal wrote:
> >> Hi Peter,
> >>
> >> On 8 June 2015 at 14:02, Peter Chen <peter.chen@freescale.com> wrote:
> >> > On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
> >> >> H Peter, Felipe,
> >> >>
> >> >> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
> >> >> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
> >> >> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
> >> >> >> > Hi Felipe,
> >> >> >> >
> >> >> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
> >> >> >> > > Hi Felipe,
> >> >> >> > >
> >> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
> >> >> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
> >> >> >> > >>>
> >> >> >> > >>> >
> >> >> >> > >>> > 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.
> >> >> >> > >>> >
> >> >> >> > >>>
> >> <snip>
> >> >>
> >> >> Kiran is unable to continue on this patch anymore, so despite my very
> >> >> limited usb experience, I would like to take this over and with your
> >> >> help, take it to closure.
> >> >>
> >> >> From what I understand, it might be possible to do this either at the
> >> >> usb controller driver (where it detects if it has enumerated in host
> >> >> mode or device mode) or in the extcon driver (where it detects VBUS
> >> >> etc..) -  which would be a cleaner / more logical way to do it?
> >> >>
> >> >> Thanks for all your help!
> >> >
> >> > I added my suggestions below:
> >> >
> >> > My opinion is: if we need this feature, we should depend on gadget's
> >> > state, eg 'configured', not vbus level. The system suspend should be
> >> > allowed if the gadget state is 'suspended' even vbus is still there.
> >> >
> >> > And when the gadget goes to suspend or reset or disconnect state, we can
> >> > go to disable this wakeup source.
> >> >
> >> > At drivers/usb/gadget/composite.c, we can know the gadget's state.
> >> >
> >> Thanks very much for your suggestion, and apologies in the delay in response.
> >>
> >> I tried to study your suggestion and the code a little bit more, and
> >> while I liked your idea quite a bit, I also realised that for the case
> >> of letting the USB charger use cases, in gadget framework, there
> >> doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
> >> would likely not be a gadget.
> >>
> >> In this case, how'd we be able to enable / disable wakeup_source for
> >> charger events?
> >
> > We may not prevent the system entering system if it is a charger, isn't
> > it, so don't need to consider USB charger.
> >
> Thanks for your comments, Peter!
> From the Android code that I see, it seems that for a charger
> connect/disconnect, some systems might require some time to react to
> the change in power, and hence there's a timed-out wakeup event for
> USB charger cases. That;s where the consideration for USB chargers
> came from.

Current udc core has an API usb_gadget_set_state set its state
according to usb ch9. For this wakeup source case, if the peripheral
has been configured, it can hold wakeup source. Otherise, just have
a timeout wake lock.
Below are some reference code:

diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index d69c355..018f534 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c

@@ -136,6 +138,16 @@ static void usb_gadget_state_work(struct
		work_struct *work)
 
        if (udc) {
		sysfs_notify(&udc->dev.kobj, NULL, "state");
		       switch (gadget->state) {
			case	USB_STATE_CONFIGURED:
			__pm_stay_awake(&x->wsource);
				break;
			default:
			__pm_wakeup_event(&x->wsource,
					USB_DEFAULT_WAKEUP_SRC_TIMEOUT);
			break;
		  }
	}


-- 

Best Regards,
Peter Chen

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

[Sumit Semwal]

Hi Peter,

On 20 July 2015 at 12:45, Peter Chen <peter.chen@freescale.com> wrote:
> On Mon, Jul 20, 2015 at 01:18:35PM +0530, Sumit Semwal wrote:
>> On 13 July 2015 at 12:30, Peter Chen <peter.chen@freescale.com> wrote:
>> > On Mon, Jul 13, 2015 at 01:26:23PM +0530, Sumit Semwal wrote:
>> >> Hi Peter,
>> >>
>> >> On 8 June 2015 at 14:02, Peter Chen <peter.chen@freescale.com> wrote:
>> >> > On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
>> >> >> H Peter, Felipe,
>> >> >>
>> >> >> On 14 January 2015 at 06:56, Peter Chen <peter.chen@freescale.com> wrote:
>> >> >> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
>> >> >> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
>> >> >> >> > Hi Felipe,
>> >> >> >> >
>> >> >> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@linaro.org> wrote:
>> >> >> >> > > Hi Felipe,
>> >> >> >> > >
>> >> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@ti.com> wrote:
>> >> >> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>> >> >> >> > >>>
>> >> >> >> > >>> >
>> >> >> >> > >>> > 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.
>> >> >> >> > >>> >
>> >> >> >> > >>>
>> >> <snip>
>> >> >>
>> >> >> Kiran is unable to continue on this patch anymore, so despite my very
>> >> >> limited usb experience, I would like to take this over and with your
>> >> >> help, take it to closure.
>> >> >>
>> >> >> From what I understand, it might be possible to do this either at the
>> >> >> usb controller driver (where it detects if it has enumerated in host
>> >> >> mode or device mode) or in the extcon driver (where it detects VBUS
>> >> >> etc..) -  which would be a cleaner / more logical way to do it?
>> >> >>
>> >> >> Thanks for all your help!
>> >> >
>> >> > I added my suggestions below:
>> >> >
>> >> > My opinion is: if we need this feature, we should depend on gadget's
>> >> > state, eg 'configured', not vbus level. The system suspend should be
>> >> > allowed if the gadget state is 'suspended' even vbus is still there.
>> >> >
>> >> > And when the gadget goes to suspend or reset or disconnect state, we can
>> >> > go to disable this wakeup source.
>> >> >
>> >> > At drivers/usb/gadget/composite.c, we can know the gadget's state.
>> >> >
>> >> Thanks very much for your suggestion, and apologies in the delay in response.
>> >>
>> >> I tried to study your suggestion and the code a little bit more, and
>> >> while I liked your idea quite a bit, I also realised that for the case
>> >> of letting the USB charger use cases, in gadget framework, there
>> >> doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
>> >> would likely not be a gadget.
>> >>
>> >> In this case, how'd we be able to enable / disable wakeup_source for
>> >> charger events?
>> >
>> > We may not prevent the system entering system if it is a charger, isn't
>> > it, so don't need to consider USB charger.
>> >
>> Thanks for your comments, Peter!
>> From the Android code that I see, it seems that for a charger
>> connect/disconnect, some systems might require some time to react to
>> the change in power, and hence there's a timed-out wakeup event for
>> USB charger cases. That;s where the consideration for USB chargers
>> came from.
>
> Current udc core has an API usb_gadget_set_state set its state
> according to usb ch9. For this wakeup source case, if the peripheral
> has been configured, it can hold wakeup source. Otherise, just have
> a timeout wake lock.
Thank you for your kind hint. Let me toy with this change and update.

> Below are some reference code:
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index d69c355..018f534 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
>
> @@ -136,6 +138,16 @@ static void usb_gadget_state_work(struct
>                 work_struct *work)
>
>         if (udc) {
>                 sysfs_notify(&udc->dev.kobj, NULL, "state");
>                        switch (gadget->state) {
>                         case    USB_STATE_CONFIGURED:
>                         __pm_stay_awake(&x->wsource);
>                                 break;
>                         default:
>                         __pm_wakeup_event(&x->wsource,
>                                         USB_DEFAULT_WAKEUP_SRC_TIMEOUT);
>                         break;
>                   }
>         }
>
>
> --
>
> Best Regards,
> Peter Chen

Best regards,
Sumit.