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

[Felipe Balbi <balbi@ti.com>]

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

On Tue, Sep 02, 2014 at 05:17:08PM +0530, Kiran Kumar Raparthy wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> 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 <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: Arve Hj�nnev�g <arve@android.com>
> Cc: Benoit Goby <benoit@android.com>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> [kiran: Added context to commit message.
> 	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).
> 	changed to "disabled by default" from "enable by default".
> 	sys entry(module param) field modified to otg_wakeupsource.
> 	included Todd's refactoring logic.
> 	Introduced get_phy_hook to handle otgws_xceiv per-PHY.
> 	otgws_nb moved to otg_wakeupsource_init function.
> 	wakeupsource handling implemeted per-PHY(in phy core).
> 	modified Kconfig help text]
> Signed-off-by: Kiran Raparthy <kiran.kumar@linaro.org>
> ---
>  drivers/usb/phy/Kconfig            |   8 +++
>  drivers/usb/phy/Makefile           |   1 +
>  drivers/usb/phy/otg-wakeupsource.c | 140 +++++++++++++++++++++++++++++++++++++
>  drivers/usb/phy/phy.c              |  42 +++++++++++
>  include/linux/usb/phy.h            |   8 +++
>  5 files changed, 199 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 0000000..fca2010
> --- /dev/null
> +++ b/drivers/usb/phy/otg-wakeupsource.c
> @@ -0,0 +1,140 @@
> +/*
> + * 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 <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/pm_wakeup.h>
> +#include <linux/spinlock.h>
> +#include <linux/usb/otg.h>
> +
> +bool enabled = false;
> +
> +static DEFINE_SPINLOCK(otgws_spinlock);
> +static struct usb_phy *get_phy_hook(void);

how many times do I have to say that this needs to be per-PHY ? This
means that you shouldn't have a global PHY pointer. Embed this directly
into struct usb_phy and have phy-core handle this.

-- 
balbi

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