* Re: [PATCH 5/7] PM: PM notifier error injection
From: Rafael J. Wysocki @ 2011-07-08 17:43 UTC (permalink / raw)
To: Akinobu Mita; +Cc: akpm, linux-kernel, linux-pm
In-Reply-To: <CAC5umyifMvVTG29JyvBtc_YThorK=AZHZypheUvbtvmShnbTog@mail.gmail.com>
On Friday, July 08, 2011, Akinobu Mita wrote:
> >> --- a/kernel/power/main.c
> >> +++ b/kernel/power/main.c
> >> @@ -42,6 +42,36 @@ int pm_notifier_call_chain(unsigned long val)
> >> return notifier_to_errno(ret);
> >> }
> >>
> >> +#ifdef CONFIG_PM_NOTIFIER_ERROR_INJECTION
> >> +
> >> +static struct err_inject_notifier_block err_inject_pm_notifier = {
> >> + .actions = {
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_HIBERNATION_PREPARE) },
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_SUSPEND_PREPARE) },
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_RESTORE_PREPARE) },
> >> + {}
> >
> > Why have you omitted the PM_POST_* actions?
>
> Because the callbacks of PM_POST_* are not supposed to fail (i.e.
> the return value of pm_notifier_call() is ignored).
OK, fair enough.
> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -1043,6 +1043,14 @@ config CPU_NOTIFIER_ERROR_INJECTION
> >> # echo 0 > /sys/devices/system/cpu/cpu1/online
> >> bash: echo: write error: Operation not permitted
> >>
> >> +config PM_NOTIFIER_ERROR_INJECTION
> >> + bool "PM notifier error injection"
> >> + depends on PM_DEBUG && NOTIFIER_ERROR_INJECTION
> >> + help
> >> + This option provides the ability to inject artifical errors to
> >> + PM notifier chain callbacks. It is controlled through debugfs
> >> + interface under /sys/kernel/debug/pm-notifier-error-inject/
> >
> > I'm not sure the help is necessary. I think it should be selected
> > automatically if both PM_DEBUG and NOTIFIER_ERROR_INJECTION are set.
>
> OK, I'll try to tweak the option to be better.
Thanks!
Rafael
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Kevin Hilman @ 2011-07-08 17:20 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <Pine.LNX.4.44L0.1107081033000.2208-100000@iolanthe.rowland.org>
Alan Stern <stern@rowland.harvard.edu> writes:
> On Fri, 8 Jul 2011, Rafael J. Wysocki wrote:
>
>> On Friday, July 08, 2011, Kevin Hilman wrote:
>> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>> >
>> > > From: Rafael J. Wysocki <rjw@sisk.pl>
>> > >
>> > > Make generic PM domains support system-wide power transitions
>> > > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
>> > > poweroff and restore callbacks to be associated with struct
>> > > generic_pm_domain objects and make pm_genpd_init() use them as
>> > > appropriate.
>> > >
>> > > The new callbacks do nothing for devices belonging to power domains
>> > > that were powered down at run time (before the transition).
>> >
>> > Thinking about this some more, how is a driver supposed to reconfigure
>> > wakeups during suspend if it has already been runtime suspended?
>>
>> If the device belongs to a PM domain that has been powered off, it
>> won't be notified.
>>
>> > For example, assume a device where device_may_wakeup() == false. This
>> > means wakeups during *suspend* are disabled, but wakeups wakeups are
>> > assumed to enabled when it is runtime suspended.
>> >
>> > So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
>> > and then system suspend comes along.
>> >
>> > With this current patch, the driver will never receive any callbacks, so
>> > it can never disable its wakeups.
>> >
>> > Am I missing something?
>>
>> As I said above, this only happens with devices that belog to PM domains
>> that were powered off before system suspend has started, so the problem
>> is limited to devices that wakeup is signaled on behalf of even when they
>> have no power.
Which on OMAP, is *all* devices, so that's a pretty major limitation. :)
>> So this is a limitation, but not affecting all platforms.
>>
>> There are a few ways to avoid this limitation I can think of:
>> (1) Add a "make me operational during system suspend" flag to struct dev_pm_info
>> and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
>> core, or pm_genpd_prepare()).
>
> What's to prevent the device from being runtime-suspended again before
> the wakeup setting can be changed?
>
>> (2) Add a "my .prepare() is safe to run if device is not accessible" flag to
>> struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
>> devices regardless of whether or not their PM domains are off.
>> (3) Call .prepare() from all drivers unconditionally during system suspend
>> (and probably .complete() too) in the hope they won't access inaccessible
>> devices.
Like Alan's comment above for (1), I think the same applies for (3)
since runtime PM transitions can still happen between .prepare() and
.suspend()
Kevin
>> Probably, there's more.
>
> In the PM domain's suspend code, do a runtime resume if the wakeup
> setting needs to be changed, rather than simply skipping over the
> device.
>
>> In any case I think it's material for future work.
>
> Alan Stern
^ permalink raw reply
* Re: [PATCH 5/7] PM: PM notifier error injection
From: Akinobu Mita @ 2011-07-08 15:56 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: akpm, linux-kernel, linux-pm
In-Reply-To: <201107072314.59052.rjw@sisk.pl>
>> --- a/kernel/power/main.c
>> +++ b/kernel/power/main.c
>> @@ -42,6 +42,36 @@ int pm_notifier_call_chain(unsigned long val)
>> return notifier_to_errno(ret);
>> }
>>
>> +#ifdef CONFIG_PM_NOTIFIER_ERROR_INJECTION
>> +
>> +static struct err_inject_notifier_block err_inject_pm_notifier = {
>> + .actions = {
>> + { ERR_INJECT_NOTIFIER_ACTION(PM_HIBERNATION_PREPARE) },
>> + { ERR_INJECT_NOTIFIER_ACTION(PM_SUSPEND_PREPARE) },
>> + { ERR_INJECT_NOTIFIER_ACTION(PM_RESTORE_PREPARE) },
>> + {}
>
> Why have you omitted the PM_POST_* actions?
Because the callbacks of PM_POST_* are not supposed to fail (i.e.
the return value of pm_notifier_call() is ignored).
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1043,6 +1043,14 @@ config CPU_NOTIFIER_ERROR_INJECTION
>> # echo 0 > /sys/devices/system/cpu/cpu1/online
>> bash: echo: write error: Operation not permitted
>>
>> +config PM_NOTIFIER_ERROR_INJECTION
>> + bool "PM notifier error injection"
>> + depends on PM_DEBUG && NOTIFIER_ERROR_INJECTION
>> + help
>> + This option provides the ability to inject artifical errors to
>> + PM notifier chain callbacks. It is controlled through debugfs
>> + interface under /sys/kernel/debug/pm-notifier-error-inject/
>
> I'm not sure the help is necessary. I think it should be selected
> automatically if both PM_DEBUG and NOTIFIER_ERROR_INJECTION are set.
OK, I'll try to tweak the option to be better.
^ permalink raw reply
* Re: [PATCH 1/7] pm: improve error code of pm_notifier_call_chain()
From: Akinobu Mita @ 2011-07-08 15:50 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-s390, Jiri Kosina, Heiko Carstens, linux-kernel,
Martin Schwidefsky, linux390, akpm, linux-pm
In-Reply-To: <201107072306.02708.rjw@sisk.pl>
2011/7/8 Rafael J. Wysocki <rjw@sisk.pl>:
> On Monday, July 04, 2011, Akinobu Mita wrote:
>> 2011/7/4 Pavel Machek <pavel@ucw.cz>:
>> > On Sun 2011-07-03 23:16:15, Akinobu Mita wrote:
>> >> This enables pm_notifier_call_chain() to get the actual error code
>> >> in the callback rather than always assume -EINVAL by converting all
>> >> PM notifier calls to return encapsulate error code with
>> >> notifier_from_errno().
>> >>
>> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> >> Cc: Pavel Machek <pavel@ucw.cz>
>> >
>> > Nothing obviously wrong here.
>>
>> Thanks. Can I add your ACK for this patch?
>
> Do you want me to take this patch?
Yes, please merge it to your tree.
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Alan Stern @ 2011-07-08 14:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <201107081124.17463.rjw@sisk.pl>
On Fri, 8 Jul 2011, Rafael J. Wysocki wrote:
> On Friday, July 08, 2011, Kevin Hilman wrote:
> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> >
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > >
> > > Make generic PM domains support system-wide power transitions
> > > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> > > poweroff and restore callbacks to be associated with struct
> > > generic_pm_domain objects and make pm_genpd_init() use them as
> > > appropriate.
> > >
> > > The new callbacks do nothing for devices belonging to power domains
> > > that were powered down at run time (before the transition).
> >
> > Thinking about this some more, how is a driver supposed to reconfigure
> > wakeups during suspend if it has already been runtime suspended?
>
> If the device belongs to a PM domain that has been powered off, it
> won't be notified.
>
> > For example, assume a device where device_may_wakeup() == false. This
> > means wakeups during *suspend* are disabled, but wakeups wakeups are
> > assumed to enabled when it is runtime suspended.
> >
> > So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
> > and then system suspend comes along.
> >
> > With this current patch, the driver will never receive any callbacks, so
> > it can never disable its wakeups.
> >
> > Am I missing something?
>
> As I said above, this only happens with devices that belog to PM domains
> that were powered off before system suspend has started, so the problem
> is limited to devices that wakeup is signaled on behalf of even when they
> have no power.
>
> So this is a limitation, but not affecting all platforms.
>
> There are a few ways to avoid this limitation I can think of:
> (1) Add a "make me operational during system suspend" flag to struct dev_pm_info
> and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
> core, or pm_genpd_prepare()).
What's to prevent the device from being runtime-suspended again before
the wakeup setting can be changed?
> (2) Add a "my .prepare() is safe to run if device is not accessible" flag to
> struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
> devices regardless of whether or not their PM domains are off.
> (3) Call .prepare() from all drivers unconditionally during system suspend
> (and probably .complete() too) in the hope they won't access inaccessible
> devices.
> Probably, there's more.
In the PM domain's suspend code, do a runtime resume if the wakeup
setting needs to be changed, rather than simply skipping over the
device.
> In any case I think it's material for future work.
Alan Stern
^ permalink raw reply
* Re: [RFC] Unprepare callback for cpuidle_device
From: Deepthi Dharwar @ 2011-07-08 13:03 UTC (permalink / raw)
To: asinghal; +Cc: linux-pm, johlstei
In-Reply-To: <82b596dce7b84db206d3afb6da564fb9.squirrel@www.codeaurora.org>
Hi Amar,
On Friday 08 July 2011 01:22 AM, asinghal@codeaurora.org wrote:
> Hello Trinabh,
> i cannot use the enter callback due to the following reason:
>
> the residency calculation(tick)nohz_get_sleep_length) and the idle state
> selection happens in the menu governor. The enter callback is called with
> the selected state.
One would first execute prepare then call select and enter .
So in the newer proposed code, there is no prepare routine. One would
just execute select() and then enter() (Prepare functionality is part of the
enter routine itself) Is it not possible to cancel the timer,
calculate the execute nohz_get_sleep in enter routine again,
then select the idle state depending on the time . Automatically promote or demote
idle state based on the latest value returned in the driver ?
> So cancelling the hrtimer that would affect the residency value calculated
> in the menu governor, in the enter callback is not possible. The timer
> needs to be cancelled before the select call is made.
>
> thanks,
> amar
>
>
>
>> On 07/06/2011 04:23 PM, asinghal@codeaurora.org wrote:
>>> We plan to use high resolution timers in one of our modules, with the
>>> requirement that we cancel these timers when the cpu goes idle and
>>> restart
>>> them when the cpu comes out of idle.
>>>
>>> We are cancelling the timers in cpuidle prepare callback. The problem is
>>> that if the need_resched() call in drivers/cpuidle/cpuidle.c returns
>>> true,
>>> how do we restart the timer? If the call returns false, we can restart
>>> the
>>> timer in the cpuidle enter callback.
>>
>> Hi Amar,
>>
>> I think you should not use cpuidle prepare callback at all. It may be
>> removed soon (see https://lkml.org/lkml/2011/6/6/261) and I think
>> there are better ways to achieve what you are trying to do.
>>
>> I think everything should go into the enter routines (the idle routines
>> provided by the driver). That way you would not have to worry about
>> need_resched() in cpuidle.c. Also it would be a cleaner implementation
>> as you wouldn't touch generic cpuidle code.
>>
>>>
>>> The solution to the problem that we have in mind is adding an unprepare
>>> callback to the cpuidle_device struct, and calling it if needs_resched()
>>> returns true. Another option is to implement deferred timers for
>>> hrtimers.
>>> Which of the two options is the better solution, or is there another
>>> feasible alternative?
>>
>> As i said, everything should go inside enter routine and
>> you wouldn't have to use/implement prepare/unprepare callbacks.
>>
>> Thanks
>> -Trinabh
>>
>
>
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm
Regards,
Deepthi
^ permalink raw reply
* [Update][PATCH 3/5] PM / Domains: Rework locking
From: Rafael J. Wysocki @ 2011-07-08 10:21 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107062253.48558.rjw@sisk.pl>
From: Rafael J. Wysocki <rjw@sisk.pl>
CUrrently, the .start_device() and .stop_device() callbacks from
struct generic_pm_domain() as well as the device drivers' runtime PM
callbacks used by the generic PM domains code are executed under
the generic PM domain lock. This, unfortunately, is prone to
deadlocks, for example if a device and its parent are boths members
of the same PM domain. For this reason, it would be better if the
PM domains code didn't execute device callbacks under the lock.
Rework the locking in the generic PM domains code so that the lock
is dropped for the execution of device callbacks. To this end,
introduce PM domains states reflecting the current status of a PM
domain and such that the PM domain lock cannot be acquired if the
status is GPD_STATE_BUSY. Make threads attempting to acquire a PM
domain's lock wait until the status changes to either
GPD_STATE_ACTIVE or GPD_STATE_POWER_OFF.
This change by itself doesn't fix the deadlock problem mentioned
above, but the mechanism introduced by it will be used for for this
purpose by a subsequent patch.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
pm_genpd_prepare() has to check pending wakeup requests before it changes
prepared_count, because otherwise the domain won't be powered on in
response to the wakeup request.
Thanks,
Rafael
---
drivers/base/power/domain.c | 249 +++++++++++++++++++++++++++++++-------------
include/linux/pm_domain.h | 10 +
2 files changed, 185 insertions(+), 74 deletions(-)
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,8 +11,11 @@
#include <linux/device.h>
-#define GPD_IN_SUSPEND 1
-#define GPD_POWER_OFF 2
+enum gpd_status {
+ GPD_STATE_ACTIVE = 0, /* PM domain is active */
+ GPD_STATE_BUSY, /* Something is happening to the PM domain */
+ GPD_STATE_POWER_OFF, /* PM domain is off */
+};
struct dev_power_governor {
bool (*power_down_ok)(struct dev_pm_domain *domain);
@@ -29,7 +32,8 @@ struct generic_pm_domain {
struct work_struct power_off_work;
unsigned int in_progress; /* Number of devices being suspended now */
unsigned int sd_count; /* Number of subdomains with power "on" */
- bool power_is_off; /* Whether or not power has been removed */
+ enum gpd_status status; /* Current state of the domain */
+ wait_queue_head_t status_wait_queue;
unsigned int device_count; /* Number of devices */
unsigned int suspended_count; /* System suspend device counter */
unsigned int prepared_count; /* Suspend counter of prepared devices */
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -13,6 +13,8 @@
#include <linux/pm_domain.h>
#include <linux/slab.h>
#include <linux/err.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
#ifdef CONFIG_PM
@@ -30,6 +32,34 @@ static void genpd_sd_counter_dec(struct
genpd->sd_count--;
}
+static void genpd_acquire_lock(struct generic_pm_domain *genpd)
+{
+ DEFINE_WAIT(wait);
+
+ mutex_lock(&genpd->lock);
+ /*
+ * Wait for the domain to transition into either the active,
+ * or the power off state.
+ */
+ for (;;) {
+ prepare_to_wait(&genpd->status_wait_queue, &wait,
+ TASK_UNINTERRUPTIBLE);
+ if (genpd->status != GPD_STATE_BUSY)
+ break;
+ mutex_unlock(&genpd->lock);
+
+ schedule();
+
+ mutex_lock(&genpd->lock);
+ }
+ finish_wait(&genpd->status_wait_queue, &wait);
+}
+
+static void genpd_release_lock(struct generic_pm_domain *genpd)
+{
+ mutex_unlock(&genpd->lock);
+}
+
/**
* pm_genpd_poweron - Restore power to a given PM domain and its parents.
* @genpd: PM domain to power up.
@@ -39,22 +69,50 @@ static void genpd_sd_counter_dec(struct
*/
static int pm_genpd_poweron(struct generic_pm_domain *genpd)
{
+ struct generic_pm_domain *parent = genpd->parent;
+ DEFINE_WAIT(wait);
int ret = 0;
start:
- if (genpd->parent)
- mutex_lock(&genpd->parent->lock);
- mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+ if (parent) {
+ mutex_lock(&parent->lock);
+ mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ mutex_lock(&genpd->lock);
+ }
+ /*
+ * Wait for the domain to transition into either the active,
+ * or the power off state.
+ */
+ for (;;) {
+ prepare_to_wait(&genpd->status_wait_queue, &wait,
+ TASK_UNINTERRUPTIBLE);
+ if (genpd->status != GPD_STATE_BUSY)
+ break;
+ mutex_unlock(&genpd->lock);
+ if (parent)
+ mutex_unlock(&parent->lock);
+
+ schedule();
- if (!genpd->power_is_off
+ if (parent) {
+ mutex_lock(&parent->lock);
+ mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ mutex_lock(&genpd->lock);
+ }
+ }
+ finish_wait(&genpd->status_wait_queue, &wait);
+
+ if (genpd->status == GPD_STATE_ACTIVE
|| (genpd->prepared_count > 0 && genpd->suspend_power_off))
goto out;
- if (genpd->parent && genpd->parent->power_is_off) {
+ if (parent && parent->status != GPD_STATE_ACTIVE) {
mutex_unlock(&genpd->lock);
- mutex_unlock(&genpd->parent->lock);
+ mutex_unlock(&parent->lock);
- ret = pm_genpd_poweron(genpd->parent);
+ ret = pm_genpd_poweron(parent);
if (ret)
return ret;
@@ -67,14 +125,14 @@ static int pm_genpd_poweron(struct gener
goto out;
}
- genpd->power_is_off = false;
- if (genpd->parent)
- genpd->parent->sd_count++;
+ genpd->status = GPD_STATE_ACTIVE;
+ if (parent)
+ parent->sd_count++;
out:
mutex_unlock(&genpd->lock);
- if (genpd->parent)
- mutex_unlock(&genpd->parent->lock);
+ if (parent)
+ mutex_unlock(&parent->lock);
return ret;
}
@@ -90,6 +148,7 @@ static int pm_genpd_poweron(struct gener
*/
static int __pm_genpd_save_device(struct dev_list_entry *dle,
struct generic_pm_domain *genpd)
+ __releases(&genpd->lock) __acquires(&genpd->lock)
{
struct device *dev = dle->dev;
struct device_driver *drv = dev->driver;
@@ -98,6 +157,8 @@ static int __pm_genpd_save_device(struct
if (dle->need_restore)
return 0;
+ mutex_unlock(&genpd->lock);
+
if (drv && drv->pm && drv->pm->runtime_suspend) {
if (genpd->start_device)
genpd->start_device(dev);
@@ -108,6 +169,8 @@ static int __pm_genpd_save_device(struct
genpd->stop_device(dev);
}
+ mutex_lock(&genpd->lock);
+
if (!ret)
dle->need_restore = true;
@@ -121,6 +184,7 @@ static int __pm_genpd_save_device(struct
*/
static void __pm_genpd_restore_device(struct dev_list_entry *dle,
struct generic_pm_domain *genpd)
+ __releases(&genpd->lock) __acquires(&genpd->lock)
{
struct device *dev = dle->dev;
struct device_driver *drv = dev->driver;
@@ -128,6 +192,8 @@ static void __pm_genpd_restore_device(st
if (!dle->need_restore)
return;
+ mutex_unlock(&genpd->lock);
+
if (drv && drv->pm && drv->pm->runtime_resume) {
if (genpd->start_device)
genpd->start_device(dev);
@@ -138,6 +204,8 @@ static void __pm_genpd_restore_device(st
genpd->stop_device(dev);
}
+ mutex_lock(&genpd->lock);
+
dle->need_restore = false;
}
@@ -150,13 +218,14 @@ static void __pm_genpd_restore_device(st
* the @genpd's devices' drivers and remove power from @genpd.
*/
static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+ __releases(&genpd->lock) __acquires(&genpd->lock)
{
struct generic_pm_domain *parent;
struct dev_list_entry *dle;
unsigned int not_suspended;
int ret;
- if (genpd->power_is_off || genpd->prepared_count > 0)
+ if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
return 0;
if (genpd->sd_count > 0)
@@ -175,22 +244,36 @@ static int pm_genpd_poweroff(struct gene
return -EAGAIN;
}
+ genpd->status = GPD_STATE_BUSY;
+
list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
ret = __pm_genpd_save_device(dle, genpd);
if (ret)
goto err_dev;
}
+ mutex_unlock(&genpd->lock);
+
+ parent = genpd->parent;
+ if (parent) {
+ genpd_acquire_lock(parent);
+ mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ mutex_lock(&genpd->lock);
+ }
+
if (genpd->power_off)
genpd->power_off(genpd);
- genpd->power_is_off = true;
+ genpd->status = GPD_STATE_POWER_OFF;
+ wake_up_all(&genpd->status_wait_queue);
- parent = genpd->parent;
if (parent) {
genpd_sd_counter_dec(parent);
if (parent->sd_count == 0)
queue_work(pm_wq, &parent->power_off_work);
+
+ genpd_release_lock(parent);
}
return 0;
@@ -199,6 +282,9 @@ static int pm_genpd_poweroff(struct gene
list_for_each_entry_continue(dle, &genpd->dev_list, node)
__pm_genpd_restore_device(dle, genpd);
+ genpd->status = GPD_STATE_ACTIVE;
+ wake_up_all(&genpd->status_wait_queue);
+
return ret;
}
@@ -212,13 +298,9 @@ static void genpd_power_off_work_fn(stru
genpd = container_of(work, struct generic_pm_domain, power_off_work);
- if (genpd->parent)
- mutex_lock(&genpd->parent->lock);
- mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+ genpd_acquire_lock(genpd);
pm_genpd_poweroff(genpd);
- mutex_unlock(&genpd->lock);
- if (genpd->parent)
- mutex_unlock(&genpd->parent->lock);
+ genpd_release_lock(genpd);
}
/**
@@ -239,23 +321,17 @@ static int pm_genpd_runtime_suspend(stru
if (IS_ERR(genpd))
return -EINVAL;
- if (genpd->parent)
- mutex_lock(&genpd->parent->lock);
- mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-
if (genpd->stop_device) {
int ret = genpd->stop_device(dev);
if (ret)
- goto out;
+ return ret;
}
+
+ genpd_acquire_lock(genpd);
genpd->in_progress++;
pm_genpd_poweroff(genpd);
genpd->in_progress--;
-
- out:
- mutex_unlock(&genpd->lock);
- if (genpd->parent)
- mutex_unlock(&genpd->parent->lock);
+ genpd_release_lock(genpd);
return 0;
}
@@ -276,9 +352,6 @@ static void __pm_genpd_runtime_resume(st
break;
}
}
-
- if (genpd->start_device)
- genpd->start_device(dev);
}
/**
@@ -304,9 +377,15 @@ static int pm_genpd_runtime_resume(struc
if (ret)
return ret;
- mutex_lock(&genpd->lock);
+ genpd_acquire_lock(genpd);
+ genpd->status = GPD_STATE_BUSY;
__pm_genpd_runtime_resume(dev, genpd);
- mutex_unlock(&genpd->lock);
+ genpd->status = GPD_STATE_ACTIVE;
+ wake_up_all(&genpd->status_wait_queue);
+ genpd_release_lock(genpd);
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
return 0;
}
@@ -339,7 +418,7 @@ static void pm_genpd_sync_poweroff(struc
{
struct generic_pm_domain *parent = genpd->parent;
- if (genpd->power_is_off)
+ if (genpd->status == GPD_STATE_POWER_OFF)
return;
if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
@@ -348,7 +427,7 @@ static void pm_genpd_sync_poweroff(struc
if (genpd->power_off)
genpd->power_off(genpd);
- genpd->power_is_off = true;
+ genpd->status = GPD_STATE_POWER_OFF;
if (parent) {
genpd_sd_counter_dec(parent);
pm_genpd_sync_poweroff(parent);
@@ -375,32 +454,41 @@ static int pm_genpd_prepare(struct devic
if (IS_ERR(genpd))
return -EINVAL;
- mutex_lock(&genpd->lock);
+ /*
+ * If a wakeup request is pending for the device, it should be woken up
+ * at this point and a system wakeup event should be reported if it's
+ * set up to wake up the system from sleep states.
+ */
+ pm_runtime_get_noresume(dev);
+ if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+ pm_wakeup_event(dev, 0);
+
+ if (pm_wakeup_pending()) {
+ pm_runtime_put_sync(dev);
+ return -EBUSY;
+ }
+
+ genpd_acquire_lock(genpd);
if (genpd->prepared_count++ == 0)
- genpd->suspend_power_off = genpd->power_is_off;
+ genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
+
+ genpd_release_lock(genpd);
if (genpd->suspend_power_off) {
- mutex_unlock(&genpd->lock);
+ pm_runtime_put_noidle(dev);
return 0;
}
/*
- * If the device is in the (runtime) "suspended" state, call
- * .start_device() for it, if defined.
- */
- if (pm_runtime_suspended(dev))
- __pm_genpd_runtime_resume(dev, genpd);
-
- /*
- * Do not check if runtime resume is pending at this point, because it
- * has been taken care of already and if pm_genpd_poweron() ran at this
- * point as a result of the check, it would deadlock.
+ * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
+ * so pm_genpd_poweron() will return immediately, but if the device
+ * is suspended (e.g. it's been stopped by .stop_device()), we need
+ * to make it operational.
*/
+ pm_runtime_resume(dev);
__pm_runtime_disable(dev, false);
- mutex_unlock(&genpd->lock);
-
ret = pm_generic_prepare(dev);
if (ret) {
mutex_lock(&genpd->lock);
@@ -409,7 +497,10 @@ static int pm_genpd_prepare(struct devic
genpd->suspend_power_off = false;
mutex_unlock(&genpd->lock);
+ pm_runtime_enable(dev);
}
+
+ pm_runtime_put_sync(dev);
return ret;
}
@@ -726,7 +817,7 @@ static int pm_genpd_restore_noirq(struct
* guaranteed that this function will never run twice in parallel for
* the same PM domain, so it is not necessary to use locking here.
*/
- genpd->power_is_off = true;
+ genpd->status = GPD_STATE_POWER_OFF;
if (genpd->suspend_power_off) {
/*
* The boot kernel might put the domain into the power on state,
@@ -836,9 +927,9 @@ int pm_genpd_add_device(struct generic_p
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
- mutex_lock(&genpd->lock);
+ genpd_acquire_lock(genpd);
- if (genpd->power_is_off) {
+ if (genpd->status == GPD_STATE_POWER_OFF) {
ret = -EINVAL;
goto out;
}
@@ -870,7 +961,7 @@ int pm_genpd_add_device(struct generic_p
spin_unlock_irq(&dev->power.lock);
out:
- mutex_unlock(&genpd->lock);
+ genpd_release_lock(genpd);
return ret;
}
@@ -891,7 +982,7 @@ int pm_genpd_remove_device(struct generi
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
- mutex_lock(&genpd->lock);
+ genpd_acquire_lock(genpd);
if (genpd->prepared_count > 0) {
ret = -EAGAIN;
@@ -915,7 +1006,7 @@ int pm_genpd_remove_device(struct generi
}
out:
- mutex_unlock(&genpd->lock);
+ genpd_release_lock(genpd);
return ret;
}
@@ -934,9 +1025,19 @@ int pm_genpd_add_subdomain(struct generi
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
return -EINVAL;
- mutex_lock(&genpd->lock);
+ start:
+ genpd_acquire_lock(genpd);
+ mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
- if (genpd->power_is_off && !new_subdomain->power_is_off) {
+ if (new_subdomain->status != GPD_STATE_POWER_OFF
+ && new_subdomain->status != GPD_STATE_ACTIVE) {
+ mutex_unlock(&new_subdomain->lock);
+ genpd_release_lock(genpd);
+ goto start;
+ }
+
+ if (genpd->status == GPD_STATE_POWER_OFF
+ && new_subdomain->status != GPD_STATE_POWER_OFF) {
ret = -EINVAL;
goto out;
}
@@ -948,17 +1049,14 @@ int pm_genpd_add_subdomain(struct generi
}
}
- mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
-
list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
new_subdomain->parent = genpd;
- if (!subdomain->power_is_off)
+ if (subdomain->status != GPD_STATE_POWER_OFF)
genpd->sd_count++;
- mutex_unlock(&new_subdomain->lock);
-
out:
- mutex_unlock(&genpd->lock);
+ mutex_unlock(&new_subdomain->lock);
+ genpd_release_lock(genpd);
return ret;
}
@@ -977,7 +1075,8 @@ int pm_genpd_remove_subdomain(struct gen
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
return -EINVAL;
- mutex_lock(&genpd->lock);
+ start:
+ genpd_acquire_lock(genpd);
list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
if (subdomain != target)
@@ -985,9 +1084,16 @@ int pm_genpd_remove_subdomain(struct gen
mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
+ if (subdomain->status != GPD_STATE_POWER_OFF
+ && subdomain->status != GPD_STATE_ACTIVE) {
+ mutex_unlock(&subdomain->lock);
+ genpd_release_lock(genpd);
+ goto start;
+ }
+
list_del(&subdomain->sd_node);
subdomain->parent = NULL;
- if (!subdomain->power_is_off)
+ if (subdomain->status != GPD_STATE_POWER_OFF)
genpd_sd_counter_dec(genpd);
mutex_unlock(&subdomain->lock);
@@ -996,7 +1102,7 @@ int pm_genpd_remove_subdomain(struct gen
break;
}
- mutex_unlock(&genpd->lock);
+ genpd_release_lock(genpd);
return ret;
}
@@ -1022,7 +1128,8 @@ void pm_genpd_init(struct generic_pm_dom
INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
genpd->in_progress = 0;
genpd->sd_count = 0;
- genpd->power_is_off = is_off;
+ genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
+ init_waitqueue_head(&genpd->status_wait_queue);
genpd->device_count = 0;
genpd->suspended_count = 0;
genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Rafael J. Wysocki @ 2011-07-08 9:24 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <871uy1d380.fsf@ti.com>
On Friday, July 08, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Make generic PM domains support system-wide power transitions
> > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> > poweroff and restore callbacks to be associated with struct
> > generic_pm_domain objects and make pm_genpd_init() use them as
> > appropriate.
> >
> > The new callbacks do nothing for devices belonging to power domains
> > that were powered down at run time (before the transition).
>
> Thinking about this some more, how is a driver supposed to reconfigure
> wakeups during suspend if it has already been runtime suspended?
If the device belongs to a PM domain that has been powered off, it
won't be notified.
> For example, assume a device where device_may_wakeup() == false. This
> means wakeups during *suspend* are disabled, but wakeups wakeups are
> assumed to enabled when it is runtime suspended.
>
> So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
> and then system suspend comes along.
>
> With this current patch, the driver will never receive any callbacks, so
> it can never disable its wakeups.
>
> Am I missing something?
As I said above, this only happens with devices that belog to PM domains
that were powered off before system suspend has started, so the problem
is limited to devices that wakeup is signaled on behalf of even when they
have no power.
So this is a limitation, but not affecting all platforms.
There are a few ways to avoid this limitation I can think of:
(1) Add a "make me operational during system suspend" flag to struct dev_pm_info
and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
core, or pm_genpd_prepare()).
(2) Add a "my .prepare() is safe to run if device is not accessible" flag to
struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
devices regardless of whether or not their PM domains are off.
(3) Call .prepare() from all drivers unconditionally during system suspend
(and probably .complete() too) in the hope they won't access inaccessible
devices.
Probably, there's more.
In any case I think it's material for future work.
Thanks,
Rafael
^ permalink raw reply
* Re: [patch 1/1] [PATCH] include storage keys in hibernation image.
From: Martin Schwidefsky @ 2011-07-08 8:29 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-s390, linux-kernel, linux-pm, Jiri Slaby
In-Reply-To: <201107072336.26151.rjw@sisk.pl>
On Thu, 7 Jul 2011 23:36:25 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Wednesday, June 15, 2011, Martin Schwidefsky wrote:
> > On Tue, 14 Jun 2011 22:50:14 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> ...
> >
> > Well before the preallocation kicked in we don't know which are the relevant
> > storage keys. The only other option would be to store all of them which I
> > consider to be a inferior solution.
>
> I've been thinking about that recently a bit.
>
> Don't we need to restore the keys of the page frames that aren't copied
> into the image too?
Pages that are free (= not part of the image) will get a zero storage key
once they are allocated again. So no, we do not need to include these
keys into the hibernation image.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply
* Re: [PATCHv3] ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
From: Chanwoo Choi @ 2011-07-08 6:19 UTC (permalink / raw)
To: Kukjin Kim
Cc: 'Sylwester Nawrocki', 'linux-samsung-soc',
'linux-kernel', 'Kyungmin Park',
'MyungJoo Ham', linux-pm
In-Reply-To: <000e01cc3d2f$eca5d530$c5f17f90$%kim@samsung.com>
Kukjin Kim wrote:
> Chanwoo Choi wrote:
>> Use the generic power domains support to implement support for
>> power domain on EXYNOS4210.
>>
>> I refer to the following patch to implement what configure
>> the clock-gating control register for block to turn off/on:
>> http://git.infradead.org/users/kmpark/linux-2.6-
>> samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
>>
> I think, this should be moved under "---"
OK, I will move it.
>
>> This patch is based on following "pm-domains" branch.
>> http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-
>> 2.6.git;a=shortlog;h=refs/heads/pm-domains
>>
> Same as above.
OK.
>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
>
> Kyungmin Park?
My mistake. I will modify it.
>
>> ---
>> arch/arm/mach-exynos4/Kconfig | 1 +
>> arch/arm/mach-exynos4/Makefile | 1 +
>> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 53 ++++++
>> arch/arm/mach-exynos4/include/mach/regs-clock.h | 8 +
>> arch/arm/mach-exynos4/mach-nuri.c | 18 ++
>> arch/arm/mach-exynos4/pm-exynos4210.c | 184
>> ++++++++++++++++++++
>> 6 files changed, 265 insertions(+), 0 deletions(-)
>> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>>
>> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
>> index 1435fc3..c5357b5 100644
>> --- a/arch/arm/mach-exynos4/Kconfig
>> +++ b/arch/arm/mach-exynos4/Kconfig
>> @@ -183,6 +183,7 @@ config MACH_NURI
>> select EXYNOS4_SETUP_SDHCI
>> select EXYNOS4_SETUP_USB_PHY
>> select SAMSUNG_DEV_PWM
>> + select PM_GENERIC_DOMAINS if PM
>
> Do you _really_ think this should be under MACH_NURI?
>
This patch apply the generic power-domain to NURI board.
If other board(EXYNOS4 series) use the generic power-domain,
I think that PM_GENERIC_DOMAINS config should be moved under CPU_EXYNOS4210 config.
If you prefer to move it under CPU_EXYNOS4210, I will modify it.
>> help
>> Machine support for Samsung Mobile NURI Board.
>>
>> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
>> index 60fe5ec..0e9461f 100644
>> --- a/arch/arm/mach-exynos4/Makefile
>> +++ b/arch/arm/mach-exynos4/Makefile
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-
>> eint.o dma.o
>> obj-$(CONFIG_PM) += pm.o sleep.o
>> obj-$(CONFIG_CPU_FREQ) += cpufreq.o
>> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>> +obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
>>
>> obj-$(CONFIG_SMP) += platsmp.o headsmp.o
>>
>> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> new file mode 100644
>> index 0000000..ab09034
>> --- /dev/null
>> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> @@ -0,0 +1,53 @@
>> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
>
> According to your file name, should be linux/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h ?
This patch support only power-domain of EXYNOS4210, so I named "pm-exynos4210.h"
If EXYNOS4 series board will be released, I will modify include file name
from "pm-exynos4210.h" to "pm-exynos4.h" to support all EXYNOS4 series.
But, I don't know next EXYNOS4 series and then named "pm-exynos4210.h".
Do you want to change from "pm-exynos4210.h" to "pm-exynos4.h"?
>
>> + *
>> + * Exynos4 Power management support
>
> Is this for EXYNOS4 or only EXYNOS4210?
>
Only EXYNOS4210. I will fix it.
>> + *
>> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.com
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef PM_EXYNOS4210_H
>> +#define PM_EXYNOS4210_H
>> +
>> +#include <linux/pm_domain.h>
>> +#include <plat/pd.h>
>> +
>> +struct platform_device;
>> +
>> +struct exynos4210_pm_domain {
>> + struct generic_pm_domain genpd;
>> +
>> + const char *name;
>> + void __iomem *base;
>> + u32 mask;
>> + int boot_on;
>> +};
>> +
>> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
>> + struct generic_pm_domain *pd)
>> +{
>> + return container_of(pd, struct exynos4210_pm_domain, genpd);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +extern struct exynos4210_pm_domain exynos4210_pd_mfc;
>> +extern struct exynos4210_pm_domain exynos4210_pd_g3d;
>> +extern struct exynos4210_pm_domain exynos4210_pd_lcd0;
>> +extern struct exynos4210_pm_domain exynos4210_pd_lcd1;
>> +extern struct exynos4210_pm_domain exynos4210_pd_tv;
>> +extern struct exynos4210_pm_domain exynos4210_pd_cam;
>> +extern struct exynos4210_pm_domain exynos4210_pd_gps;
>> +
>> +extern void exynos4210_init_pm_domain(struct exynos4210_pm_domain
>> *exynos4210_pd);
>> +extern void exynos4210_add_device_to_domain(struct exynos4210_pm_domain
>> *exynos4210_pd,
>> + struct platform_device *pdev);
>> +#else
>> +#define exynos4210_init_pm_domain(pd) do { } while(0)
>> +#define exynos4_add_device_to_domain(pd, pdev) do { } while(0)
>> +#endif /* CONFIG_PM */
>> +
>> +#endif /* PM_EXYNOS4210_H */
>> diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-
>> exynos4/include/mach/regs-clock.h
>> index 6e311c1..bc0fda9 100644
>> --- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
>> +++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
>> @@ -171,6 +171,14 @@
>> #define S5P_CLKDIV_BUS_GPLR_SHIFT (4)
>> #define S5P_CLKDIV_BUS_GPLR_MASK (0x7 <<
>> S5P_CLKDIV_BUS_GPLR_SHIFT)
>>
>> +#define S5P_CLKGATE_BLOCK_CAM (1<<0)
>
> The blank should be added around "<<" like (1 << 0)
Ok, I will modify it.
>
>> +#define S5P_CLKGATE_BLOCK_TV (1<<1)
>> +#define S5P_CLKGATE_BLOCK_MFC (1<<2)
>> +#define S5P_CLKGATE_BLOCK_G3D (1<<3)
>> +#define S5P_CLKGATE_BLOCK_LCD0 (1<<4)
>> +#define S5P_CLKGATE_BLOCK_LCD1 (1<<5)
>> +#define S5P_CLKGATE_BLOCK_GPS (1<<7)
>
> Same...
Ok.
>
>> +
>> /* Compatibility defines and inclusion */
>>
>> #include <mach/regs-pmu.h>
>
> Hmm...how about if this is required, just add the inclusion in c file?
>
> Actually, you added above in pm-exynos4210.c...
It was added before this patch, so I didn't consider it.
If you want to remove it, I apply this patch about removal of inclusion(mach/regs-pmu.h).
>
>> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-
>> nuri.c
>> index 642702b..816c6f4 100644
>> --- a/arch/arm/mach-exynos4/mach-nuri.c
>> +++ b/arch/arm/mach-exynos4/mach-nuri.c
>> @@ -37,6 +37,7 @@
>> #include <plat/iic.h>
>>
>> #include <mach/map.h>
>> +#include <mach/pm-exynos4210.h>
>>
>> /* Following are default values for UCON, ULCON and UFCON UART registers */
>> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
>> @@ -376,6 +377,21 @@ static struct platform_device *nuri_devices[] __initdata = {
>> &nuri_backlight_device,
>> };
>>
>> +static void __init nuri_power_domain_init(void)
>> +{
>> + /* Initialize power-domain */
>> + exynos4210_init_pm_domain(&exynos4210_pd_mfc);
>> + exynos4210_init_pm_domain(&exynos4210_pd_g3d);
>> + exynos4210_init_pm_domain(&exynos4210_pd_lcd0);
>> + exynos4210_init_pm_domain(&exynos4210_pd_lcd1);
>> + exynos4210_init_pm_domain(&exynos4210_pd_tv);
>> + exynos4210_init_pm_domain(&exynos4210_pd_cam);
>> + exynos4210_init_pm_domain(&exynos4210_pd_gps);
>
> Hmm, is this needed only on NURI?
As mentioned above, this patch support only NURI board.
If the generic power-domain to support EXYNOS4210 is used on other board,
I think that it should apply the board file of other board using EXYNOS4210.
>
>> +
>> + /* Add device to power-domain */
>> + exynos4210_add_device_to_domain(&exynos4210_pd_lcd0,
>> &nuri_lcd_device);
>> +}
>> +
>> static void __init nuri_map_io(void)
>> {
>> s5p_init_io(NULL, 0, S5P_VA_CHIPID);
>> @@ -398,6 +414,8 @@ static void __init nuri_machine_init(void)
>>
>> /* Last */
>> platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
>> +
>> + nuri_power_domain_init();
>> }
>>
>> MACHINE_START(NURI, "NURI")
>> diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-
>> exynos4/pm-exynos4210.c
>> new file mode 100644
>> index 0000000..f44dcef
>> --- /dev/null
>> +++ b/arch/arm/mach-exynos4/pm-exynos4210.c
>> @@ -0,0 +1,184 @@
>> +/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
>> + *
>> + * Exynos4210 Power management support
>> + *
>> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.com
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/pm.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/slab.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/io.h>
>> +
>> +#include <mach/regs-pmu.h>
>
> As I said, so no need to add this inclusion in <mach/regs-clock.h>
Ok.
>
>> +#include <mach/regs-clock.h>
>> +#include <mach/pm-exynos4210.h>
>> +
>> +#ifdef CONFIG_PM
>> +static DEFINE_SPINLOCK(clkgate_block_lock);
>> +
>> +static int pd_power_down(struct generic_pm_domain *genpd)
>> +{
>> + struct exynos4210_pm_domain *exynos4210_pd =
>> to_exynos4210_pd(genpd);
>> + u32 timeout;
>> +
>> + /* Disable the power of power-domain */
>> + __raw_writel(0, exynos4210_pd->base);
>> +
>> + /* Wait max 1ms */
>> + timeout = 10;
>> + while (__raw_readl(exynos4210_pd->base + 0x4) &
>> S5P_INT_LOCAL_PWR_EN) {
>> + if (timeout == 0) {
>> + printk(KERN_ERR "Power domain %s disable failed.\n",
>> + exynos4210_pd->name);
>> + return -ETIMEDOUT;
>> + }
>> + timeout--;
>> + udelay(100);
>> + }
>> +
>> + /* Configure the clock-gating control register for block to turn off */
>> + if (exynos4210_pd->mask) {
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + spin_lock_irqsave(&clkgate_block_lock, flags);
>> + reg = __raw_readl(S5P_CLKGATE_BLOCK);
>> + reg &= ~exynos4210_pd->mask;
>> + __raw_writel(reg, S5P_CLKGATE_BLOCK);
>> + spin_unlock_irqrestore(&clkgate_block_lock, flags);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int pd_power_up(struct generic_pm_domain *genpd)
>> +{
>> + struct exynos4210_pm_domain *exynos4210_pd =
>> to_exynos4210_pd(genpd);
>> + u32 timeout;
>> +
>> + /* Enable power domain */
>> + __raw_writel(S5P_INT_LOCAL_PWR_EN, exynos4210_pd->base);
>> +
>> + /* Wait max 1ms */
>> + timeout = 10;
>> + while ((__raw_readl(exynos4210_pd->base + 0x4) &
>> S5P_INT_LOCAL_PWR_EN)
>> + != S5P_INT_LOCAL_PWR_EN) {
>> + if (timeout == 0) {
>> + printk(KERN_ERR "Power domain %s enable failed.\n",
>> + exynos4210_pd->name);
>> + return -ETIMEDOUT;
>> + }
>> + timeout--;
>> + udelay(100);
>> + }
>> +
>> + /* Configure the clock-gating control register for block to turn on */
>> + if (exynos4210_pd->mask) {
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + spin_lock_irqsave(&clkgate_block_lock, flags);
>> + reg = __raw_readl(S5P_CLKGATE_BLOCK);
>> + reg |= exynos4210_pd->mask;
>> + __raw_writel(reg, S5P_CLKGATE_BLOCK);
>> + spin_unlock_irqrestore(&clkgate_block_lock, flags);
>> + }
>> +
>> + return 0;
>> +}
>
> Would be better if you could add some common function for pd_power_up and _down...
What do you mean about common function?
The generic Power-domain framework provides callback function to control power-domain,
so I implement the control function to support power-domain of EXYNOS4210.
We need the additional common function for EXYNOS4210 ???
You can see the below codes:
+ genpd->stop_device = pm_clk_suspend;
+ genpd->start_device = pm_clk_resume;
+ genpd->active_wakeup = pd_active_wakeup;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
>
> I wonder why we need to add this for handling power domain not update current pd.
As you said, the generic power-domain framework isn't updated to Mainline,
But, this patch prepares to support the subsequent power-domain framework,
To review this patch, I posted it.
> As you know, they are having same/similar purpose and functions...
>
I don't understand your comment exactly.
Don't your agree to use the generic Power-domain framework for SAMSUNG SoC?
or Again, please decribe your comment in detail.
>> +
>> +static bool pd_active_wakeup(struct device *dev)
>> +{
>> + return true;
>> +}
>> +
>> +void exynos4210_init_pm_domain(struct exynos4210_pm_domain
>> *exynos4210_pd)
>> +{
>> + struct generic_pm_domain *genpd;
>> +
>> + if (!exynos4210_pd)
>> + return;
>> +
>> + genpd = &exynos4210_pd->genpd;
>> +
>> + pm_genpd_init(genpd, NULL, false);
>> + genpd->stop_device = pm_clk_suspend;
>> + genpd->start_device = pm_clk_resume;
>> + genpd->active_wakeup = pd_active_wakeup;
>> + genpd->power_off = pd_power_down;
>> + genpd->power_on = pd_power_up;
>> +
>> + if (exynos4210_pd->boot_on)
>> + pd_power_up(&exynos4210_pd->genpd);
>> +}
>> +
>> +void exynos4210_add_device_to_domain(struct exynos4210_pm_domain
>> *exynos4210_pd,
>> + struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> +
>> + if (exynos4210_pd && pdev)
>> + pm_genpd_add_device(&exynos4210_pd->genpd, dev);
>> +}
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_mfc = {
>> + .name = "PD_MFC",
>> + .base = S5P_PMU_MFC_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_MFC,
>> + .boot_on = 0,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_g3d = {
>> + .name = "PD_G3D",
>> + .base = S5P_PMU_G3D_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_G3D,
>> + .boot_on = 0,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_lcd0 = {
>> + .name = "PD_LCD0",
>> + .base = S5P_PMU_LCD0_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_LCD0,
>> + .boot_on = 1,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_lcd1 = {
>> + .name = "PD_LCD1",
>> + .base = S5P_PMU_LCD1_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_LCD1,
>> + .boot_on = 0,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_tv = {
>> + .name = "PD_TV",
>> + .base = S5P_PMU_TV_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_TV,
>> + .boot_on = 0,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_cam = {
>> + .name = "PD_CAM",
>> + .base = S5P_PMU_CAM_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_CAM,
>> + .boot_on = 0,
>> +};
>> +
>> +struct exynos4210_pm_domain exynos4210_pd_gps = {
>> + .name = "PD_GPS",
>> + .base = S5P_PMU_GPS_CONF,
>> + .mask = S5P_CLKGATE_BLOCK_GPS,
>> + .boot_on = 0,
>> +};
>> +
>> +#endif /* CONFIG_PM */
>> --
>> 1.7.0.4
>
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>
Thanks,
Best regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCHv3] ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
From: Kukjin Kim @ 2011-07-08 5:28 UTC (permalink / raw)
To: 'Chanwoo Choi', 'Rafael J. Wysocki'
Cc: 'linux-samsung-soc', 'linux-kernel',
'Sylwester Nawrocki', 'Kyungmin Park',
'MyungJoo Ham', linux-pm
In-Reply-To: <4E165C64.8090808@samsung.com>
Chanwoo Choi wrote:
>
> Use the generic power domains support to implement support for
> power domain on EXYNOS4210.
>
> I refer to the following patch to implement what configure
> the clock-gating control register for block to turn off/on:
> http://git.infradead.org/users/kmpark/linux-2.6-
> samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
>
I think, this should be moved under "---"
> This patch is based on following "pm-domains" branch.
> http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-
> 2.6.git;a=shortlog;h=refs/heads/pm-domains
>
Same as above.
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
Kyungmin Park?
> ---
> arch/arm/mach-exynos4/Kconfig | 1 +
> arch/arm/mach-exynos4/Makefile | 1 +
> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 53 ++++++
> arch/arm/mach-exynos4/include/mach/regs-clock.h | 8 +
> arch/arm/mach-exynos4/mach-nuri.c | 18 ++
> arch/arm/mach-exynos4/pm-exynos4210.c | 184
> ++++++++++++++++++++
> 6 files changed, 265 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>
> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
> index 1435fc3..c5357b5 100644
> --- a/arch/arm/mach-exynos4/Kconfig
> +++ b/arch/arm/mach-exynos4/Kconfig
> @@ -183,6 +183,7 @@ config MACH_NURI
> select EXYNOS4_SETUP_SDHCI
> select EXYNOS4_SETUP_USB_PHY
> select SAMSUNG_DEV_PWM
> + select PM_GENERIC_DOMAINS if PM
Do you _really_ think this should be under MACH_NURI?
> help
> Machine support for Samsung Mobile NURI Board.
>
> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
> index 60fe5ec..0e9461f 100644
> --- a/arch/arm/mach-exynos4/Makefile
> +++ b/arch/arm/mach-exynos4/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-
> eint.o dma.o
> obj-$(CONFIG_PM) += pm.o sleep.o
> obj-$(CONFIG_CPU_FREQ) += cpufreq.o
> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> +obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
>
> obj-$(CONFIG_SMP) += platsmp.o headsmp.o
>
> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> new file mode 100644
> index 0000000..ab09034
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> @@ -0,0 +1,53 @@
> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
According to your file name, should be linux/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h ?
> + *
> + * Exynos4 Power management support
Is this for EXYNOS4 or only EXYNOS4210?
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef PM_EXYNOS4210_H
> +#define PM_EXYNOS4210_H
> +
> +#include <linux/pm_domain.h>
> +#include <plat/pd.h>
> +
> +struct platform_device;
> +
> +struct exynos4210_pm_domain {
> + struct generic_pm_domain genpd;
> +
> + const char *name;
> + void __iomem *base;
> + u32 mask;
> + int boot_on;
> +};
> +
> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
> + struct generic_pm_domain *pd)
> +{
> + return container_of(pd, struct exynos4210_pm_domain, genpd);
> +}
> +
> +#ifdef CONFIG_PM
> +extern struct exynos4210_pm_domain exynos4210_pd_mfc;
> +extern struct exynos4210_pm_domain exynos4210_pd_g3d;
> +extern struct exynos4210_pm_domain exynos4210_pd_lcd0;
> +extern struct exynos4210_pm_domain exynos4210_pd_lcd1;
> +extern struct exynos4210_pm_domain exynos4210_pd_tv;
> +extern struct exynos4210_pm_domain exynos4210_pd_cam;
> +extern struct exynos4210_pm_domain exynos4210_pd_gps;
> +
> +extern void exynos4210_init_pm_domain(struct exynos4210_pm_domain
> *exynos4210_pd);
> +extern void exynos4210_add_device_to_domain(struct exynos4210_pm_domain
> *exynos4210_pd,
> + struct platform_device *pdev);
> +#else
> +#define exynos4210_init_pm_domain(pd) do { } while(0)
> +#define exynos4_add_device_to_domain(pd, pdev) do { } while(0)
> +#endif /* CONFIG_PM */
> +
> +#endif /* PM_EXYNOS4210_H */
> diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-
> exynos4/include/mach/regs-clock.h
> index 6e311c1..bc0fda9 100644
> --- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
> +++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
> @@ -171,6 +171,14 @@
> #define S5P_CLKDIV_BUS_GPLR_SHIFT (4)
> #define S5P_CLKDIV_BUS_GPLR_MASK (0x7 <<
> S5P_CLKDIV_BUS_GPLR_SHIFT)
>
> +#define S5P_CLKGATE_BLOCK_CAM (1<<0)
The blank should be added around "<<" like (1 << 0)
> +#define S5P_CLKGATE_BLOCK_TV (1<<1)
> +#define S5P_CLKGATE_BLOCK_MFC (1<<2)
> +#define S5P_CLKGATE_BLOCK_G3D (1<<3)
> +#define S5P_CLKGATE_BLOCK_LCD0 (1<<4)
> +#define S5P_CLKGATE_BLOCK_LCD1 (1<<5)
> +#define S5P_CLKGATE_BLOCK_GPS (1<<7)
Same...
> +
> /* Compatibility defines and inclusion */
>
> #include <mach/regs-pmu.h>
Hmm...how about if this is required, just add the inclusion in c file?
Actually, you added above in pm-exynos4210.c...
> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-
> nuri.c
> index 642702b..816c6f4 100644
> --- a/arch/arm/mach-exynos4/mach-nuri.c
> +++ b/arch/arm/mach-exynos4/mach-nuri.c
> @@ -37,6 +37,7 @@
> #include <plat/iic.h>
>
> #include <mach/map.h>
> +#include <mach/pm-exynos4210.h>
>
> /* Following are default values for UCON, ULCON and UFCON UART registers */
> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> @@ -376,6 +377,21 @@ static struct platform_device *nuri_devices[] __initdata = {
> &nuri_backlight_device,
> };
>
> +static void __init nuri_power_domain_init(void)
> +{
> + /* Initialize power-domain */
> + exynos4210_init_pm_domain(&exynos4210_pd_mfc);
> + exynos4210_init_pm_domain(&exynos4210_pd_g3d);
> + exynos4210_init_pm_domain(&exynos4210_pd_lcd0);
> + exynos4210_init_pm_domain(&exynos4210_pd_lcd1);
> + exynos4210_init_pm_domain(&exynos4210_pd_tv);
> + exynos4210_init_pm_domain(&exynos4210_pd_cam);
> + exynos4210_init_pm_domain(&exynos4210_pd_gps);
Hmm, is this needed only on NURI?
> +
> + /* Add device to power-domain */
> + exynos4210_add_device_to_domain(&exynos4210_pd_lcd0,
> &nuri_lcd_device);
> +}
> +
> static void __init nuri_map_io(void)
> {
> s5p_init_io(NULL, 0, S5P_VA_CHIPID);
> @@ -398,6 +414,8 @@ static void __init nuri_machine_init(void)
>
> /* Last */
> platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
> +
> + nuri_power_domain_init();
> }
>
> MACHINE_START(NURI, "NURI")
> diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-
> exynos4/pm-exynos4210.c
> new file mode 100644
> index 0000000..f44dcef
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/pm-exynos4210.c
> @@ -0,0 +1,184 @@
> +/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
> + *
> + * Exynos4210 Power management support
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/pm.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/io.h>
> +
> +#include <mach/regs-pmu.h>
As I said, so no need to add this inclusion in <mach/regs-clock.h>
> +#include <mach/regs-clock.h>
> +#include <mach/pm-exynos4210.h>
> +
> +#ifdef CONFIG_PM
> +static DEFINE_SPINLOCK(clkgate_block_lock);
> +
> +static int pd_power_down(struct generic_pm_domain *genpd)
> +{
> + struct exynos4210_pm_domain *exynos4210_pd =
> to_exynos4210_pd(genpd);
> + u32 timeout;
> +
> + /* Disable the power of power-domain */
> + __raw_writel(0, exynos4210_pd->base);
> +
> + /* Wait max 1ms */
> + timeout = 10;
> + while (__raw_readl(exynos4210_pd->base + 0x4) &
> S5P_INT_LOCAL_PWR_EN) {
> + if (timeout == 0) {
> + printk(KERN_ERR "Power domain %s disable failed.\n",
> + exynos4210_pd->name);
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + udelay(100);
> + }
> +
> + /* Configure the clock-gating control register for block to turn off */
> + if (exynos4210_pd->mask) {
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&clkgate_block_lock, flags);
> + reg = __raw_readl(S5P_CLKGATE_BLOCK);
> + reg &= ~exynos4210_pd->mask;
> + __raw_writel(reg, S5P_CLKGATE_BLOCK);
> + spin_unlock_irqrestore(&clkgate_block_lock, flags);
> + }
> +
> + return 0;
> +}
> +
> +static int pd_power_up(struct generic_pm_domain *genpd)
> +{
> + struct exynos4210_pm_domain *exynos4210_pd =
> to_exynos4210_pd(genpd);
> + u32 timeout;
> +
> + /* Enable power domain */
> + __raw_writel(S5P_INT_LOCAL_PWR_EN, exynos4210_pd->base);
> +
> + /* Wait max 1ms */
> + timeout = 10;
> + while ((__raw_readl(exynos4210_pd->base + 0x4) &
> S5P_INT_LOCAL_PWR_EN)
> + != S5P_INT_LOCAL_PWR_EN) {
> + if (timeout == 0) {
> + printk(KERN_ERR "Power domain %s enable failed.\n",
> + exynos4210_pd->name);
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + udelay(100);
> + }
> +
> + /* Configure the clock-gating control register for block to turn on */
> + if (exynos4210_pd->mask) {
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&clkgate_block_lock, flags);
> + reg = __raw_readl(S5P_CLKGATE_BLOCK);
> + reg |= exynos4210_pd->mask;
> + __raw_writel(reg, S5P_CLKGATE_BLOCK);
> + spin_unlock_irqrestore(&clkgate_block_lock, flags);
> + }
> +
> + return 0;
> +}
Would be better if you could add some common function for pd_power_up and _down...
I wonder why we need to add this for handling power domain not update current pd.
As you know, they are having same/similar purpose and functions...
> +
> +static bool pd_active_wakeup(struct device *dev)
> +{
> + return true;
> +}
> +
> +void exynos4210_init_pm_domain(struct exynos4210_pm_domain
> *exynos4210_pd)
> +{
> + struct generic_pm_domain *genpd;
> +
> + if (!exynos4210_pd)
> + return;
> +
> + genpd = &exynos4210_pd->genpd;
> +
> + pm_genpd_init(genpd, NULL, false);
> + genpd->stop_device = pm_clk_suspend;
> + genpd->start_device = pm_clk_resume;
> + genpd->active_wakeup = pd_active_wakeup;
> + genpd->power_off = pd_power_down;
> + genpd->power_on = pd_power_up;
> +
> + if (exynos4210_pd->boot_on)
> + pd_power_up(&exynos4210_pd->genpd);
> +}
> +
> +void exynos4210_add_device_to_domain(struct exynos4210_pm_domain
> *exynos4210_pd,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> +
> + if (exynos4210_pd && pdev)
> + pm_genpd_add_device(&exynos4210_pd->genpd, dev);
> +}
> +
> +struct exynos4210_pm_domain exynos4210_pd_mfc = {
> + .name = "PD_MFC",
> + .base = S5P_PMU_MFC_CONF,
> + .mask = S5P_CLKGATE_BLOCK_MFC,
> + .boot_on = 0,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_g3d = {
> + .name = "PD_G3D",
> + .base = S5P_PMU_G3D_CONF,
> + .mask = S5P_CLKGATE_BLOCK_G3D,
> + .boot_on = 0,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_lcd0 = {
> + .name = "PD_LCD0",
> + .base = S5P_PMU_LCD0_CONF,
> + .mask = S5P_CLKGATE_BLOCK_LCD0,
> + .boot_on = 1,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_lcd1 = {
> + .name = "PD_LCD1",
> + .base = S5P_PMU_LCD1_CONF,
> + .mask = S5P_CLKGATE_BLOCK_LCD1,
> + .boot_on = 0,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_tv = {
> + .name = "PD_TV",
> + .base = S5P_PMU_TV_CONF,
> + .mask = S5P_CLKGATE_BLOCK_TV,
> + .boot_on = 0,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_cam = {
> + .name = "PD_CAM",
> + .base = S5P_PMU_CAM_CONF,
> + .mask = S5P_CLKGATE_BLOCK_CAM,
> + .boot_on = 0,
> +};
> +
> +struct exynos4210_pm_domain exynos4210_pd_gps = {
> + .name = "PD_GPS",
> + .base = S5P_PMU_GPS_CONF,
> + .mask = S5P_CLKGATE_BLOCK_GPS,
> + .boot_on = 0,
> +};
> +
> +#endif /* CONFIG_PM */
> --
> 1.7.0.4
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* [PATCH] runtime-pm: consistent utilization of deferred_resume
From: Liu, ShuoX @ 2011-07-08 4:57 UTC (permalink / raw)
To: Brown, Len, pavel@ucw.cz, rjw@sisk.pl, gregkh@suse.de
Cc: linux-pm@lists.linux-foundation.org, linux-kernel@vger.kernel.org
>From 10bb3c851a0a9c9ca4d552743a371e3cfda81075 Mon Sep 17 00:00:00 2001
From: ShuoX Liu <shuox.liu@intel.com>
Date: Thu, 7 Jul 2011 15:59:06 +0800
Subject: [PATCH] runtime-pm: consistent utilization of deferred_resume
dev->power.deferred_resume is used as a bool typically, so change
one assignment to false from 0, like other places.
Signed-off-by: ShuoX Liu <shuox.liu@intel.com>
---
drivers/base/power/runtime.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 0d4587b..af3d1a5 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -388,7 +388,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
retval = rpm_callback(callback, dev);
if (retval) {
__update_runtime_status(dev, RPM_ACTIVE);
- dev->power.deferred_resume = 0;
+ dev->power.deferred_resume = false;
if (retval == -EAGAIN || retval == -EBUSY)
dev->power.runtime_error = 0;
else
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 00/10] mm: Linux VM Infrastructure to support Memory Power Management
From: david @ 2011-07-08 1:32 UTC (permalink / raw)
To: Pekka Enberg
Cc: Paul E. McKenney, KAMEZAWA Hiroyuki, linux-kernel, Dave Hansen,
linux-mm, thomas.abraham, Ankita Garg, linux-pm,
Christoph Lameter, linux-arm-kernel, Arjan van de Ven
In-Reply-To: <alpine.DEB.2.00.1107072058390.5978@tiger>
On Thu, 7 Jul 2011, Pekka Enberg wrote:
> On Wed, 6 Jul 2011, Pekka Enberg wrote:
>>> Why does the allocator need to know about address boundaries? Why
>>> isn't it enough to make the page allocator and reclaim policies favor
>>> using
>>> memory from lower addresses as aggressively as possible? That'd mean
>>> we'd favor the first memory banks and could keep the remaining ones
>>> powered off as much as possible.
>>>
>>> IOW, why do we need to support scenarios such as this:
>>>
>>> bank 0 bank 1 bank 2 bank3
>>> | online | offline | online | offline |
>>
> On Wed, 6 Jul 2011, david@lang.hm wrote:
>> I believe that there are memory allocations that cannot be moved after they
>> are made (think about regions allocated to DMA from hardware where the
>> hardware has already been given the address space to DMA into)
>>
>> As a result, you may not be able to take bank 2 offline, so your option is
>> to either leave banks 0-2 all online, or support emptying bank 1 and taking
>> it offline.
>
> But drivers allocate DMA memory for hardware during module load and stay
> pinned there until the driver is unloaded, no? So in practice DMA buffers are
> going to be in banks 0-1?
that depends on when the device was initialized. it is common for them to
be in the beginning, but with hotplug, who knows.
David Lang
^ permalink raw reply
* [PATCHv3] ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
From: Chanwoo Choi @ 2011-07-08 1:24 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-samsung-soc, linux-kernel, Sylwester Nawrocki,
Kyungmin Park, Kukjin Kim, MyungJoo Ham, linux-pm
Use the generic power domains support to implement support for
power domain on EXYNOS4210.
I refer to the following patch to implement what configure
the clock-gating control register for block to turn off/on:
http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
This patch is based on following "pm-domains" branch.
http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=shortlog;h=refs/heads/pm-domains
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
---
arch/arm/mach-exynos4/Kconfig | 1 +
arch/arm/mach-exynos4/Makefile | 1 +
arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 53 ++++++
arch/arm/mach-exynos4/include/mach/regs-clock.h | 8 +
arch/arm/mach-exynos4/mach-nuri.c | 18 ++
arch/arm/mach-exynos4/pm-exynos4210.c | 184 ++++++++++++++++++++
6 files changed, 265 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index 1435fc3..c5357b5 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -183,6 +183,7 @@ config MACH_NURI
select EXYNOS4_SETUP_SDHCI
select EXYNOS4_SETUP_USB_PHY
select SAMSUNG_DEV_PWM
+ select PM_GENERIC_DOMAINS if PM
help
Machine support for Samsung Mobile NURI Board.
diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 60fe5ec..0e9461f 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-eint.o dma.o
obj-$(CONFIG_PM) += pm.o sleep.o
obj-$(CONFIG_CPU_FREQ) += cpufreq.o
obj-$(CONFIG_CPU_IDLE) += cpuidle.o
+obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
obj-$(CONFIG_SMP) += platsmp.o headsmp.o
diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
new file mode 100644
index 0000000..ab09034
--- /dev/null
+++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
@@ -0,0 +1,53 @@
+/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
+ *
+ * Exynos4 Power management support
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef PM_EXYNOS4210_H
+#define PM_EXYNOS4210_H
+
+#include <linux/pm_domain.h>
+#include <plat/pd.h>
+
+struct platform_device;
+
+struct exynos4210_pm_domain {
+ struct generic_pm_domain genpd;
+
+ const char *name;
+ void __iomem *base;
+ u32 mask;
+ int boot_on;
+};
+
+static inline struct exynos4210_pm_domain *to_exynos4210_pd(
+ struct generic_pm_domain *pd)
+{
+ return container_of(pd, struct exynos4210_pm_domain, genpd);
+}
+
+#ifdef CONFIG_PM
+extern struct exynos4210_pm_domain exynos4210_pd_mfc;
+extern struct exynos4210_pm_domain exynos4210_pd_g3d;
+extern struct exynos4210_pm_domain exynos4210_pd_lcd0;
+extern struct exynos4210_pm_domain exynos4210_pd_lcd1;
+extern struct exynos4210_pm_domain exynos4210_pd_tv;
+extern struct exynos4210_pm_domain exynos4210_pd_cam;
+extern struct exynos4210_pm_domain exynos4210_pd_gps;
+
+extern void exynos4210_init_pm_domain(struct exynos4210_pm_domain *exynos4210_pd);
+extern void exynos4210_add_device_to_domain(struct exynos4210_pm_domain *exynos4210_pd,
+ struct platform_device *pdev);
+#else
+#define exynos4210_init_pm_domain(pd) do { } while(0)
+#define exynos4_add_device_to_domain(pd, pdev) do { } while(0)
+#endif /* CONFIG_PM */
+
+#endif /* PM_EXYNOS4210_H */
diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-exynos4/include/mach/regs-clock.h
index 6e311c1..bc0fda9 100644
--- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
+++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
@@ -171,6 +171,14 @@
#define S5P_CLKDIV_BUS_GPLR_SHIFT (4)
#define S5P_CLKDIV_BUS_GPLR_MASK (0x7 << S5P_CLKDIV_BUS_GPLR_SHIFT)
+#define S5P_CLKGATE_BLOCK_CAM (1<<0)
+#define S5P_CLKGATE_BLOCK_TV (1<<1)
+#define S5P_CLKGATE_BLOCK_MFC (1<<2)
+#define S5P_CLKGATE_BLOCK_G3D (1<<3)
+#define S5P_CLKGATE_BLOCK_LCD0 (1<<4)
+#define S5P_CLKGATE_BLOCK_LCD1 (1<<5)
+#define S5P_CLKGATE_BLOCK_GPS (1<<7)
+
/* Compatibility defines and inclusion */
#include <mach/regs-pmu.h>
diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
index 642702b..816c6f4 100644
--- a/arch/arm/mach-exynos4/mach-nuri.c
+++ b/arch/arm/mach-exynos4/mach-nuri.c
@@ -37,6 +37,7 @@
#include <plat/iic.h>
#include <mach/map.h>
+#include <mach/pm-exynos4210.h>
/* Following are default values for UCON, ULCON and UFCON UART registers */
#define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
@@ -376,6 +377,21 @@ static struct platform_device *nuri_devices[] __initdata = {
&nuri_backlight_device,
};
+static void __init nuri_power_domain_init(void)
+{
+ /* Initialize power-domain */
+ exynos4210_init_pm_domain(&exynos4210_pd_mfc);
+ exynos4210_init_pm_domain(&exynos4210_pd_g3d);
+ exynos4210_init_pm_domain(&exynos4210_pd_lcd0);
+ exynos4210_init_pm_domain(&exynos4210_pd_lcd1);
+ exynos4210_init_pm_domain(&exynos4210_pd_tv);
+ exynos4210_init_pm_domain(&exynos4210_pd_cam);
+ exynos4210_init_pm_domain(&exynos4210_pd_gps);
+
+ /* Add device to power-domain */
+ exynos4210_add_device_to_domain(&exynos4210_pd_lcd0, &nuri_lcd_device);
+}
+
static void __init nuri_map_io(void)
{
s5p_init_io(NULL, 0, S5P_VA_CHIPID);
@@ -398,6 +414,8 @@ static void __init nuri_machine_init(void)
/* Last */
platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
+
+ nuri_power_domain_init();
}
MACHINE_START(NURI, "NURI")
diff --git a/arch/arm/mach-exynos4/pm-exynos4210.c b/arch/arm/mach-exynos4/pm-exynos4210.c
new file mode 100644
index 0000000..f44dcef
--- /dev/null
+++ b/arch/arm/mach-exynos4/pm-exynos4210.c
@@ -0,0 +1,184 @@
+/* linux/arch/arm/mach-exynos4/pm-exynos4210.c
+ *
+ * Exynos4210 Power management support
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/pm.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/io.h>
+
+#include <mach/regs-pmu.h>
+#include <mach/regs-clock.h>
+#include <mach/pm-exynos4210.h>
+
+#ifdef CONFIG_PM
+static DEFINE_SPINLOCK(clkgate_block_lock);
+
+static int pd_power_down(struct generic_pm_domain *genpd)
+{
+ struct exynos4210_pm_domain *exynos4210_pd = to_exynos4210_pd(genpd);
+ u32 timeout;
+
+ /* Disable the power of power-domain */
+ __raw_writel(0, exynos4210_pd->base);
+
+ /* Wait max 1ms */
+ timeout = 10;
+ while (__raw_readl(exynos4210_pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN) {
+ if (timeout == 0) {
+ printk(KERN_ERR "Power domain %s disable failed.\n",
+ exynos4210_pd->name);
+ return -ETIMEDOUT;
+ }
+ timeout--;
+ udelay(100);
+ }
+
+ /* Configure the clock-gating control register for block to turn off */
+ if (exynos4210_pd->mask) {
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&clkgate_block_lock, flags);
+ reg = __raw_readl(S5P_CLKGATE_BLOCK);
+ reg &= ~exynos4210_pd->mask;
+ __raw_writel(reg, S5P_CLKGATE_BLOCK);
+ spin_unlock_irqrestore(&clkgate_block_lock, flags);
+ }
+
+ return 0;
+}
+
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+ struct exynos4210_pm_domain *exynos4210_pd = to_exynos4210_pd(genpd);
+ u32 timeout;
+
+ /* Enable power domain */
+ __raw_writel(S5P_INT_LOCAL_PWR_EN, exynos4210_pd->base);
+
+ /* Wait max 1ms */
+ timeout = 10;
+ while ((__raw_readl(exynos4210_pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN)
+ != S5P_INT_LOCAL_PWR_EN) {
+ if (timeout == 0) {
+ printk(KERN_ERR "Power domain %s enable failed.\n",
+ exynos4210_pd->name);
+ return -ETIMEDOUT;
+ }
+ timeout--;
+ udelay(100);
+ }
+
+ /* Configure the clock-gating control register for block to turn on */
+ if (exynos4210_pd->mask) {
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&clkgate_block_lock, flags);
+ reg = __raw_readl(S5P_CLKGATE_BLOCK);
+ reg |= exynos4210_pd->mask;
+ __raw_writel(reg, S5P_CLKGATE_BLOCK);
+ spin_unlock_irqrestore(&clkgate_block_lock, flags);
+ }
+
+ return 0;
+}
+
+static bool pd_active_wakeup(struct device *dev)
+{
+ return true;
+}
+
+void exynos4210_init_pm_domain(struct exynos4210_pm_domain *exynos4210_pd)
+{
+ struct generic_pm_domain *genpd;
+
+ if (!exynos4210_pd)
+ return;
+
+ genpd = &exynos4210_pd->genpd;
+
+ pm_genpd_init(genpd, NULL, false);
+ genpd->stop_device = pm_clk_suspend;
+ genpd->start_device = pm_clk_resume;
+ genpd->active_wakeup = pd_active_wakeup;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
+
+ if (exynos4210_pd->boot_on)
+ pd_power_up(&exynos4210_pd->genpd);
+}
+
+void exynos4210_add_device_to_domain(struct exynos4210_pm_domain *exynos4210_pd,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ if (exynos4210_pd && pdev)
+ pm_genpd_add_device(&exynos4210_pd->genpd, dev);
+}
+
+struct exynos4210_pm_domain exynos4210_pd_mfc = {
+ .name = "PD_MFC",
+ .base = S5P_PMU_MFC_CONF,
+ .mask = S5P_CLKGATE_BLOCK_MFC,
+ .boot_on = 0,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_g3d = {
+ .name = "PD_G3D",
+ .base = S5P_PMU_G3D_CONF,
+ .mask = S5P_CLKGATE_BLOCK_G3D,
+ .boot_on = 0,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_lcd0 = {
+ .name = "PD_LCD0",
+ .base = S5P_PMU_LCD0_CONF,
+ .mask = S5P_CLKGATE_BLOCK_LCD0,
+ .boot_on = 1,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_lcd1 = {
+ .name = "PD_LCD1",
+ .base = S5P_PMU_LCD1_CONF,
+ .mask = S5P_CLKGATE_BLOCK_LCD1,
+ .boot_on = 0,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_tv = {
+ .name = "PD_TV",
+ .base = S5P_PMU_TV_CONF,
+ .mask = S5P_CLKGATE_BLOCK_TV,
+ .boot_on = 0,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_cam = {
+ .name = "PD_CAM",
+ .base = S5P_PMU_CAM_CONF,
+ .mask = S5P_CLKGATE_BLOCK_CAM,
+ .boot_on = 0,
+};
+
+struct exynos4210_pm_domain exynos4210_pd_gps = {
+ .name = "PD_GPS",
+ .base = S5P_PMU_GPS_CONF,
+ .mask = S5P_CLKGATE_BLOCK_GPS,
+ .boot_on = 0,
+};
+
+#endif /* CONFIG_PM */
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v2] ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
From: Chanwoo Choi @ 2011-07-08 0:58 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-samsung-soc, linux-kernel, Sylwester Nawrocki,
Kyungmin Park, Kukjin Kim, Myungjoo Ham, linux-pm
In-Reply-To: <201107072323.44155.rjw@sisk.pl>
Hi,
Rafael J. Wysocki wrote:
> Hi,
>
> On Monday, June 27, 2011, Chanwoo Choi wrote:
>> Use the generic power domains support to implement support for
>> power domain on EXYNOS4210.
>>
>> I refer to the following patch to implement what configure
>> the clock-gating control register for block to turn off/on:
>> http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
>>
>> This patch is based on following "pm-domains" branch.
>> http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=shortlog;h=refs/heads/pm-domains
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
>> ---
>> arch/arm/mach-exynos4/Kconfig | 1 +
>> arch/arm/mach-exynos4/Makefile | 1 +
>> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 69 ++++++++
>> arch/arm/mach-exynos4/include/mach/regs-clock.h | 8 +
>> arch/arm/mach-exynos4/mach-nuri.c | 20 +++
>> arch/arm/mach-exynos4/pm-exynos4210.c | 170 ++++++++++++++++++++
>> arch/arm/plat-samsung/include/plat/pd.h | 3 +-
>> 7 files changed, 271 insertions(+), 1 deletions(-)
>> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>>
>> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
>> index 1435fc3..266e7f6 100644
>> --- a/arch/arm/mach-exynos4/Kconfig
>> +++ b/arch/arm/mach-exynos4/Kconfig
>> @@ -183,6 +183,7 @@ config MACH_NURI
>> select EXYNOS4_SETUP_SDHCI
>> select EXYNOS4_SETUP_USB_PHY
>> select SAMSUNG_DEV_PWM
>> + select PM_GENERIC_DOMAINS
>
> That should be "select PM_GENERIC_DOMAINS if PM". Otherwise it will be
> selected even for CONFIG_PM unset.
OK, I will modify it.
>
>> help
>> Machine support for Samsung Mobile NURI Board.
>>
>> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
>> index 60fe5ec..0e9461f 100644
>> --- a/arch/arm/mach-exynos4/Makefile
>> +++ b/arch/arm/mach-exynos4/Makefile
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-eint.o dma.o
>> obj-$(CONFIG_PM) += pm.o sleep.o
>> obj-$(CONFIG_CPU_FREQ) += cpufreq.o
>> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
>> +obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
>>
>> obj-$(CONFIG_SMP) += platsmp.o headsmp.o
>>
>> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> new file mode 100644
>> index 0000000..ff924a8
>> --- /dev/null
>> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
>> @@ -0,0 +1,69 @@
>> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
>> + *
>> + * Exynos4 Power management support
>> + *
>> + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.com
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef __PM_EXYNOS4210_H__
>> +#define __PM_EXYNOS4210_H__
>> +
>> +#include <linux/pm_domain.h>
>> +#include <plat/pd.h>
>> +
>> +struct platform_device;
>> +
>> +struct exynos4210_pm_domain {
>> + struct generic_pm_domain genpd;
>> +
>> + const char *name;
>> + void __iomem *base;
>> + unsigned long gate_mask;
>> + int boot_on;
>> +};
>> +
>> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
>> + struct generic_pm_domain *pd)
>> +{
>> + return container_of(pd, struct exynos4210_pm_domain, genpd);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +extern struct exynos4210_pm_domain exynos4210_pd_list[PD_MAX_NUM];
>> +#define EXYNOS4210_MFC (&exynos4210_pd_list[PD_MFC])
>> +#define EXYNOS4210_G3D (&exynos4210_pd_list[PD_G3D])
>> +#define EXYNOS4210_LCD0 (&exynos4210_pd_list[PD_LCD0])
>> +#define EXYNOS4210_LCD1 (&exynos4210_pd_list[PD_LCD1])
>> +#define EXYNOS4210_TV (&exynos4210_pd_list[PD_TV])
>> +#define EXYNOS4210_CAM (&exynos4210_pd_list[PD_CAM])
>> +#define EXYNOS4210_GPS (&exynos4210_pd_list[PD_GPS])
>> +
>> +int exynos4210_pd_init(struct exynos4210_pm_domain *domain);
>> +int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
>> + struct platform_device *pdev);
>> +#else
>> +#define EXYNOS4210_MFC NULL
>> +#define EXYNOS4210_G3D NULL
>> +#define EXYNOS4210_LCD0 NULL
>> +#define EXYNOS4210_LCD1 NULL
>> +#define EXYNOS4210_TV NULL
>> +#define EXYNOS4210_CAM NULL
>> +#define EXYNOS4210_GPS NULL
>> +
>> +static inline int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
>> +{
>> + return 0;
>> +}
>> +static inline int exynos4_add_device_to_domain(struct exynos4210_pm_domain *domain,
>> + struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_PM */
>> +
>> +#endif /* __PM_EXYNOS4210_H__ */
>> diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-exynos4/include/mach/regs-clock.h
>> index 6e311c1..bc0fda9 100644
>> --- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
>> +++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
>> @@ -171,6 +171,14 @@
>> #define S5P_CLKDIV_BUS_GPLR_SHIFT (4)
>> #define S5P_CLKDIV_BUS_GPLR_MASK (0x7 << S5P_CLKDIV_BUS_GPLR_SHIFT)
>>
>> +#define S5P_CLKGATE_BLOCK_CAM (1<<0)
>> +#define S5P_CLKGATE_BLOCK_TV (1<<1)
>> +#define S5P_CLKGATE_BLOCK_MFC (1<<2)
>> +#define S5P_CLKGATE_BLOCK_G3D (1<<3)
>> +#define S5P_CLKGATE_BLOCK_LCD0 (1<<4)
>> +#define S5P_CLKGATE_BLOCK_LCD1 (1<<5)
>> +#define S5P_CLKGATE_BLOCK_GPS (1<<7)
>> +
>> /* Compatibility defines and inclusion */
>>
>> #include <mach/regs-pmu.h>
>> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
>> index 642702b..805c6d5 100644
>> --- a/arch/arm/mach-exynos4/mach-nuri.c
>> +++ b/arch/arm/mach-exynos4/mach-nuri.c
>> @@ -37,6 +37,7 @@
>> #include <plat/iic.h>
>>
>> #include <mach/map.h>
>> +#include <mach/pm-exynos4210.h>
>>
>> /* Following are default values for UCON, ULCON and UFCON UART registers */
>> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
>> @@ -376,6 +377,23 @@ static struct platform_device *nuri_devices[] __initdata = {
>> &nuri_backlight_device,
>> };
>>
>> +static void __init nuri_power_domain_init(void)
>> +{
>> + int i;
>> +
>> + for (i = 0 ; i < PD_MAX_NUM ; i++)
>> + exynos4210_pd_init(&exynos4210_pd_list[i]);
>> +
>> + /* Add device to power-domain */
>> + exynos4210_add_device_to_domain(EXYNOS4210_MFC, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_G3D, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_LCD0, &nuri_lcd_device);
>> + exynos4210_add_device_to_domain(EXYNOS4210_LCD1, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_TV, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_CAM, NULL);
>> + exynos4210_add_device_to_domain(EXYNOS4210_GPS, NULL);
>
> It seems the calls with the second NULL argument are redundant.
>
I will fix it.
> The rest looks OK to me.
>
> Thanks,
> Rafael
>
I will resend the modified patch according to your comment and your "pm-domains" branch.
Thanks,
Chanwoo Choi
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Kevin Hilman @ 2011-07-08 0:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <201106290144.01186.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Make generic PM domains support system-wide power transitions
> (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> poweroff and restore callbacks to be associated with struct
> generic_pm_domain objects and make pm_genpd_init() use them as
> appropriate.
>
> The new callbacks do nothing for devices belonging to power domains
> that were powered down at run time (before the transition).
Thinking about this some more, how is a driver supposed to reconfigure
wakeups during suspend if it has already been runtime suspended?
For example, assume a device where device_may_wakeup() == false. This
means wakeups during *suspend* are disabled, but wakeups wakeups are
assumed to enabled when it is runtime suspended.
So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
and then system suspend comes along.
With this current patch, the driver will never receive any callbacks, so
it can never disable its wakeups.
Am I missing something?
Kevin
^ permalink raw reply
* Re: [PATCH] power: introduce Charger-Manager
From: MyungJoo Ham @ 2011-07-08 0:16 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, linux-kernel, Greg Kroah-Hartman, dg77.kim,
Kyungmin Park, Anton Vorontsov, linux-pm
In-Reply-To: <201107072318.45500.rjw@sisk.pl>
2011/7/8 Rafael J. Wysocki <rjw@sisk.pl>:
> On Thursday, June 30, 2011, MyungJoo Ham wrote:
>> Charger Manager provides in-kernel battery charger management that
>> requires temperature monitoring during both normal and suspend-to-RAM states
>> and where each battery may have multiple chargers attached and the userland
>> wants to look at the aggregated information of the multiple chargers.
>>
>> For the discussions about the need for in-suspend monitoring, please
>> refer to the discussions of suspend-again in PM:
>> v1 https://lists.linux-foundation.org/pipermail/linux-pm/2011-April/031052.html
>> v2 https://lists.linux-foundation.org/pipermail/linux-pm/2011-April/031111.html
>> v3 https://lists.linux-foundation.org/pipermail/linux-pm/2011-May/031267.html
>> v4 https://lists.linux-foundation.org/pipermail/linux-pm/2011-May/031357.html
>> v5 (last, applied) https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031561.html
>>
>> To see the usage example, please refer to:
>> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/charger-manager
>> In this git branch, a test code for Exynos4-NURI is shown.
>>
>> Charger Manager is a platform_driver with power-supply-class entries.
>> An instance of Charger Manager (a platform-device created with
>> Charger-Manager) represents a battery with chargers. If there are multiple
>> batteries with their own chargers acting independently in a system,
>> the system may need multiple instances of Charger Manager.
>>
>> Charger Manager glues multiple charger-related frameworks (regulators of
>> chargers, power-supply-class from chargers and fuel-gauge, RTC,
>> suspend-again, ...) together to provide aggregated information and
>> transparent battery monitoring to userspace.
>>
>> Because battery health monitoring should be done even when suspended,
>> it needs to wake up and suspend periodically. Thus, userspace battery
>> monitoring may incur too much overhead; every device and task is waked
>> up periodically. Charger Manager uses suspend-again (in next PM) to provide
>> in-suspend monitoring. Multiple chargers (e.g., USB, wireless, and solar
>> panels) may be included as pairs of a regulator and a power-supply-class
>> per charger. Charger Manager provides power-supply-class aggregating
>> information from multiple chargers and a fuel-gauge and UEVENT notifying
>> status changes. Multiple instances of Charger Manager enable multiple
>> batteries.
>>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> I have tried to review this patch, but it is _huge_. Is there any reasonable
> way to split it into a series of smaller patches that would be easier to
> comprehend?
>
> Rafael
>
Ah.. ok..
I'll try to split it into multiple patches by introducing major functions first.
Thanks.
- 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: [patch 1/1] [PATCH] include storage keys in hibernation image.
From: Rafael J. Wysocki @ 2011-07-07 21:36 UTC (permalink / raw)
To: Martin Schwidefsky; +Cc: linux-s390, linux-kernel, linux-pm, Jiri Slaby
In-Reply-To: <20110615093629.07f01779@mschwide>
On Wednesday, June 15, 2011, Martin Schwidefsky wrote:
> On Tue, 14 Jun 2011 22:50:14 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
...
>
> Well before the preallocation kicked in we don't know which are the relevant
> storage keys. The only other option would be to store all of them which I
> consider to be a inferior solution.
I've been thinking about that recently a bit.
Don't we need to restore the keys of the page frames that aren't copied
into the image too?
Rafael
^ permalink raw reply
* Re: [PATCH v2] ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
From: Rafael J. Wysocki @ 2011-07-07 21:23 UTC (permalink / raw)
To: Chanwoo Choi
Cc: linux-samsung-soc, linux-kernel, Sylwester Nawrocki,
Kyungmin Park, Kukjin Kim, Myungjoo Ham, linux-pm
In-Reply-To: <4E0824B1.6070501@samsung.com>
Hi,
On Monday, June 27, 2011, Chanwoo Choi wrote:
> Use the generic power domains support to implement support for
> power domain on EXYNOS4210.
>
> I refer to the following patch to implement what configure
> the clock-gating control register for block to turn off/on:
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/39a81876d034dcbdc2a4c4c4b847b3b49e38870c
>
> This patch is based on following "pm-domains" branch.
> http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=shortlog;h=refs/heads/pm-domains
>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
> ---
> arch/arm/mach-exynos4/Kconfig | 1 +
> arch/arm/mach-exynos4/Makefile | 1 +
> arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 69 ++++++++
> arch/arm/mach-exynos4/include/mach/regs-clock.h | 8 +
> arch/arm/mach-exynos4/mach-nuri.c | 20 +++
> arch/arm/mach-exynos4/pm-exynos4210.c | 170 ++++++++++++++++++++
> arch/arm/plat-samsung/include/plat/pd.h | 3 +-
> 7 files changed, 271 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
>
> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
> index 1435fc3..266e7f6 100644
> --- a/arch/arm/mach-exynos4/Kconfig
> +++ b/arch/arm/mach-exynos4/Kconfig
> @@ -183,6 +183,7 @@ config MACH_NURI
> select EXYNOS4_SETUP_SDHCI
> select EXYNOS4_SETUP_USB_PHY
> select SAMSUNG_DEV_PWM
> + select PM_GENERIC_DOMAINS
That should be "select PM_GENERIC_DOMAINS if PM". Otherwise it will be
selected even for CONFIG_PM unset.
> help
> Machine support for Samsung Mobile NURI Board.
>
> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
> index 60fe5ec..0e9461f 100644
> --- a/arch/arm/mach-exynos4/Makefile
> +++ b/arch/arm/mach-exynos4/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_CPU_EXYNOS4210) += setup-i2c0.o irq-eint.o dma.o
> obj-$(CONFIG_PM) += pm.o sleep.o
> obj-$(CONFIG_CPU_FREQ) += cpufreq.o
> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> +obj-$(CONFIG_CPU_EXYNOS4210) += pm-exynos4210.o
>
> obj-$(CONFIG_SMP) += platsmp.o headsmp.o
>
> diff --git a/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> new file mode 100644
> index 0000000..ff924a8
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> @@ -0,0 +1,69 @@
> +/* linux/arch/arm/mach-exynos4/include/mach/pm-exynos4.h
> + *
> + * Exynos4 Power management support
> + *
> + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __PM_EXYNOS4210_H__
> +#define __PM_EXYNOS4210_H__
> +
> +#include <linux/pm_domain.h>
> +#include <plat/pd.h>
> +
> +struct platform_device;
> +
> +struct exynos4210_pm_domain {
> + struct generic_pm_domain genpd;
> +
> + const char *name;
> + void __iomem *base;
> + unsigned long gate_mask;
> + int boot_on;
> +};
> +
> +static inline struct exynos4210_pm_domain *to_exynos4210_pd(
> + struct generic_pm_domain *pd)
> +{
> + return container_of(pd, struct exynos4210_pm_domain, genpd);
> +}
> +
> +#ifdef CONFIG_PM
> +extern struct exynos4210_pm_domain exynos4210_pd_list[PD_MAX_NUM];
> +#define EXYNOS4210_MFC (&exynos4210_pd_list[PD_MFC])
> +#define EXYNOS4210_G3D (&exynos4210_pd_list[PD_G3D])
> +#define EXYNOS4210_LCD0 (&exynos4210_pd_list[PD_LCD0])
> +#define EXYNOS4210_LCD1 (&exynos4210_pd_list[PD_LCD1])
> +#define EXYNOS4210_TV (&exynos4210_pd_list[PD_TV])
> +#define EXYNOS4210_CAM (&exynos4210_pd_list[PD_CAM])
> +#define EXYNOS4210_GPS (&exynos4210_pd_list[PD_GPS])
> +
> +int exynos4210_pd_init(struct exynos4210_pm_domain *domain);
> +int exynos4210_add_device_to_domain(struct exynos4210_pm_domain *domain,
> + struct platform_device *pdev);
> +#else
> +#define EXYNOS4210_MFC NULL
> +#define EXYNOS4210_G3D NULL
> +#define EXYNOS4210_LCD0 NULL
> +#define EXYNOS4210_LCD1 NULL
> +#define EXYNOS4210_TV NULL
> +#define EXYNOS4210_CAM NULL
> +#define EXYNOS4210_GPS NULL
> +
> +static inline int exynos4210_pd_init(struct exynos4210_pm_domain *domain)
> +{
> + return 0;
> +}
> +static inline int exynos4_add_device_to_domain(struct exynos4210_pm_domain *domain,
> + struct platform_device *pdev)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_PM */
> +
> +#endif /* __PM_EXYNOS4210_H__ */
> diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-exynos4/include/mach/regs-clock.h
> index 6e311c1..bc0fda9 100644
> --- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
> +++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
> @@ -171,6 +171,14 @@
> #define S5P_CLKDIV_BUS_GPLR_SHIFT (4)
> #define S5P_CLKDIV_BUS_GPLR_MASK (0x7 << S5P_CLKDIV_BUS_GPLR_SHIFT)
>
> +#define S5P_CLKGATE_BLOCK_CAM (1<<0)
> +#define S5P_CLKGATE_BLOCK_TV (1<<1)
> +#define S5P_CLKGATE_BLOCK_MFC (1<<2)
> +#define S5P_CLKGATE_BLOCK_G3D (1<<3)
> +#define S5P_CLKGATE_BLOCK_LCD0 (1<<4)
> +#define S5P_CLKGATE_BLOCK_LCD1 (1<<5)
> +#define S5P_CLKGATE_BLOCK_GPS (1<<7)
> +
> /* Compatibility defines and inclusion */
>
> #include <mach/regs-pmu.h>
> diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
> index 642702b..805c6d5 100644
> --- a/arch/arm/mach-exynos4/mach-nuri.c
> +++ b/arch/arm/mach-exynos4/mach-nuri.c
> @@ -37,6 +37,7 @@
> #include <plat/iic.h>
>
> #include <mach/map.h>
> +#include <mach/pm-exynos4210.h>
>
> /* Following are default values for UCON, ULCON and UFCON UART registers */
> #define NURI_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> @@ -376,6 +377,23 @@ static struct platform_device *nuri_devices[] __initdata = {
> &nuri_backlight_device,
> };
>
> +static void __init nuri_power_domain_init(void)
> +{
> + int i;
> +
> + for (i = 0 ; i < PD_MAX_NUM ; i++)
> + exynos4210_pd_init(&exynos4210_pd_list[i]);
> +
> + /* Add device to power-domain */
> + exynos4210_add_device_to_domain(EXYNOS4210_MFC, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_G3D, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_LCD0, &nuri_lcd_device);
> + exynos4210_add_device_to_domain(EXYNOS4210_LCD1, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_TV, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_CAM, NULL);
> + exynos4210_add_device_to_domain(EXYNOS4210_GPS, NULL);
It seems the calls with the second NULL argument are redundant.
The rest looks OK to me.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] power: introduce Charger-Manager
From: Rafael J. Wysocki @ 2011-07-07 21:18 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, linux-kernel, Greg Kroah-Hartman, dg77.kim,
Kyungmin Park, Anton Vorontsov, linux-pm
In-Reply-To: <1309424069-15965-1-git-send-email-myungjoo.ham@samsung.com>
On Thursday, June 30, 2011, MyungJoo Ham wrote:
> Charger Manager provides in-kernel battery charger management that
> requires temperature monitoring during both normal and suspend-to-RAM states
> and where each battery may have multiple chargers attached and the userland
> wants to look at the aggregated information of the multiple chargers.
>
> For the discussions about the need for in-suspend monitoring, please
> refer to the discussions of suspend-again in PM:
> v1 https://lists.linux-foundation.org/pipermail/linux-pm/2011-April/031052.html
> v2 https://lists.linux-foundation.org/pipermail/linux-pm/2011-April/031111.html
> v3 https://lists.linux-foundation.org/pipermail/linux-pm/2011-May/031267.html
> v4 https://lists.linux-foundation.org/pipermail/linux-pm/2011-May/031357.html
> v5 (last, applied) https://lists.linux-foundation.org/pipermail/linux-pm/2011-June/031561.html
>
> To see the usage example, please refer to:
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/charger-manager
> In this git branch, a test code for Exynos4-NURI is shown.
>
> Charger Manager is a platform_driver with power-supply-class entries.
> An instance of Charger Manager (a platform-device created with
> Charger-Manager) represents a battery with chargers. If there are multiple
> batteries with their own chargers acting independently in a system,
> the system may need multiple instances of Charger Manager.
>
> Charger Manager glues multiple charger-related frameworks (regulators of
> chargers, power-supply-class from chargers and fuel-gauge, RTC,
> suspend-again, ...) together to provide aggregated information and
> transparent battery monitoring to userspace.
>
> Because battery health monitoring should be done even when suspended,
> it needs to wake up and suspend periodically. Thus, userspace battery
> monitoring may incur too much overhead; every device and task is waked
> up periodically. Charger Manager uses suspend-again (in next PM) to provide
> in-suspend monitoring. Multiple chargers (e.g., USB, wireless, and solar
> panels) may be included as pairs of a regulator and a power-supply-class
> per charger. Charger Manager provides power-supply-class aggregating
> information from multiple chargers and a fuel-gauge and UEVENT notifying
> status changes. Multiple instances of Charger Manager enable multiple
> batteries.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
I have tried to review this patch, but it is _huge_. Is there any reasonable
way to split it into a series of smaller patches that would be easier to
comprehend?
Rafael
^ permalink raw reply
* Re: [PATCH 5/7] PM: PM notifier error injection
From: Rafael J. Wysocki @ 2011-07-07 21:14 UTC (permalink / raw)
To: Akinobu Mita; +Cc: akpm, linux-kernel, linux-pm
In-Reply-To: <1309702581-16863-6-git-send-email-akinobu.mita@gmail.com>
On Sunday, July 03, 2011, Akinobu Mita wrote:
> This provides the ability to inject artifical errors to PM notifier
> chain callbacks. It is controlled through debugfs interface under
> /sys/kernel/debug/pm-notifier-error-inject/
>
> Each of the files in the directory represents an event which can be
> failed and contains the error code. If the notifier call chain should
> be failed with some events notified, write the error code to the files.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-pm@lists.linux-foundation.org
> ---
> kernel/power/main.c | 30 ++++++++++++++++++++++++++++++
> lib/Kconfig.debug | 8 ++++++++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 6c601f8..04b3774 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -42,6 +42,36 @@ int pm_notifier_call_chain(unsigned long val)
> return notifier_to_errno(ret);
> }
>
> +#ifdef CONFIG_PM_NOTIFIER_ERROR_INJECTION
> +
> +static struct err_inject_notifier_block err_inject_pm_notifier = {
> + .actions = {
> + { ERR_INJECT_NOTIFIER_ACTION(PM_HIBERNATION_PREPARE) },
> + { ERR_INJECT_NOTIFIER_ACTION(PM_SUSPEND_PREPARE) },
> + { ERR_INJECT_NOTIFIER_ACTION(PM_RESTORE_PREPARE) },
> + {}
Why have you omitted the PM_POST_* actions?
> + }
> +};
> +
> +static int __init err_inject_pm_notifier_init(void)
> +{
> + int err;
> +
> + err = err_inject_notifier_block_init(&err_inject_pm_notifier,
> + "pm-notifier-error-inject", -1);
> + if (err)
> + return err;
> +
> + err = register_pm_notifier(&err_inject_pm_notifier.nb);
> + if (err)
> + err_inject_notifier_block_cleanup(&err_inject_pm_notifier);
> +
> + return err;
> +}
> +late_initcall(err_inject_pm_notifier_init);
> +
> +#endif /* CONFIG_PM_NOTIFIER_ERROR_INJECTION */
> +
> /* If set, devices may be suspended and resumed asynchronously. */
> int pm_async_enabled = 1;
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d944b32..3ffb38b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1043,6 +1043,14 @@ config CPU_NOTIFIER_ERROR_INJECTION
> # echo 0 > /sys/devices/system/cpu/cpu1/online
> bash: echo: write error: Operation not permitted
>
> +config PM_NOTIFIER_ERROR_INJECTION
> + bool "PM notifier error injection"
> + depends on PM_DEBUG && NOTIFIER_ERROR_INJECTION
> + help
> + This option provides the ability to inject artifical errors to
> + PM notifier chain callbacks. It is controlled through debugfs
> + interface under /sys/kernel/debug/pm-notifier-error-inject/
I'm not sure the help is necessary. I think it should be selected
automatically if both PM_DEBUG and NOTIFIER_ERROR_INJECTION are set.
> +
> config CPU_NOTIFIER_ERROR_INJECT
> tristate "CPU notifier error injection module"
> depends on HOTPLUG_CPU && DEBUG_KERNEL
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v3 2/3] PM / DEVFREQ: add example governors
From: Rafael J. Wysocki @ 2011-07-07 21:08 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, Kyungmin Park,
linux-pm, Thomas Gleixner
In-Reply-To: <CAJ0PZbQ0ENk-vEebqiD=DGHZVtswHQkPre8wUqM2vwR_r+V=Uw@mail.gmail.com>
On Monday, July 04, 2011, MyungJoo Ham wrote:
> On Mon, Jul 4, 2011 at 5:43 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Monday, July 04, 2011, MyungJoo Ham wrote:
> >> Hello,
> >>
> >> 2011/7/3 Rafael J. Wysocki <rjw@sisk.pl>:
> >> > Hi,
> >> >
> >> > On Friday, May 27, 2011, MyungJoo Ham wrote:
> >> >> Three CPUFREQ-like governors are provided as examples.
> >> >>
> >> >> powersave: use the lowest frequency possible. The user (device) should
> >> >> set the polling_ms as 0 because polling is useless for this governor.
> >> >>
> >> >> performance: use the highest freqeuncy possible. The user (device)
> >> >> should set the polling_ms as 0 because polling is useless for this
> >> >> governor.
> >> >>
> >> >> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.
> >> >>
> >> >> 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
> >> >> DEVFREQ manually with OPP entry updates or set polling_ms for powersave
> >> >> , performance, or any other "static" governors.
> >> >>
> >> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> []
> >> >> +
> >> >> + /* Set the desired frequency based on the load */
> >> >> + a = (unsigned long long) stat.busy_time * stat.current_frequency;
> >> >
> >> > What's the purpose of the conversion?
> >>
> >> Assuming that the work speed of a device is proportional to its
> >> frequency, it measures the amount of work done.
> >> It's time * work/time. For example, during the last 10 second, if the
> >> busy_time was 5 sec and frequency was 10MHz,
> >> it's "50M", which is same as 20MHz and 2.5 sec.
> >
> > I understand that, but my question was why you're doing a forced conversion
> > to (unsigned long long).
>
> Ah.. that was for the 64bit operations.
>
> Both busy_time and current_frequency are 32bit and current_frequency
> may be a big number.
>
> Thus, in order to get "freq" value without losing bits (e.g., if
> current_frequency = 1GHz and busy_time = 8000, we get an overflow
> without 64bit operations), I've inserted 64bit operations with the
> conversion. For the cosmetic reasons, it appears that "u64" looks
> better though.
You wouldn't need the explicit type casting if you did
a = stat.busy_time;
a *= stat.current_frequency;
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 1/7] pm: improve error code of pm_notifier_call_chain()
From: Rafael J. Wysocki @ 2011-07-07 21:06 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-s390, Jiri Kosina, Heiko Carstens, linux-kernel,
Martin Schwidefsky, linux390, akpm, linux-pm
In-Reply-To: <CAC5umyibWo===0Lr5Wz2moOvnf0qiAvFLo4--sLCRmx4sJrdRA@mail.gmail.com>
On Monday, July 04, 2011, Akinobu Mita wrote:
> 2011/7/4 Pavel Machek <pavel@ucw.cz>:
> > On Sun 2011-07-03 23:16:15, Akinobu Mita wrote:
> >> This enables pm_notifier_call_chain() to get the actual error code
> >> in the callback rather than always assume -EINVAL by converting all
> >> PM notifier calls to return encapsulate error code with
> >> notifier_from_errno().
> >>
> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> Cc: Pavel Machek <pavel@ucw.cz>
> >
> > Nothing obviously wrong here.
>
> Thanks. Can I add your ACK for this patch?
Do you want me to take this patch?
Rafael
^ permalink raw reply
* [GIT PULL] Power management fix for 3.0
From: Rafael J. Wysocki @ 2011-07-07 20:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux PM mailing list, LKML, Matthew Garrett
Hi Linus,
Please pull a power management fix for 3.0 from:
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-fixes
It fixes an old bug, but such that causes a BUG_ON() to trigger and it is
-stable material.
kernel/power/snapshot.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
---------------
Rafael J. Wysocki (1):
PM / Hibernate: Fix free_unnecessary_pages()
^ permalink raw reply
* [Update][PATCH 5/5] PM / Domains: Do not restore all devices on power off error
From: Rafael J. Wysocki @ 2011-07-07 20:01 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107062301.22277.rjw@sisk.pl>
From: Rafael J. Wysocki <rjw@sisk.pl>
Since every device in a PM domain has its own need_restore
flag, which is set by __pm_genpd_save_device(), there's no need to
walk the domain's device list and restore all devices on an error
from one of the drivers' .runtime_suspend() callbacks.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
This update is on top of the [4/5] update I've just sent. It makes the code
more straightforward IMHO.
Thanks,
Rafael
---
drivers/base/power/domain.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -269,8 +269,10 @@ static int pm_genpd_poweroff(struct gene
list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
ret = __pm_genpd_save_device(dle, genpd);
- if (ret)
- goto err_dev;
+ if (ret) {
+ genpd_set_active(genpd);
+ goto out;
+ }
if (genpd_abort_poweroff(genpd))
goto out;
@@ -311,13 +313,6 @@ static int pm_genpd_poweroff(struct gene
genpd->poweroff_task = NULL;
wake_up_all(&genpd->status_wait_queue);
return ret;
-
- err_dev:
- list_for_each_entry_continue(dle, &genpd->dev_list, node)
- __pm_genpd_restore_device(dle, genpd);
-
- genpd_set_active(genpd);
- goto out;
}
/**
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox