Re: [RFC v3 1/2] usb: phy: Hold wakeupsource when USB is enumerated in peripheral mode

2014-09-08 Thread Felipe Balbi 
Hi,

On Mon, Sep 08, 2014 at 07:48:06PM +0530, Kiran Raparthy wrote:
> On 8 September 2014 19:08, Felipe Balbi  wrote:
> 
> > Hi,
> >
> > On Mon, Sep 08, 2014 at 03:39:23PM +0530, Kiran Kumar Raparthy wrote:
> > > From: Todd Poynor 
> > >
> > > usb: phy: Hold wakeupsource when USB is enumerated in peripheral mode
> > >
> > > Purpose of this is to prevent the system to enter into suspend state
> > from USB
> > > peripheral traffic by hodling a wakeupsource when USB is connected and
> > > enumerated in peripheral mode(say adb).
> > >
> > > Disabled by default, can enable with:
> > >echo Y > /sys/module/otg_wakeupsource/parameters/enabled
> > >
> > > Cc: Felipe Balbi 
> > > Cc: Greg Kroah-Hartman 
> > > Cc: linux-ker...@vger.kernel.org
> > > Cc: linux-usb@vger.kernel.org
> > > Cc: Android Kernel Team 
> > > Cc: John Stultz 
> > > Cc: Sumit Semwal 
> > > Cc: Arve Hj�nnev�g 
> > > Cc: Benoit Goby 
> > > Signed-off-by: Todd Poynor 
> > > [kiran: Added context to commit message, squished build fixes
> > > from Benoit Goby and Arve Hj�nnev�g, changed wakelocks usage
> > > to wakeupsource, merged Todd's refactoring logic and simplified
> > > the structures and code and addressed community feedback]
> > > Signed-off-by: Kiran Raparthy 
> > > ---
> > > v3:
> > > * As per the feedback,no global phy pointer used.
> > > * called the one-liner wakeupsource handling calls
> > >   directly instead of indirect functions implemented in v2.
> > > * Removed indirect function get_phy_hook and used usb_get_phy
> > >   to get the phy handle..
> > >
> > > v2:
> > > * wakeupsource handling implemeted per-PHY
> > > * Implemented wakeupsource handling calls in phy
> > > * included Todd's refactoring logic.
> > >
> > > v1:
> > > * changed to "disabled by default" from "enable by default".
> > > * Kconfig help text modified
> > > * Included better commit text
> > > * otgws_nb moved to otg_wakeupsource_init function
> > > * Introduced get_phy_hook to handle otgws_xceiv per-PHY
> > >
> > > RFC:
> > > * Included build fix from Benoit Goby and Arve Hj�nnev�g
> > > * Removed lock->held field in driver as this mechanism is
> > >   provided in wakeupsource driver.
> > > * wakelock(wl) terminology replaced with wakeup_source(ws).
> > >
> > >  drivers/usb/phy/Kconfig|   8 +++
> > >  drivers/usb/phy/Makefile   |   1 +
> > >  drivers/usb/phy/otg-wakeupsource.c | 136
> > +
> > >  include/linux/usb/phy.h|   4 ++
> > >  4 files changed, 149 insertions(+)
> > >  create mode 100644 drivers/usb/phy/otg-wakeupsource.c
> > >
> > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> > > index e253fa0..d9ddd85 100644
> > > --- a/drivers/usb/phy/Kconfig
> > > +++ b/drivers/usb/phy/Kconfig
> > > @@ -6,6 +6,14 @@ menu "USB Physical Layer drivers"
> > >  config USB_PHY
> > >   def_bool n
> > >
> > > +config USB_OTG_WAKEUPSOURCE
> > > + bool "Hold wakeupsource when USB is enumerated in peripheral mode"
> > > + depends on PM_SLEEP
> > > + select USB_PHY
> > > + help
> > > +   Prevent the system going into automatic suspend while
> > > +   it is attached as a USB peripheral by holding a wakeupsource.
> > > +
> > >  #
> > >  # USB Transceiver Drivers
> > >  #
> > > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> > > index 24a9133..ca2fbaf 100644
> > > --- a/drivers/usb/phy/Makefile
> > > +++ b/drivers/usb/phy/Makefile
> > > @@ -3,6 +3,7 @@
> > >  #
> > >  obj-$(CONFIG_USB_PHY)+= phy.o
> > >  obj-$(CONFIG_OF) += of.o
> > > +obj-$(CONFIG_USB_OTG_WAKEUPSOURCE)   += otg-wakeupsource.o
> > >
> > >  # transceiver drivers, keep the list sorted
> > >
> > > diff --git a/drivers/usb/phy/otg-wakeupsource.c
> > b/drivers/usb/phy/otg-wakeupsource.c
> > > new file mode 100644
> > > index 000..d9a1720
> > > --- /dev/null
> > > +++ b/drivers/usb/phy/otg-wakeupsource.c
> > > @@ -0,0 +1,136 @@
> > > +/*
> > > + * otg-wakeupsource.c
> > > + *
> > > + * Copyright (C) 2011 Google, Inc.
> > > + *
> > > + * This software is licensed under the terms of the GNU General Public
> > > + * License version 2, as published by the Free Software Foundation, and
> > > + * may be copied, distributed, and modified under those terms.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +bool enabled = false;
> > > +
> > > +static DEFINE_SPINLOCK(otgws_spinlock);
> >
> > why do you continue to ignore my comment that this should be built
> > *into* struct usb_phy so it's a per-PHY setting ? Is this some sort of a
> > joke that I'm n

Re: [RFC v3 1/2] usb: phy: Hold wakeupsource when USB is enumerated in peripheral mode

2014-09-08 Thread Felipe Balbi 
Hi,

On Mon, Sep 08, 2014 at 03:39:23PM +0530, Kiran Kumar Raparthy wrote:
> From: Todd Poynor 
> 
> usb: phy: Hold wakeupsource when USB is enumerated in peripheral mode
> 
> Purpose of this is to prevent the system to enter into suspend state from USB
> peripheral traffic by hodling a wakeupsource when USB is connected and
> enumerated in peripheral mode(say adb).
> 
> Disabled by default, can enable with:
>echo Y > /sys/module/otg_wakeupsource/parameters/enabled
> 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> Cc: Android Kernel Team 
> Cc: John Stultz 
> Cc: Sumit Semwal 
> Cc: Arve Hj�nnev�g 
> Cc: Benoit Goby 
> Signed-off-by: Todd Poynor 
> [kiran: Added context to commit message, squished build fixes
> from Benoit Goby and Arve Hj�nnev�g, changed wakelocks usage
> to wakeupsource, merged Todd's refactoring logic and simplified
> the structures and code and addressed community feedback]
> Signed-off-by: Kiran Raparthy 
> ---
> v3:
> * As per the feedback,no global phy pointer used.
> * called the one-liner wakeupsource handling calls
>   directly instead of indirect functions implemented in v2.
> * Removed indirect function get_phy_hook and used usb_get_phy
>   to get the phy handle..
> 
> v2:
> * wakeupsource handling implemeted per-PHY
> * Implemented wakeupsource handling calls in phy
> * included Todd's refactoring logic.
> 
> v1:
> * changed to "disabled by default" from "enable by default".
> * Kconfig help text modified
> * Included better commit text
> * otgws_nb moved to otg_wakeupsource_init function
> * Introduced get_phy_hook to handle otgws_xceiv per-PHY
> 
> RFC:
> * Included build fix from Benoit Goby and Arve Hj�nnev�g
> * Removed lock->held field in driver as this mechanism is
>   provided in wakeupsource driver.
> * wakelock(wl) terminology replaced with wakeup_source(ws).
> 
>  drivers/usb/phy/Kconfig|   8 +++
>  drivers/usb/phy/Makefile   |   1 +
>  drivers/usb/phy/otg-wakeupsource.c | 136 
> +
>  include/linux/usb/phy.h|   4 ++
>  4 files changed, 149 insertions(+)
>  create mode 100644 drivers/usb/phy/otg-wakeupsource.c
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index e253fa0..d9ddd85 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -6,6 +6,14 @@ menu "USB Physical Layer drivers"
>  config USB_PHY
>   def_bool n
>  
> +config USB_OTG_WAKEUPSOURCE
> + bool "Hold wakeupsource when USB is enumerated in peripheral mode"
> + depends on PM_SLEEP
> + select USB_PHY
> + help
> +   Prevent the system going into automatic suspend while
> +   it is attached as a USB peripheral by holding a wakeupsource.
> +
>  #
>  # USB Transceiver Drivers
>  #
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 24a9133..ca2fbaf 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -3,6 +3,7 @@
>  #
>  obj-$(CONFIG_USB_PHY)+= phy.o
>  obj-$(CONFIG_OF) += of.o
> +obj-$(CONFIG_USB_OTG_WAKEUPSOURCE)   += otg-wakeupsource.o
>  
>  # transceiver drivers, keep the list sorted
>  
> diff --git a/drivers/usb/phy/otg-wakeupsource.c 
> b/drivers/usb/phy/otg-wakeupsource.c
> new file mode 100644
> index 000..d9a1720
> --- /dev/null
> +++ b/drivers/usb/phy/otg-wakeupsource.c
> @@ -0,0 +1,136 @@
> +/*
> + * otg-wakeupsource.c
> + *
> + * Copyright (C) 2011 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +bool enabled = false;
> +
> +static DEFINE_SPINLOCK(otgws_spinlock);

why do you continue to ignore my comment that this should be built
*into* struct usb_phy so it's a per-PHY setting ? Is this some sort of a
joke that I'm not getting ?

> +static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long 
> event)
> +{
> + unsigned long irqflags;
> +
> + spin_lock_irqsave(&otgws_spinlock, irqflags);
> +
> + if (!enabled) {
> + __pm_relax(&otgws_xceiv->wsource);
> + spin_unlock_irqrestore(&otgws_spinlock, irqflags);
> + return;
> + }
> +
> + switch (event) {
> + case USB_EVENT_VBUS:
> + case USB_EVENT_ENUMERATED:
> + __pm_stay_awake(&otgws_xceiv->wsource);
> + break;
> +
> + case USB_EVENT_NONE:
> + case USB_EVENT_ID:
> + case USB_EVENT_

[RFC v3 1/2] usb: phy: Hold wakeupsource when USB is enumerated in peripheral mode

2014-09-08 Thread Kiran Kumar Raparthy 
From: Todd Poynor 

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

Purpose of this is to prevent the system to enter into suspend state from USB
peripheral traffic by hodling a wakeupsource when USB is connected and
enumerated in peripheral mode(say adb).

Disabled by default, can enable with:
   echo Y > /sys/module/otg_wakeupsource/parameters/enabled

Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-ker...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: Android Kernel Team 
Cc: John Stultz 
Cc: Sumit Semwal 
Cc: Arve Hj�nnev�g 
Cc: Benoit Goby 
Signed-off-by: Todd Poynor 
[kiran: Added context to commit message, squished build fixes
from Benoit Goby and Arve Hj�nnev�g, changed wakelocks usage
to wakeupsource, merged Todd's refactoring logic and simplified
the structures and code and addressed community feedback]
Signed-off-by: Kiran Raparthy 
---
v3:
* As per the feedback,no global phy pointer used.
* called the one-liner wakeupsource handling calls
  directly instead of indirect functions implemented in v2.
* Removed indirect function get_phy_hook and used usb_get_phy
  to get the phy handle..

v2:
* wakeupsource handling implemeted per-PHY
* Implemented wakeupsource handling calls in phy
* included Todd's refactoring logic.

v1:
* changed to "disabled by default" from "enable by default".
* Kconfig help text modified
* Included better commit text
* otgws_nb moved to otg_wakeupsource_init function
* Introduced get_phy_hook to handle otgws_xceiv per-PHY

RFC:
* Included build fix from Benoit Goby and Arve Hj�nnev�g
* Removed lock->held field in driver as this mechanism is
  provided in wakeupsource driver.
* wakelock(wl) terminology replaced with wakeup_source(ws).

 drivers/usb/phy/Kconfig|   8 +++
 drivers/usb/phy/Makefile   |   1 +
 drivers/usb/phy/otg-wakeupsource.c | 136 +
 include/linux/usb/phy.h|   4 ++
 4 files changed, 149 insertions(+)
 create mode 100644 drivers/usb/phy/otg-wakeupsource.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index e253fa0..d9ddd85 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -6,6 +6,14 @@ menu "USB Physical Layer drivers"
 config USB_PHY
def_bool n
 
+config USB_OTG_WAKEUPSOURCE
+   bool "Hold wakeupsource when USB is enumerated in peripheral mode"
+   depends on PM_SLEEP
+   select USB_PHY
+   help
+ Prevent the system going into automatic suspend while
+ it is attached as a USB peripheral by holding a wakeupsource.
+
 #
 # USB Transceiver Drivers
 #
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 24a9133..ca2fbaf 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -3,6 +3,7 @@
 #
 obj-$(CONFIG_USB_PHY)  += phy.o
 obj-$(CONFIG_OF)   += of.o
+obj-$(CONFIG_USB_OTG_WAKEUPSOURCE) += otg-wakeupsource.o
 
 # transceiver drivers, keep the list sorted
 
diff --git a/drivers/usb/phy/otg-wakeupsource.c 
b/drivers/usb/phy/otg-wakeupsource.c
new file mode 100644
index 000..d9a1720
--- /dev/null
+++ b/drivers/usb/phy/otg-wakeupsource.c
@@ -0,0 +1,136 @@
+/*
+ * otg-wakeupsource.c
+ *
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+bool enabled = false;
+
+static DEFINE_SPINLOCK(otgws_spinlock);
+
+static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long 
event)
+{
+   unsigned long irqflags;
+
+   spin_lock_irqsave(&otgws_spinlock, irqflags);
+
+   if (!enabled) {
+   __pm_relax(&otgws_xceiv->wsource);
+   spin_unlock_irqrestore(&otgws_spinlock, irqflags);
+   return;
+   }
+
+   switch (event) {
+   case USB_EVENT_VBUS:
+   case USB_EVENT_ENUMERATED:
+   __pm_stay_awake(&otgws_xceiv->wsource);
+   break;
+
+   case USB_EVENT_NONE:
+   case USB_EVENT_ID:
+   case USB_EVENT_CHARGER:
+   __pm_relax(&otgws_xceiv->wsource);
+   break;
+
+   default:
+   break;
+   }
+
+   spin_unlock_irqrestore(&otgws_spinlock, irqflags);
+}
+
+static int otgws_otg_notifications(struct notifier_block *nb,
+   unsigned long event, void *unused)
+{
+   static struct usb_phy *otgws_xceiv;
+
+   otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
+
+   if (IS_ERR(otgws_xceiv)) {
+   pr_err("%