* [PATCH 1/2 v2] PM: Change PM subsys_data lock type into spinlock
From: Rafael J. Wysocki @ 2011-08-21 19:10 UTC (permalink / raw)
To: linux-sh; +Cc: Linux PM mailing list, LKML
In-Reply-To: <201108212109.30399.rjw@sisk.pl>
From: Rafael J. Wysocki <rjw@sisk.pl>
The lock member of struct pm_subsys_data is of type struct mutex,
which is a problem, because the suspend and resume routines
defined in drivers/base/power/clock_ops.c cannot be executed
with interrupts disabled for this reason. Modify
struct pm_subsys_data so that its lock member is a spinlock.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/base/power/clock_ops.c | 35 +++++++++++++++++++++--------------
drivers/base/power/common.c | 2 +-
include/linux/pm.h | 2 +-
3 files changed, 23 insertions(+), 16 deletions(-)
Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -438,7 +438,7 @@ struct pm_domain_data {
};
struct pm_subsys_data {
- struct mutex lock;
+ spinlock_t lock;
unsigned int refcount;
#ifdef CONFIG_PM_CLK
struct list_head clock_list;
Index: linux/drivers/base/power/clock_ops.c
===================================================================
--- linux.orig/drivers/base/power/clock_ops.c
+++ linux/drivers/base/power/clock_ops.c
@@ -43,6 +43,7 @@ int pm_clk_add(struct device *dev, const
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
if (!psd)
return -EINVAL;
@@ -63,9 +64,9 @@ int pm_clk_add(struct device *dev, const
}
}
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_add_tail(&ce->node, &psd->clock_list);
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
return 0;
}
@@ -109,11 +110,12 @@ void pm_clk_remove(struct device *dev, c
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
if (!psd)
return;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry(ce, &psd->clock_list, node) {
if (!con_id && !ce->con_id) {
@@ -127,7 +129,7 @@ void pm_clk_remove(struct device *dev, c
}
}
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
}
/**
@@ -169,16 +171,17 @@ void pm_clk_destroy(struct device *dev)
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce, *c;
+ unsigned long flags;
if (!psd)
return;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry_safe_reverse(ce, c, &psd->clock_list, node)
__pm_clk_remove(ce);
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
dev_pm_put_subsys_data(dev);
}
@@ -212,13 +215,14 @@ int pm_clk_suspend(struct device *dev)
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
dev_dbg(dev, "%s()\n", __func__);
if (!psd)
return 0;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry_reverse(ce, &psd->clock_list, node) {
if (ce->status == PCE_STATUS_NONE)
@@ -230,7 +234,7 @@ int pm_clk_suspend(struct device *dev)
}
}
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
return 0;
}
@@ -243,13 +247,14 @@ int pm_clk_resume(struct device *dev)
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
dev_dbg(dev, "%s()\n", __func__);
if (!psd)
return 0;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry(ce, &psd->clock_list, node) {
if (ce->status == PCE_STATUS_NONE)
@@ -261,7 +266,7 @@ int pm_clk_resume(struct device *dev)
}
}
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
return 0;
}
@@ -336,6 +341,7 @@ int pm_clk_suspend(struct device *dev)
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
dev_dbg(dev, "%s()\n", __func__);
@@ -343,12 +349,12 @@ int pm_clk_suspend(struct device *dev)
if (!psd || !dev->driver)
return 0;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry_reverse(ce, &psd->clock_list, node)
clk_disable(ce->clk);
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
return 0;
}
@@ -361,6 +367,7 @@ int pm_clk_resume(struct device *dev)
{
struct pm_subsys_data *psd = dev_to_psd(dev);
struct pm_clock_entry *ce;
+ unsigned long flags;
dev_dbg(dev, "%s()\n", __func__);
@@ -368,12 +375,12 @@ int pm_clk_resume(struct device *dev)
if (!psd || !dev->driver)
return 0;
- mutex_lock(&psd->lock);
+ spin_lock_irqsave(&psd->lock, flags);
list_for_each_entry(ce, &psd->clock_list, node)
clk_enable(ce->clk);
- mutex_unlock(&psd->lock);
+ spin_unlock_irqrestore(&psd->lock, flags);
return 0;
}
Index: linux/drivers/base/power/common.c
===================================================================
--- linux.orig/drivers/base/power/common.c
+++ linux/drivers/base/power/common.c
@@ -34,7 +34,7 @@ int dev_pm_get_subsys_data(struct device
if (dev->power.subsys_data) {
dev->power.subsys_data->refcount++;
} else {
- mutex_init(&psd->lock);
+ spin_lock_init(&psd->lock);
psd->refcount = 1;
dev->power.subsys_data = psd;
pm_clk_init(dev);
^ permalink raw reply
* [PATCH 0/2 v2] sh-sci / PM: Fix problem with runtime PM callbacks run with interrupts off
From: Rafael J. Wysocki @ 2011-08-21 19:09 UTC (permalink / raw)
To: linux-sh; +Cc: Linux PM mailing list, LKML
In-Reply-To: <201108202131.19479.rjw@sisk.pl>
Hi,
> The sh-sci driver uses pm_runtime_get/put_sync() in such a way
> that they may be run with interrupts off and cause the (recently
> added) might_sleep_if() to trigger in rpm_suspend/resume().
>
> To avoid that, it's necessary to set the SCI device's power.irq_safe
> flag to indicate that it's runtime PM callbacks may be executed with
> interrupts off safely. However, the sh-sci driver needs to be able to
> clear that flag sometimes, so a new runtime PM helper for doing that
> is needed.
The previous version of this patchset was not very good as Alan pointed out,
so hopefully this one will be better.
[1/2] - Change PM subsys_data lock type into spinlock.
[2/2] - Make sh-sci use power.irq_safe to indicate that runtime PM callbacks
may be run with interrupts off.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 1/2] PM / Runtime: Introduce pm_runtime_irq_unsafe()
From: Rafael J. Wysocki @ 2011-08-21 18:09 UTC (permalink / raw)
To: Alan Stern; +Cc: Linux PM mailing list, LKML, linux-sh
In-Reply-To: <Pine.LNX.4.44L0.1108211048060.12143-100000@netrider.rowland.org>
On Sunday, August 21, 2011, Alan Stern wrote:
> On Sat, 20 Aug 2011, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Add a helper function allowing drivers and subsystems to clear
> > the power.irq_safe device flag.
>
> > --- linux.orig/drivers/base/power/runtime.c
> > +++ linux/drivers/base/power/runtime.c
> > @@ -1109,22 +1109,23 @@ void pm_runtime_no_callbacks(struct devi
> > EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
> >
> > /**
> > - * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
> > + * __pm_runtime_irq_safe - Manipulate a device's power.irq_safe flag.
> > * @dev: Device to handle
> > + * @irq_safe: Whether or not to leave interrupts disabled during callbacks.
> > *
> > - * Set the power.irq_safe flag, which tells the PM core that the
> > + * Set or unset the power.irq_safe flag, which tells the PM core that the
> > * ->runtime_suspend() and ->runtime_resume() callbacks for this device should
> > * always be invoked with the spinlock held and interrupts disabled. It also
> > * causes the parent's usage counter to be permanently incremented, preventing
> > * the parent from runtime suspending -- otherwise an irq-safe child might have
> > * to wait for a non-irq-safe parent.
> > */
> > -void pm_runtime_irq_safe(struct device *dev)
> > +void __pm_runtime_irq_safe(struct device *dev, bool irq_safe)
> > {
> > if (dev->parent)
> > pm_runtime_get_sync(dev->parent);
> > spin_lock_irq(&dev->power.lock);
> > - dev->power.irq_safe = 1;
> > + dev->power.irq_safe = irq_safe;
> > spin_unlock_irq(&dev->power.lock);
>
> It's not quite this easy. There are two important aspects that must be
> considered.
>
> Firstly, I originally envisioned pm_runtime_irq_safe() being called
> just once, before the device is enabled for runtime PM. If you allow
> the flag to be turned on and off like this, you raise the possibility
> of races with runtime PM callbacks. (That is, if a callback occurs at
> about the same time as the irq_safe flag is changed, nobody can predict
> whether the callback will be invoked with interrupts enabled.) Maybe
> that's something the driver needs to take care of, but it should at
> least be mentioned in the documentation.
Good point. Perhaps I should make it possible only if runtime PM is
disabled.
> Secondly, this doesn't manage the parent's usage counter correctly.
Right, I forgot about that.
> Do the pm_runtime_get_sync(dev->parent) at the beginning only when the
> irq_safe flag was off and is being turned on. And at the end, if the
> irq_safe flag was on and is being turned off, do
> pm_runtime_put_sync(dev->parent). See pm_runtime_remove() for why this
> matters. (Also update the documentation; the change to the parent
> isn't necessarily permanent any more.)
I'll try to figure out an alternative approach without the $subject change
first. If I can't, I'll revisit this idea.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-21 18:05 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110821082509.GA10380@opensource.wolfsonmicro.com>
On Sunday, August 21, 2011, Mark Brown wrote:
> On Sat, Aug 20, 2011 at 09:14:37PM +0200, Rafael J. Wysocki wrote:
>
> > I guess you mean the driver here and I'm not really sure it can.
> > For instance, the driver may not know what configuration it works in,
> > e.g. is there a power domain or a hierarchy of those and how much time
> > it takes to power them all down and up and what the power break even is.
>
> I don't understand why the driver would need to know what situation it's
> in. I'd been working on the basis that the idea was that the driver
> said what the constraints it has are and then some code with a more
> system wide view would make the actual decisions for things outside the
> driver domian.
That's correct, but in order to figure out a "sensible default"
the driver generally would need to know what the expectations with
respect to it are. Otherwise it can very well generate a random
number and use that.
Thanks,
Rafael
^ permalink raw reply
* Re: [GIT PULL pm-next] freezer: fix various bugs and simplify implementation
From: Rafael J. Wysocki @ 2011-08-21 18:03 UTC (permalink / raw)
To: Tejun Heo
Cc: arnd, paul, lizf, linux-kernel, oleg, Martin Schwidefsky,
linux-pm
In-Reply-To: <20110821091200.GB24151@htj.dyndns.org>
On Sunday, August 21, 2011, Tejun Heo wrote:
> Hello, Rafafel.
>
> On Sat, Aug 20, 2011 at 06:33:38PM +0200, Rafael J. Wysocki wrote:
> > > ssh://master.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
> >
> > Pulled and stored in the pm-freezer branch in my tree, and merged into
> > the linux-next branch.
>
> Cool.
>
> > > FYI, this patchset will cause a conflict with s390 TIF flag fix patch.
> > > The conflict is trivial and Stephen should be able to handle it
> > > without any problem. Also, I'm planning on doing some further work on
> > > cgroup freezer and then will try to bridge it with job control. If
> > > that plan fans out, I might ask Oleg to pull from the pm tree.
> >
> > I'm not sure if Linus likes it. He generally doesn't want the trees
> > that he pulls from to be entangled this way.
>
> The job control portion has to go through Linus anyway, so let's see
> how that flies.
>
> > > This shouldn't matter too much either way but it *might* be a good idea to
> > > keep this line of patches in a separate branch.
> >
> > I'm going to keep it in the pm-freezer branch anyway (there may be patches
> > on top of it, though)
>
> Yeah, I'm pretty sure it will need some fix too.
Speaking of which, the addition of might_sleep() to try_to_freeze()
causes a badly looking backtrace to appear during reboot on ARM,
so I'd prefer it to go into __refrigerator().
Please tell me what you think of the patch below.
Rafael
---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / Freezer: Move might_sleep() from try_to_freeze()
There are some code paths that call try_to_freeze() from interrupt
context, but doing so they know that the current process cannot
possible be freezing (e.g. during reboot on ARM). However, the
recently added might_sleep() annotation in try_to_freeze()
triggers in those cases, making it look like there were bugs in
those places, which really isn't the case.
Therefore move might_sleep() from try_to_freeze() to
__refrigerator() so that it doesn't produce false positives.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/freezer.h | 1 -
kernel/freezer.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
Index: linux/include/linux/freezer.h
===================================================================
--- linux.orig/include/linux/freezer.h
+++ linux/include/linux/freezer.h
@@ -41,7 +41,6 @@ extern void thaw_processes(void);
static inline bool try_to_freeze(void)
{
- might_sleep();
if (likely(!freezing(current)))
return false;
return __refrigerator(false);
Index: linux/kernel/freezer.c
===================================================================
--- linux.orig/kernel/freezer.c
+++ linux/kernel/freezer.c
@@ -54,6 +54,8 @@ bool __refrigerator(bool check_kthr_stop
bool was_frozen = false;
long save;
+ might_sleep();
+
/*
* No point in checking freezing() again - the caller already did.
* Proceed to enter FROZEN.
^ permalink raw reply
* Re: [PATCH 1/2] PM / Runtime: Introduce pm_runtime_irq_unsafe()
From: Alan Stern @ 2011-08-21 14:55 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM mailing list, LKML, linux-sh
In-Reply-To: <201108202132.13304.rjw@sisk.pl>
On Sat, 20 Aug 2011, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Add a helper function allowing drivers and subsystems to clear
> the power.irq_safe device flag.
> --- linux.orig/drivers/base/power/runtime.c
> +++ linux/drivers/base/power/runtime.c
> @@ -1109,22 +1109,23 @@ void pm_runtime_no_callbacks(struct devi
> EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
>
> /**
> - * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
> + * __pm_runtime_irq_safe - Manipulate a device's power.irq_safe flag.
> * @dev: Device to handle
> + * @irq_safe: Whether or not to leave interrupts disabled during callbacks.
> *
> - * Set the power.irq_safe flag, which tells the PM core that the
> + * Set or unset the power.irq_safe flag, which tells the PM core that the
> * ->runtime_suspend() and ->runtime_resume() callbacks for this device should
> * always be invoked with the spinlock held and interrupts disabled. It also
> * causes the parent's usage counter to be permanently incremented, preventing
> * the parent from runtime suspending -- otherwise an irq-safe child might have
> * to wait for a non-irq-safe parent.
> */
> -void pm_runtime_irq_safe(struct device *dev)
> +void __pm_runtime_irq_safe(struct device *dev, bool irq_safe)
> {
> if (dev->parent)
> pm_runtime_get_sync(dev->parent);
> spin_lock_irq(&dev->power.lock);
> - dev->power.irq_safe = 1;
> + dev->power.irq_safe = irq_safe;
> spin_unlock_irq(&dev->power.lock);
It's not quite this easy. There are two important aspects that must be
considered.
Firstly, I originally envisioned pm_runtime_irq_safe() being called
just once, before the device is enabled for runtime PM. If you allow
the flag to be turned on and off like this, you raise the possibility
of races with runtime PM callbacks. (That is, if a callback occurs at
about the same time as the irq_safe flag is changed, nobody can predict
whether the callback will be invoked with interrupts enabled.) Maybe
that's something the driver needs to take care of, but it should at
least be mentioned in the documentation.
Secondly, this doesn't manage the parent's usage counter correctly.
Do the pm_runtime_get_sync(dev->parent) at the beginning only when the
irq_safe flag was off and is being turned on. And at the end, if the
irq_safe flag was on and is being turned off, do
pm_runtime_put_sync(dev->parent). See pm_runtime_remove() for why this
matters. (Also update the documentation; the change to the parent
isn't necessarily permanent any more.)
Alan Stern
^ permalink raw reply
* Re: [GIT PULL pm-next] freezer: fix various bugs and simplify implementation
From: Tejun Heo @ 2011-08-21 9:12 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: arnd, paul, lizf, linux-kernel, oleg, Martin Schwidefsky,
linux-pm
In-Reply-To: <201108201833.39089.rjw@sisk.pl>
Hello, Rafafel.
On Sat, Aug 20, 2011 at 06:33:38PM +0200, Rafael J. Wysocki wrote:
> > ssh://master.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
>
> Pulled and stored in the pm-freezer branch in my tree, and merged into
> the linux-next branch.
Cool.
> > FYI, this patchset will cause a conflict with s390 TIF flag fix patch.
> > The conflict is trivial and Stephen should be able to handle it
> > without any problem. Also, I'm planning on doing some further work on
> > cgroup freezer and then will try to bridge it with job control. If
> > that plan fans out, I might ask Oleg to pull from the pm tree.
>
> I'm not sure if Linus likes it. He generally doesn't want the trees
> that he pulls from to be entangled this way.
The job control portion has to go through Linus anyway, so let's see
how that flies.
> > This shouldn't matter too much either way but it *might* be a good idea to
> > keep this line of patches in a separate branch.
>
> I'm going to keep it in the pm-freezer branch anyway (there may be patches
> on top of it, though)
Yeah, I'm pretty sure it will need some fix too.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Mark Brown @ 2011-08-21 8:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <201108202114.37502.rjw@sisk.pl>
On Sat, Aug 20, 2011 at 09:14:37PM +0200, Rafael J. Wysocki wrote:
> I guess you mean the driver here and I'm not really sure it can.
> For instance, the driver may not know what configuration it works in,
> e.g. is there a power domain or a hierarchy of those and how much time
> it takes to power them all down and up and what the power break even is.
I don't understand why the driver would need to know what situation it's
in. I'd been working on the basis that the idea was that the driver
said what the constraints it has are and then some code with a more
system wide view would make the actual decisions for things outside the
driver domian.
^ permalink raw reply
* [PATCH] Add regulator driver for the bq2407x family of charger ICs
From: Heiko Stübner @ 2011-08-20 20:24 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: linux-pm
This driver controls a TI bq2407x charger attached via GPIOs.
The provided current regulator can enable/disable charging and
select between 100 mA, 500 mA and a machine specific current limit.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/regulator/Kconfig | 8 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/bq2407x.c | 205 +++++++++++++++++++++++++++++++++++++
include/linux/regulator/bq2407x.h | 36 +++++++
4 files changed, 250 insertions(+), 0 deletions(-)
create mode 100644 drivers/regulator/bq2407x.c
create mode 100644 include/linux/regulator/bq2407x.h
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c7fd2c0..921e271 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -72,6 +72,14 @@ config REGULATOR_BQ24022
charging select between 100 mA and 500 mA charging current
limit.
+config REGULATOR_BQ2407x
+ tristate "TI bq2407x Li-Ion Charger IC"
+ help
+ This driver controls a TI bq2407x Charger attached via
+ GPIOs. The provided current regulator can enable/disable
+ charging select between 100 mA, 500 mA and a machine specific
+ charging current limit.
+
config REGULATOR_MAX1586
tristate "Maxim 1586/1587 voltage regulator"
depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 040d5aa..ce65493 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
obj-$(CONFIG_REGULATOR_BQ24022) += bq24022.o
+obj-$(CONFIG_REGULATOR_BQ2407x) += bq2407x.o
obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
diff --git a/drivers/regulator/bq2407x.c b/drivers/regulator/bq2407x.c
new file mode 100644
index 0000000..2338540
--- /dev/null
+++ b/drivers/regulator/bq2407x.c
@@ -0,0 +1,205 @@
+/*
+ * Support for TI bq2407x USB-friendly
+ * Li-Ion Charger connected via GPIOs.
+ *
+ * Copyright (c) 2011 Heiko Stuebner
+ *
+ * based on the bq24022 driver
+ * Copyright (c) 2008 Philipp Zabel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/regulator/bq2407x.h>
+#include <linux/regulator/driver.h>
+
+
+static int bq2407x_set_current_limit(struct regulator_dev *rdev,
+ int min_uA, int max_uA)
+{
+ struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
+
+ if (pdata->max_uA && pdata->max_uA > 500000
+ && max_uA >= pdata->max_uA) {
+ dev_dbg(rdev_get_dev(rdev),
+ "setting current limit to %d mA\n",
+ pdata->max_uA / 1000);
+ gpio_set_value(pdata->gpio_en2, 1);
+ gpio_set_value(pdata->gpio_en1, 0);
+ } else if (max_uA >= 500000) {
+ dev_dbg(rdev_get_dev(rdev),
+ "setting current limit to 500 mA\n");
+ gpio_set_value(pdata->gpio_en2, 0);
+ gpio_set_value(pdata->gpio_en1, 1);
+ } else if (max_uA >= 100000) {
+ dev_dbg(rdev_get_dev(rdev),
+ "setting current limit to 100 mA\n");
+ gpio_set_value(pdata->gpio_en2, 0);
+ gpio_set_value(pdata->gpio_en1, 0);
+ } else {
+ dev_dbg(rdev_get_dev(rdev),
+ "setting current limit to 0 mA\n");
+ gpio_set_value(pdata->gpio_en2, 1);
+ gpio_set_value(pdata->gpio_en1, 1);
+ }
+
+ /* REVISIT: maybe return error if min_uA != 0 ? */
+ return 0;
+}
+
+static int bq2407x_get_current_limit(struct regulator_dev *rdev)
+{
+ struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
+
+ int en2 = gpio_get_value(pdata->gpio_en2);
+ int en1 = gpio_get_value(pdata->gpio_en1);
+
+ if (en2 && en1)
+ return 0;
+ else if (en2 && !en1)
+ return pdata->max_uA;
+ else if (!en2 && en1)
+ return 500000;
+ else
+ return 100000;
+}
+
+static int bq2407x_enable(struct regulator_dev *rdev)
+{
+ struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
+
+ dev_dbg(rdev_get_dev(rdev), "enabling charger\n");
+
+ gpio_set_value(pdata->gpio_nce, 0);
+ return 0;
+}
+
+static int bq2407x_disable(struct regulator_dev *rdev)
+{
+ struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
+
+ dev_dbg(rdev_get_dev(rdev), "disabling charger\n");
+
+ gpio_set_value(pdata->gpio_nce, 1);
+ return 0;
+}
+
+static int bq2407x_is_enabled(struct regulator_dev *rdev)
+{
+ struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
+
+ return !gpio_get_value(pdata->gpio_nce);
+}
+
+static struct regulator_ops bq2407x_ops = {
+ .set_current_limit = bq2407x_set_current_limit,
+ .get_current_limit = bq2407x_get_current_limit,
+ .enable = bq2407x_enable,
+ .disable = bq2407x_disable,
+ .is_enabled = bq2407x_is_enabled,
+};
+
+static struct regulator_desc bq2407x_desc = {
+ .name = "bq2407x",
+ .ops = &bq2407x_ops,
+ .type = REGULATOR_CURRENT,
+ .owner = THIS_MODULE,
+};
+
+static int __init bq2407x_probe(struct platform_device *pdev)
+{
+ struct bq2407x_mach_info *pdata = pdev->dev.platform_data;
+ struct regulator_dev *bq2407x;
+ int ret;
+
+ if (!pdata || !pdata->gpio_nce || !pdata->gpio_en1 || !pdata->gpio_en2)
+ return -EINVAL;
+
+ ret = gpio_request(pdata->gpio_nce, "ncharge_en");
+ if (ret) {
+ dev_dbg(&pdev->dev, "couldn't request nCE GPIO: %d\n",
+ pdata->gpio_nce);
+ goto err_ce;
+ }
+ ret = gpio_request(pdata->gpio_en2, "charge_mode_en2");
+ if (ret) {
+ dev_dbg(&pdev->dev, "couldn't request EN2 GPIO: %d\n",
+ pdata->gpio_en2);
+ goto err_en2;
+ }
+ ret = gpio_request(pdata->gpio_en1, "charge_mode_en1");
+ if (ret) {
+ dev_dbg(&pdev->dev, "couldn't request EN1 GPIO: %d\n",
+ pdata->gpio_en1);
+ goto err_en1;
+ }
+ ret = gpio_direction_output(pdata->gpio_en2, 0);
+ ret = gpio_direction_output(pdata->gpio_en1, 0);
+ ret = gpio_direction_output(pdata->gpio_nce, 1);
+
+ bq2407x = regulator_register(&bq2407x_desc, &pdev->dev,
+ pdata->init_data, pdata);
+ if (IS_ERR(bq2407x)) {
+ dev_dbg(&pdev->dev, "couldn't register regulator\n");
+ ret = PTR_ERR(bq2407x);
+ goto err_reg;
+ }
+ platform_set_drvdata(pdev, bq2407x);
+ dev_dbg(&pdev->dev, "registered regulator\n");
+
+ return 0;
+err_reg:
+ gpio_free(pdata->gpio_en1);
+err_en1:
+ gpio_free(pdata->gpio_en2);
+err_en2:
+ gpio_free(pdata->gpio_nce);
+err_ce:
+ return ret;
+}
+
+static int __devexit bq2407x_remove(struct platform_device *pdev)
+{
+ struct bq2407x_mach_info *pdata = pdev->dev.platform_data;
+ struct regulator_dev *bq2407x = platform_get_drvdata(pdev);
+
+ regulator_unregister(bq2407x);
+ gpio_free(pdata->gpio_en1);
+ gpio_free(pdata->gpio_en2);
+ gpio_free(pdata->gpio_nce);
+
+ return 0;
+}
+
+static struct platform_driver bq2407x_driver = {
+ .driver = {
+ .name = "bq2407x",
+ },
+ .remove = __devexit_p(bq2407x_remove),
+};
+
+static int __init bq2407x_init(void)
+{
+ return platform_driver_probe(&bq2407x_driver, bq2407x_probe);
+}
+
+static void __exit bq2407x_exit(void)
+{
+ platform_driver_unregister(&bq2407x_driver);
+}
+
+module_init(bq2407x_init);
+module_exit(bq2407x_exit);
+
+MODULE_AUTHOR("Heiko Stuebner");
+MODULE_DESCRIPTION("TI bq2407x Li-Ion Charger driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/bq2407x.h b/include/linux/regulator/bq2407x.h
new file mode 100644
index 0000000..8145150
--- /dev/null
+++ b/include/linux/regulator/bq2407x.h
@@ -0,0 +1,36 @@
+/*
+ * Support for TI bq2407x 1.5A USB-friendly
+ * Li-Ion Charger connected via GPIOs.
+ *
+ * Copyright (c) 2011 Heiko Stuebner
+ *
+ * based on the bq24022 driver
+ * Copyright (c) 2008 Philipp Zabel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+struct regulator_init_data;
+
+/**
+ * bq2407x_mach_info - platform data for bq2407x
+ * @gpio_nce: GPIO line connected to the nCE pin, used to control charging
+ * @gpio_en2: GPIO line connected to the EN2 pin, used to limit charging
+ * @gpio_en1: GPIO line connected to the EN1 pin, used to limit charging
+ * @max_uA: maximum current defined by resistor on ILIM connector
+ * Modes of operation:
+ * EN2 = 0, EN1 = 0: 100mA
+ * EN2 = 0, EN1 = 1: 500mA
+ * EN2 = 1, EN1 = 0: max_current
+ * EN2 = 1, EN1 = 1: Standby (usb suspend)
+ */
+struct bq2407x_mach_info {
+ int gpio_nce;
+ int gpio_en2;
+ int gpio_en1;
+ int max_uA;
+ struct regulator_init_data *init_data;
+};
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/2] sh-sci / PM: Use power.irq_safe
From: Rafael J. Wysocki @ 2011-08-20 19:33 UTC (permalink / raw)
To: linux-sh; +Cc: Linux PM mailing list, LKML
In-Reply-To: <201108202131.19479.rjw@sisk.pl>
From: Rafael J. Wysocki <rjw@sisk.pl>
Since sci_port_enable() and sci_port_disable() may be run with
interrupts off and they execute pm_runtime_get_sync() and
pm_runtime_put_sync(), respectively, the SCI device's
power.irq_safe flags has to be used to indicate that it is safe
to execute runtime PM callbacks for this device with interrupts off.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/tty/serial/sh-sci.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index: linux/drivers/tty/serial/sh-sci.c
===================================================================
--- linux.orig/drivers/tty/serial/sh-sci.c
+++ linux/drivers/tty/serial/sh-sci.c
@@ -1582,11 +1582,15 @@ static int sci_startup(struct uart_port
dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
+ pm_runtime_irq_safe(port->dev);
+
sci_port_enable(s);
ret = sci_request_irq(s);
- if (unlikely(ret < 0))
+ if (unlikely(ret < 0)) {
+ pm_runtime_irq_unsafe(port->dev);
return ret;
+ }
sci_request_dma(port);
@@ -1609,6 +1613,8 @@ static void sci_shutdown(struct uart_por
sci_free_irq(s);
sci_port_disable(s);
+
+ pm_runtime_irq_unsafe(port->dev);
}
static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
^ permalink raw reply
* [PATCH 1/2] PM / Runtime: Introduce pm_runtime_irq_unsafe()
From: Rafael J. Wysocki @ 2011-08-20 19:32 UTC (permalink / raw)
To: linux-sh; +Cc: Linux PM mailing list, LKML
In-Reply-To: <201108202131.19479.rjw@sisk.pl>
From: Rafael J. Wysocki <rjw@sisk.pl>
Add a helper function allowing drivers and subsystems to clear
the power.irq_safe device flag.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
Documentation/power/runtime_pm.txt | 4 ++++
drivers/base/power/runtime.c | 9 +++++----
include/linux/pm_runtime.h | 13 ++++++++++++-
3 files changed, 21 insertions(+), 5 deletions(-)
Index: linux/include/linux/pm_runtime.h
===================================================================
--- linux.orig/include/linux/pm_runtime.h
+++ linux/include/linux/pm_runtime.h
@@ -40,7 +40,7 @@ extern int pm_generic_runtime_idle(struc
extern int pm_generic_runtime_suspend(struct device *dev);
extern int pm_generic_runtime_resume(struct device *dev);
extern void pm_runtime_no_callbacks(struct device *dev);
-extern void pm_runtime_irq_safe(struct device *dev);
+extern void __pm_runtime_irq_safe(struct device *dev, bool irq_safe);
extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
@@ -102,6 +102,16 @@ static inline void pm_runtime_mark_last_
ACCESS_ONCE(dev->power.last_busy) = jiffies;
}
+static inline void pm_runtime_irq_safe(struct device *dev)
+{
+ __pm_runtime_irq_safe(dev, true);
+}
+
+static inline void pm_runtime_irq_unsafe(struct device *dev)
+{
+ __pm_runtime_irq_safe(dev, false);
+}
+
#else /* !CONFIG_PM_RUNTIME */
static inline int __pm_runtime_idle(struct device *dev, int rpmflags)
@@ -143,6 +153,7 @@ static inline int pm_generic_runtime_sus
static inline int pm_generic_runtime_resume(struct device *dev) { return 0; }
static inline void pm_runtime_no_callbacks(struct device *dev) {}
static inline void pm_runtime_irq_safe(struct device *dev) {}
+static inline void pm_runtime_irq_unsafe(struct device *dev) {}
static inline bool pm_runtime_callbacks_present(struct device *dev) { return false; }
static inline void pm_runtime_mark_last_busy(struct device *dev) {}
Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -1109,22 +1109,23 @@ void pm_runtime_no_callbacks(struct devi
EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
/**
- * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
+ * __pm_runtime_irq_safe - Manipulate a device's power.irq_safe flag.
* @dev: Device to handle
+ * @irq_safe: Whether or not to leave interrupts disabled during callbacks.
*
- * Set the power.irq_safe flag, which tells the PM core that the
+ * Set or unset the power.irq_safe flag, which tells the PM core that the
* ->runtime_suspend() and ->runtime_resume() callbacks for this device should
* always be invoked with the spinlock held and interrupts disabled. It also
* causes the parent's usage counter to be permanently incremented, preventing
* the parent from runtime suspending -- otherwise an irq-safe child might have
* to wait for a non-irq-safe parent.
*/
-void pm_runtime_irq_safe(struct device *dev)
+void __pm_runtime_irq_safe(struct device *dev, bool irq_safe)
{
if (dev->parent)
pm_runtime_get_sync(dev->parent);
spin_lock_irq(&dev->power.lock);
- dev->power.irq_safe = 1;
+ dev->power.irq_safe = irq_safe;
spin_unlock_irq(&dev->power.lock);
}
EXPORT_SYMBOL_GPL(pm_runtime_irq_safe);
Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -434,6 +434,10 @@ drivers/base/power/runtime.c and include
suspend and resume callbacks (but not the idle callback) to be invoked
with interrupts disabled
+ void pm_runtime_irq_unsafe(struct device *dev);
+ - clear the power.irq_safe flag for the device, causing the runtime-PM
+ callbacks to be invoked with interrupts enabled
+
void pm_runtime_mark_last_busy(struct device *dev);
- set the power.last_busy field to the current time
^ permalink raw reply
* [PATCH 0/2] sh-sci / PM: Fix problem with runtime PM callbacks run with interrupts off
From: Rafael J. Wysocki @ 2011-08-20 19:31 UTC (permalink / raw)
To: linux-sh; +Cc: Linux PM mailing list, LKML
Hi,
The sh-sci driver uses pm_runtime_get/put_sync() in such a way
that they may be run with interrupts off and cause the (recently
added) might_sleep_if() to trigger in rpm_suspend/resume().
To avoid that, it's necessary to set the SCI device's power.irq_safe
flag to indicate that it's runtime PM callbacks may be executed with
interrupts off safely. However, the sh-sci driver needs to be able to
clear that flag sometimes, so a new runtime PM helper for doing that
is needed.
[1/2] - Add pm_runtime_irq_unsafe() for clearing the power.irq_safe device flag.
[2/2] - Make sh-sci use power.irq_safe to indicate that runtime PM callbacks
may be run with interrupts off.
Thanks,
Rafael
^ permalink raw reply
* [PATCH] PM / Domains: Preliminary support for devices with power.irq_safe set
From: Rafael J. Wysocki @ 2011-08-20 19:24 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: LKML, linux-sh
From: Rafael J. Wysocki <rjw@sisk.pl>
The generic PM domains framework currently doesn't work with devices
whose power.irq_safe flag is set, because runtime PM callbacks for
such devices are run with interrupts disabled and the callbacks
provided by the generic PM domains framework use domain mutexes
and may sleep. However, such devices very well may belong to
power domains on some systems, so the generic PM domains framework
should take them into account.
For this reason, make modify the generic PM domains framework so
that the domain .power_off() and .power_on() callbacks are never
executed for a domain containing devices with power.irq_safe set,
although the .stop_device() and .start_device() callbacks are
still run for them.
Additionally, introduce a flag allowing the creator of a
struct generic_pm_domain object to indicate that its .stop_device()
and .start_device() callbacks may be run in interrupt context
(might_sleep_if() triggers if that flag is not set and one of those
callbacks is run in interrupt context).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
arch/arm/mach-shmobile/pm-sh7372.c | 1 +
drivers/base/power/domain.c | 19 ++++++++++++++++++-
include/linux/pm_domain.h | 1 +
3 files changed, 20 insertions(+), 1 deletion(-)
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -309,7 +309,8 @@ static int pm_genpd_poweroff(struct gene
not_suspended = 0;
list_for_each_entry(pdd, &genpd->dev_list, list_node)
- if (pdd->dev->driver && !pm_runtime_suspended(pdd->dev))
+ if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
+ || pdd->dev->power.irq_safe))
not_suspended++;
if (not_suspended > genpd->in_progress)
@@ -417,12 +418,21 @@ static int pm_genpd_runtime_suspend(stru
if (IS_ERR(genpd))
return -EINVAL;
+ might_sleep_if(!genpd->dev_irq_safe);
+
if (genpd->stop_device) {
int ret = genpd->stop_device(dev);
if (ret)
return ret;
}
+ /*
+ * If power.irq_safe is set, this routine will be run with interrupts
+ * off, so it can't use mutexes.
+ */
+ if (dev->power.irq_safe)
+ return 0;
+
mutex_lock(&genpd->lock);
genpd->in_progress++;
pm_genpd_poweroff(genpd);
@@ -452,6 +462,12 @@ static int pm_genpd_runtime_resume(struc
if (IS_ERR(genpd))
return -EINVAL;
+ might_sleep_if(!genpd->dev_irq_safe);
+
+ /* If power.irq_safe, the PM domain is never powered off. */
+ if (dev->power.irq_safe)
+ goto out;
+
mutex_lock(&genpd->lock);
ret = __pm_genpd_poweron(genpd);
if (ret) {
@@ -483,6 +499,7 @@ static int pm_genpd_runtime_resume(struc
wake_up_all(&genpd->status_wait_queue);
mutex_unlock(&genpd->lock);
+ out:
if (genpd->start_device)
genpd->start_device(dev);
Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -42,6 +42,7 @@ struct generic_pm_domain {
unsigned int suspended_count; /* System suspend device counter */
unsigned int prepared_count; /* Suspend counter of prepared devices */
bool suspend_power_off; /* Power status before system suspend */
+ bool dev_irq_safe; /* Device callbacks are IRQ-safe */
int (*power_off)(struct generic_pm_domain *domain);
int (*power_on)(struct generic_pm_domain *domain);
int (*start_device)(struct device *dev);
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -103,6 +103,7 @@ void sh7372_init_pm_domain(struct sh7372
pm_genpd_init(genpd, NULL, false);
genpd->stop_device = pm_clk_suspend;
genpd->start_device = pm_clk_resume;
+ genpd->dev_irq_safe = true;
genpd->active_wakeup = pd_active_wakeup;
genpd->power_off = pd_power_down;
genpd->power_on = pd_power_up;
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-20 19:18 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110820172237.GB27040@opensource.wolfsonmicro.com>
On Saturday, August 20, 2011, Mark Brown wrote:
> On Sat, Aug 20, 2011 at 06:51:42PM +0200, Rafael J. Wysocki wrote:
> > On Saturday, August 20, 2011, Mark Brown wrote:
>
> > > Coming at this from the embedded perspective modifying the kernel just
> > > isn't an issue. It's quite depressing in other cases too but some of
> > > the circumstances you mentioned in previous messages are sensible do
> > > make sense to me. It does seem like this is a system specific problem
> > > which we should be able to enable as part of the support for those
> > > systems.
>
> > I don't think that's platform-specific in general. For example, there are
> > devices that don't really belong to any media-streaming-alike framework
> > and we may (and probably will) want to use PM QoS with them, because they
> > may be included in PM domains and influence power management of other
> > devices. Think of a serial console.
>
> Using PM QoS doesn't seem platform specific of course. Having userspace
> need to go in and do per-device overrides in order to get things working
> does.
>
> > > > without modifying the kernel. Also, it will help to test and debug new drivers
> > > > and subsystems.
>
> > > If it were a debugfs facility I'd not be concerned.
>
> > If that's going to be per-device, it really is much easier to put it into
> > sysfs (we already have per-device PM debug interfaces in there). It may
> > depend on CONFIG_PM_ADVANCED_DEBUG or something like this, though, at least
> > to start with.
>
> Yeah, the debugfs device attachment stuff is slightly annoying but it's
> fairly straightforward to create an appropriate heirachy - there's
> several subsystems doing that sort of stuff by using dev_name().
I'm not a big fan of that, sorry. Besides, as I said, we already have
debug PM interfaces in sysfs, so I don't see the reason not to add
another one.
> > > That's not really a problem - if people are adding their own crazy
> > > interfaces it's clear that they've done that. It'll show up as a red
> > > flag to anyone looking at their stuff and this will create pressure on
> > > them to fix their code or at least do a better job for the next thing.
>
> > > The goal isn't to tie people's hands to stop them doing silly things,
> > > it's to make it clear that that is what they're doing.
>
> > The same applies to using kernel interfaces. If someone uses a sane
> > interface for doing crazy stuff, that's their problem and should show
> > up as a red flag just as well.
>
> The big problem I'm seeing here is that there's nothing about having
> userspace do all the knob twiddling that looks crazy just from looking
> at the system. The controls are there and in the standard kernel
> interface, it's not like there's any ABIs or large piles of in kernel
> code that need to be added (if anything the in kernel code should look
> simpler as there's less going on).
Well, I'm kind of not seeing that as a big problem. At least, this is not
a technical issue, but a social one.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-20 19:14 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110820170403.GA27040@opensource.wolfsonmicro.com>
On Saturday, August 20, 2011, Mark Brown wrote:
> On Sat, Aug 20, 2011 at 06:34:34PM +0200, Rafael J. Wysocki wrote:
> > On Saturday, August 20, 2011, Mark Brown wrote:
>
> > > Any sort of media streaming would be an obvious example - the
> > > application picks the amount of data it buffers and how often it's
> > > notified of progress depending on the usage which then controls how
> > > quickly the system needs to handle things.
>
> > Well, what about other types of devices?
>
> Other than the input case (which is a latency issue - there's two
> components, one is how much data is delivered for things like
> touchscreens which stream and the other is how quickly the first data is
> delivered) nothing immediately springs to mind but this may just be a
> product of what I'm most familiar with. I don't really see this as a
> problem, for a lot of devices it's probably the case that the device can
> figure out something sensible to do without any help.
I guess you mean the driver here and I'm not really sure it can.
For instance, the driver may not know what configuration it works in,
e.g. is there a power domain or a hierarchy of those and how much time
it takes to power them all down and up and what the power break even is.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Mark Brown @ 2011-08-20 17:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <201108201851.42839.rjw@sisk.pl>
On Sat, Aug 20, 2011 at 06:51:42PM +0200, Rafael J. Wysocki wrote:
> On Saturday, August 20, 2011, Mark Brown wrote:
> > Coming at this from the embedded perspective modifying the kernel just
> > isn't an issue. It's quite depressing in other cases too but some of
> > the circumstances you mentioned in previous messages are sensible do
> > make sense to me. It does seem like this is a system specific problem
> > which we should be able to enable as part of the support for those
> > systems.
> I don't think that's platform-specific in general. For example, there are
> devices that don't really belong to any media-streaming-alike framework
> and we may (and probably will) want to use PM QoS with them, because they
> may be included in PM domains and influence power management of other
> devices. Think of a serial console.
Using PM QoS doesn't seem platform specific of course. Having userspace
need to go in and do per-device overrides in order to get things working
does.
> > > without modifying the kernel. Also, it will help to test and debug new drivers
> > > and subsystems.
> > If it were a debugfs facility I'd not be concerned.
> If that's going to be per-device, it really is much easier to put it into
> sysfs (we already have per-device PM debug interfaces in there). It may
> depend on CONFIG_PM_ADVANCED_DEBUG or something like this, though, at least
> to start with.
Yeah, the debugfs device attachment stuff is slightly annoying but it's
fairly straightforward to create an appropriate heirachy - there's
several subsystems doing that sort of stuff by using dev_name().
> > That's not really a problem - if people are adding their own crazy
> > interfaces it's clear that they've done that. It'll show up as a red
> > flag to anyone looking at their stuff and this will create pressure on
> > them to fix their code or at least do a better job for the next thing.
> > The goal isn't to tie people's hands to stop them doing silly things,
> > it's to make it clear that that is what they're doing.
> The same applies to using kernel interfaces. If someone uses a sane
> interface for doing crazy stuff, that's their problem and should show
> up as a red flag just as well.
The big problem I'm seeing here is that there's nothing about having
userspace do all the knob twiddling that looks crazy just from looking
at the system. The controls are there and in the standard kernel
interface, it's not like there's any ABIs or large piles of in kernel
code that need to be added (if anything the in kernel code should look
simpler as there's less going on).
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Mark Brown @ 2011-08-20 17:04 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <201108201834.34741.rjw@sisk.pl>
On Sat, Aug 20, 2011 at 06:34:34PM +0200, Rafael J. Wysocki wrote:
> On Saturday, August 20, 2011, Mark Brown wrote:
> > Any sort of media streaming would be an obvious example - the
> > application picks the amount of data it buffers and how often it's
> > notified of progress depending on the usage which then controls how
> > quickly the system needs to handle things.
> Well, what about other types of devices?
Other than the input case (which is a latency issue - there's two
components, one is how much data is delivered for things like
touchscreens which stream and the other is how quickly the first data is
delivered) nothing immediately springs to mind but this may just be a
product of what I'm most familiar with. I don't really see this as a
problem, for a lot of devices it's probably the case that the device can
figure out something sensible to do without any help.
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-20 16:51 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110820103121.GA21457@opensource.wolfsonmicro.com>
On Saturday, August 20, 2011, Mark Brown wrote:
> On Sat, Aug 20, 2011 at 11:35:58AM +0200, Rafael J. Wysocki wrote:
>
> > I'm not sure what you mean by "exposing per-device wakeup latency constraints
> > to user space". I simply thought it might be useful to allow user space to
> > add and remove those, in analogy to what it can do with the currently available
> > PM QoS constraints.
>
> I linked to the mails from Kevin... To be honest I'm rather surprised
> this stuff is getting exposed directly to userspace with write support.
>
> > My point is that this won't prevent kernel subsystems from adding their
> > own PM QoS constraints to devices and the constraints added by user space
> > will only have effect if they make the devices stay active for longer
> > times. In consequence, they may only increase the energy usage of the system
> > and shouldn't affect functionality.
>
> Well, clearly that's not the only use - demand for increased energy
> consumption by itself from users is generally quite low :)
>
> > The reason why I think this may be useful is that I can readily imagine
> > scenarios in which user space (think a distribution or system vendor) will
> > want to do something that hasn't been anticipated by kernel developers
> > and the ability to add per-device PM QoS constraints will allow it to do that
>
> Coming at this from the embedded perspective modifying the kernel just
> isn't an issue. It's quite depressing in other cases too but some of
> the circumstances you mentioned in previous messages are sensible do
> make sense to me. It does seem like this is a system specific problem
> which we should be able to enable as part of the support for those
> systems.
I don't think that's platform-specific in general. For example, there are
devices that don't really belong to any media-streaming-alike framework
and we may (and probably will) want to use PM QoS with them, because they
may be included in PM domains and influence power management of other
devices. Think of a serial console.
> > without modifying the kernel. Also, it will help to test and debug new drivers
> > and subsystems.
>
> If it were a debugfs facility I'd not be concerned.
If that's going to be per-device, it really is much easier to put it into
sysfs (we already have per-device PM debug interfaces in there). It may
depend on CONFIG_PM_ADVANCED_DEBUG or something like this, though, at least
to start with.
> > I don't want to prevent kernel subsystems from using their own interfaces to
> > acquire user-level input that translates into PM QoS constraints. I simply
> > think it won't _hurt_ to provide user space with an additional level of
> > control over per-device PM QoS constraints.
>
> My experience has been that once you start providing an interface people
> will find it, start using it and it takes a lot of work to convince them
> that they ought to do something better. This is totally reasonable from
> when you look at things from their point of view, especially people who
> aren't used to being able to improve anything except their own driver.
There are ways to make people do right things. :-)
Anyway, if an interface is provided, people are free to use it and it
kind of is their problem if they do that for unreasonable things. That
might be an issue for us if we were to remove that interface going forward,
but I'm not sure if that's going to ever happen.
> > > I'm additionally concerned that if we expose this stuff directly to userspace
> > > that's an open invitation to driver authors to not even bother trying to make
> > > the kernel figure this stuff out by itself and to instead tie the system
> > > together with magic userspace.
>
> > As I said, I don't think we can prevent that from happening anyway.
> > Worst case people will add strange interfaces to drivers making them
> > incompatible with anything except for their crazy user space.
>
> That's not really a problem - if people are adding their own crazy
> interfaces it's clear that they've done that. It'll show up as a red
> flag to anyone looking at their stuff and this will create pressure on
> them to fix their code or at least do a better job for the next thing.
>
> The goal isn't to tie people's hands to stop them doing silly things,
> it's to make it clear that that is what they're doing.
The same applies to using kernel interfaces. If someone uses a sane
interface for doing crazy stuff, that's their problem and should show
up as a red flag just as well.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-20 16:34 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110820152959.GA22424@opensource.wolfsonmicro.com>
On Saturday, August 20, 2011, Mark Brown wrote:
> On Sat, Aug 20, 2011 at 09:48:25AM -0400, Alan Stern wrote:
>
> > No, I as wasking about driver- and subsystem-specific interfaces to
> > userspace that translate into things users are already doing. Kevin's
> > example was a touchscreen (although that was really an example of
> > setting a power usage level, not a latency constraint). Do you have
> > any others?
>
> Any sort of media streaming would be an obvious example - the
> application picks the amount of data it buffers and how often it's
> notified of progress depending on the usage which then controls how
> quickly the system needs to handle things.
Well, what about other types of devices?
^ permalink raw reply
* Re: [GIT PULL pm-next] freezer: fix various bugs and simplify implementation
From: Rafael J. Wysocki @ 2011-08-20 16:33 UTC (permalink / raw)
To: Tejun Heo
Cc: arnd, paul, lizf, linux-kernel, oleg, Martin Schwidefsky,
linux-pm
In-Reply-To: <20110820094434.GA24151@htj.dyndns.org>
Hi,
On Saturday, August 20, 2011, Tejun Heo wrote:
> Hello, Rafael.
>
> Please consider pulling from the following branch to receive freezer
> fixes and simplifications patchset[1]. The only changes from the
> posting are addition of acked-by's, minor patch description updates
> and rebase on top of pm-next (af0fa9cb79 "PM / Hibernate: Include
> storage keys in hibernation image on s390").
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
>
> The HEAD should be 7b5b95b3f51a28ec008a295e5247436637220f41. If
> git.korg doesn't show it yet. Please pull from master.korg.
>
> ssh://master.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
Pulled and stored in the pm-freezer branch in my tree, and merged into
the linux-next branch.
> FYI, this patchset will cause a conflict with s390 TIF flag fix patch.
> The conflict is trivial and Stephen should be able to handle it
> without any problem. Also, I'm planning on doing some further work on
> cgroup freezer and then will try to bridge it with job control. If
> that plan fans out, I might ask Oleg to pull from the pm tree.
I'm not sure if Linus likes it. He generally doesn't want the trees
that he pulls from to be entangled this way.
> This shouldn't matter too much either way but it *might* be a good idea to
> keep this line of patches in a separate branch.
I'm going to keep it in the pm-freezer branch anyway (there may be patches
on top of it, though)
Thanks,
Rafael
> [1] http://thread.gmane.org/gmane.linux.kernel/1181594
>
> Tejun Heo (16):
> freezer: fix current->state restoration race in refrigerator()
> freezer: don't unnecessarily set PF_NOFREEZE explicitly
> freezer: unexport refrigerator() and update try_to_freeze() slightly
> freezer: implement and use kthread_freezable_should_stop()
> freezer: rename thaw_process() to __thaw_task() and simplify the implementation
> freezer: make exiting tasks properly unfreezable
> freezer: don't distinguish nosig tasks on thaw
> freezer: use dedicated lock instead of task_lock() + memory barrier
> freezer: make freezing indicate freeze condition in effect
> freezer: fix set_freezable[_with_signal]() race
> freezer: kill PF_FREEZING
> freezer: clean up freeze_processes() failure path
> cgroup_freezer: prepare for removal of TIF_FREEZE
> freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE
> freezer: remove now unused TIF_FREEZE
> freezer: remove should_send_signal() and update frozen()
>
> Documentation/power/freezing-of-tasks.txt | 14 +-
> arch/alpha/include/asm/thread_info.h | 2 -
> arch/arm/include/asm/thread_info.h | 2 -
> arch/avr32/include/asm/thread_info.h | 2 -
> arch/blackfin/include/asm/thread_info.h | 2 -
> arch/cris/include/asm/thread_info.h | 2 -
> arch/frv/include/asm/thread_info.h | 2 -
> arch/h8300/include/asm/thread_info.h | 2 -
> arch/ia64/include/asm/thread_info.h | 2 -
> arch/m32r/include/asm/thread_info.h | 2 -
> arch/m68k/include/asm/thread_info.h | 1 -
> arch/microblaze/include/asm/thread_info.h | 2 -
> arch/mips/include/asm/thread_info.h | 2 -
> arch/mn10300/include/asm/thread_info.h | 2 -
> arch/parisc/include/asm/thread_info.h | 2 -
> arch/powerpc/include/asm/thread_info.h | 2 -
> arch/s390/include/asm/thread_info.h | 2 -
> arch/sh/include/asm/thread_info.h | 2 -
> arch/sparc/include/asm/thread_info_32.h | 2 -
> arch/sparc/include/asm/thread_info_64.h | 2 -
> arch/um/include/asm/thread_info.h | 2 -
> arch/unicore32/include/asm/thread_info.h | 2 -
> arch/x86/include/asm/thread_info.h | 2 -
> arch/xtensa/include/asm/thread_info.h | 2 -
> drivers/bluetooth/btmrvl_main.c | 2 -
> drivers/mfd/twl4030-irq.c | 3 -
> drivers/mfd/twl6030-irq.c | 2 -
> drivers/net/irda/stir4200.c | 2 +-
> drivers/platform/x86/thinkpad_acpi.c | 15 +--
> drivers/staging/rts_pstor/rtsx.c | 2 -
> fs/btrfs/async-thread.c | 2 +-
> fs/btrfs/disk-io.c | 8 +-
> fs/ext4/super.c | 3 +-
> fs/fs-writeback.c | 4 +-
> fs/gfs2/log.c | 4 +-
> fs/gfs2/quota.c | 4 +-
> fs/jbd/journal.c | 2 +-
> fs/jbd2/journal.c | 2 +-
> fs/jfs/jfs_logmgr.c | 2 +-
> fs/jfs/jfs_txnmgr.c | 4 +-
> fs/nilfs2/segment.c | 2 +-
> fs/xfs/linux-2.6/xfs_buf.c | 2 +-
> include/linux/freezer.h | 78 +++++--------
> include/linux/kthread.h | 1 +
> include/linux/sched.h | 3 +-
> kernel/cgroup_freezer.c | 51 ++++-----
> kernel/exit.c | 8 +-
> kernel/fork.c | 1 -
> kernel/freezer.c | 179 +++++++++++++++++------------
> kernel/kthread.c | 25 ++++
> kernel/power/hibernate.c | 15 +--
> kernel/power/process.c | 65 ++++-------
> kernel/power/user.c | 4 +-
> mm/backing-dev.c | 8 +-
> 54 files changed, 247 insertions(+), 315 deletions(-)
>
> --
> tejun
>
>
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Mark Brown @ 2011-08-20 15:30 UTC (permalink / raw)
To: Alan Stern; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <Pine.LNX.4.44L0.1108200943500.26680-100000@netrider.rowland.org>
On Sat, Aug 20, 2011 at 09:48:25AM -0400, Alan Stern wrote:
> No, I as wasking about driver- and subsystem-specific interfaces to
> userspace that translate into things users are already doing. Kevin's
> example was a touchscreen (although that was really an example of
> setting a power usage level, not a latency constraint). Do you have
> any others?
Any sort of media streaming would be an obvious example - the
application picks the amount of data it buffers and how often it's
notified of progress depending on the usage which then controls how
quickly the system needs to handle things.
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Alan Stern @ 2011-08-20 13:48 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110820062543.GA5011@opensource.wolfsonmicro.com>
On Sat, 20 Aug 2011, Mark Brown wrote:
> On Fri, Aug 19, 2011 at 10:24:19PM -0400, Alan Stern wrote:
> > On Sat, 20 Aug 2011, Mark Brown wrote:
>
> > > interfaces and let the subsystem and driver translate these into actual
> > > wakeup latency constraints:
>
> > > https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032422.html
> > > https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032428.html
>
> > > This is much easier for users as it translates into something they're
> > > actually doing (and in most cases the driver can make it Just Work) and
> > > it means that off the shelf applications will end up tuning the system
> > > appropriately by themselves. I'm additionally concerned that if we
> > > expose this stuff directly to userspace that's an open invitation to
> > > driver authors to not even bother trying to make the kernel figure this
> > > stuff out by itself and to instead tie the system together with magic
> > > userspace.
>
> > Can you give a couple of examples to illustrate these points? I think
> > it would help a lot to make the conversation more concrete.
>
> Examples of what? Latency constraints from drivers? That'd be things
> like Kevin listed in the second message linked above - the kernel knows
> it needs to wake up within a given time period in order to have time to
> do what it needs to do in response to a given wake source such as
> filling a buffer before it underflows.
No, I as wasking about driver- and subsystem-specific interfaces to
userspace that translate into things users are already doing. Kevin's
example was a touchscreen (although that was really an example of
setting a power usage level, not a latency constraint). Do you have
any others?
Alan Stern
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Mark Brown @ 2011-08-20 10:31 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <201108201135.58886.rjw@sisk.pl>
On Sat, Aug 20, 2011 at 11:35:58AM +0200, Rafael J. Wysocki wrote:
> I'm not sure what you mean by "exposing per-device wakeup latency constraints
> to user space". I simply thought it might be useful to allow user space to
> add and remove those, in analogy to what it can do with the currently available
> PM QoS constraints.
I linked to the mails from Kevin... To be honest I'm rather surprised
this stuff is getting exposed directly to userspace with write support.
> My point is that this won't prevent kernel subsystems from adding their
> own PM QoS constraints to devices and the constraints added by user space
> will only have effect if they make the devices stay active for longer
> times. In consequence, they may only increase the energy usage of the system
> and shouldn't affect functionality.
Well, clearly that's not the only use - demand for increased energy
consumption by itself from users is generally quite low :)
> The reason why I think this may be useful is that I can readily imagine
> scenarios in which user space (think a distribution or system vendor) will
> want to do something that hasn't been anticipated by kernel developers
> and the ability to add per-device PM QoS constraints will allow it to do that
Coming at this from the embedded perspective modifying the kernel just
isn't an issue. It's quite depressing in other cases too but some of
the circumstances you mentioned in previous messages are sensible do
make sense to me. It does seem like this is a system specific problem
which we should be able to enable as part of the support for those
systems.
> without modifying the kernel. Also, it will help to test and debug new drivers
> and subsystems.
If it were a debugfs facility I'd not be concerned.
> I don't want to prevent kernel subsystems from using their own interfaces to
> acquire user-level input that translates into PM QoS constraints. I simply
> think it won't _hurt_ to provide user space with an additional level of
> control over per-device PM QoS constraints.
My experience has been that once you start providing an interface people
will find it, start using it and it takes a lot of work to convince them
that they ought to do something better. This is totally reasonable from
when you look at things from their point of view, especially people who
aren't used to being able to improve anything except their own driver.
> > I'm additionally concerned that if we expose this stuff directly to userspace
> > that's an open invitation to driver authors to not even bother trying to make
> > the kernel figure this stuff out by itself and to instead tie the system
> > together with magic userspace.
> As I said, I don't think we can prevent that from happening anyway.
> Worst case people will add strange interfaces to drivers making them
> incompatible with anything except for their crazy user space.
That's not really a problem - if people are adding their own crazy
interfaces it's clear that they've done that. It'll show up as a red
flag to anyone looking at their stuff and this will create pressure on
them to fix their code or at least do a better job for the next thing.
The goal isn't to tie people's hands to stop them doing silly things,
it's to make it clear that that is what they're doing.
^ permalink raw reply
* [GIT PULL pm-next] freezer: fix various bugs and simplify implementation
From: Tejun Heo @ 2011-08-20 9:44 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: arnd, paul, lizf, linux-kernel, oleg, Martin Schwidefsky,
linux-pm
Hello, Rafael.
Please consider pulling from the following branch to receive freezer
fixes and simplifications patchset[1]. The only changes from the
posting are addition of acked-by's, minor patch description updates
and rebase on top of pm-next (af0fa9cb79 "PM / Hibernate: Include
storage keys in hibernation image on s390").
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
The HEAD should be 7b5b95b3f51a28ec008a295e5247436637220f41. If
git.korg doesn't show it yet. Please pull from master.korg.
ssh://master.kernel.org/pub/scm/linux/kernel/git/tj/misc.git freezer
FYI, this patchset will cause a conflict with s390 TIF flag fix patch.
The conflict is trivial and Stephen should be able to handle it
without any problem. Also, I'm planning on doing some further work on
cgroup freezer and then will try to bridge it with job control. If
that plan fans out, I might ask Oleg to pull from the pm tree. This
shouldn't matter too much either way but it *might* be a good idea to
keep this line of patches in a separate branch.
Thank you.
[1] http://thread.gmane.org/gmane.linux.kernel/1181594
Tejun Heo (16):
freezer: fix current->state restoration race in refrigerator()
freezer: don't unnecessarily set PF_NOFREEZE explicitly
freezer: unexport refrigerator() and update try_to_freeze() slightly
freezer: implement and use kthread_freezable_should_stop()
freezer: rename thaw_process() to __thaw_task() and simplify the implementation
freezer: make exiting tasks properly unfreezable
freezer: don't distinguish nosig tasks on thaw
freezer: use dedicated lock instead of task_lock() + memory barrier
freezer: make freezing indicate freeze condition in effect
freezer: fix set_freezable[_with_signal]() race
freezer: kill PF_FREEZING
freezer: clean up freeze_processes() failure path
cgroup_freezer: prepare for removal of TIF_FREEZE
freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE
freezer: remove now unused TIF_FREEZE
freezer: remove should_send_signal() and update frozen()
Documentation/power/freezing-of-tasks.txt | 14 +-
arch/alpha/include/asm/thread_info.h | 2 -
arch/arm/include/asm/thread_info.h | 2 -
arch/avr32/include/asm/thread_info.h | 2 -
arch/blackfin/include/asm/thread_info.h | 2 -
arch/cris/include/asm/thread_info.h | 2 -
arch/frv/include/asm/thread_info.h | 2 -
arch/h8300/include/asm/thread_info.h | 2 -
arch/ia64/include/asm/thread_info.h | 2 -
arch/m32r/include/asm/thread_info.h | 2 -
arch/m68k/include/asm/thread_info.h | 1 -
arch/microblaze/include/asm/thread_info.h | 2 -
arch/mips/include/asm/thread_info.h | 2 -
arch/mn10300/include/asm/thread_info.h | 2 -
arch/parisc/include/asm/thread_info.h | 2 -
arch/powerpc/include/asm/thread_info.h | 2 -
arch/s390/include/asm/thread_info.h | 2 -
arch/sh/include/asm/thread_info.h | 2 -
arch/sparc/include/asm/thread_info_32.h | 2 -
arch/sparc/include/asm/thread_info_64.h | 2 -
arch/um/include/asm/thread_info.h | 2 -
arch/unicore32/include/asm/thread_info.h | 2 -
arch/x86/include/asm/thread_info.h | 2 -
arch/xtensa/include/asm/thread_info.h | 2 -
drivers/bluetooth/btmrvl_main.c | 2 -
drivers/mfd/twl4030-irq.c | 3 -
drivers/mfd/twl6030-irq.c | 2 -
drivers/net/irda/stir4200.c | 2 +-
drivers/platform/x86/thinkpad_acpi.c | 15 +--
drivers/staging/rts_pstor/rtsx.c | 2 -
fs/btrfs/async-thread.c | 2 +-
fs/btrfs/disk-io.c | 8 +-
fs/ext4/super.c | 3 +-
fs/fs-writeback.c | 4 +-
fs/gfs2/log.c | 4 +-
fs/gfs2/quota.c | 4 +-
fs/jbd/journal.c | 2 +-
fs/jbd2/journal.c | 2 +-
fs/jfs/jfs_logmgr.c | 2 +-
fs/jfs/jfs_txnmgr.c | 4 +-
fs/nilfs2/segment.c | 2 +-
fs/xfs/linux-2.6/xfs_buf.c | 2 +-
include/linux/freezer.h | 78 +++++--------
include/linux/kthread.h | 1 +
include/linux/sched.h | 3 +-
kernel/cgroup_freezer.c | 51 ++++-----
kernel/exit.c | 8 +-
kernel/fork.c | 1 -
kernel/freezer.c | 179 +++++++++++++++++------------
kernel/kthread.c | 25 ++++
kernel/power/hibernate.c | 15 +--
kernel/power/process.c | 65 ++++-------
kernel/power/user.c | 4 +-
mm/backing-dev.c | 8 +-
54 files changed, 247 insertions(+), 315 deletions(-)
--
tejun
^ permalink raw reply
* Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints
From: Rafael J. Wysocki @ 2011-08-20 9:35 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux PM mailing list, linux-omap, Jean Pihet, markgross
In-Reply-To: <20110819231419.GA3981@sirena.org.uk>
On Saturday, August 20, 2011, Mark Brown wrote:
> On Fri, Aug 19, 2011 at 10:42:20PM +0200, Rafael J. Wysocki wrote:
>
> > I really wouldn't like the discussion to go in circles.
>
> > First, please tell me what in particular you are objecting to,
> > because I don't think that's any of the patches that have been
> > sent to the linux-pm list to date.
>
> The original issue that Kevin raised and CCed me in on was the idea of
> exposing raw per-device wakeup latency constraints to userspace; it
> seems much better to expose user level requirements via subsystem
> interfaces and let the subsystem and driver translate these into actual
> wakeup latency constraints:
>
> https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032422.html
> https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032428.html
OK, so let's clarify things.
I'm not sure what you mean by "exposing per-device wakeup latency constraints
to user space". I simply thought it might be useful to allow user space to
add and remove those, in analogy to what it can do with the currently available
PM QoS constraints.
My point is that this won't prevent kernel subsystems from adding their
own PM QoS constraints to devices and the constraints added by user space
will only have effect if they make the devices stay active for longer
times. In consequence, they may only increase the energy usage of the system
and shouldn't affect functionality.
The reason why I think this may be useful is that I can readily imagine
scenarios in which user space (think a distribution or system vendor) will
want to do something that hasn't been anticipated by kernel developers
and the ability to add per-device PM QoS constraints will allow it to do that
without modifying the kernel. Also, it will help to test and debug new drivers
and subsystems.
I don't want to prevent kernel subsystems from using their own interfaces to
acquire user-level input that translates into PM QoS constraints. I simply
think it won't _hurt_ to provide user space with an additional level of
control over per-device PM QoS constraints.
> This is much easier for users as it translates into something they're
> actually doing (and in most cases the driver can make it Just Work) and
> it means that off the shelf applications will end up tuning the system
> appropriately by themselves.
The _end_ users really don't care. They'll use whatever settings the user
interface exposes to them. Moreover, even if there is an interface for
user space to add per-device PM QoS constraints, that doesn't mean the
user space is _required_ to use it. If it doesn't, everything will work
as you describe.
> I'm additionally concerned that if we expose this stuff directly to userspace
> that's an open invitation to driver authors to not even bother trying to make
> the kernel figure this stuff out by itself and to instead tie the system
> together with magic userspace.
As I said, I don't think we can prevent that from happening anyway.
Worst case people will add strange interfaces to drivers making them
incompatible with anything except for their crazy user space.
Thanks,
Rafael
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox