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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4855631.a4dcc0Gn36@vostro.rjw.lan>
Date:	Mon, 26 Nov 2012 02:16:20 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Huang Ying <ying.huang@...el.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Zhang Rui <rui.zhang@...el.com>,
	"Svahn, Kai" <kai.svahn@...el.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"Lan, Tianyu" <tianyu.lan@...el.com>,
	"Zheng, Lv" <lv.zheng@...el.com>, Aaron Lu <aaron.lu@...el.com>,
	Grant Likely <grant.likely@...retlab.ca>
Subject: Re: [PATCH 1/2] ACPI / PM: Allow attach/detach routines to change device power states

On Monday, November 26, 2012 09:07:27 AM Huang Ying wrote:
> On Mon, 2012-11-26 at 02:00 +0100, Rafael J. Wysocki wrote:
> > On Monday, November 26, 2012 08:43:10 AM Huang Ying wrote:
> > > On Sun, 2012-11-25 at 15:55 +0100, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > 
> > > > Make it possible to ask the routines used for adding/removing devices
> > > > to/from the general ACPI PM domain, acpi_dev_pm_attach() and
> > > > acpi_dev_pm_detach(), respectively, to change the power states of
> > > > devices so that they are put into the full-power state automatically
> > > > by acpi_dev_pm_attach() and into the lowest-power state available
> > > > automatically by acpi_dev_pm_detach().
> > > > 
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > ---
> > > >  drivers/acpi/device_pm.c |   28 ++++++++++++++++++++++++----
> > > >  include/linux/acpi.h     |   11 +++++++----
> > > >  2 files changed, 31 insertions(+), 8 deletions(-)
> > > > 
> > > > Index: linux/drivers/acpi/device_pm.c
> > > > ===================================================================
> > > > --- linux.orig/drivers/acpi/device_pm.c
> > > > +++ linux/drivers/acpi/device_pm.c
> > > > @@ -599,10 +599,12 @@ static struct dev_pm_domain acpi_general
> > > >  /**
> > > >   * acpi_dev_pm_attach - Prepare device for ACPI power management.
> > > >   * @dev: Device to prepare.
> > > > + * @power_on: Whether or not to power on the device.
> > > >   *
> > > >   * If @dev has a valid ACPI handle that has a valid struct acpi_device object
> > > >   * attached to it, install a wakeup notification handler for the device and
> > > > - * add it to the general ACPI PM domain.
> > > > + * add it to the general ACPI PM domain.  If @power_on is set, the device will
> > > > + * be put into the ACPI D0 state before the function returns.
> > > >   *
> > > >   * This assumes that the @dev's bus type uses generic power management callbacks
> > > >   * (or doesn't use any power management callbacks at all).
> > > > @@ -610,7 +612,7 @@ static struct dev_pm_domain acpi_general
> > > >   * Callers must ensure proper synchronization of this function with power
> > > >   * management callbacks.
> > > >   */
> > > > -int acpi_dev_pm_attach(struct device *dev)
> > > > +int acpi_dev_pm_attach(struct device *dev, bool power_on)
> > > >  {
> > > >  	struct acpi_device *adev = acpi_dev_pm_get_node(dev);
> > > >  
> > > > @@ -622,6 +624,10 @@ int acpi_dev_pm_attach(struct device *de
> > > >  
> > > >  	acpi_add_pm_notifier(adev, acpi_wakeup_device, dev);
> > > >  	dev->pm_domain = &acpi_general_pm_domain;
> > > > +	if (power_on) {
> > > > +		acpi_dev_pm_full_power(adev);
> > > > +		__acpi_device_run_wake(adev, false);
> > > > +	}
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
> > > > @@ -629,20 +635,34 @@ EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
> > > >  /**
> > > >   * acpi_dev_pm_detach - Remove ACPI power management from the device.
> > > >   * @dev: Device to take care of.
> > > > + * @power_off: Whether or not to try to remove power from the device.
> > > >   *
> > > >   * Remove the device from the general ACPI PM domain and remove its wakeup
> > > > - * notifier.
> > > > + * notifier.  If @power_off is set, additionally remove power from the device if
> > > > + * possible.
> > > >   *
> > > >   * Callers must ensure proper synchronization of this function with power
> > > >   * management callbacks.
> > > >   */
> > > > -void acpi_dev_pm_detach(struct device *dev)
> > > > +void acpi_dev_pm_detach(struct device *dev, bool power_off)
> > > >  {
> > > >  	struct acpi_device *adev = acpi_dev_pm_get_node(dev);
> > > >  
> > > >  	if (adev && dev->pm_domain == &acpi_general_pm_domain) {
> > > >  		dev->pm_domain = NULL;
> > > >  		acpi_remove_pm_notifier(adev, acpi_wakeup_device);
> > > > +		if (power_off) {
> > > > +			/*
> > > > +			 * If the device's PM QoS resume latency limit or flags
> > > > +			 * have been exposed to user space, they have to be
> > > > +			 * hidden at this point, so that they don't affect the
> > > > +			 * choice of the low-power state to put the device into.
> > > > +			 */
> > > > +			dev_pm_qos_hide_latency_limit(dev);
> > > > +			dev_pm_qos_hide_flags(dev);
> > > 
> > > NO_POWER_OFF flag is ignored here.  Is it possible for some device (or
> > > corresponding ACPI method) has broken D3cold implementation, so that the
> > > user need a way to disable it?
> > 
> > We are removing the driver here and we haven't exposed the flags, have we?
> 
> IMHO, the flag may be exposed by the bus instead of device driver.
> Because some bus can determine whether D3cold is supported by the
> device.  If the sysfs file for the flag is exposed by the bus, user set
> it and we silently ignore it, will it cause confusing for the user?

OK, put it in a different way: If the entity that calls acpi_dev_pm_attach()
exposes the flags _before_ calling it, then acpi_dev_pm_detach() should not
hide the flags.  Otherwise, it should hide them.  Currently, the only user
of acpi_dev_pm_attach() (that will be the platform bus type) will not expose
the flags before calling that routine, so it is OK for acpi_dev_pm_detach()
to remove them.

Is there anything I'm missing here?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ