lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:	Mon, 8 Sep 2014 09:21:28 -0500
From:	Felipe Balbi <balbi@...com>
To:	Kiran Raparthy <kiran.kumar@...aro.org>
CC:	Felipe Balbi <balbi@...com>, LKML <linux-kernel@...r.kernel.org>,
	Todd Poynor <toddpoynor@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<linux-usb@...r.kernel.org>,
	Android Kernel Team <kernel-team@...roid.com>,
	John Stultz <john.stultz@...aro.org>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	Arve Hj�nnev�g <arve@...roid.com>,
	Benoit Goby <benoit@...roid.com>
Subject: Re: [RFC v3 1/2] usb: phy: Hold wakeupsource when USB is enumerated
 in peripheral mode

Hi,

On Mon, Sep 08, 2014 at 07:48:06PM +0530, Kiran Raparthy wrote:
> On 8 September 2014 19:08, Felipe Balbi <balbi@...com> wrote:
> 
> > Hi,
> >
> > On Mon, Sep 08, 2014 at 03:39:23PM +0530, Kiran Kumar Raparthy wrote:
> > > From: Todd Poynor <toddpoynor@...gle.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@...com>
> > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > Cc: linux-kernel@...r.kernel.org
> > > Cc: linux-usb@...r.kernel.org
> > > Cc: Android Kernel Team <kernel-team@...roid.com>
> > > Cc: John Stultz <john.stultz@...aro.org>
> > > Cc: Sumit Semwal <sumit.semwal@...aro.org>
> > > Cc: Arve Hj�nnev�g <arve@...roid.com>
> > > Cc: Benoit Goby <benoit@...roid.com>
> > > Signed-off-by: Todd Poynor <toddpoynor@...gle.com>
> > > [kiran: Added context to commit message, squished build fixes
> > > from Benoit Goby and Arve Hj�nnev�g, changed wakelocks usage
> > > to wakeupsource, merged Todd's refactoring logic and simplified
> > > the structures and code and addressed community feedback]
> > > Signed-off-by: Kiran Raparthy <kiran.kumar@...aro.org>
> > > ---
> > > 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 0000000..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 <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);
> >
> > 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 ?
> >
> > Hi Balbi,
> Thanks for taking time in providing the valuable input.
> I have changed everything per-PHY where ever global phy pointer is used.
> Since otgws_spinlock is not dealing with any phy related parameter,i have
> not included that in struct usb_phy.
> Sorry,i was not expecting that you are referring to otgws_spinlock.
> I'll modify as per your suggestion and resubmit the patch.

while at that, also drop that enabled boolean too.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists