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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Android Kernel Team <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Benoit Goby <[email protected]>
[Original patch in Android from Todd]
Cc: Todd Poynor <[email protected]>
Signed-off-by: Kiran Raparthy <[email protected]>
Signed-off-by: Felipe Balbi <[email protected]>
---
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
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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Android Kernel Team <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Benoit Goby <[email protected]>
[Original patch in Android from Todd]
Cc: Todd Poynor <[email protected]>
Signed-off-by: Kiran Raparthy <[email protected]>
---
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
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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Android Kernel Team <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Benoit Goby <[email protected]>
[Original patch in Android from Todd]
Cc: Todd Poynor <[email protected]>
Signed-off-by: Kiran Raparthy <[email protected]>
---
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
>
> 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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Android Kernel Team <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Arve Hjønnevåg <[email protected]>
> Cc: Benoit Goby <[email protected]>
> [Original patch in Android from Todd]
> Cc: Todd Poynor <[email protected]>
> Signed-off-by: Kiran Raparthy <[email protected]>
> ---
> 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 [email protected] More majordomo info at
> http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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
Hi Felipe,
On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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
H Peter, Felipe,
On 14 January 2015 at 06:56, Peter Chen <[email protected]> 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 <[email protected]> wrote:
>> > > Hi Felipe,
>> > >
>> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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 [email protected]
> 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
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 <[email protected]> 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 <[email protected]> wrote:
> >> > > Hi Felipe,
> >> > >
> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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
On Mon, Jul 13, 2015 at 01:26:23PM +0530, Sumit Semwal wrote:
> Hi Peter,
>
> On 8 June 2015 at 14:02, Peter Chen <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> >> >> > > Hi Felipe,
> >> >> > >
> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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
Hi Peter,
On 8 June 2015 at 14:02, Peter Chen <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
>> >> > > Hi Felipe,
>> >> > >
>> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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.
On Mon, Jul 20, 2015 at 01:18:35PM +0530, Sumit Semwal wrote:
> On 13 July 2015 at 12:30, Peter Chen <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> >> >> >> > > Hi Felipe,
> >> >> >> > >
> >> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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
On 13 July 2015 at 12:30, Peter Chen <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
>> >> >> > > Hi Felipe,
>> >> >> > >
>> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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.
Hi Peter,
On 20 July 2015 at 12:45, Peter Chen <[email protected]> wrote:
> On Mon, Jul 20, 2015 at 01:18:35PM +0530, Sumit Semwal wrote:
>> On 13 July 2015 at 12:30, Peter Chen <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
>> >> >> >> > > Hi Felipe,
>> >> >> >> > >
>> >> >> >> > > On 25 November 2014 at 20:15, Felipe Balbi <[email protected]> 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.