* Re: [PATCH v4 1/2] Input: enable i8042-level wakeup control
From: Dmitry Torokhov @ 2011-08-17 7:03 UTC (permalink / raw)
To: Daniel Drake; +Cc: linux-pm, dilinger, linux-input
In-Reply-To: <CAMLZHHTie_u40NZ40o+LCwffk=3LbY96YTwxQt5wcVtbTc53qg@mail.gmail.com>
Hi Daniel,
On Thu, Aug 11, 2011 at 07:02:17PM +0100, Daniel Drake wrote:
> Hi Dmitry,
>
> On Fri, Aug 5, 2011 at 4:24 PM, Daniel Drake <dsd@laptop.org> wrote:
> > I've started looking at this, but my first problem is that when the
> > input layer asks atkbd to turn off LEDs, atkbd schedules a workqueue
> > item to undertake this work. However, as there is no synchronization,
> > this work only seems to get executed during or after system resume.
> > This makes it hard to catch with such a scheme. Any ideas?
>
> I've looked closer, and am keeping my eyes open for other options too.
>
> During suspend of the input layer, input_dev_suspend() calls
> input_dev_toggle(dev, false) which turns off LEDs and sounds.
>
> On the XO laptops, we don't have LEDs, nor sounds, and we don't have
> num lock, scroll lock or caps lock keys. So the suspend code does
> nothing - there are no lights to turn off. So far so good.
>
> Moving to the resume case: input_dev_resume() calls
> input_reset_device(). This calls input_dev_toggle(dev, true), which
> asks atkbd to turn the non-existent keyboard LEDs off. I guess this is
> harmless and is unlikely to be the cause of the problems which led us
> to need to disable this code in light of the following:
>
> After calling input_dev_toggle(), input_dev_resume() does this:
>
> /*
> * Keys that have been pressed at suspend time are unlikely
> * to be still pressed when we resume.
> */
> spin_lock_irq(&dev->event_lock);
> input_dev_release_keys(dev);
> spin_unlock_irq(&dev->event_lock);
>
> We definitely don't want this to happen in our case, where we have
> maintained full control and power of the device during suspend/resume.
>
> This particular important code snippet would not even have been
> disabled by your suggestion of blocking data transfer at the
> i8042-level.
>
> So, I think we need a new approach at solving this, and I think it has
> to be one that interacts at the input_dev layer.
OK, so we have the following scenario:
- we do not need to worry about SND and LED on OLPC;
- we do not need to worry about releasing the key that was pressed
when we went to sleep because they system would not go to sleep while
there is a key pressed. So the code in input_dev_resume() won't
actually release any keys unless a key was held pressed and we forced
system to sleep somehow.
- we need to make sure we do not loose key press (and following events)
when we are waking up.
Can atkbd driver stash away serio byte data (when not in command mode)
and replay it when pm notifier signals us that resume process is done?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: 3.0-rc2 failed s2ram: Freezing of tasks failed after 20.00 seconds
From: Carlos R. Mafra @ 2011-08-16 22:45 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM mailing list, LKML
In-Reply-To: <201108162320.28297.rjw@sisk.pl>
On Tue, 16 Aug 2011 at 23:20:28 +0200, Rafael J. Wysocki wrote:
> On Tuesday, August 16, 2011, Carlos R. Mafra wrote:
> > On Tue, 16 Aug 2011 at 20:13:55 +0200, Rafael J. Wysocki wrote:
> > > On Tuesday, August 16, 2011, Carlos R. Mafra wrote:
> > > > I started testing 3.0-rc2 yesterday and after a few successful suspend to ram
> > > > it did not suspend anymore and I got this:
> > > >
> > > > PM: Syncing filesystems ... done.
> > > > PM: Preparing system for mem sleep
> > > > Freezing user space processes ...
> > > > Freezing of tasks failed after 20.00 seconds (1 tasks refusing to freeze, wq_busy=0):
> > > > udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00800004
> > > > ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> > > > ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> > > > 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> > > > Call Trace:
> > > > [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> > > > [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> > > > [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> > > > [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> > > > [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> > > > [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> > > > [<ffffffff81059afb>] flush_work+0x2b/0x40
> > > > [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> > > > [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> > > > [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> > > > [<ffffffff81106052>] check_disk_change+0x32/0x80
> > > > [<ffffffff812ee649>] sd_open+0xb9/0x190
> > > > [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> > > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > > [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> > > > [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> > > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > > [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> > > > [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> > > > [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> > > > [<ffffffff810e2ab1>] do_last+0xb1/0x800
> > > > [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> > > > [<ffffffff81229077>] ? kobject_put+0x27/0x60
> > > > [<ffffffff812ced62>] ? put_device+0x12/0x20
> > > > [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> > > > [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> > > > [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> > > > [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> > > > [<ffffffff810d522b>] sys_open+0x1b/0x20
> > > > [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
> > > >
> > > > Restarting tasks ... done.
> > > > video LNXVIDEO:01: Restoring backlight state
> > > > acpid: 1 client rule loaded
> > > > EXT4-fs (sda2): re-mounted. Opts: discard,commit=600
> > > > INFO: task udisks-daemon:5848 blocked for more than 120 seconds.
> > > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > > > udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00000004
> > > > ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> > > > ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> > > > 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> > > > Call Trace:
> > > > [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> > > > [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> > > > [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> > > > [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> > > > [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> > > > [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> > > > [<ffffffff81059afb>] flush_work+0x2b/0x40
> > > > [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> > > > [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> > > > [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> > > > [<ffffffff81106052>] check_disk_change+0x32/0x80
> > > > [<ffffffff812ee649>] sd_open+0xb9/0x190
> > > > [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> > > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > > [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> > > > [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> > > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > > [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> > > > [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> > > > [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> > > > [<ffffffff810e2ab1>] do_last+0xb1/0x800
> > > > [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> > > > [<ffffffff81229077>] ? kobject_put+0x27/0x60
> > > > [<ffffffff812ced62>] ? put_device+0x12/0x20
> > > > [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> > > > [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> > > > [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> > > > [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> > > > [<ffffffff810d522b>] sys_open+0x1b/0x20
> > > > [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
> > > >
> > > >
> > > > and there are some more of these traces in the logs, but they all look the
> > > > same.
> > >
> > > It looks like udisks-daemon is waiting for a completion that's
> > > never completed.
> > >
> > > Do you use any removable storage devices?
> >
> > Yes, external harddisks connected via USB. But none were mounted
> > when I closed the lid to suspend.
>
> Were they connected to the USB ports?
No.
> > PS: I made a mistake in the Subject:, the kernel is 3.1-rc2 and
> > not 3.0-rc2.
>
> Did 3.0 work correctly?
I didn't test 3.0, but 2.6.39 always worked.
^ permalink raw reply
* Re: [PATCH 07/15] PM QoS: add a global notification mechanism for the device constraints
From: Rafael J. Wysocki @ 2011-08-16 21:44 UTC (permalink / raw)
To: jean.pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <1313502198-9298-8-git-send-email-j-pihet@ti.com>
Hi,
On Tuesday, August 16, 2011, jean.pihet@newoldbits.com wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> Add a global notification chain that gets called upon changes to the
> aggregated constraint value for any device.
> The notification callbacks are passing the full constraint request data
> in order for the callees to have access to it. The current use is for the
> platform low-level code to access the target device of the constraint.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> drivers/base/power/qos.c | 84 ++++++++++++++++++++++++++++++++++++----------
> include/linux/pm_qos.h | 11 ++++++
> kernel/power/qos.c | 2 +-
> 3 files changed, 78 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 304d68d..b52b3e8 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -8,6 +8,12 @@
> *
> * This QoS design is best effort based. Dependents register their QoS needs.
> * Watchers register to keep track of the current QoS needs of the system.
> + * Watchers can register different types of notification callbacks:
> + * . a per-device notification callback using the dev_pm_qos_*_notifier API.
> + * The notification chain data is stored in the per-device constraint
> + * data struct.
> + * . a system-wide notification callback using the dev_pm_qos_*_global_notifier
> + * API. The notification chain data is stored in a static variable.
> *
> * Note about the per-device constraint data struct allocation:
> * . The per-device constraints data struct ptr is tored into the device
> @@ -36,8 +42,32 @@
>
>
> static DEFINE_MUTEX(dev_pm_qos_mtx);
> +static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
> static void dev_pm_qos_constraints_allocate(struct device *dev);
>
> +/*
> + * Update the constraints list using the PM QoS core code and
> + * if needed call the per-device and the global notification callbacks
> + */
Well, if you add kerneldoc comments, please make them follow the standard,
even if that's a static function.
> +static int _apply_constraint(struct dev_pm_qos_request *req,
The initial underscore looks odd and is not really necessary. Please drop it.
The rest of the patch looks fine.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 06/15] PM QoS: implement the per-device PM QoS constraints
From: Rafael J. Wysocki @ 2011-08-16 21:40 UTC (permalink / raw)
To: jean.pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <1313502198-9298-7-git-send-email-j-pihet@ti.com>
Hi,
On Tuesday, August 16, 2011, jean.pihet@newoldbits.com wrote:
> From: Jean Pihet <j-pihet@ti.com>
>
> Implement the per-device PM QoS constraints by creating a device
> PM QoS API, which calls the PM QoS constraints management core code.
>
> The per-device latency constraints data strctures are stored
> in the device dev_pm_info struct.
>
> The device PM code calls the init and destroy of the per-device constraints
> data struct in order to support the dynamic insertion and removal of the
> devices in the system.
>
> To minimize the data usage by the per-device constraints, the data struct
> is only allocated at the first call to dev_pm_qos_add_request.
> The data is later free'd when the device is removed from the system.
> A global mutex protects the constraints users from the data being
> allocated and free'd.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> drivers/base/power/Makefile | 4 +-
> drivers/base/power/main.c | 3 +
> drivers/base/power/qos.c | 291 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm.h | 9 ++
> include/linux/pm_qos.h | 42 ++++++
> 5 files changed, 347 insertions(+), 2 deletions(-)
> create mode 100644 drivers/base/power/qos.c
>
> diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
> index 2639ae7..b707447 100644
> --- a/drivers/base/power/Makefile
> +++ b/drivers/base/power/Makefile
> @@ -1,4 +1,4 @@
> -obj-$(CONFIG_PM) += sysfs.o generic_ops.o
> +obj-$(CONFIG_PM) += sysfs.o generic_ops.o qos.o
> obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
> obj-$(CONFIG_PM_RUNTIME) += runtime.o
> obj-$(CONFIG_PM_TRACE_RTC) += trace.o
> @@ -6,4 +6,4 @@ obj-$(CONFIG_PM_OPP) += opp.o
> obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o
> obj-$(CONFIG_HAVE_CLK) += clock_ops.o
>
> -ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
> \ No newline at end of file
> +ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index a854591..956443f 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -22,6 +22,7 @@
> #include <linux/mutex.h>
> #include <linux/pm.h>
> #include <linux/pm_runtime.h>
> +#include <linux/pm_qos.h>
> #include <linux/resume-trace.h>
> #include <linux/interrupt.h>
> #include <linux/sched.h>
> @@ -97,6 +98,7 @@ void device_pm_add(struct device *dev)
> dev_name(dev->parent));
> list_add_tail(&dev->power.entry, &dpm_list);
> mutex_unlock(&dpm_list_mtx);
> + dev_pm_qos_constraints_init(dev);
> }
>
> /**
> @@ -107,6 +109,7 @@ void device_pm_remove(struct device *dev)
> {
> pr_debug("PM: Removing info for %s:%s\n",
> dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
> + dev_pm_qos_constraints_destroy(dev);
> complete_all(&dev->power.completion);
> mutex_lock(&dpm_list_mtx);
> list_del_init(&dev->power.entry);
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> new file mode 100644
> index 0000000..304d68d
> --- /dev/null
> +++ b/drivers/base/power/qos.c
> @@ -0,0 +1,291 @@
> +/*
Please add a copyright notice.
> + * This module exposes the interface to kernel space for specifying
> + * per-device PM QoS dependencies. It provides infrastructure for registration
> + * of:
> + *
> + * Dependents on a QoS value : register requests
> + * Watchers of QoS value : get notified when target QoS value changes
> + *
> + * This QoS design is best effort based. Dependents register their QoS needs.
> + * Watchers register to keep track of the current QoS needs of the system.
> + *
> + * Note about the per-device constraint data struct allocation:
> + * . The per-device constraints data struct ptr is tored into the device
> + * dev_pm_info.
> + * . To minimize the data usage by the per-device constraints, the data struct
> + * is only allocated at the first call to dev_pm_qos_add_request.
> + * . The data is later free'd when the device is removed from the system.
> + * . The constraints_state variable from dev_pm_info tracks the data struct
> + * allocation state:
> + * DEV_PM_QOS_NO_DEVICE: No device present or device removed, no data
> + * allocated,
> + * DEV_PM_QOS_DEVICE_PRESENT: Device present, data not allocated and will be
> + * allocated at the first call to dev_pm_qos_add_request,
> + * DEV_PM_QOS_ALLOCATED: Device present, data allocated. The per-device
> + * PM QoS constraints framework is operational and constraints can be
> + * added, updated or removed using the dev_pm_qos_* API.
> + * . A global mutex protects the constraints users from the data being
> + * allocated and free'd.
> + */
> +
> +#include <linux/pm_qos.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +
> +
> +static DEFINE_MUTEX(dev_pm_qos_mtx);
> +static void dev_pm_qos_constraints_allocate(struct device *dev);
This header wouldn't be necessary if you put the definition of
dev_pm_qos_constraints_allocate() before dev_pm_qos_add_request().
> +
> +/**
> + * dev_pm_qos_add_request - inserts new qos request into the list
> + * @dev: target device for the constraint
> + * @req: pointer to a preallocated handle
> + * @value: defines the qos request
> + *
> + * This function inserts a new entry in the device constraints list of
> + * requested qos performance characteristics. It recomputes the aggregate
> + * QoS expectations of parameters and initializes the dev_pm_qos_request
> + * handle. Caller needs to save this handle for later use in updates and
> + * removal.
> + */
> +void dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
> + s32 value)
> +{
> + if (!dev || !req) /*guard against callers passing in null */
> + return;
> +
> + if (dev_pm_qos_request_active(req)) {
> + WARN(1, KERN_ERR "dev_pm_qos_add_request() called for already "
> + "added request\n");
> + return;
> + }
> +
> + /* Allocate the constraints struct on the first call to add_request */
> + dev_pm_qos_constraints_allocate(dev);
I would remove the locking from dev_pm_qos_constraints_allocate() and
put it under the lock below. The code would be cleaner this way.
Also, the name is slightly misleading, because it suggests that the
allocation happens every time, so I'd move the condition from there
into the caller too.
The remaning part of the patch looks OK to me, but I'll have another
look at it tomorrow, when I'm less tired.
Thanks,
Rafael
^ permalink raw reply
* Re: 3.0-rc2 failed s2ram: Freezing of tasks failed after 20.00 seconds
From: Rafael J. Wysocki @ 2011-08-16 21:20 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: Linux PM mailing list, LKML
In-Reply-To: <20110816211358.GA2251@Pilar.site>
On Tuesday, August 16, 2011, Carlos R. Mafra wrote:
> On Tue, 16 Aug 2011 at 20:13:55 +0200, Rafael J. Wysocki wrote:
> > On Tuesday, August 16, 2011, Carlos R. Mafra wrote:
> > > I started testing 3.0-rc2 yesterday and after a few successful suspend to ram
> > > it did not suspend anymore and I got this:
> > >
> > > PM: Syncing filesystems ... done.
> > > PM: Preparing system for mem sleep
> > > Freezing user space processes ...
> > > Freezing of tasks failed after 20.00 seconds (1 tasks refusing to freeze, wq_busy=0):
> > > udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00800004
> > > ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> > > ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> > > 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> > > Call Trace:
> > > [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> > > [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> > > [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> > > [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> > > [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> > > [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> > > [<ffffffff81059afb>] flush_work+0x2b/0x40
> > > [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> > > [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> > > [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> > > [<ffffffff81106052>] check_disk_change+0x32/0x80
> > > [<ffffffff812ee649>] sd_open+0xb9/0x190
> > > [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> > > [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> > > [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> > > [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> > > [<ffffffff810e2ab1>] do_last+0xb1/0x800
> > > [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> > > [<ffffffff81229077>] ? kobject_put+0x27/0x60
> > > [<ffffffff812ced62>] ? put_device+0x12/0x20
> > > [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> > > [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> > > [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> > > [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> > > [<ffffffff810d522b>] sys_open+0x1b/0x20
> > > [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
> > >
> > > Restarting tasks ... done.
> > > video LNXVIDEO:01: Restoring backlight state
> > > acpid: 1 client rule loaded
> > > EXT4-fs (sda2): re-mounted. Opts: discard,commit=600
> > > INFO: task udisks-daemon:5848 blocked for more than 120 seconds.
> > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > > udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00000004
> > > ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> > > ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> > > 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> > > Call Trace:
> > > [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> > > [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> > > [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> > > [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> > > [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> > > [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> > > [<ffffffff81059afb>] flush_work+0x2b/0x40
> > > [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> > > [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> > > [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> > > [<ffffffff81106052>] check_disk_change+0x32/0x80
> > > [<ffffffff812ee649>] sd_open+0xb9/0x190
> > > [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> > > [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> > > [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> > > [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> > > [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> > > [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> > > [<ffffffff810e2ab1>] do_last+0xb1/0x800
> > > [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> > > [<ffffffff81229077>] ? kobject_put+0x27/0x60
> > > [<ffffffff812ced62>] ? put_device+0x12/0x20
> > > [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> > > [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> > > [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> > > [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> > > [<ffffffff810d522b>] sys_open+0x1b/0x20
> > > [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
> > >
> > >
> > > and there are some more of these traces in the logs, but they all look the
> > > same.
> >
> > It looks like udisks-daemon is waiting for a completion that's
> > never completed.
> >
> > Do you use any removable storage devices?
>
> Yes, external harddisks connected via USB. But none were mounted
> when I closed the lid to suspend.
Were they connected to the USB ports?
> PS: I made a mistake in the Subject:, the kernel is 3.1-rc2 and
> not 3.0-rc2.
Did 3.0 work correctly?
Rafael
^ permalink raw reply
* Re: [Regression] GPF in usb_driver_release_interface during/after resume
From: Rafael J. Wysocki @ 2011-08-16 18:42 UTC (permalink / raw)
To: Alan Stern
Cc: Linux PM mailing list, linux-usb, Marcel Holtmann, LKML, Greg KH
In-Reply-To: <Pine.LNX.4.44L0.1108160956170.2028-100000@iolanthe.rowland.org>
On Tuesday, August 16, 2011, Alan Stern wrote:
> On Mon, 15 Aug 2011, Rafael J. Wysocki wrote:
>
> > Hi,
> >
> > I get the following general protection fault sometimes (but sufficiently
> > often for it to be annoying) during resume from system suspend on Toshiba
> > Portege R500:
> >
> > [ 330.839651] PM: Finishing wakeup.
> > [ 330.839656] Restarting tasks ...
> > [ 330.839742] e1000e 0000:01:00.0: PME# enabled
> > [ 330.840121] usb 2-2: USB disconnect, device number 3
> > [ 330.841626] option1 ttyUSB0: GSM modem (1-port) converter now disconnected from ttyUSB0
> > [ 330.841835] option 2-2:1.0: device disconnected
> > [ 330.842626] option1 ttyUSB1: GSM modem (1-port) converter now disconnected from ttyUSB1
> > [ 330.842770] option 2-2:1.1: device disconnected
> > [ 330.843506] option1 ttyUSB2: GSM modem (1-port) converter now disconnected from ttyUSB2
> > [ 330.843648] option 2-2:1.2: device disconnected
> > [ 330.857550] done.
> > [ 330.857591] video LNXVIDEO:00: Restoring backlight state
> > [ 330.915075] option1 ttyUSB3: GSM modem (1-port) converter now disconnected from ttyUSB3
> > [ 330.924372] option 2-2:1.3: device disconnected
> > [ 330.964520] usb 5-2: USB disconnect, device number 4
> > [ 331.218859] general protection fault: 0000 [#1] PREEMPT SMP
> > [ 331.218889] CPU 0
> > [ 331.218894] Modules linked in: bnep cryptd aes_x86_64 aes_generic xt_tcpudp xt_pkttype af_packet snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw xt_NOTRACK ipt_REJECT iptable_raw iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables ipv6 cpufreq_conservative cpufreq_ondemand cpufreq_userspace cpufreq_powersave acpi_cpufreq microcode freq_table mperf fuse dm_mod sr_mod cdrom arc4 joydev btusb bluetooth option usb_wwan iwl4965 pcmcia iwl_legacy mac80211 psmouse snd_hda_codec_realtek usbhid serio_raw sdhci_pci usb_storage hid usbserial sdhci firewire_ohci snd_hda_intel yenta_socket snd_hda_codec pcmci
a_rsrc sg firewire_core mmc_core snd_hwdep pcmcia_core crc_itu_t cfg80211 snd_pcm iTCO_wdt iTCO_vendor_support snd_timer snd soundcore e1000e rfkill snd_page_alloc toshiba_bluetooth battery!
ac
> uhci_hcd sd_mod crc_t10dif ehci_hcd usbcore rtc_cmos ext3 jbd ahci libahci libata fan processor thermal
> > [ 331.219207]
> > [ 331.219216] Pid: 906, comm: khubd Not tainted 2.6.39+ #388 TOSHIBA PORTEGE R500/Portable PC
> > [ 331.219237] RIP: 0010:[<ffffffffa00c5bd1>] [<ffffffffa00c5bd1>] usb_driver_release_interface+0x32/0x9f [usbcore]
> > [ 331.219289] RSP: 0018:ffff880078899be0 EFLAGS: 00010282
> > [ 331.219301] RAX: 0000000000000000 RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000000006
> > [ 331.219314] RDX: 0000000000000006 RSI: 00000000000001e3 RDI: ffffffffa0011020
> > [ 331.219327] RBP: ffff880078899c00 R08: ffff880078899be0 R09: 09f911029d74e35b
> > [ 331.219341] R10: ffff880078899bb0 R11: 0000000000000000 R12: ffff88007a5e6548
> > [ 331.219354] R13: ffff880040a8d000 R14: ffffffffa0011020 R15: ffffffffa00110b8
> > [ 331.219368] FS: 0000000000000000(0000) GS:ffff88007f400000(0000) knlGS:0000000000000000
> > [ 331.219382] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > [ 331.219394] CR2: 00007f4b2fd01080 CR3: 0000000079cf9000 CR4: 00000000000006f0
> > [ 331.219407] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 331.219420] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > [ 331.219434] Process khubd (pid: 906, threadinfo ffff880078898000, task ffff88003798ee80)
> > [ 331.219448] Stack:
> > [ 331.219455] ffff880078899c00 ffff880037070388 ffff88007a5e6548 ffff880040a8d000
> > [ 331.219477] ffff880078899c30 ffffffffa000fdb4 ffff88007a5e6578 ffff88007a5e6578
> > [ 331.219497] ffff88007a5e6548 ffff88007856c188 ffff880078899c70 ffffffffa00c5af0
> > [ 331.219518] Call Trace:
> > [ 331.219535] [<ffffffffa000fdb4>] btusb_disconnect+0x9c/0xc2 [btusb]
> > [ 331.219566] [<ffffffffa00c5af0>] usb_unbind_interface+0x5a/0x109 [usbcore]
> > [ 331.219590] [<ffffffff812a3818>] __device_release_driver+0x81/0xca
> > [ 331.219606] [<ffffffff812a387a>] device_release_driver+0x19/0x26
> > [ 331.219622] [<ffffffff812a33e1>] bus_remove_device+0xc2/0xd3
> > [ 331.219637] [<ffffffff812a0e96>] device_del+0x12b/0x17a
> > [ 331.219666] [<ffffffffa00c477f>] usb_disable_device+0x8a/0x176 [usbcore]
> > [ 331.219695] [<ffffffffa00bdac4>] usb_disconnect+0xcd/0x137 [usbcore]
> > [ 331.219724] [<ffffffffa00bf1c2>] hub_thread+0x74b/0x113e [usbcore]
> > [ 331.219745] [<ffffffff81033ab7>] ? get_parent_ip+0x11/0x41
> > [ 331.219760] [<ffffffff8102c835>] ? need_resched+0x1e/0x28
> > [ 331.219777] [<ffffffff81057f49>] ? wake_up_bit+0x25/0x25
> > [ 331.219804] [<ffffffffa00bea77>] ? usb_new_device+0x1ca/0x1ca [usbcore]
> > [ 331.219821] [<ffffffff81057aa9>] kthread+0x7d/0x85
> > [ 331.219838] [<ffffffff8136af54>] kernel_thread_helper+0x4/0x10
> > [ 331.219854] [<ffffffff81369a44>] ? retint_restore_args+0xe/0xe
> > [ 331.219870] [<ffffffff81057a2c>] ? __init_kthread_worker+0x56/0x56
> > [ 331.219885] [<ffffffff8136af50>] ? gs_change+0xb/0xb
> > [ 331.219896] Code: 54 53 48 89 f3 48 83 ec 08 be e3 01 00 00 48 85 db 74 0a 48 85 ff 75 13 be e6 01 00 00 48 c7 c7 9c 25 0d a0 e8 b6 6d f7 e0 eb 65
> > [ 331.219983] 8b 83 20 01 00 00 48 85 c0 74 59 48 81 c7 98 00 00 00 48 39
> > [ 331.220031] RIP [<ffffffffa00c5bd1>] usb_driver_release_interface+0x32/0x9f [usbcore]
> > [ 331.220065] RSP <ffff880078899be0>
> > [ 331.240771] ---[ end trace be6a0c945bcc56b9 ]---
> >
> > This started to happen during the 3.0 development cycle, as far as I can say.
> >
> > According to GDB, usb_driver_release_interface+0x32 is line 484 in
> > drivers/usb/core/driver.c:
> >
> > void usb_driver_release_interface(struct usb_driver *driver,
> > struct usb_interface *iface)
> > {
> > struct device *dev = &iface->dev;
> >
> > /* this should never happen, don't release something that's not ours */
> > ---> if (!dev->driver || dev->driver != &driver->drvwrap.driver)
> >
> > so this looks like a use-after-free to me and since it doesn't occur every
> > time, there seems to be a race condition in the USB stack.
>
> That's what it looks like to me too. But I don't see how iface could
> have been freed at this point, because the USB core still has a
> reference to it.
>
> > I tried to figure it out myself, but apparently it's too tricky to me. :-)
>
> Does the same thing happen if you simply unload the btusb driver,
> without suspending?
No, it doesn't. It looks like suspend/resume causes the device to be
removed and probed at the same time somehow.
> Also, can you add some debugging statements to
> drivers/bluetooth/btusb.c? in btusb_disconnect(), it would be good to
> see the values of intf, data, data->intf, and data->isoc. In addition
> (with caution), data->intf->dev.driver and data->isoc->dev.driver.
Sure, I'll do that.
Thanks,
Rafael
^ permalink raw reply
* Re: 3.0-rc2 failed s2ram: Freezing of tasks failed after 20.00 seconds
From: Rafael J. Wysocki @ 2011-08-16 18:13 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: Linux PM mailing list, LKML
In-Reply-To: <20110816094245.GA2042@Pilar.site>
On Tuesday, August 16, 2011, Carlos R. Mafra wrote:
> I started testing 3.0-rc2 yesterday and after a few successful suspend to ram
> it did not suspend anymore and I got this:
>
> PM: Syncing filesystems ... done.
> PM: Preparing system for mem sleep
> Freezing user space processes ...
> Freezing of tasks failed after 20.00 seconds (1 tasks refusing to freeze, wq_busy=0):
> udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00800004
> ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> Call Trace:
> [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> [<ffffffff81059afb>] flush_work+0x2b/0x40
> [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> [<ffffffff81106052>] check_disk_change+0x32/0x80
> [<ffffffff812ee649>] sd_open+0xb9/0x190
> [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> [<ffffffff810e2ab1>] do_last+0xb1/0x800
> [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> [<ffffffff81229077>] ? kobject_put+0x27/0x60
> [<ffffffff812ced62>] ? put_device+0x12/0x20
> [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> [<ffffffff810d522b>] sys_open+0x1b/0x20
> [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
>
> Restarting tasks ... done.
> video LNXVIDEO:01: Restoring backlight state
> acpid: 1 client rule loaded
> EXT4-fs (sda2): re-mounted. Opts: discard,commit=600
> INFO: task udisks-daemon:5848 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> udisks-daemon D ffff8800a641e5d0 0 5848 5845 0x00000004
> ffff8800a6741928 0000000000000082 ffff880000000000 00000000000105c0
> ffff8800a641e200 00000000000105c0 ffff8800a6741fd8 00000000000105c0
> 00000000000105c0 ffff8800a6740000 ffff8800a6741fd8 00000000000105c0
> Call Trace:
> [<ffffffff814cf355>] schedule_timeout+0x1c5/0x230
> [<ffffffff814ce8c9>] ? schedule+0x399/0x8a0
> [<ffffffff814ce3e0>] wait_for_common+0xc0/0x160
> [<ffffffff8103ec60>] ? try_to_wake_up+0x290/0x290
> [<ffffffff814d10ba>] ? _raw_spin_unlock_irq+0x2a/0x40
> [<ffffffff814ce528>] wait_for_completion+0x18/0x20
> [<ffffffff81059afb>] flush_work+0x2b/0x40
> [<ffffffff81058fd0>] ? do_work_for_cpu+0x30/0x30
> [<ffffffff8105a076>] flush_delayed_work+0x46/0x50
> [<ffffffff8121e6b6>] disk_clear_events+0x76/0x110
> [<ffffffff81106052>] check_disk_change+0x32/0x80
> [<ffffffff812ee649>] sd_open+0xb9/0x190
> [<ffffffff81106f81>] __blkdev_get+0x91/0x3d0
> [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> [<ffffffff8110730e>] blkdev_get+0x4e/0x340
> [<ffffffff810e1b17>] ? do_lookup+0xb7/0x380
> [<ffffffff81107600>] ? blkdev_get+0x340/0x340
> [<ffffffff8110765d>] blkdev_open+0x5d/0x80
> [<ffffffff810d3d80>] __dentry_open+0x130/0x320
> [<ffffffff810d5021>] nameidata_to_filp+0x71/0x80
> [<ffffffff810e2ab1>] do_last+0xb1/0x800
> [<ffffffff810e4603>] path_openat+0xd3/0x3f0
> [<ffffffff81229077>] ? kobject_put+0x27/0x60
> [<ffffffff812ced62>] ? put_device+0x12/0x20
> [<ffffffff810e4984>] do_filp_open+0x44/0xa0
> [<ffffffff810f0a44>] ? alloc_fd+0xf4/0x150
> [<ffffffff810d512c>] do_sys_open+0xfc/0x1e0
> [<ffffffff810d3a86>] ? filp_close+0x56/0x80
> [<ffffffff810d522b>] sys_open+0x1b/0x20
> [<ffffffff814d1c3b>] system_call_fastpath+0x16/0x1b
>
>
> and there are some more of these traces in the logs, but they all look the
> same.
It looks like udisks-daemon is waiting for a completion that's
never completed.
Do you use any removable storage devices?
Rafael
^ permalink raw reply
* Re: [PATCH v5 2/3] PM / DEVFREQ: add basic governors
From: Turquette, Mike @ 2011-08-16 18:11 UTC (permalink / raw)
To: myungjoo.ham
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
Thomas Gleixner
In-Reply-To: <CAJ0PZbTh-YZB9kTNw3uAhukU-2fJ2g5KRy8HC+1B00TjoxQn2Q@mail.gmail.com>
On Tue, Aug 16, 2011 at 1:52 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Thu, Aug 11, 2011 at 10:35 AM, Turquette, Mike <mturquette@ti.com> wrote:
> []
>>
>> Polling is the only practical use for devfreq, assuming a QoS API
>> exists for DVFS. As such powersave and performance governors should
>> be removed.
>
> Although powersave/performance governors may seem useless, they are
> used as basis on measuring the usefulness of DVFS mechanism of
> specific devices. If a device is going to use DVFS, we can test the
> device with them to find out the potential power save (compare
> powersave to performance) and the performance deterioration (compared
> to performance). Often, in testing phase, QA teams use performance to
> find out any issues with DVFS features (in CPUFREQ). Users may simply
> want to use performance governor in some cases (power is not an issue
> sometimes).
Fair enough. Keeping them around for testing is sensible.
> Using QoS APIs simply to set "minimum" or "maximum" is possible.
> However, they are not that straightforward; e.g., how should we set
> "DMA latency" to be fixed at the minimum frequency regardless of
> others, or how should we set "Network latency" to be fixed at the
> maximum frequency? especially without knowing the specifications of
> each DVFS-capable device (such as available frequencies, valid latency
> values, ...).
>
>>
>>> userspace: use the user specified frequency stored at
>>> devfreq.user_set_freq. With sysfs support in the following patch, a user
>>> may set the value with the sysfs interface.
>>>
>>> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.
>>
>> I won't repeate everything from patch 1 of this series, but the
>> governors should implement the queue/loop logic in the same way that
>> CPUfreq does, and the individual devices should have their own
>> delayed_work.
>
> First, in case where we want to let each DVFS-capable device have
> exact polling frequency (up to jiffy resolution), we only need to set
> polling_interval = jiffies_to_msecs(1);.
That requires a source code change for anyone that wants to do it. My
main complaint with this method is that it is restrictive to begin
with and the whole method for determining the next_polling time
reproduces what workqueues already give us.
> In case of CPUFREQ, there would be only one polling loop at most for
> each core. However, in case of DEVFREQ, there could be multiple
> polling loops at a core if CPUFREQ-like looping logic is introduced.
> Why don't we reduce that overhead while their function is same, it is
> easily doable, and it reduces redundancy?
I'm afraid I don't follow. I was thinking of having a single wq loop
for each device. Under what conditions would a single device have
multiple wq loops operating against it?
>>> When a user updates OPP entries (enable/disable/add), OPP framework
>>> automatically notifies DEVFREQ to update operating frequency
>>> accordingly. Thus, DEVFREQ users (device drivers) do not need to update
>>
>> It would be nice if OPP library "notified" devfreq but it does not
>> today. OPP library needs notifiers and devfreq can provide handlers
>> for them.
>
> That's why devfreq_update() is added in the patch. While DEVFREQ is
> the only one requiring notifications from OPP, do you think we may
> incur the overhead of notifier at OPP by replacing devfreq_update with
> notifier? If we somehow add another module that requires notifications
> from OPP for frequency availability changes, we will need to implement
> notifier at OPP side, but not just yet, I guess: (discussed before at
> https://lists.linux-foundation.org/pipermail/linux-pm/2011-July/032053.html
> )
Reading that thread makes me think that we really should implement
notifiers in the OPP library. An obvious user of OPP notifiers would
be CPUfreq. I think it is safe to say that there may be
implementations of devfreq and CPUfreq that live side-by-side in the
near future; OPPs might be enabled/disabled dynamically, which means
both of them need callbacks. Better to abstract it out early, I
think.
>>
> []
>>> ---
>>> drivers/base/power/devfreq.c | 100 ++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/devfreq.h | 8 +++
>>> 2 files changed, 108 insertions(+), 0 deletions(-)
>>
>> Governors should be split out into their own file, especially since
>> they need to grow to include polling/queueing logic.
>
> We will need to decide where to settle devfreq core, drivers, and
> governors first. Would /drivers/devfreq/ be appropriate?
I think GKH already ACK'd drivers/devfreq in a previous thread:
https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032537.html
> []
>>> +
>>> + /* Set MAX if it's busy enough */
>>> + if (stat.busy_time * 100 >
>>> + stat.total_time * DFSO_UPTHRESHOLD) {
>>
>> Thresholds should not be constants, but should be tuneable parameters,
>> per-device. This is yet another reason for revising the existing
>> relationship between devfreq core code, governors and devices.
>>
>
> I agree. I think I should add governor specific "setup" value at
> devfreq_add_device(); modifying the interface from
>
> extern int devfreq_add_device(struct device *dev, struct
> devfreq_dev_profile *profile, struct devfreq_governor *governor);
> ==>
> extern int devfreq_add_device(struct device *dev, struct
> devfreq_dev_profile *profile, struct devfreq_governor *governor, void
> *gov_data);
>
> where gov_data is fed to struct devfreq's data field.
It would be nice for the threshold values to be run-time tunable via
sysfs. CPUfreq does this well today for ondemand/conservative
governors and it really helps when doing power/performance tuning.
Regards,
Mike
>>> + *freq = UINT_MAX;
>>> + return 0;
>>> + }
>>> +
>>> + /* Set MAX if we do not know the initial frequency */
>>> + if (stat.current_frequency == 0) {
>>> + *freq = UINT_MAX;
>>> + return 0;
>>> + }
>>> +
>>> + /* Keep the current frequency */
>>> + if (stat.busy_time * 100 >
>>> + stat.total_time * (DFSO_UPTHRESHOLD - DFSO_DOWNDIFFERENCTIAL)) {
>>
>> Same as above.
> Yes.
>
>>
>>> + *freq = stat.current_frequency;
>>> + return 0;
>>> + }
>>> +
>>> + /* Set the desired frequency based on the load */
>>> + a = stat.busy_time;
>>> + a *= stat.current_frequency;
>>> + b = div_u64(a, stat.total_time);
>>> + b *= 100;
>>> + b = div_u64(b, (DFSO_UPTHRESHOLD - DFSO_DOWNDIFFERENCTIAL / 2));
>>
>> Same as above.
> Yes.
>
>>
>> Regards,
>> Mike
>>
>
>
> Cheers!
> MyungJoo
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
^ permalink raw reply
* Re: PM / hibernate xfs lock up / xfs_reclaim_inodes_ag
From: Rafael J. Wysocki @ 2011-08-16 18:05 UTC (permalink / raw)
To: Christoph; +Cc: Linux PM mailing list, Dave Chinner, xfs
In-Reply-To: <4E4A64D4.4000005@u-club.de>
On Tuesday, August 16, 2011, Christoph wrote:
> On 10.08.2011 23:43, Pavel Machek wrote:
> > Hi!
> >
> >>>> Why don't you simply submit a patch to do that?
> >>>
> >>> a) I don't know how to test suspend/hibernate
> >>> b) I don't have any hardware I can test it on.
>
>
> Hi!
>
> I do apologize fmy silence but I've been in a serious productions the last
> weeks.
>
> If there any patches to test, let me know. I can run some decent stress tests.
Well, please try the one attached to
https://bugzilla.kernel.org/show_bug.cgi?id=33622#c10
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 05/15] PM QoS: generalize and export the constraints management code
From: Rafael J. Wysocki @ 2011-08-16 18:01 UTC (permalink / raw)
To: markgross; +Cc: Mark Brown, Linux PM mailing list, linux-omap, Jean Pihet
In-Reply-To: <20110816174557.GA17027@gvim.org>
On Tuesday, August 16, 2011, mark gross wrote:
> On Tue, Aug 16, 2011 at 08:44:19AM +0200, Jean Pihet wrote:
> > On Tue, Aug 16, 2011 at 6:08 AM, mark gross <markgross@thegnar.org> wrote:
> > > On Sun, Aug 14, 2011 at 03:37:43PM +0200, Rafael J. Wysocki wrote:
> > >> On Sunday, August 14, 2011, Jean Pihet wrote:
> > >> > Hi Rafael, Mark,
> > >> >
> > >> > On Sat, Aug 13, 2011 at 10:34 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > >> > > On Saturday, August 13, 2011, mark gross wrote:
> > >> > >> On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pihet@newoldbits.com wrote:
> > >> > >> > From: Jean Pihet <j-pihet@ti.com>
> > >> > >> >
> > >> > >> > In preparation for the per-device constratins support:
> > >> > >> > - rename update_target to pm_qos_update_target
> > >> > >> > - generalize and export pm_qos_update_target for usage by the upcoming
> > >> > >> > per-device latency constraints framework:
> > >> > >> > . operate on struct pm_qos_constraints for constraints management,
> > >> > >> > . introduce an 'action' parameter for constraints add/update/remove,
> > >> > >> > . the return value indicates if the aggregated constraint value has
> > >> > >> > changed,
> > >> > >> > - update the internal code to operate on struct pm_qos_constraints
> > >> > >> > - add a NULL pointer check in the API functions
> > >> > >> >
> > >> > >> > Signed-off-by: Jean Pihet <j-pihet@ti.com>
> > >> > ...
> > >> > >> > +/* Action requested to pm_qos_update_target */
> > >> > >> > +enum pm_qos_req_action {
> > >> > >> > + PM_QOS_ADD_REQ, /* Add a new request */
> > >> > >> > + PM_QOS_UPDATE_REQ, /* Update an existing request */
> > >> > >> > + PM_QOS_REMOVE_REQ /* Remove an existing request */
> > >> > >> > +};
> > >> > >> > +
> > >> > >>
> > >> > >> What do you need this enum for? The function names *_update_*, *_add_*,
> > >> > >> and *_remove_* seem to be pretty redundant if you have to pass an enum
> > >> > >> that could possibly conflict with the function name.
> > >> > >>
> > >> > >> > #ifdef CONFIG_PM
> > >> > >> > +int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
> > >> > >> > + enum pm_qos_req_action action, int value);
> > >> > >> The action for update_target better damn well be "PM_QOS_UPDATE_REQ" or
> > >> > >> there is something strange going on.... BTW what shold this function do
> > >> > >> if the pm_qos_req_action was *not* the UPDATE one?
> > >> >
> > >> > The meaning of pm_qos_update_target is 'update the PM QoS target
> > >> > constraints lists'. As described in the changelog the intention of
> > >> > this patch is to implement the constraints lists management logic in
> > >> > update_target and simplify the API functions (add/update/remove). It
> > >> > is also exported for the upcoming (patch 06/15]) to use it as well.
> > >>
> > >> The enums are fine by me and they allow us to simplify the code
> > >> quite a bit.
> > >>
> > > Ok, but they look a bit sloppy to me as we now have an API that says
> > > "add" we can actually pass in an enum that says "remove".
> > We have an API that says 'update target' that we pass in a parameter
> > that says 'add request', 'update request' or 'remove request'.
> > If it is required I could just rename the internal function
> > update_target, in a later patch.
>
> You are right. I thought I saw the enum added to the other API's for
> some reason. Still, with this update we have an API overloaded through
> that enum parameter providing 2 add, 2 delete and 2 update API's Each
> pair doing about the same thing.
>
> To me it feels like a baby step toward an ioctl type of API that I don't
> like. I'm not going to fight about it any more but I don't like such
> API's as they tend to get hard to read and get out of control.
>
> please rethink this a little.
For real _users_, the API is still "add", "update" and "remove",
but _internally_ those functions use the same "worker" routine with an
argument specifying the operation to carry out. This reduces code
duplication quite a bit and it quite common in the kernel.
Thanks,
Rafael
^ permalink raw reply
* Re: [patch 1/1] [PATCH] include storage keys in hibernation image.
From: Rafael J. Wysocki @ 2011-08-16 17:56 UTC (permalink / raw)
To: Martin Schwidefsky; +Cc: linux-s390, linux-pm, Jiri Slaby, linux-kernel
In-Reply-To: <20110816141541.237704940@de.ibm.com>
On Tuesday, August 16, 2011, Martin Schwidefsky wrote:
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
>
> For s390 there is one additional byte associated with each page,
> the storage key. This byte contains the referenced and changed
> bits and needs to be included into the hibernation image.
> If the storage keys are not restored to their previous state all
> original pages would appear to be dirty. This can cause
> inconsistencies e.g. with read-only filesystems.
>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-pm@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-s390@vger.kernel.org
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
OK, I don't have any complaints. Do you want me to take this
patch or do you want to push it through the s390 tree?
Rafael
> ---
>
> arch/s390/Kconfig | 1
> arch/s390/kernel/suspend.c | 118 ++++++++++++++++++++++++++++++++++++++++
> arch/s390/kernel/swsusp_asm64.S | 3 +
> include/linux/suspend.h | 34 +++++++++++
> kernel/power/Kconfig | 3 +
> kernel/power/snapshot.c | 18 ++++++
> 6 files changed, 177 insertions(+)
>
> Index: hibernate-2.6/arch/s390/Kconfig
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/Kconfig 2011-08-09 17:40:09.220010373 +0200
> +++ hibernate-2.6/arch/s390/Kconfig 2011-08-16 16:06:09.845540931 +0200
> @@ -91,6 +91,7 @@
> select HAVE_ARCH_MUTEX_CPU_RELAX
> select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
> select HAVE_RCU_TABLE_FREE if SMP
> + select ARCH_SAVE_PAGE_KEYS if HIBERNATION
> select ARCH_INLINE_SPIN_TRYLOCK
> select ARCH_INLINE_SPIN_TRYLOCK_BH
> select ARCH_INLINE_SPIN_LOCK
> Index: hibernate-2.6/arch/s390/kernel/suspend.c
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/kernel/suspend.c 2009-09-24 09:06:44.000000000 +0200
> +++ hibernate-2.6/arch/s390/kernel/suspend.c 2011-08-16 16:06:09.845540931 +0200
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/pfn.h>
> +#include <linux/mm.h>
> #include <asm/system.h>
>
> /*
> @@ -14,6 +15,123 @@
> */
> extern const void __nosave_begin, __nosave_end;
>
> +/*
> + * The restore of the saved pages in an hibernation image will set
> + * the change and referenced bits in the storage key for each page.
> + * Overindication of the referenced bits after an hibernation cycle
> + * does not cause any harm but the overindication of the change bits
> + * would cause trouble.
> + * Use the ARCH_SAVE_PAGE_KEYS hooks to save the storage key of each
> + * page to the most significant byte of the associated page frame
> + * number in the hibernation image.
> + */
> +
> +/*
> + * Key storage is allocated as a linked list of pages.
> + * The size of the keys array is (PAGE_SIZE - sizeof(long))
> + */
> +struct page_key_data {
> + struct page_key_data *next;
> + unsigned char data[];
> +};
> +
> +#define PAGE_KEY_DATA_SIZE (PAGE_SIZE - sizeof(struct page_key_data *))
> +
> +static struct page_key_data *page_key_data;
> +static struct page_key_data *page_key_rp, *page_key_wp;
> +static unsigned long page_key_rx, page_key_wx;
> +
> +/*
> + * For each page in the hibernation image one additional byte is
> + * stored in the most significant byte of the page frame number.
> + * On suspend no additional memory is required but on resume the
> + * keys need to be memorized until the page data has been restored.
> + * Only then can the storage keys be set to their old state.
> + */
> +unsigned long page_key_additional_pages(unsigned long pages)
> +{
> + return DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> +}
> +
> +/*
> + * Free page_key_data list of arrays.
> + */
> +void page_key_free(void)
> +{
> + struct page_key_data *pkd;
> +
> + while (page_key_data) {
> + pkd = page_key_data;
> + page_key_data = pkd->next;
> + free_page((unsigned long) pkd);
> + }
> +}
> +
> +/*
> + * Allocate page_key_data list of arrays with enough room to store
> + * one byte for each page in the hibernation image.
> + */
> +int page_key_alloc(unsigned long pages)
> +{
> + struct page_key_data *pk;
> + unsigned long size;
> +
> + size = DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> + while (size--) {
> + pk = (struct page_key_data *) get_zeroed_page(GFP_KERNEL);
> + if (!pk) {
> + page_key_free();
> + return -ENOMEM;
> + }
> + pk->next = page_key_data;
> + page_key_data = pk;
> + }
> + page_key_rp = page_key_wp = page_key_data;
> + page_key_rx = page_key_wx = 0;
> + return 0;
> +}
> +
> +/*
> + * Save the storage key into the upper 8 bits of the page frame number.
> + */
> +void page_key_read(unsigned long *pfn)
> +{
> + unsigned long addr;
> +
> + addr = (unsigned long) page_address(pfn_to_page(*pfn));
> + *(unsigned char *) pfn = (unsigned char) page_get_storage_key(addr);
> +}
> +
> +/*
> + * Extract the storage key from the upper 8 bits of the page frame number
> + * and store it in the page_key_data list of arrays.
> + */
> +void page_key_memorize(unsigned long *pfn)
> +{
> + page_key_wp->data[page_key_wx] = *(unsigned char *) pfn;
> + *(unsigned char *) pfn = 0;
> + if (++page_key_wx < PAGE_KEY_DATA_SIZE)
> + return;
> + page_key_wp = page_key_wp->next;
> + page_key_wx = 0;
> +}
> +
> +/*
> + * Get the next key from the page_key_data list of arrays and set the
> + * storage key of the page referred by @address. If @address refers to
> + * a "safe" page the swsusp_arch_resume code will transfer the storage
> + * key from the buffer page to the original page.
> + */
> +void page_key_write(void *address)
> +{
> + page_set_storage_key((unsigned long) address,
> + page_key_rp->data[page_key_rx], 0);
> + if (++page_key_rx >= PAGE_KEY_DATA_SIZE)
> + return;
> + page_key_rp = page_key_rp->next;
> + page_key_rx = 0;
> +}
> +
> int pfn_is_nosave(unsigned long pfn)
> {
> unsigned long nosave_begin_pfn = PFN_DOWN(__pa(&__nosave_begin));
> Index: hibernate-2.6/arch/s390/kernel/swsusp_asm64.S
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/kernel/swsusp_asm64.S 2011-08-09 17:40:09.228010544 +0200
> +++ hibernate-2.6/arch/s390/kernel/swsusp_asm64.S 2011-08-16 16:06:09.845540931 +0200
> @@ -136,11 +136,14 @@
> 0:
> lg %r2,8(%r1)
> lg %r4,0(%r1)
> + iske %r0,%r4
> lghi %r3,PAGE_SIZE
> lghi %r5,PAGE_SIZE
> 1:
> mvcle %r2,%r4,0
> jo 1b
> + lg %r2,8(%r1)
> + sske %r0,%r2
> lg %r1,16(%r1)
> ltgr %r1,%r1
> jnz 0b
> Index: hibernate-2.6/include/linux/suspend.h
> ===================================================================
> --- hibernate-2.6.orig/include/linux/suspend.h 2011-08-09 17:40:10.164030551 +0200
> +++ hibernate-2.6/include/linux/suspend.h 2011-08-16 16:06:09.845540931 +0200
> @@ -334,4 +334,38 @@
> }
> #endif
>
> +#ifdef CONFIG_ARCH_SAVE_PAGE_KEYS
> +/*
> + * The ARCH_SAVE_PAGE_KEYS functions can be used by an architecture
> + * to save/restore additional information to/from the array of page
> + * frame numbers in the hibernation image. For s390 this is used to
> + * save and restore the storage key for each page that is included
> + * in the hibernation image.
> + */
> +unsigned long page_key_additional_pages(unsigned long pages);
> +int page_key_alloc(unsigned long pages);
> +void page_key_free(void);
> +void page_key_read(unsigned long *pfn);
> +void page_key_memorize(unsigned long *pfn);
> +void page_key_write(void *address);
> +
> +#else /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
> +
> +static inline unsigned long page_key_additional_pages(unsigned long pages)
> +{
> + return 0;
> +}
> +
> +static inline int page_key_alloc(unsigned long pages)
> +{
> + return 0;
> +}
> +
> +static inline void page_key_free(void) {}
> +static inline void page_key_read(unsigned long *pfn) {}
> +static inline void page_key_memorize(unsigned long *pfn) {}
> +static inline void page_key_write(void *address) {}
> +
> +#endif /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
> +
> #endif /* _LINUX_SUSPEND_H */
> Index: hibernate-2.6/kernel/power/Kconfig
> ===================================================================
> --- hibernate-2.6.orig/kernel/power/Kconfig 2011-08-09 17:40:10.192031153 +0200
> +++ hibernate-2.6/kernel/power/Kconfig 2011-08-16 16:06:09.845540931 +0200
> @@ -65,6 +65,9 @@
>
> For more information take a look at <file:Documentation/power/swsusp.txt>.
>
> +config ARCH_SAVE_PAGE_KEYS
> + bool
> +
> config PM_STD_PARTITION
> string "Default resume partition"
> depends on HIBERNATION
> Index: hibernate-2.6/kernel/power/snapshot.c
> ===================================================================
> --- hibernate-2.6.orig/kernel/power/snapshot.c 2011-07-08 11:25:06.293203483 +0200
> +++ hibernate-2.6/kernel/power/snapshot.c 2011-08-16 16:06:09.845540931 +0200
> @@ -1339,6 +1339,9 @@
> count += highmem;
> count -= totalreserve_pages;
>
> + /* Add number of pages required for page keys (s390 only). */
> + size += page_key_additional_pages(saveable);
> +
> /* Compute the maximum number of saveable pages to leave in memory. */
> max_size = (count - (size + PAGES_FOR_IO)) / 2
> - 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);
> @@ -1662,6 +1665,8 @@
> buf[j] = memory_bm_next_pfn(bm);
> if (unlikely(buf[j] == BM_END_OF_MAP))
> break;
> + /* Save page key for data page (s390 only). */
> + page_key_read(buf + j);
> }
> }
>
> @@ -1821,6 +1826,9 @@
> if (unlikely(buf[j] == BM_END_OF_MAP))
> break;
>
> + /* Extract and buffer page key for data page (s390 only). */
> + page_key_memorize(buf + j);
> +
> if (memory_bm_pfn_present(bm, buf[j]))
> memory_bm_set_bit(bm, buf[j]);
> else
> @@ -2223,6 +2231,11 @@
> if (error)
> return error;
>
> + /* Allocate buffer for page keys. */
> + error = page_key_alloc(nr_copy_pages);
> + if (error)
> + return error;
> +
> } else if (handle->cur <= nr_meta_pages + 1) {
> error = unpack_orig_pfns(buffer, ©_bm);
> if (error)
> @@ -2243,6 +2256,8 @@
> }
> } else {
> copy_last_highmem_page();
> + /* Restore page key for data page (s390 only). */
> + page_key_write(handle->buffer);
> handle->buffer = get_buffer(&orig_bm, &ca);
> if (IS_ERR(handle->buffer))
> return PTR_ERR(handle->buffer);
> @@ -2264,6 +2279,9 @@
> void snapshot_write_finalize(struct snapshot_handle *handle)
> {
> copy_last_highmem_page();
> + /* Restore page key for data page (s390 only). */
> + page_key_write(handle->buffer);
> + page_key_free();
> /* Free only if we have loaded the image entirely */
> if (handle->cur > 1 && handle->cur > nr_meta_pages + nr_copy_pages) {
> memory_bm_free(&orig_bm, PG_UNSAFE_CLEAR);
>
>
>
^ permalink raw reply
* Re: [PATCH 05/15] PM QoS: generalize and export the constraints management code
From: mark gross @ 2011-08-16 17:45 UTC (permalink / raw)
To: Jean Pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <CAORVsuXDp4vYGg9BCLkJm6iMPobSCtE9s_1j4zNV5frKjQY1jg@mail.gmail.com>
On Tue, Aug 16, 2011 at 08:44:19AM +0200, Jean Pihet wrote:
> On Tue, Aug 16, 2011 at 6:08 AM, mark gross <markgross@thegnar.org> wrote:
> > On Sun, Aug 14, 2011 at 03:37:43PM +0200, Rafael J. Wysocki wrote:
> >> On Sunday, August 14, 2011, Jean Pihet wrote:
> >> > Hi Rafael, Mark,
> >> >
> >> > On Sat, Aug 13, 2011 at 10:34 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > > On Saturday, August 13, 2011, mark gross wrote:
> >> > >> On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pihet@newoldbits.com wrote:
> >> > >> > From: Jean Pihet <j-pihet@ti.com>
> >> > >> >
> >> > >> > In preparation for the per-device constratins support:
> >> > >> > - rename update_target to pm_qos_update_target
> >> > >> > - generalize and export pm_qos_update_target for usage by the upcoming
> >> > >> > per-device latency constraints framework:
> >> > >> > . operate on struct pm_qos_constraints for constraints management,
> >> > >> > . introduce an 'action' parameter for constraints add/update/remove,
> >> > >> > . the return value indicates if the aggregated constraint value has
> >> > >> > changed,
> >> > >> > - update the internal code to operate on struct pm_qos_constraints
> >> > >> > - add a NULL pointer check in the API functions
> >> > >> >
> >> > >> > Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> > ...
> >> > >> > +/* Action requested to pm_qos_update_target */
> >> > >> > +enum pm_qos_req_action {
> >> > >> > + PM_QOS_ADD_REQ, /* Add a new request */
> >> > >> > + PM_QOS_UPDATE_REQ, /* Update an existing request */
> >> > >> > + PM_QOS_REMOVE_REQ /* Remove an existing request */
> >> > >> > +};
> >> > >> > +
> >> > >>
> >> > >> What do you need this enum for? The function names *_update_*, *_add_*,
> >> > >> and *_remove_* seem to be pretty redundant if you have to pass an enum
> >> > >> that could possibly conflict with the function name.
> >> > >>
> >> > >> > #ifdef CONFIG_PM
> >> > >> > +int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
> >> > >> > + enum pm_qos_req_action action, int value);
> >> > >> The action for update_target better damn well be "PM_QOS_UPDATE_REQ" or
> >> > >> there is something strange going on.... BTW what shold this function do
> >> > >> if the pm_qos_req_action was *not* the UPDATE one?
> >> >
> >> > The meaning of pm_qos_update_target is 'update the PM QoS target
> >> > constraints lists'. As described in the changelog the intention of
> >> > this patch is to implement the constraints lists management logic in
> >> > update_target and simplify the API functions (add/update/remove). It
> >> > is also exported for the upcoming (patch 06/15]) to use it as well.
> >>
> >> The enums are fine by me and they allow us to simplify the code
> >> quite a bit.
> >>
> > Ok, but they look a bit sloppy to me as we now have an API that says
> > "add" we can actually pass in an enum that says "remove".
> We have an API that says 'update target' that we pass in a parameter
> that says 'add request', 'update request' or 'remove request'.
> If it is required I could just rename the internal function
> update_target, in a later patch.
You are right. I thought I saw the enum added to the other API's for
some reason. Still, with this update we have an API overloaded through
that enum parameter providing 2 add, 2 delete and 2 update API's Each
pair doing about the same thing.
To me it feels like a baby step toward an ioctl type of API that I don't
like. I'm not going to fight about it any more but I don't like such
API's as they tend to get hard to read and get out of control.
please rethink this a little.
--mark
^ permalink raw reply
* Re: [PATCH 12/15] OMAP4: powerdomain data: add wake-up latency figures
From: Santosh @ 2011-08-16 14:58 UTC (permalink / raw)
To: Jean Pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <CAORVsuUtJ05AHrT0C3rUg_2iUH=NfKdWpqqzoEZms9pYXR6u6A@mail.gmail.com>
On Tuesday 16 August 2011 08:08 PM, Jean Pihet wrote:
> On Tue, Aug 16, 2011 at 4:26 PM, Santosh<santosh.shilimkar@ti.com> wrote:
>> On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
>>>
>>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>>
>>> This patch adds wake up latency numbers for OMAP4. Note that these are
>>> preliminary numbers and need to be relooked.
>>>
>>> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
>>>
>>> The INACTIVE state is added as unsupported.
>>>
>> In that case, don't add that support in first place. When INA support is
>> getting added, you can update these as well.
> No. A value is needed for all states, even if unsupported at the
> moment. Omitting a value causes it to be set to '0', which means 'no
> latency'.
>
What I am saying is don't add "PWRDM_FUNC_PWRST_INACTIVE" which is
not supported. Then you won't even have that state and no need
of latency number for that in the current series.
Regards
Santosh
^ permalink raw reply
* Re: [PATCH 12/15] OMAP4: powerdomain data: add wake-up latency figures
From: Jean Pihet @ 2011-08-16 14:38 UTC (permalink / raw)
To: Santosh
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <4E4A7E25.8020701@ti.com>
On Tue, Aug 16, 2011 at 4:26 PM, Santosh <santosh.shilimkar@ti.com> wrote:
> On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
>>
>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> This patch adds wake up latency numbers for OMAP4. Note that these are
>> preliminary numbers and need to be relooked.
>>
>> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> The INACTIVE state is added as unsupported.
>>
> In that case, don't add that support in first place. When INA support is
> getting added, you can update these as well.
No. A value is needed for all states, even if unsupported at the
moment. Omitting a value causes it to be set to '0', which means 'no
latency'.
Jean
^ permalink raw reply
* Re: [PATCH 11/15] OMAP3: powerdomain data: add wake-up latency figures
From: Santosh @ 2011-08-16 14:37 UTC (permalink / raw)
To: Jean Pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <CAORVsuWPp2SWi-SqHe8Pjoi3MgRriA74TwhsNEUS2L4s9iiqDg@mail.gmail.com>
On Tuesday 16 August 2011 08:04 PM, Jean Pihet wrote:
> On Tue, Aug 16, 2011 at 4:25 PM, Santosh<santosh.shilimkar@ti.com> wrote:
>> On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
>>>
>>> From: Jean Pihet<j-pihet@ti.com>
>>>
>>> Figures are added to the power domains structs.
>>>
>>> Note: the figures are preliminary figures. More accurate measurements
>>> are needed. Also the conditions of measurements shall be investigated
>>> and described.
>>>
>>> Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
>>> on MPU, CORE and PER.
>>>
>>> Signed-off-by: Jean Pihet<j-pihet@ti.com>
>>> ---
>>> arch/arm/mach-omap2/powerdomains3xxx_data.c | 77
>>> +++++++++++++++++++++++++++
>>> 1 files changed, 77 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c
>>> b/arch/arm/mach-omap2/powerdomains3xxx_data.c
>>> index 469a920..64446e7 100644
>>> --- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
>>> +++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
>>> @@ -31,6 +31,13 @@
>>>
>>> /*
>>> * Powerdomains
>>> + *
>>> + * The wakeup_lat values are derived from measurements on
>>> + * the actual target.
>>> + *
>>> + * Note: the latency figures are preliminary and only used
>>> + * for the constraints framework validation.
>>> + * Actual figures and measurements conditions shall be added.
>>> */
>>>
>>> static struct powerdomain iva2_pwrdm = {
>>> @@ -52,6 +59,13 @@ static struct powerdomain iva2_pwrdm = {
>>> [2] = PWRSTS_OFF_ON,
>>> [3] = PWRSTS_ON,
>>> },
>>> + .wakeup_lat = {
>>> + [PWRDM_FUNC_PWRST_OFF] = 1100,
>>> + [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
>>> + [PWRDM_FUNC_PWRST_CSWR] = 350,
>>> + [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
>>
>> This can easily derived from the PWRST flag instead of hardcoding
>> it this way. Also note that INACTIVE PD isn't supported in mainline
>> yet because of voltage-domain dependency planned changes.
>>
>>> + [PWRDM_FUNC_PWRST_ON] = 0,
>>
>> All of the PD structures are manually coded. This whole file is
>> auto-generated and even these field generation needs to follow
>> that path.
> Ok I need to check that. Any pointer on how to generate those?
>
The scripts needs to be updated to generate this additional information.
Will send you some pointer off the list.
Regards
Santosh
^ permalink raw reply
* Re: [PATCH 11/15] OMAP3: powerdomain data: add wake-up latency figures
From: Jean Pihet @ 2011-08-16 14:34 UTC (permalink / raw)
To: Santosh
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <4E4A7DC9.1040500@ti.com>
On Tue, Aug 16, 2011 at 4:25 PM, Santosh <santosh.shilimkar@ti.com> wrote:
> On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
>>
>> From: Jean Pihet<j-pihet@ti.com>
>>
>> Figures are added to the power domains structs.
>>
>> Note: the figures are preliminary figures. More accurate measurements
>> are needed. Also the conditions of measurements shall be investigated
>> and described.
>>
>> Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
>> on MPU, CORE and PER.
>>
>> Signed-off-by: Jean Pihet<j-pihet@ti.com>
>> ---
>> arch/arm/mach-omap2/powerdomains3xxx_data.c | 77
>> +++++++++++++++++++++++++++
>> 1 files changed, 77 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c
>> b/arch/arm/mach-omap2/powerdomains3xxx_data.c
>> index 469a920..64446e7 100644
>> --- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
>> +++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
>> @@ -31,6 +31,13 @@
>>
>> /*
>> * Powerdomains
>> + *
>> + * The wakeup_lat values are derived from measurements on
>> + * the actual target.
>> + *
>> + * Note: the latency figures are preliminary and only used
>> + * for the constraints framework validation.
>> + * Actual figures and measurements conditions shall be added.
>> */
>>
>> static struct powerdomain iva2_pwrdm = {
>> @@ -52,6 +59,13 @@ static struct powerdomain iva2_pwrdm = {
>> [2] = PWRSTS_OFF_ON,
>> [3] = PWRSTS_ON,
>> },
>> + .wakeup_lat = {
>> + [PWRDM_FUNC_PWRST_OFF] = 1100,
>> + [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
>> + [PWRDM_FUNC_PWRST_CSWR] = 350,
>> + [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
>
> This can easily derived from the PWRST flag instead of hardcoding
> it this way. Also note that INACTIVE PD isn't supported in mainline
> yet because of voltage-domain dependency planned changes.
>
>> + [PWRDM_FUNC_PWRST_ON] = 0,
>
> All of the PD structures are manually coded. This whole file is
> auto-generated and even these field generation needs to follow
> that path.
Ok I need to check that. Any pointer on how to generate those?
Jean
^ permalink raw reply
* Re: [PATCH 12/15] OMAP4: powerdomain data: add wake-up latency figures
From: Santosh @ 2011-08-16 14:26 UTC (permalink / raw)
To: jean.pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <1313502198-9298-13-git-send-email-j-pihet@ti.com>
On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
> From: Vishwanath BS<vishwanath.bs@ti.com>
>
> This patch adds wake up latency numbers for OMAP4. Note that these are
> preliminary numbers and need to be relooked.
>
> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
>
> The INACTIVE state is added as unsupported.
>
In that case, don't add that support in first place. When INA support is
getting added, you can update these as well.
^ permalink raw reply
* Re: [PATCH 11/15] OMAP3: powerdomain data: add wake-up latency figures
From: Santosh @ 2011-08-16 14:25 UTC (permalink / raw)
To: jean.pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <1313502198-9298-12-git-send-email-j-pihet@ti.com>
On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
> From: Jean Pihet<j-pihet@ti.com>
>
> Figures are added to the power domains structs.
>
> Note: the figures are preliminary figures. More accurate measurements
> are needed. Also the conditions of measurements shall be investigated
> and described.
>
> Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
> on MPU, CORE and PER.
>
> Signed-off-by: Jean Pihet<j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/powerdomains3xxx_data.c | 77 +++++++++++++++++++++++++++
> 1 files changed, 77 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c b/arch/arm/mach-omap2/powerdomains3xxx_data.c
> index 469a920..64446e7 100644
> --- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
> +++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
> @@ -31,6 +31,13 @@
>
> /*
> * Powerdomains
> + *
> + * The wakeup_lat values are derived from measurements on
> + * the actual target.
> + *
> + * Note: the latency figures are preliminary and only used
> + * for the constraints framework validation.
> + * Actual figures and measurements conditions shall be added.
> */
>
> static struct powerdomain iva2_pwrdm = {
> @@ -52,6 +59,13 @@ static struct powerdomain iva2_pwrdm = {
> [2] = PWRSTS_OFF_ON,
> [3] = PWRSTS_ON,
> },
> + .wakeup_lat = {
> + [PWRDM_FUNC_PWRST_OFF] = 1100,
> + [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
> + [PWRDM_FUNC_PWRST_CSWR] = 350,
> + [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
This can easily derived from the PWRST flag instead of hardcoding
it this way. Also note that INACTIVE PD isn't supported in mainline
yet because of voltage-domain dependency planned changes.
> + [PWRDM_FUNC_PWRST_ON] = 0,
All of the PD structures are manually coded. This whole file is
auto-generated and even these field generation needs to follow
that path.
^ permalink raw reply
* Re: [PATCH 15/15] OMAP2+: cpuidle only influences the MPU state
From: Santosh @ 2011-08-16 14:16 UTC (permalink / raw)
To: jean.pihet
Cc: markgross, Mark Brown, Linux PM mailing list, linux-omap,
Jean Pihet
In-Reply-To: <1313502198-9298-16-git-send-email-j-pihet@ti.com>
Jean,
On Tuesday 16 August 2011 07:13 PM, jean.pihet@newoldbits.com wrote:
> From: Jean Pihet<j-pihet@ti.com>
>
> Since cpuidle is a CPU centric framework it decides the MPU
> next power state based on the MPU exit_latency and target_residency
> figures.
>
> The rest of the power domains get their next power state programmed
> from the devices PM QoS framework, via the devices wake-up latency
> constraints.
>
> Note: the exit_latency and target_residency figures of the MPU
> include the MPU itself and the peripherals needed for the MPU to
> execute instructions (e.g. main memory, caches, IRQ controller,
> MMU etc). Some of those peripherals can belong to other power domains
> than the MPU subsystem and so the corresponding latencies must be
> included in this figure.
>
> Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
> on MPU, CORE and PER.
>
> Signed-off-by: Jean Pihet<j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 42 +++++++++++-------------------------
> arch/arm/mach-omap2/pm.h | 17 +++++++++++++-
> 2 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 4bf6e6e..b43d1d2 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -37,26 +37,26 @@
> #ifdef CONFIG_CPU_IDLE
>
[....]
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 4e166ad..aca3b6c 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -43,9 +43,22 @@ static inline int omap4_opp_init(void)
> * omap3_pm_init_cpuidle
> */
> struct cpuidle_params {
> - u32 exit_latency; /* exit_latency = sleep + wake-up latencies */
> + /*
> + * exit_latency = sleep + wake-up latencies of the MPU,
> + * which include the MPU itself and the peripherals needed
> + * for the MPU to execute instructions (e.g. main memory,
> + * caches, IRQ controller, MMU etc). Some of those peripherals
> + * can belong to other power domains than the MPU subsystem and so
> + * the corresponding latencies must be included in this figure.
> + */
This is exactly was my point in the last comment on this patch.
If you are adding up those numbers then it's no long MPU PD
latency but the system C-state latency.
And I say again, the latency numbers above are not for MPU alone
as you have done in this hunk. So be clear here. Either you provide
way to dynamically add the latency numbers to MPU latency which
is fixed for a OPP or call this as system latency etc.
> /*
> - * The latencies/thresholds for various C states have
> + * The MPU latencies/thresholds for various C states have
> * to be configured from the respective board files.
> * These are some default values (which might not provide
> * the best power savings) used on boards which do not
> * pass these details from the board file.
> */
> static struct cpuidle_params cpuidle_params_table[] = {
> - /* C1 */
> + /* C1 . MPU WFI + Core active */
> {2 + 2, 5, 1},
> - /* C2 */
> + /* C2 . MPU WFI + Core inactive */
> {10 + 10, 30, 1},
Regards
Santosh
^ permalink raw reply
* [patch 1/1] [PATCH] include storage keys in hibernation image.
From: Martin Schwidefsky @ 2011-08-16 14:14 UTC (permalink / raw)
To: linux-pm, linux-kernel, linux-s390, Rafael J. Wysocki,
Pavel Machek, Jiri
Cc: Martin Schwidefsky
In-Reply-To: <20110816141441.898339466@de.ibm.com>
[-- Attachment #1: 201-hibernate-storage-keys.diff --]
[-- Type: text/plain, Size: 9951 bytes --]
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
For s390 there is one additional byte associated with each page,
the storage key. This byte contains the referenced and changed
bits and needs to be included into the hibernation image.
If the storage keys are not restored to their previous state all
original pages would appear to be dirty. This can cause
inconsistencies e.g. with read-only filesystems.
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-pm@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
arch/s390/Kconfig | 1
arch/s390/kernel/suspend.c | 118 ++++++++++++++++++++++++++++++++++++++++
arch/s390/kernel/swsusp_asm64.S | 3 +
include/linux/suspend.h | 34 +++++++++++
kernel/power/Kconfig | 3 +
kernel/power/snapshot.c | 18 ++++++
6 files changed, 177 insertions(+)
Index: hibernate-2.6/arch/s390/Kconfig
===================================================================
--- hibernate-2.6.orig/arch/s390/Kconfig 2011-08-09 17:40:09.220010373 +0200
+++ hibernate-2.6/arch/s390/Kconfig 2011-08-16 16:06:09.845540931 +0200
@@ -91,6 +91,7 @@
select HAVE_ARCH_MUTEX_CPU_RELAX
select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
select HAVE_RCU_TABLE_FREE if SMP
+ select ARCH_SAVE_PAGE_KEYS if HIBERNATION
select ARCH_INLINE_SPIN_TRYLOCK
select ARCH_INLINE_SPIN_TRYLOCK_BH
select ARCH_INLINE_SPIN_LOCK
Index: hibernate-2.6/arch/s390/kernel/suspend.c
===================================================================
--- hibernate-2.6.orig/arch/s390/kernel/suspend.c 2009-09-24 09:06:44.000000000 +0200
+++ hibernate-2.6/arch/s390/kernel/suspend.c 2011-08-16 16:06:09.845540931 +0200
@@ -7,6 +7,7 @@
*/
#include <linux/pfn.h>
+#include <linux/mm.h>
#include <asm/system.h>
/*
@@ -14,6 +15,123 @@
*/
extern const void __nosave_begin, __nosave_end;
+/*
+ * The restore of the saved pages in an hibernation image will set
+ * the change and referenced bits in the storage key for each page.
+ * Overindication of the referenced bits after an hibernation cycle
+ * does not cause any harm but the overindication of the change bits
+ * would cause trouble.
+ * Use the ARCH_SAVE_PAGE_KEYS hooks to save the storage key of each
+ * page to the most significant byte of the associated page frame
+ * number in the hibernation image.
+ */
+
+/*
+ * Key storage is allocated as a linked list of pages.
+ * The size of the keys array is (PAGE_SIZE - sizeof(long))
+ */
+struct page_key_data {
+ struct page_key_data *next;
+ unsigned char data[];
+};
+
+#define PAGE_KEY_DATA_SIZE (PAGE_SIZE - sizeof(struct page_key_data *))
+
+static struct page_key_data *page_key_data;
+static struct page_key_data *page_key_rp, *page_key_wp;
+static unsigned long page_key_rx, page_key_wx;
+
+/*
+ * For each page in the hibernation image one additional byte is
+ * stored in the most significant byte of the page frame number.
+ * On suspend no additional memory is required but on resume the
+ * keys need to be memorized until the page data has been restored.
+ * Only then can the storage keys be set to their old state.
+ */
+unsigned long page_key_additional_pages(unsigned long pages)
+{
+ return DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
+}
+
+/*
+ * Free page_key_data list of arrays.
+ */
+void page_key_free(void)
+{
+ struct page_key_data *pkd;
+
+ while (page_key_data) {
+ pkd = page_key_data;
+ page_key_data = pkd->next;
+ free_page((unsigned long) pkd);
+ }
+}
+
+/*
+ * Allocate page_key_data list of arrays with enough room to store
+ * one byte for each page in the hibernation image.
+ */
+int page_key_alloc(unsigned long pages)
+{
+ struct page_key_data *pk;
+ unsigned long size;
+
+ size = DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
+ while (size--) {
+ pk = (struct page_key_data *) get_zeroed_page(GFP_KERNEL);
+ if (!pk) {
+ page_key_free();
+ return -ENOMEM;
+ }
+ pk->next = page_key_data;
+ page_key_data = pk;
+ }
+ page_key_rp = page_key_wp = page_key_data;
+ page_key_rx = page_key_wx = 0;
+ return 0;
+}
+
+/*
+ * Save the storage key into the upper 8 bits of the page frame number.
+ */
+void page_key_read(unsigned long *pfn)
+{
+ unsigned long addr;
+
+ addr = (unsigned long) page_address(pfn_to_page(*pfn));
+ *(unsigned char *) pfn = (unsigned char) page_get_storage_key(addr);
+}
+
+/*
+ * Extract the storage key from the upper 8 bits of the page frame number
+ * and store it in the page_key_data list of arrays.
+ */
+void page_key_memorize(unsigned long *pfn)
+{
+ page_key_wp->data[page_key_wx] = *(unsigned char *) pfn;
+ *(unsigned char *) pfn = 0;
+ if (++page_key_wx < PAGE_KEY_DATA_SIZE)
+ return;
+ page_key_wp = page_key_wp->next;
+ page_key_wx = 0;
+}
+
+/*
+ * Get the next key from the page_key_data list of arrays and set the
+ * storage key of the page referred by @address. If @address refers to
+ * a "safe" page the swsusp_arch_resume code will transfer the storage
+ * key from the buffer page to the original page.
+ */
+void page_key_write(void *address)
+{
+ page_set_storage_key((unsigned long) address,
+ page_key_rp->data[page_key_rx], 0);
+ if (++page_key_rx >= PAGE_KEY_DATA_SIZE)
+ return;
+ page_key_rp = page_key_rp->next;
+ page_key_rx = 0;
+}
+
int pfn_is_nosave(unsigned long pfn)
{
unsigned long nosave_begin_pfn = PFN_DOWN(__pa(&__nosave_begin));
Index: hibernate-2.6/arch/s390/kernel/swsusp_asm64.S
===================================================================
--- hibernate-2.6.orig/arch/s390/kernel/swsusp_asm64.S 2011-08-09 17:40:09.228010544 +0200
+++ hibernate-2.6/arch/s390/kernel/swsusp_asm64.S 2011-08-16 16:06:09.845540931 +0200
@@ -136,11 +136,14 @@
0:
lg %r2,8(%r1)
lg %r4,0(%r1)
+ iske %r0,%r4
lghi %r3,PAGE_SIZE
lghi %r5,PAGE_SIZE
1:
mvcle %r2,%r4,0
jo 1b
+ lg %r2,8(%r1)
+ sske %r0,%r2
lg %r1,16(%r1)
ltgr %r1,%r1
jnz 0b
Index: hibernate-2.6/include/linux/suspend.h
===================================================================
--- hibernate-2.6.orig/include/linux/suspend.h 2011-08-09 17:40:10.164030551 +0200
+++ hibernate-2.6/include/linux/suspend.h 2011-08-16 16:06:09.845540931 +0200
@@ -334,4 +334,38 @@
}
#endif
+#ifdef CONFIG_ARCH_SAVE_PAGE_KEYS
+/*
+ * The ARCH_SAVE_PAGE_KEYS functions can be used by an architecture
+ * to save/restore additional information to/from the array of page
+ * frame numbers in the hibernation image. For s390 this is used to
+ * save and restore the storage key for each page that is included
+ * in the hibernation image.
+ */
+unsigned long page_key_additional_pages(unsigned long pages);
+int page_key_alloc(unsigned long pages);
+void page_key_free(void);
+void page_key_read(unsigned long *pfn);
+void page_key_memorize(unsigned long *pfn);
+void page_key_write(void *address);
+
+#else /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
+
+static inline unsigned long page_key_additional_pages(unsigned long pages)
+{
+ return 0;
+}
+
+static inline int page_key_alloc(unsigned long pages)
+{
+ return 0;
+}
+
+static inline void page_key_free(void) {}
+static inline void page_key_read(unsigned long *pfn) {}
+static inline void page_key_memorize(unsigned long *pfn) {}
+static inline void page_key_write(void *address) {}
+
+#endif /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
+
#endif /* _LINUX_SUSPEND_H */
Index: hibernate-2.6/kernel/power/Kconfig
===================================================================
--- hibernate-2.6.orig/kernel/power/Kconfig 2011-08-09 17:40:10.192031153 +0200
+++ hibernate-2.6/kernel/power/Kconfig 2011-08-16 16:06:09.845540931 +0200
@@ -65,6 +65,9 @@
For more information take a look at <file:Documentation/power/swsusp.txt>.
+config ARCH_SAVE_PAGE_KEYS
+ bool
+
config PM_STD_PARTITION
string "Default resume partition"
depends on HIBERNATION
Index: hibernate-2.6/kernel/power/snapshot.c
===================================================================
--- hibernate-2.6.orig/kernel/power/snapshot.c 2011-07-08 11:25:06.293203483 +0200
+++ hibernate-2.6/kernel/power/snapshot.c 2011-08-16 16:06:09.845540931 +0200
@@ -1339,6 +1339,9 @@
count += highmem;
count -= totalreserve_pages;
+ /* Add number of pages required for page keys (s390 only). */
+ size += page_key_additional_pages(saveable);
+
/* Compute the maximum number of saveable pages to leave in memory. */
max_size = (count - (size + PAGES_FOR_IO)) / 2
- 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);
@@ -1662,6 +1665,8 @@
buf[j] = memory_bm_next_pfn(bm);
if (unlikely(buf[j] == BM_END_OF_MAP))
break;
+ /* Save page key for data page (s390 only). */
+ page_key_read(buf + j);
}
}
@@ -1821,6 +1826,9 @@
if (unlikely(buf[j] == BM_END_OF_MAP))
break;
+ /* Extract and buffer page key for data page (s390 only). */
+ page_key_memorize(buf + j);
+
if (memory_bm_pfn_present(bm, buf[j]))
memory_bm_set_bit(bm, buf[j]);
else
@@ -2223,6 +2231,11 @@
if (error)
return error;
+ /* Allocate buffer for page keys. */
+ error = page_key_alloc(nr_copy_pages);
+ if (error)
+ return error;
+
} else if (handle->cur <= nr_meta_pages + 1) {
error = unpack_orig_pfns(buffer, ©_bm);
if (error)
@@ -2243,6 +2256,8 @@
}
} else {
copy_last_highmem_page();
+ /* Restore page key for data page (s390 only). */
+ page_key_write(handle->buffer);
handle->buffer = get_buffer(&orig_bm, &ca);
if (IS_ERR(handle->buffer))
return PTR_ERR(handle->buffer);
@@ -2264,6 +2279,9 @@
void snapshot_write_finalize(struct snapshot_handle *handle)
{
copy_last_highmem_page();
+ /* Restore page key for data page (s390 only). */
+ page_key_write(handle->buffer);
+ page_key_free();
/* Free only if we have loaded the image entirely */
if (handle->cur > 1 && handle->cur > nr_meta_pages + nr_copy_pages) {
memory_bm_free(&orig_bm, PG_UNSAFE_CLEAR);
^ permalink raw reply
* [patch 0/1] [patch 0/1] include storage keys in hibernation image
From: Martin Schwidefsky @ 2011-08-16 14:14 UTC (permalink / raw)
To: linux-pm, linux-kernel, linux-s390, Rafael J. Wysocki,
Pavel Machek, Jiri
Hi Rafael,
Version 2 of the storage key patch for hibernation on s390.
I've added the following changes as you requested:
1) moved the s390 specific code to arch/s390/kernel/suspend.c
2) introduced CONFIG_ARCH_SAVE_PAGE_KEYS and added a select to
the main s390 Kconfig file
3) added the empty default functions to include/linux/suspend.h
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply
* Re: [Regression] GPF in usb_driver_release_interface during/after resume
From: Alan Stern @ 2011-08-16 14:06 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, linux-usb, Marcel Holtmann, LKML, Greg KH
In-Reply-To: <201108152252.49863.rjw@sisk.pl>
On Mon, 15 Aug 2011, Rafael J. Wysocki wrote:
> Hi,
>
> I get the following general protection fault sometimes (but sufficiently
> often for it to be annoying) during resume from system suspend on Toshiba
> Portege R500:
>
> [ 330.839651] PM: Finishing wakeup.
> [ 330.839656] Restarting tasks ...
> [ 330.839742] e1000e 0000:01:00.0: PME# enabled
> [ 330.840121] usb 2-2: USB disconnect, device number 3
> [ 330.841626] option1 ttyUSB0: GSM modem (1-port) converter now disconnected from ttyUSB0
> [ 330.841835] option 2-2:1.0: device disconnected
> [ 330.842626] option1 ttyUSB1: GSM modem (1-port) converter now disconnected from ttyUSB1
> [ 330.842770] option 2-2:1.1: device disconnected
> [ 330.843506] option1 ttyUSB2: GSM modem (1-port) converter now disconnected from ttyUSB2
> [ 330.843648] option 2-2:1.2: device disconnected
> [ 330.857550] done.
> [ 330.857591] video LNXVIDEO:00: Restoring backlight state
> [ 330.915075] option1 ttyUSB3: GSM modem (1-port) converter now disconnected from ttyUSB3
> [ 330.924372] option 2-2:1.3: device disconnected
> [ 330.964520] usb 5-2: USB disconnect, device number 4
> [ 331.218859] general protection fault: 0000 [#1] PREEMPT SMP
> [ 331.218889] CPU 0
> [ 331.218894] Modules linked in: bnep cryptd aes_x86_64 aes_generic xt_tcpudp xt_pkttype af_packet snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw xt_NOTRACK ipt_REJECT iptable_raw iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables ipv6 cpufreq_conservative cpufreq_ondemand cpufreq_userspace cpufreq_powersave acpi_cpufreq microcode freq_table mperf fuse dm_mod sr_mod cdrom arc4 joydev btusb bluetooth option usb_wwan iwl4965 pcmcia iwl_legacy mac80211 psmouse snd_hda_codec_realtek usbhid serio_raw sdhci_pci usb_storage hid usbserial sdhci firewire_ohci snd_hda_intel yenta_socket snd_hda_codec pcmcia_
rsrc sg firewire_core mmc_core snd_hwdep pcmcia_core crc_itu_t cfg80211 snd_pcm iTCO_wdt iTCO_vendor_support snd_timer snd soundcore e1000e rfkill snd_page_alloc toshiba_bluetooth battery a!
c uhci_hcd sd_mod crc_t10dif ehci_hcd usbcore rtc_cmos ext3 jbd ahci libahci libata fan processor thermal
> [ 331.219207]
> [ 331.219216] Pid: 906, comm: khubd Not tainted 2.6.39+ #388 TOSHIBA PORTEGE R500/Portable PC
> [ 331.219237] RIP: 0010:[<ffffffffa00c5bd1>] [<ffffffffa00c5bd1>] usb_driver_release_interface+0x32/0x9f [usbcore]
> [ 331.219289] RSP: 0018:ffff880078899be0 EFLAGS: 00010282
> [ 331.219301] RAX: 0000000000000000 RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000000006
> [ 331.219314] RDX: 0000000000000006 RSI: 00000000000001e3 RDI: ffffffffa0011020
> [ 331.219327] RBP: ffff880078899c00 R08: ffff880078899be0 R09: 09f911029d74e35b
> [ 331.219341] R10: ffff880078899bb0 R11: 0000000000000000 R12: ffff88007a5e6548
> [ 331.219354] R13: ffff880040a8d000 R14: ffffffffa0011020 R15: ffffffffa00110b8
> [ 331.219368] FS: 0000000000000000(0000) GS:ffff88007f400000(0000) knlGS:0000000000000000
> [ 331.219382] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 331.219394] CR2: 00007f4b2fd01080 CR3: 0000000079cf9000 CR4: 00000000000006f0
> [ 331.219407] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 331.219420] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 331.219434] Process khubd (pid: 906, threadinfo ffff880078898000, task ffff88003798ee80)
> [ 331.219448] Stack:
> [ 331.219455] ffff880078899c00 ffff880037070388 ffff88007a5e6548 ffff880040a8d000
> [ 331.219477] ffff880078899c30 ffffffffa000fdb4 ffff88007a5e6578 ffff88007a5e6578
> [ 331.219497] ffff88007a5e6548 ffff88007856c188 ffff880078899c70 ffffffffa00c5af0
> [ 331.219518] Call Trace:
> [ 331.219535] [<ffffffffa000fdb4>] btusb_disconnect+0x9c/0xc2 [btusb]
> [ 331.219566] [<ffffffffa00c5af0>] usb_unbind_interface+0x5a/0x109 [usbcore]
> [ 331.219590] [<ffffffff812a3818>] __device_release_driver+0x81/0xca
> [ 331.219606] [<ffffffff812a387a>] device_release_driver+0x19/0x26
> [ 331.219622] [<ffffffff812a33e1>] bus_remove_device+0xc2/0xd3
> [ 331.219637] [<ffffffff812a0e96>] device_del+0x12b/0x17a
> [ 331.219666] [<ffffffffa00c477f>] usb_disable_device+0x8a/0x176 [usbcore]
> [ 331.219695] [<ffffffffa00bdac4>] usb_disconnect+0xcd/0x137 [usbcore]
> [ 331.219724] [<ffffffffa00bf1c2>] hub_thread+0x74b/0x113e [usbcore]
> [ 331.219745] [<ffffffff81033ab7>] ? get_parent_ip+0x11/0x41
> [ 331.219760] [<ffffffff8102c835>] ? need_resched+0x1e/0x28
> [ 331.219777] [<ffffffff81057f49>] ? wake_up_bit+0x25/0x25
> [ 331.219804] [<ffffffffa00bea77>] ? usb_new_device+0x1ca/0x1ca [usbcore]
> [ 331.219821] [<ffffffff81057aa9>] kthread+0x7d/0x85
> [ 331.219838] [<ffffffff8136af54>] kernel_thread_helper+0x4/0x10
> [ 331.219854] [<ffffffff81369a44>] ? retint_restore_args+0xe/0xe
> [ 331.219870] [<ffffffff81057a2c>] ? __init_kthread_worker+0x56/0x56
> [ 331.219885] [<ffffffff8136af50>] ? gs_change+0xb/0xb
> [ 331.219896] Code: 54 53 48 89 f3 48 83 ec 08 be e3 01 00 00 48 85 db 74 0a 48 85 ff 75 13 be e6 01 00 00 48 c7 c7 9c 25 0d a0 e8 b6 6d f7 e0 eb 65
> [ 331.219983] 8b 83 20 01 00 00 48 85 c0 74 59 48 81 c7 98 00 00 00 48 39
> [ 331.220031] RIP [<ffffffffa00c5bd1>] usb_driver_release_interface+0x32/0x9f [usbcore]
> [ 331.220065] RSP <ffff880078899be0>
> [ 331.240771] ---[ end trace be6a0c945bcc56b9 ]---
>
> This started to happen during the 3.0 development cycle, as far as I can say.
>
> According to GDB, usb_driver_release_interface+0x32 is line 484 in
> drivers/usb/core/driver.c:
>
> void usb_driver_release_interface(struct usb_driver *driver,
> struct usb_interface *iface)
> {
> struct device *dev = &iface->dev;
>
> /* this should never happen, don't release something that's not ours */
> ---> if (!dev->driver || dev->driver != &driver->drvwrap.driver)
>
> so this looks like a use-after-free to me and since it doesn't occur every
> time, there seems to be a race condition in the USB stack.
That's what it looks like to me too. But I don't see how iface could
have been freed at this point, because the USB core still has a
reference to it.
> I tried to figure it out myself, but apparently it's too tricky to me. :-)
Does the same thing happen if you simply unload the btusb driver,
without suspending?
Also, can you add some debugging statements to
drivers/bluetooth/btusb.c? in btusb_disconnect(), it would be good to
see the values of intf, data, data->intf, and data->isoc. In addition
(with caution), data->intf->dev.driver and data->isoc->dev.driver.
Alan Stern
^ permalink raw reply
* [PATCH 15/15] OMAP2+: cpuidle only influences the MPU state
From: jean.pihet @ 2011-08-16 13:43 UTC (permalink / raw)
To: Mark Brown, Kevin Hilman, markgross, Linux PM mailing list; +Cc: Jean Pihet
In-Reply-To: <1313502198-9298-1-git-send-email-j-pihet@ti.com>
From: Jean Pihet <j-pihet@ti.com>
Since cpuidle is a CPU centric framework it decides the MPU
next power state based on the MPU exit_latency and target_residency
figures.
The rest of the power domains get their next power state programmed
from the devices PM QoS framework, via the devices wake-up latency
constraints.
Note: the exit_latency and target_residency figures of the MPU
include the MPU itself and the peripherals needed for the MPU to
execute instructions (e.g. main memory, caches, IRQ controller,
MMU etc). Some of those peripherals can belong to other power domains
than the MPU subsystem and so the corresponding latencies must be
included in this figure.
Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
on MPU, CORE and PER.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 42 +++++++++++-------------------------
arch/arm/mach-omap2/pm.h | 17 +++++++++++++-
2 files changed, 28 insertions(+), 31 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4bf6e6e..b43d1d2 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -37,26 +37,26 @@
#ifdef CONFIG_CPU_IDLE
/*
- * The latencies/thresholds for various C states have
+ * The MPU latencies/thresholds for various C states have
* to be configured from the respective board files.
* These are some default values (which might not provide
* the best power savings) used on boards which do not
* pass these details from the board file.
*/
static struct cpuidle_params cpuidle_params_table[] = {
- /* C1 */
+ /* C1 . MPU WFI + Core active */
{2 + 2, 5, 1},
- /* C2 */
+ /* C2 . MPU WFI + Core inactive */
{10 + 10, 30, 1},
- /* C3 */
+ /* C3 . MPU CSWR + Core inactive */
{50 + 50, 300, 1},
- /* C4 */
+ /* C4 . MPU OFF + Core inactive */
{1500 + 1800, 4000, 1},
- /* C5 */
+ /* C5 . MPU RET + Core RET */
{2500 + 7500, 12000, 1},
- /* C6 */
+ /* C6 . MPU OFF + Core RET */
{3000 + 8500, 15000, 1},
- /* C7 */
+ /* C7 . MPU OFF + Core OFF */
{10000 + 30000, 300000, 1},
};
#define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
@@ -64,7 +64,6 @@ static struct cpuidle_params cpuidle_params_table[] = {
/* Mach specific information to be recorded in the C-state driver_data */
struct omap3_idle_statedata {
u32 mpu_state;
- u32 core_state;
u8 valid;
};
struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
@@ -98,7 +97,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
{
struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
struct timespec ts_preidle, ts_postidle, ts_idle;
- u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
+ u32 mpu_state = cx->mpu_state;
/* Used to keep track of the total time in idle */
getnstimeofday(&ts_preidle);
@@ -107,7 +106,6 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
local_fiq_disable();
pwrdm_set_next_pwrst(mpu_pd, mpu_state);
- pwrdm_set_next_pwrst(core_pd, core_state);
if (omap_irq_pending() || need_resched())
goto return_sleep_time;
@@ -156,6 +154,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
u32 mpu_deepest_state = PWRDM_POWER_RET;
u32 core_deepest_state = PWRDM_POWER_RET;
+ u32 core_next_state = pwrdm_read_next_pwrst(core_pd);
if (enable_off_mode) {
mpu_deepest_state = PWRDM_POWER_OFF;
@@ -171,7 +170,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
/* Check if current state is valid */
if ((cx->valid) &&
(cx->mpu_state >= mpu_deepest_state) &&
- (cx->core_state >= core_deepest_state)) {
+ (core_next_state >= core_deepest_state)) {
return curr;
} else {
int idx = OMAP3_NUM_STATES - 1;
@@ -196,7 +195,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
cx = cpuidle_get_statedata(&dev->states[idx]);
if ((cx->valid) &&
(cx->mpu_state >= mpu_deepest_state) &&
- (cx->core_state >= core_deepest_state)) {
+ (core_next_state >= core_deepest_state)) {
next = &dev->states[idx];
break;
}
@@ -242,19 +241,11 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
}
/*
- * FIXME: we currently manage device-specific idle states
- * for PER and CORE in combination with CPU-specific
- * idle states. This is wrong, and device-specific
- * idle management needs to be separated out into
- * its own code.
- */
-
- /*
* Prevent PER off if CORE is not in retention or off as this
* would disable PER wakeups completely.
*/
cx = cpuidle_get_statedata(state);
- core_next_state = cx->core_state;
+ core_next_state = pwrdm_read_next_pwrst(core_pd);
per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
if ((per_next_state == PWRDM_POWER_OFF) &&
(core_next_state > PWRDM_POWER_RET))
@@ -346,32 +337,26 @@ int __init omap3_idle_init(void)
dev->safe_state = &dev->states[0];
cx->valid = 1; /* C1 is always valid */
cx->mpu_state = PWRDM_POWER_ON;
- cx->core_state = PWRDM_POWER_ON;
/* C2 . MPU WFI + Core inactive */
cx = _fill_cstate(dev, 1, "MPU ON + CORE ON");
cx->mpu_state = PWRDM_POWER_ON;
- cx->core_state = PWRDM_POWER_ON;
/* C3 . MPU CSWR + Core inactive */
cx = _fill_cstate(dev, 2, "MPU RET + CORE ON");
cx->mpu_state = PWRDM_POWER_RET;
- cx->core_state = PWRDM_POWER_ON;
/* C4 . MPU OFF + Core inactive */
cx = _fill_cstate(dev, 3, "MPU OFF + CORE ON");
cx->mpu_state = PWRDM_POWER_OFF;
- cx->core_state = PWRDM_POWER_ON;
/* C5 . MPU RET + Core RET */
cx = _fill_cstate(dev, 4, "MPU RET + CORE RET");
cx->mpu_state = PWRDM_POWER_RET;
- cx->core_state = PWRDM_POWER_RET;
/* C6 . MPU OFF + Core RET */
cx = _fill_cstate(dev, 5, "MPU OFF + CORE RET");
cx->mpu_state = PWRDM_POWER_OFF;
- cx->core_state = PWRDM_POWER_RET;
/* C7 . MPU OFF + Core OFF */
cx = _fill_cstate(dev, 6, "MPU OFF + CORE OFF");
@@ -386,7 +371,6 @@ int __init omap3_idle_init(void)
__func__);
}
cx->mpu_state = PWRDM_POWER_OFF;
- cx->core_state = PWRDM_POWER_OFF;
dev->state_count = OMAP3_NUM_STATES;
if (cpuidle_register_device(dev)) {
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 4e166ad..aca3b6c 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -43,9 +43,22 @@ static inline int omap4_opp_init(void)
* omap3_pm_init_cpuidle
*/
struct cpuidle_params {
- u32 exit_latency; /* exit_latency = sleep + wake-up latencies */
+ /*
+ * exit_latency = sleep + wake-up latencies of the MPU,
+ * which include the MPU itself and the peripherals needed
+ * for the MPU to execute instructions (e.g. main memory,
+ * caches, IRQ controller, MMU etc). Some of those peripherals
+ * can belong to other power domains than the MPU subsystem and so
+ * the corresponding latencies must be included in this figure.
+ */
+ u32 exit_latency;
+ /*
+ * target_residency: required amount of time in the C state
+ * to break even on energy cost
+ */
u32 target_residency;
- u8 valid; /* validates the C-state */
+ /* validates the C-state on the given board */
+ u8 valid;
};
#if defined(CONFIG_PM) && defined(CONFIG_CPU_IDLE)
--
1.7.4.1
^ permalink raw reply related
* [PATCH 14/15] OMAP: PM CONSTRAINTS: implement the devices wake-up latency constraints
From: jean.pihet @ 2011-08-16 13:43 UTC (permalink / raw)
To: Mark Brown, Kevin Hilman, markgross, Linux PM mailing list; +Cc: Jean Pihet
In-Reply-To: <1313502198-9298-1-git-send-email-j-pihet@ti.com>
From: Jean Pihet <j-pihet@ti.com>
Implement the devices wake-up latency constraints using the global
device PM QoS notification handler which applies the constraints to the
underlying layer by calling the corresponding function at hwmod level.
Note: the bus throughput function is implemented but currently is
a no-op. A new PM QoS class for the bus throughput needs to be
added.
Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/plat-omap/include/plat/omap-pm.h | 128 ---------------------
arch/arm/plat-omap/omap-pm-constraints.c | 173 +++++++++++++---------------
arch/arm/plat-omap/omap-pm-noop.c | 89 ---------------
3 files changed, 80 insertions(+), 310 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
index 0840df8..d276082 100644
--- a/arch/arm/plat-omap/include/plat/omap-pm.h
+++ b/arch/arm/plat-omap/include/plat/omap-pm.h
@@ -62,136 +62,8 @@ void omap_pm_if_exit(void);
* Device-driver-originated constraints (via board-*.c files, platform_data)
*/
-
-/**
- * omap_pm_set_max_mpu_wakeup_lat - set the maximum MPU wakeup latency
- * @dev: struct device * requesting the constraint
- * @t: maximum MPU wakeup latency in microseconds
- *
- * Request that the maximum interrupt latency for the MPU to be no
- * greater than @t microseconds. "Interrupt latency" in this case is
- * defined as the elapsed time from the occurrence of a hardware or
- * timer interrupt to the time when the device driver's interrupt
- * service routine has been entered by the MPU.
- *
- * It is intended that underlying PM code will use this information to
- * determine what power state to put the MPU powerdomain into, and
- * possibly the CORE powerdomain as well, since interrupt handling
- * code currently runs from SDRAM. Advanced PM or board*.c code may
- * also configure interrupt controller priorities, OCP bus priorities,
- * CPU speed(s), etc.
- *
- * This function will not affect device wakeup latency, e.g., time
- * elapsed from when a device driver enables a hardware device with
- * clk_enable(), to when the device is ready for register access or
- * other use. To control this device wakeup latency, use
- * omap_pm_set_max_dev_wakeup_lat()
- *
- * Multiple calls to omap_pm_set_max_mpu_wakeup_lat() will replace the
- * previous t value. To remove the latency target for the MPU, call
- * with t = -1.
- *
- * XXX This constraint will be deprecated soon in favor of the more
- * general omap_pm_set_max_dev_wakeup_lat()
- *
- * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
- * is not satisfiable, or 0 upon success.
- */
-int omap_pm_set_max_mpu_wakeup_lat(struct device *dev, long t);
-
-
-/**
- * omap_pm_set_min_bus_tput - set minimum bus throughput needed by device
- * @dev: struct device * requesting the constraint
- * @tbus_id: interconnect to operate on (OCP_{INITIATOR,TARGET}_AGENT)
- * @r: minimum throughput (in KiB/s)
- *
- * Request that the minimum data throughput on the OCP interconnect
- * attached to device @dev interconnect agent @tbus_id be no less
- * than @r KiB/s.
- *
- * It is expected that the OMAP PM or bus code will use this
- * information to set the interconnect clock to run at the lowest
- * possible speed that satisfies all current system users. The PM or
- * bus code will adjust the estimate based on its model of the bus, so
- * device driver authors should attempt to specify an accurate
- * quantity for their device use case, and let the PM or bus code
- * overestimate the numbers as necessary to handle request/response
- * latency, other competing users on the system, etc. On OMAP2/3, if
- * a driver requests a minimum L4 interconnect speed constraint, the
- * code will also need to add an minimum L3 interconnect speed
- * constraint,
- *
- * Multiple calls to omap_pm_set_min_bus_tput() will replace the
- * previous rate value for this device. To remove the interconnect
- * throughput restriction for this device, call with r = 0.
- *
- * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
- * is not satisfiable, or 0 upon success.
- */
int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r);
-
-/**
- * omap_pm_set_max_dev_wakeup_lat - set the maximum device enable latency
- * @req_dev: struct device * requesting the constraint, or NULL if none
- * @dev: struct device * to set the constraint one
- * @t: maximum device wakeup latency in microseconds
- *
- * Request that the maximum amount of time necessary for a device @dev
- * to become accessible after its clocks are enabled should be no
- * greater than @t microseconds. Specifically, this represents the
- * time from when a device driver enables device clocks with
- * clk_enable(), to when the register reads and writes on the device
- * will succeed. This function should be called before clk_disable()
- * is called, since the power state transition decision may be made
- * during clk_disable().
- *
- * It is intended that underlying PM code will use this information to
- * determine what power state to put the powerdomain enclosing this
- * device into.
- *
- * Multiple calls to omap_pm_set_max_dev_wakeup_lat() will replace the
- * previous wakeup latency values for this device. To remove the
- * wakeup latency restriction for this device, call with t = -1.
- *
- * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
- * is not satisfiable, or 0 upon success.
- */
-int omap_pm_set_max_dev_wakeup_lat(struct device *req_dev, struct device *dev,
- long t);
-
-
-/**
- * omap_pm_set_max_sdma_lat - set the maximum system DMA transfer start latency
- * @dev: struct device *
- * @t: maximum DMA transfer start latency in microseconds
- *
- * Request that the maximum system DMA transfer start latency for this
- * device 'dev' should be no greater than 't' microseconds. "DMA
- * transfer start latency" here is defined as the elapsed time from
- * when a device (e.g., McBSP) requests that a system DMA transfer
- * start or continue, to the time at which data starts to flow into
- * that device from the system DMA controller.
- *
- * It is intended that underlying PM code will use this information to
- * determine what power state to put the CORE powerdomain into.
- *
- * Since system DMA transfers may not involve the MPU, this function
- * will not affect MPU wakeup latency. Use set_max_cpu_lat() to do
- * so. Similarly, this function will not affect device wakeup latency
- * -- use set_max_dev_wakeup_lat() to affect that.
- *
- * Multiple calls to set_max_sdma_lat() will replace the previous t
- * value for this device. To remove the maximum DMA latency for this
- * device, call with t = -1.
- *
- * Returns -EINVAL for an invalid argument, -ERANGE if the constraint
- * is not satisfiable, or 0 upon success.
- */
-int omap_pm_set_max_sdma_lat(struct device *dev, long t);
-
-
/**
* omap_pm_set_min_clk_rate - set minimum clock rate requested by @dev
* @dev: struct device * requesting the constraint
diff --git a/arch/arm/plat-omap/omap-pm-constraints.c b/arch/arm/plat-omap/omap-pm-constraints.c
index c8b4e4c..efec5df 100644
--- a/arch/arm/plat-omap/omap-pm-constraints.c
+++ b/arch/arm/plat-omap/omap-pm-constraints.c
@@ -17,132 +17,112 @@
#undef DEBUG
#include <linux/init.h>
+#include <linux/notifier.h>
#include <linux/cpufreq.h>
#include <linux/device.h>
#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
/* Interface documentation is in mach/omap-pm.h */
#include <plat/omap-pm.h>
#include <plat/omap_device.h>
+#include <plat/common.h>
+#include <plat/omap_hwmod.h>
static bool off_mode_enabled;
static u32 dummy_context_loss_counter;
-/*
- * Device-driver-originated constraints (via board-*.c files)
- */
+static int _dev_pm_qos_wakeup_latency_handler(struct notifier_block *,
+ unsigned long, void *);
+static struct notifier_block _dev_pm_qos_wakeup_latency_notifier = {
+ .notifier_call = _dev_pm_qos_wakeup_latency_handler,
+};
-int omap_pm_set_max_mpu_wakeup_lat(struct device *dev, long t)
+
+static int _apply_dev_pm_qos_constraint(void *req, unsigned long new_value)
{
- if (!dev || t < -1) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
- return -EINVAL;
- };
+ struct omap_device *od;
+ struct omap_hwmod *oh;
+ struct platform_device *pdev;
+ struct dev_pm_qos_request *dev_pm_qos_req = req;
- if (t == -1)
- pr_debug("OMAP PM: remove max MPU wakeup latency constraint: "
- "dev %s\n", dev_name(dev));
- else
- pr_debug("OMAP PM: add max MPU wakeup latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
+ pr_debug("OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n",
+ req, new_value);
- /*
- * For current Linux, this needs to map the MPU to a
- * powerdomain, then go through the list of current max lat
- * constraints on the MPU and find the smallest. If
- * the latency constraint has changed, the code should
- * recompute the state to enter for the next powerdomain
- * state.
- *
- * TI CDP code can call constraint_set here.
- */
+ /* Look for the platform device for the constraint target device */
+ pdev = to_platform_device(dev_pm_qos_req->dev);
- return 0;
-}
+ /* Try to catch non platform devices */
+ if (pdev->name == NULL) {
+ pr_err("%s: Error: platform device for device %s not valid\n",
+ __func__, dev_name(dev_pm_qos_req->dev));
+ return -EINVAL;
+ }
-int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r)
-{
- if (!dev || (agent_id != OCP_INITIATOR_AGENT &&
- agent_id != OCP_TARGET_AGENT)) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
+ /* Find the associated omap_device for dev */
+ od = container_of(pdev, struct omap_device, pdev);
+ if (od->hwmods_cnt != 1) {
+ pr_err("%s: Error: No unique hwmod for device %s\n",
+ __func__, dev_name(dev_pm_qos_req->dev));
return -EINVAL;
- };
+ }
- if (r == 0)
- pr_debug("OMAP PM: remove min bus tput constraint: "
- "dev %s for agent_id %d\n", dev_name(dev), agent_id);
- else
- pr_debug("OMAP PM: add min bus tput constraint: "
- "dev %s for agent_id %d: rate %ld KiB\n",
- dev_name(dev), agent_id, r);
+ /* Find the primary omap_hwmod for dev */
+ oh = od->hwmods[0];
- /*
- * This code should model the interconnect and compute the
- * required clock frequency, convert that to a VDD2 OPP ID, then
- * set the VDD2 OPP appropriately.
- *
- * TI CDP code can call constraint_set here on the VDD2 OPP.
- */
+ pr_debug("OMAP PM CONSTRAINTS: req@0x%p, dev=0x%p, new_value=%lu\n",
+ req, dev_pm_qos_req->dev, new_value);
- return 0;
+ /* Apply the constraint */
+ return omap_hwmod_set_wkup_lat_constraint(oh, dev_pm_qos_req,
+ new_value);
}
-int omap_pm_set_max_dev_wakeup_lat(struct device *req_dev, struct device *dev,
- long t)
+/* PM QoS classes handlers */
+static int _dev_pm_qos_wakeup_latency_handler(struct notifier_block *nb,
+ unsigned long new_value,
+ void *req)
{
- if (!req_dev || !dev || t < -1) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
- return -EINVAL;
- };
-
- if (t == -1)
- pr_debug("OMAP PM: remove max device latency constraint: "
- "dev %s\n", dev_name(dev));
- else
- pr_debug("OMAP PM: add max device latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
-
- /*
- * For current Linux, this needs to map the device to a
- * powerdomain, then go through the list of current max lat
- * constraints on that powerdomain and find the smallest. If
- * the latency constraint has changed, the code should
- * recompute the state to enter for the next powerdomain
- * state. Conceivably, this code should also determine
- * whether to actually disable the device clocks or not,
- * depending on how long it takes to re-enable the clocks.
- *
- * TI CDP code can call constraint_set here.
- */
-
- return 0;
+ return _apply_dev_pm_qos_constraint(req, new_value);
}
-int omap_pm_set_max_sdma_lat(struct device *dev, long t)
+/*
+ * omap_pm_set_min_bus_tput - set/release bus throughput constraints
+ * ToDo: currently is a no-op, to be converted to a PM QoS handler
+ * for the TPUT class
+ */
+int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r)
{
- if (!dev || t < -1) {
+ long t;
+ struct device *req_dev = NULL;
+
+ if (!dev || (agent_id != OCP_INITIATOR_AGENT &&
+ agent_id != OCP_TARGET_AGENT)) {
WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
return -EINVAL;
};
- if (t == -1)
- pr_debug("OMAP PM: remove max DMA latency constraint: "
- "dev %s\n", dev_name(dev));
+ /*
+ * A value of r == 0 removes the constraint. Convert it to the
+ * generic _set_dev_constraint convention (-1 for constraint removal)
+ */
+ if (r == 0)
+ t = -1;
else
- pr_debug("OMAP PM: add max DMA latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
+ t = r;
/*
- * For current Linux PM QOS params, this code should scan the
- * list of maximum CPU and DMA latencies and select the
- * smallest, then set cpu_dma_latency pm_qos_param
- * accordingly.
- *
- * For future Linux PM QOS params, with separate CPU and DMA
- * latency params, this code should just set the dma_latency param.
- *
- * TI CDP code can call constraint_set here.
+ * Assign the device for L3 or L4 interconnect to req_dev,
+ * based on the value of agent_id
*/
+ switch (agent_id) {
+ case OCP_INITIATOR_AGENT:
+ req_dev = omap2_get_l3_device();
+ break;
+ case OCP_TARGET_AGENT:
+ /* Fixme: need the device for L4 interconnect */
+ break;
+ }
return 0;
}
@@ -350,10 +330,17 @@ int __init omap_pm_if_early_init(void)
return 0;
}
-/* Must be called after clock framework is initialized */
+/* Must be called after the clock framework is initialized */
int __init omap_pm_if_init(void)
{
- return 0;
+ int ret;
+
+ ret = dev_pm_qos_add_global_notifier(
+ &_dev_pm_qos_wakeup_latency_notifier);
+ if (ret)
+ WARN(1, KERN_ERR "Cannot add global notifier for dev PM QoS\n");
+
+ return ret;
}
void omap_pm_if_exit(void)
diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c
index b0471bb2..8ad902f 100644
--- a/arch/arm/plat-omap/omap-pm-noop.c
+++ b/arch/arm/plat-omap/omap-pm-noop.c
@@ -32,35 +32,6 @@ static u32 dummy_context_loss_counter;
/*
* Device-driver-originated constraints (via board-*.c files)
*/
-
-int omap_pm_set_max_mpu_wakeup_lat(struct device *dev, long t)
-{
- if (!dev || t < -1) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
- return -EINVAL;
- };
-
- if (t == -1)
- pr_debug("OMAP PM: remove max MPU wakeup latency constraint: "
- "dev %s\n", dev_name(dev));
- else
- pr_debug("OMAP PM: add max MPU wakeup latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
-
- /*
- * For current Linux, this needs to map the MPU to a
- * powerdomain, then go through the list of current max lat
- * constraints on the MPU and find the smallest. If
- * the latency constraint has changed, the code should
- * recompute the state to enter for the next powerdomain
- * state.
- *
- * TI CDP code can call constraint_set here.
- */
-
- return 0;
-}
-
int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r)
{
if (!dev || (agent_id != OCP_INITIATOR_AGENT &&
@@ -88,66 +59,6 @@ int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r)
return 0;
}
-int omap_pm_set_max_dev_wakeup_lat(struct device *req_dev, struct device *dev,
- long t)
-{
- if (!req_dev || !dev || t < -1) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
- return -EINVAL;
- };
-
- if (t == -1)
- pr_debug("OMAP PM: remove max device latency constraint: "
- "dev %s\n", dev_name(dev));
- else
- pr_debug("OMAP PM: add max device latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
-
- /*
- * For current Linux, this needs to map the device to a
- * powerdomain, then go through the list of current max lat
- * constraints on that powerdomain and find the smallest. If
- * the latency constraint has changed, the code should
- * recompute the state to enter for the next powerdomain
- * state. Conceivably, this code should also determine
- * whether to actually disable the device clocks or not,
- * depending on how long it takes to re-enable the clocks.
- *
- * TI CDP code can call constraint_set here.
- */
-
- return 0;
-}
-
-int omap_pm_set_max_sdma_lat(struct device *dev, long t)
-{
- if (!dev || t < -1) {
- WARN(1, "OMAP PM: %s: invalid parameter(s)", __func__);
- return -EINVAL;
- };
-
- if (t == -1)
- pr_debug("OMAP PM: remove max DMA latency constraint: "
- "dev %s\n", dev_name(dev));
- else
- pr_debug("OMAP PM: add max DMA latency constraint: "
- "dev %s, t = %ld usec\n", dev_name(dev), t);
-
- /*
- * For current Linux PM QOS params, this code should scan the
- * list of maximum CPU and DMA latencies and select the
- * smallest, then set cpu_dma_latency pm_qos_param
- * accordingly.
- *
- * For future Linux PM QOS params, with separate CPU and DMA
- * latency params, this code should just set the dma_latency param.
- *
- * TI CDP code can call constraint_set here.
- */
-
- return 0;
-}
-
int omap_pm_set_min_clk_rate(struct device *dev, struct clk *c, long r)
{
if (!dev || !c || r < 0) {
--
1.7.4.1
^ permalink raw reply related
* [PATCH 13/15] OMAP2+: omap_hwmod: manage the wake-up latency constraints
From: jean.pihet @ 2011-08-16 13:43 UTC (permalink / raw)
To: Mark Brown, Kevin Hilman, markgross, Linux PM mailing list; +Cc: Jean Pihet
In-Reply-To: <1313502198-9298-1-git-send-email-j-pihet@ti.com>
From: Jean Pihet <j-pihet@ti.com>
Hwmod is queried from the OMAP_PM layer to manage the power domains
wake-up latency constraints. Hwmod retrieves the correct power domain
and if it exists it calls the corresponding power domain function.
Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/omap_hwmod.c | 26 +++++++++++++++++++++++++-
arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 ++
2 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 84cc0bd..c6b1cc9 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -143,6 +143,7 @@
#include "powerdomain.h"
#include <plat/clock.h>
#include <plat/omap_hwmod.h>
+#include <plat/omap_device.h>
#include <plat/prcm.h>
#include "cm2xxx_3xxx.h"
@@ -2618,11 +2619,34 @@ ohsps_unlock:
return ret;
}
+/*
+ * omap_hwmod_set_wkup_constraint- set/release a wake-up latency constraint
+ *
+ * @oh: struct omap_hwmod* to which the target device belongs to.
+ * @cookie: identifier of the constraints list for @oh.
+ * @min_latency: the minimum allowed wake-up latency for @oh.
+ *
+ * Returns 0 upon success.
+ */
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh,
+ void *cookie, long min_latency)
+{
+ struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
+
+ if (!pwrdm) {
+ pr_err("%s: Error: could not find powerdomain "
+ "for %s\n", __func__, oh->name);
+ return -EINVAL;
+ }
+
+ return pwrdm_set_wkup_lat_constraint(pwrdm, cookie, min_latency);
+}
+
/**
* omap_hwmod_get_context_loss_count - get lost context count
* @oh: struct omap_hwmod *
*
- * Query the powerdomain of of @oh to get the context loss
+ * Query the powerdomain of @oh to get the context loss
* count for this device.
*
* Returns the context loss count of the powerdomain assocated with @oh
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 0e329ca..75e0e7a 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -603,6 +603,8 @@ int omap_hwmod_for_each_by_class(const char *classname,
void *user);
int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh, void *cookie,
+ long min_latency);
u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
--
1.7.4.1
^ permalink raw reply related
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