Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-15 Thread Nishanth Menon 
On 11/15/2013 04:27 AM, Viresh Kumar wrote:
> On 14 November 2013 22:34, Nishanth Menon  wrote:
>> I think it is still too early to do so :(
> 
> :)
:D

> 
>> equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
>> changes for build)
>>
>> Basic tests: http://pastebin.mozilla.org/3603456 (governor is
>> functional, but governor kicks in early before i2c is resumed)
>>
>> With call stack: http://pastebin.mozilla.org/3603455 to highlight call
>> sequences
>>
>> Seems like we might want to pause governor as early in the suspend
>> sequence as possible to allow SoC and regulator stuff to suspend
>> themselves without cpufreq interfering.. just my 2 cents..
> 
> You made me spend a day on this :)
> It wasn't a day's job really but I got into a really hard to crack bug with my
> patch, I was calling __cpufreq_governor() from under write_lock_irqsave
> for cpufreq_driver_lock. And __cpufreq_governor() had:
> 
>  read_lock_irqsave(_driver_lock, flags);
> 
> I wasn't able to suspend my system: ARM, X86.. It simply stopped
> printing anything and I didn't had a clue of what's going on.. Hacked
> everything possible, even kernel/power/suspend.c to return early
> (yeah I used freezer > pm_test as well, but I wanted to return before
> freezing userspace)...
> 
> Then somehow I got to know that this is the wrong piece of code :)

Thanks a ton for your efforts in helping come with a generic solution.

> 
> But probably I have a solution now to which you can't say:

https://patchwork.kernel.org/patch/3187511/ as a link for the records :)

> 
> "I think it is still too early to do so :("
> 
> :)
> 
> Give it a try and give a Tested-by please :)
> 
Definitely - on it.. will feedback further on the patch in proposal.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-15 Thread Viresh Kumar 
On 14 November 2013 22:34, Nishanth Menon  wrote:
> I think it is still too early to do so :(

:)

> equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
> changes for build)
>
> Basic tests: http://pastebin.mozilla.org/3603456 (governor is
> functional, but governor kicks in early before i2c is resumed)
>
> With call stack: http://pastebin.mozilla.org/3603455 to highlight call
> sequences
>
> Seems like we might want to pause governor as early in the suspend
> sequence as possible to allow SoC and regulator stuff to suspend
> themselves without cpufreq interfering.. just my 2 cents..

You made me spend a day on this :)
It wasn't a day's job really but I got into a really hard to crack bug with my
patch, I was calling __cpufreq_governor() from under write_lock_irqsave
for cpufreq_driver_lock. And __cpufreq_governor() had:

 read_lock_irqsave(_driver_lock, flags);

I wasn't able to suspend my system: ARM, X86.. It simply stopped
printing anything and I didn't had a clue of what's going on.. Hacked
everything possible, even kernel/power/suspend.c to return early
(yeah I used freezer > pm_test as well, but I wanted to return before
freezing userspace)...

Then somehow I got to know that this is the wrong piece of code :)

But probably I have a solution now to which you can't say:

"I think it is still too early to do so :("

:)

Give it a try and give a Tested-by please :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-15 Thread Viresh Kumar 
On 14 November 2013 22:34, Nishanth Menon n...@ti.com wrote:
 I think it is still too early to do so :(

:)

 equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
 changes for build)

 Basic tests: http://pastebin.mozilla.org/3603456 (governor is
 functional, but governor kicks in early before i2c is resumed)

 With call stack: http://pastebin.mozilla.org/3603455 to highlight call
 sequences

 Seems like we might want to pause governor as early in the suspend
 sequence as possible to allow SoC and regulator stuff to suspend
 themselves without cpufreq interfering.. just my 2 cents..

You made me spend a day on this :)
It wasn't a day's job really but I got into a really hard to crack bug with my
patch, I was calling __cpufreq_governor() from under write_lock_irqsave
for cpufreq_driver_lock. And __cpufreq_governor() had:

 read_lock_irqsave(cpufreq_driver_lock, flags);

I wasn't able to suspend my system: ARM, X86.. It simply stopped
printing anything and I didn't had a clue of what's going on.. Hacked
everything possible, even kernel/power/suspend.c to return early
(yeah I used freezer  pm_test as well, but I wanted to return before
freezing userspace)...

Then somehow I got to know that this is the wrong piece of code :)

But probably I have a solution now to which you can't say:

I think it is still too early to do so :(

:)

Give it a try and give a Tested-by please :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-15 Thread Nishanth Menon 
On 11/15/2013 04:27 AM, Viresh Kumar wrote:
 On 14 November 2013 22:34, Nishanth Menon n...@ti.com wrote:
 I think it is still too early to do so :(
 
 :)
:D

 
 equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
 changes for build)

 Basic tests: http://pastebin.mozilla.org/3603456 (governor is
 functional, but governor kicks in early before i2c is resumed)

 With call stack: http://pastebin.mozilla.org/3603455 to highlight call
 sequences

 Seems like we might want to pause governor as early in the suspend
 sequence as possible to allow SoC and regulator stuff to suspend
 themselves without cpufreq interfering.. just my 2 cents..
 
 You made me spend a day on this :)
 It wasn't a day's job really but I got into a really hard to crack bug with my
 patch, I was calling __cpufreq_governor() from under write_lock_irqsave
 for cpufreq_driver_lock. And __cpufreq_governor() had:
 
  read_lock_irqsave(cpufreq_driver_lock, flags);
 
 I wasn't able to suspend my system: ARM, X86.. It simply stopped
 printing anything and I didn't had a clue of what's going on.. Hacked
 everything possible, even kernel/power/suspend.c to return early
 (yeah I used freezer  pm_test as well, but I wanted to return before
 freezing userspace)...
 
 Then somehow I got to know that this is the wrong piece of code :)

Thanks a ton for your efforts in helping come with a generic solution.

 
 But probably I have a solution now to which you can't say:

https://patchwork.kernel.org/patch/3187511/ as a link for the records :)

 
 I think it is still too early to do so :(
 
 :)
 
 Give it a try and give a Tested-by please :)
 
Definitely - on it.. will feedback further on the patch in proposal.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread viresh kumar 
On Friday 15 November 2013 03:30 AM, Rafael J. Wysocki wrote:
> I'm not going to apply anything like this.  If I have already, that's been a 
> mistake.
> 
> Do not mix assignments with logical operators in such outrageous ways, please.
> That's completely unreadable and confusing.

Okay... Will get it fixed for existing code as well..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Rafael J. Wysocki 
On Thursday, November 14, 2013 06:55:05 AM viresh kumar wrote:
> On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
> > arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the 
> > following
> > look equivalent?
> 
> yes.
> 
> > With this, I now see:
> 
> > [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
> > ^^^ ??
> 
> Ahh, I missed this part. I thought it will fail at some other place where 
> there
> is no error checking :), but that's not true.
> 
> Following should fix it for you and looks to be the right way as well.
> 
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index dc67fa0..30b09d3 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
> }
> }
> 
> +   if (has_target()) {
> +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
> +   (ret = __cpufreq_governor(policy, 
> CPUFREQ_GOV_LIMITS))) {


I'm not going to apply anything like this.  If I have already, that's been a 
mistake.

Do not mix assignments with logical operators in such outrageous ways, please.
That's completely unreadable and confusing.

What about:

ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
if (!ret) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
if (ret) {


> +   pr_err("%s: Failed to start governor\n", __func__);
> +   goto fail;
> +   }
> +   }
> +
> schedule_work(>update);
> 
>  fail:

Thanks!

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

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Nishanth Menon 
On 11/14/2013 10:46 AM, viresh kumar wrote:
> On Thursday 14 November 2013 07:57 PM, Nishanth Menon wrote:
>> I am guessing this is a little too early for restarting policy here
>> considering syscore_ops->resume is pretty early..
> 
> Yeah, looks like that..
> 
>> http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
>> http://pastebin.mozilla.org/3602747 is the result.
> 
> Can you try this instead of last diff I sent?
> 
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index dc67fa0..e70e906 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1324,6 +1324,15 @@ static void handle_update(struct work_struct *work)
> container_of(work, struct cpufreq_policy, update);
> unsigned int cpu = policy->cpu;
> pr_debug("handle_update for cpu %u called\n", cpu);
> +
> +   if (has_target() && !policy->governor_enabled) {
> +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
> +   (ret = __cpufreq_governor(policy, 
> CPUFREQ_GOV_LIMITS))) {
> +   pr_err("%s: Failed to start governor\n", __func__);
> +   goto fail;
> +   }
> +   }
> +
> cpufreq_update_policy(cpu);
>  }
> 
> 
I think it is still too early to do so :(

equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
changes for build)

Basic tests: http://pastebin.mozilla.org/3603456 (governor is
functional, but governor kicks in early before i2c is resumed)

With call stack: http://pastebin.mozilla.org/3603455 to highlight call
sequences

Seems like we might want to pause governor as early in the suspend
sequence as possible to allow SoC and regulator stuff to suspend
themselves without cpufreq interfering.. just my 2 cents..

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread viresh kumar 
On Thursday 14 November 2013 07:57 PM, Nishanth Menon wrote:
> I am guessing this is a little too early for restarting policy here
> considering syscore_ops->resume is pretty early..

Yeah, looks like that..

> http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
> http://pastebin.mozilla.org/3602747 is the result.

Can you try this instead of last diff I sent?


diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index dc67fa0..e70e906 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1324,6 +1324,15 @@ static void handle_update(struct work_struct *work)
container_of(work, struct cpufreq_policy, update);
unsigned int cpu = policy->cpu;
pr_debug("handle_update for cpu %u called\n", cpu);
+
+   if (has_target() && !policy->governor_enabled) {
+   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+   (ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) 
{
+   pr_err("%s: Failed to start governor\n", __func__);
+   goto fail;
+   }
+   }
+
cpufreq_update_policy(cpu);
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Nishanth Menon 
On 11/13/2013 07:25 PM, viresh kumar wrote:
> On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
>> arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
>> look equivalent?
> 
> yes.
> 
>> With this, I now see:
> 
>> [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
>> ^^^ ??
> 
> Ahh, I missed this part. I thought it will fail at some other place where 
> there
> is no error checking :), but that's not true.
> 
> Following should fix it for you and looks to be the right way as well.
> 
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index dc67fa0..30b09d3 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
> }
> }
> 
> +   if (has_target()) {
> +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
> +   (ret = __cpufreq_governor(policy, 
> CPUFREQ_GOV_LIMITS))) {
> +   pr_err("%s: Failed to start governor\n", __func__);
> +   goto fail;
> +   }
> +   }
> +
> schedule_work(>update);
> 
>  fail:
> 
I am guessing this is a little too early for restarting policy here
considering syscore_ops->resume is pretty early..

http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
http://pastebin.mozilla.org/3602747 is the result.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Nishanth Menon 
On 11/13/2013 07:25 PM, viresh kumar wrote:
 On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
 arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
 look equivalent?
 
 yes.
 
 With this, I now see:
 
 [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
 ^^^ ??
 
 Ahh, I missed this part. I thought it will fail at some other place where 
 there
 is no error checking :), but that's not true.
 
 Following should fix it for you and looks to be the right way as well.
 
 
 diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
 index dc67fa0..30b09d3 100644
 --- a/drivers/cpufreq/cpufreq.c
 +++ b/drivers/cpufreq/cpufreq.c
 @@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
 }
 }
 
 +   if (has_target()) {
 +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
 +   (ret = __cpufreq_governor(policy, 
 CPUFREQ_GOV_LIMITS))) {
 +   pr_err(%s: Failed to start governor\n, __func__);
 +   goto fail;
 +   }
 +   }
 +
 schedule_work(policy-update);
 
  fail:
 
I am guessing this is a little too early for restarting policy here
considering syscore_ops-resume is pretty early..

http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
http://pastebin.mozilla.org/3602747 is the result.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread viresh kumar 
On Thursday 14 November 2013 07:57 PM, Nishanth Menon wrote:
 I am guessing this is a little too early for restarting policy here
 considering syscore_ops-resume is pretty early..

Yeah, looks like that..

 http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
 http://pastebin.mozilla.org/3602747 is the result.

Can you try this instead of last diff I sent?


diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index dc67fa0..e70e906 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1324,6 +1324,15 @@ static void handle_update(struct work_struct *work)
container_of(work, struct cpufreq_policy, update);
unsigned int cpu = policy-cpu;
pr_debug(handle_update for cpu %u called\n, cpu);
+
+   if (has_target()  !policy-governor_enabled) {
+   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+   (ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) 
{
+   pr_err(%s: Failed to start governor\n, __func__);
+   goto fail;
+   }
+   }
+
cpufreq_update_policy(cpu);
 }


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Nishanth Menon 
On 11/14/2013 10:46 AM, viresh kumar wrote:
 On Thursday 14 November 2013 07:57 PM, Nishanth Menon wrote:
 I am guessing this is a little too early for restarting policy here
 considering syscore_ops-resume is pretty early..
 
 Yeah, looks like that..
 
 http://pastebin.mozilla.org/3602746 is the equivalent patch for v3.12
 http://pastebin.mozilla.org/3602747 is the result.
 
 Can you try this instead of last diff I sent?
 
 
 diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
 index dc67fa0..e70e906 100644
 --- a/drivers/cpufreq/cpufreq.c
 +++ b/drivers/cpufreq/cpufreq.c
 @@ -1324,6 +1324,15 @@ static void handle_update(struct work_struct *work)
 container_of(work, struct cpufreq_policy, update);
 unsigned int cpu = policy-cpu;
 pr_debug(handle_update for cpu %u called\n, cpu);
 +
 +   if (has_target()  !policy-governor_enabled) {
 +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
 +   (ret = __cpufreq_governor(policy, 
 CPUFREQ_GOV_LIMITS))) {
 +   pr_err(%s: Failed to start governor\n, __func__);
 +   goto fail;
 +   }
 +   }
 +
 cpufreq_update_policy(cpu);
  }
 
 
I think it is still too early to do so :(

equivalent patch: http://pastebin.mozilla.org/3603467 (with minor
changes for build)

Basic tests: http://pastebin.mozilla.org/3603456 (governor is
functional, but governor kicks in early before i2c is resumed)

With call stack: http://pastebin.mozilla.org/3603455 to highlight call
sequences

Seems like we might want to pause governor as early in the suspend
sequence as possible to allow SoC and regulator stuff to suspend
themselves without cpufreq interfering.. just my 2 cents..

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread Rafael J. Wysocki 
On Thursday, November 14, 2013 06:55:05 AM viresh kumar wrote:
 On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
  arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the 
  following
  look equivalent?
 
 yes.
 
  With this, I now see:
 
  [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
  ^^^ ??
 
 Ahh, I missed this part. I thought it will fail at some other place where 
 there
 is no error checking :), but that's not true.
 
 Following should fix it for you and looks to be the right way as well.
 
 
 diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
 index dc67fa0..30b09d3 100644
 --- a/drivers/cpufreq/cpufreq.c
 +++ b/drivers/cpufreq/cpufreq.c
 @@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
 }
 }
 
 +   if (has_target()) {
 +   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
 +   (ret = __cpufreq_governor(policy, 
 CPUFREQ_GOV_LIMITS))) {


I'm not going to apply anything like this.  If I have already, that's been a 
mistake.

Do not mix assignments with logical operators in such outrageous ways, please.
That's completely unreadable and confusing.

What about:

ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
if (!ret) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
if (ret) {


 +   pr_err(%s: Failed to start governor\n, __func__);
 +   goto fail;
 +   }
 +   }
 +
 schedule_work(policy-update);
 
  fail:

Thanks!

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

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-14 Thread viresh kumar 
On Friday 15 November 2013 03:30 AM, Rafael J. Wysocki wrote:
 I'm not going to apply anything like this.  If I have already, that's been a 
 mistake.
 
 Do not mix assignments with logical operators in such outrageous ways, please.
 That's completely unreadable and confusing.

Okay... Will get it fixed for existing code as well..
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-13 Thread viresh kumar 
On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
> arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
> look equivalent?

yes.

> With this, I now see:

> [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
> ^^^ ??

Ahh, I missed this part. I thought it will fail at some other place where there
is no error checking :), but that's not true.

Following should fix it for you and looks to be the right way as well.


diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index dc67fa0..30b09d3 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
}
}

+   if (has_target()) {
+   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+   (ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) 
{
+   pr_err("%s: Failed to start governor\n", __func__);
+   goto fail;
+   }
+   }
+
schedule_work(>update);

 fail:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-13 Thread Nishanth Menon 
On 11:19-20131113, Viresh Kumar wrote:
> On 12 November 2013 20:41, Nishanth Menon  wrote:
> > On 11/12/2013 12:03 AM, Viresh Kumar wrote:
[...]
> >> Can you try attached patch? I will then repost it formally...
> >
> > I tried a equivalent of this for v3.12 tag:
[..]
> > @@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
> > device *dev,
> >
> > /* If cpu is last user of policy, free policy */
> > if (cpus == 1) {
> > -   if (cpufreq_driver->target) {
> > +   if (cpufreq_driver->target && !frozen) {
> > ret = __cpufreq_governor(policy,
> > CPUFREQ_GOV_POLICY_EXIT);
> 
> This is not an equivalent of my patch :)

arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
look equivalent?
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 04548f7..a9847ce 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct device 
*dev,
return -EINVAL;
}
 
-   if (cpufreq_driver->target) {
+   if (cpufreq_driver->target && (!frozen || policy->governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err("%s: Failed to stop governor\n", __func__);
@@ -1295,7 +1295,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (cpufreq_driver->target) {
+   if (cpufreq_driver->target && !frozen) {
if ((ret = __cpufreq_governor(policy, 
CPUFREQ_GOV_START)) ||
(ret = __cpufreq_governor(policy, 
CPUFREQ_GOV_LIMITS))) {
pr_err("%s: Failed to start governor\n",


With this, I now see:
wakeup from "mem" at Sat Jan  1 00:17:45 2000
[   40.823352] PM: Syncing filesystems ... done.
[   40.848058] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   40.857869] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) 
done.
[   40.884567] smsc95xx 1-3:1.0 eth0: entering SUSPEND2 mode
[   40.955323] PM: suspend of devices complete after 81.563 msecs
[   40.967333] PM: late suspend of devices complete after 5.789 msecs
[   40.981182] PM: noirq suspend of devices complete after 7.274 msecs
[   40.988005] Disabling non-boot CPUs ...
[   41.000297] CPU1: shutdown
[   43.169193] Powerdomain (core_pwrdm) didn't enter target state 1
[   43.175681] Powerdomain (emu_pwrdm) didn't enter target state 1
[   43.182097] Powerdomain (l3init_pwrdm) didn't enter target state 1
[   43.188762] Could not enter target state in pm_suspend
[   43.194298] A possible cause could be an old bootloader - try u-boot >= 
v2012.07
[   43.203291] Enabling non-boot CPUs ...
[   43.210398] CPU1: Booted secondary processor
[   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
^^^ ??
[   43.224252] CPU1 is up
[   43.248114] PM: noirq resume of devices complete after 21.329 msecs
[   43.260582] PM: early resume of devices complete after 4.201 msecs
[   43.623307] ata1: SATA link down (SStatus 0 SControl 300)
[   44.006234] PM: resume of devices complete after 742.501 msecs
[   44.020163] Restarting tasks ... done.

but, yes, the patch does squelch the warning I saw.
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-13 Thread Nishanth Menon 
On 11:19-20131113, Viresh Kumar wrote:
 On 12 November 2013 20:41, Nishanth Menon n...@ti.com wrote:
  On 11/12/2013 12:03 AM, Viresh Kumar wrote:
[...]
  Can you try attached patch? I will then repost it formally...
 
  I tried a equivalent of this for v3.12 tag:
[..]
  @@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
  device *dev,
 
  /* If cpu is last user of policy, free policy */
  if (cpus == 1) {
  -   if (cpufreq_driver-target) {
  +   if (cpufreq_driver-target  !frozen) {
  ret = __cpufreq_governor(policy,
  CPUFREQ_GOV_POLICY_EXIT);
 
 This is not an equivalent of my patch :)

arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
look equivalent?
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 04548f7..a9847ce 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct device 
*dev,
return -EINVAL;
}
 
-   if (cpufreq_driver-target) {
+   if (cpufreq_driver-target  (!frozen || policy-governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err(%s: Failed to stop governor\n, __func__);
@@ -1295,7 +1295,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (cpufreq_driver-target) {
+   if (cpufreq_driver-target  !frozen) {
if ((ret = __cpufreq_governor(policy, 
CPUFREQ_GOV_START)) ||
(ret = __cpufreq_governor(policy, 
CPUFREQ_GOV_LIMITS))) {
pr_err(%s: Failed to start governor\n,


With this, I now see:
wakeup from mem at Sat Jan  1 00:17:45 2000
[   40.823352] PM: Syncing filesystems ... done.
[   40.848058] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   40.857869] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) 
done.
[   40.884567] smsc95xx 1-3:1.0 eth0: entering SUSPEND2 mode
[   40.955323] PM: suspend of devices complete after 81.563 msecs
[   40.967333] PM: late suspend of devices complete after 5.789 msecs
[   40.981182] PM: noirq suspend of devices complete after 7.274 msecs
[   40.988005] Disabling non-boot CPUs ...
[   41.000297] CPU1: shutdown
[   43.169193] Powerdomain (core_pwrdm) didn't enter target state 1
[   43.175681] Powerdomain (emu_pwrdm) didn't enter target state 1
[   43.182097] Powerdomain (l3init_pwrdm) didn't enter target state 1
[   43.188762] Could not enter target state in pm_suspend
[   43.194298] A possible cause could be an old bootloader - try u-boot = 
v2012.07
[   43.203291] Enabling non-boot CPUs ...
[   43.210398] CPU1: Booted secondary processor
[   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
^^^ ??
[   43.224252] CPU1 is up
[   43.248114] PM: noirq resume of devices complete after 21.329 msecs
[   43.260582] PM: early resume of devices complete after 4.201 msecs
[   43.623307] ata1: SATA link down (SStatus 0 SControl 300)
[   44.006234] PM: resume of devices complete after 742.501 msecs
[   44.020163] Restarting tasks ... done.

but, yes, the patch does squelch the warning I saw.
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-13 Thread viresh kumar 
On Wednesday 13 November 2013 08:46 PM, Nishanth Menon wrote:
 arrgh, my bad.. Apologies for the bad one.. I missed it :( Does the following
 look equivalent?

yes.

 With this, I now see:

 [   43.212714] cpufreq: cpufreq_add_policy_cpu: Failed to stop governor
 ^^^ ??

Ahh, I missed this part. I thought it will fail at some other place where there
is no error checking :), but that's not true.

Following should fix it for you and looks to be the right way as well.


diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index dc67fa0..30b09d3 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1530,6 +1530,14 @@ static void cpufreq_bp_resume(void)
}
}

+   if (has_target()) {
+   if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+   (ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) 
{
+   pr_err(%s: Failed to start governor\n, __func__);
+   goto fail;
+   }
+   }
+
schedule_work(policy-update);

 fail:

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-12 Thread Viresh Kumar 
On 12 November 2013 20:41, Nishanth Menon  wrote:
> On 11/12/2013 12:03 AM, Viresh Kumar wrote:

>> Yes the problem looks real but there are issues with this patch.
>> - It doesn't solve your problem completely, because you returned -EBUSY,
>> your suspend operation failed and we resumed immediately.
>
> Seems like there was an error handling miss somewhere - for some
> reason, it did suspend properly.

Yeah, its missing in cpufreq_cpu_callback()..

>> But I think the problem can/should be solved some other way.. Looking 
>> closely,
>> we got to the problem because we called
>>
>> __cpufreq_governor(policy, CPUFREQ_GOV_START)
>>
>> at the first place. This happened because the policy structure had more than
>> one cpu to take care of and after stopping goveronr for CPU1 it has to start 
>> it
>> again for CPU0... But this is really not required as anyway we are going to
>> suspend.
>>
>> Can you try attached patch? I will then repost it formally...
>
> I tried a equivalent of this for v3.12 tag:
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 04548f7..9ec243c 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct
> device *dev,
> return -EINVAL;
> }
>
> -   if (cpufreq_driver->target) {
> +   if (cpufreq_driver->target && (!frozen ||
> policy->governor_enabled)) {
> ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> if (ret) {
> pr_err("%s: Failed to stop governor\n", __func__);
> @@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
> device *dev,
>
> /* If cpu is last user of policy, free policy */
> if (cpus == 1) {
> -   if (cpufreq_driver->target) {
> +   if (cpufreq_driver->target && !frozen) {
> ret = __cpufreq_governor(policy,
> CPUFREQ_GOV_POLICY_EXIT);

This is not an equivalent of my patch :)

@@ -1282,7 +1282,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (has_target()) {
+   if (has_target() && !frozen) {
if ((ret = __cpufreq_governor(policy,
CPUFREQ_GOV_START)) ||
(ret =
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {


> And I see http://pastebin.mozilla.org/3528478
>
> with a WARN patch for generating call stack.

that's why you got it.. I was really surprised to see it just didn't
worked for you
and believe me it took me a lot of time understanding how isn't it
working for u.
Because I simply believed on your equivalent version and didn't looked at it
closely :)

> Finally squelched warnings with a net diff (v3.12) of
> http://pastebin.mozilla.org/3546062

we don't need that stuff in cpufreq_add_policy_cpu()

> However, ondemand is no longer functioning on resume (governor needs a
> start after being unfrozen.. and obviously by avoiding that entirely
> in frozen case.. not sure if I missed any other)..

It would be, try the right code once. :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-12 Thread Nishanth Menon 
On 11/12/2013 12:03 AM, Viresh Kumar wrote:
> Cc'ing Shawn as well.
> 
> Sorry for being really late.. I just forgot about it :(

Thanks for responding :)

> 
> On 24 October 2013 23:38, Nishanth Menon  wrote:
>> For platforms where regulators are used, regulator access tends to be
>> disabled as part of the suspend path. In SMP systems such as OMAP,
>> CPU1 is disabled as post suspend_noirq. This results in the following
>> tail end sequence of actions:
>> cpufreq_cpu_callback gets called with CPU_POST_DEAD
>> __cpufreq_remove_dev_finish is invoked as a result
>> __cpufreq_governor(policy, CPUFREQ_GOV_START) is
>> triggered
>>
>> At this point, with ondemand governor, if the cpu entered suspend path
>> at a lower OPP, this triggers a transition request. However, since
>> irqs are disabled, typically, regulator control using I2C operations
>> are not possible either, regulator operations will naturally fail
>> (even though clk_set_rate might succeed depending on the platform).
>>
>> Unfortunately, cpufreq_driver->suspend|resume is too late as well, since
>> they are invoked as part of syscore_ops (after CPU1 is hotplugged out).
>>
>> The proposal here is to use pm notifier suspend to disable any
>> requests to target, as we may very well expect this to fail at a later
>> suspend sequence.
> 
> Yes the problem looks real but there are issues with this patch.
> - It doesn't solve your problem completely, because you returned -EBUSY,
> your suspend operation failed and we resumed immediately.

Seems like there was an error handling miss somewhere - for some
reason, it did suspend properly.

> - We can't make this solution true for everybody using cpu0 driver, it should
> be platform specific.

Agreed.

> - Its not a problem of cpu0 driver but all drivers. So, probably a better idea
> would be not calling ->target() at all when drivers have marked them unusable
> in suspend path..

Ack.

> 
> But I think the problem can/should be solved some other way.. Looking closely,
> we got to the problem because we called
> 
> __cpufreq_governor(policy, CPUFREQ_GOV_START)
> 
> at the first place. This happened because the policy structure had more than
> one cpu to take care of and after stopping goveronr for CPU1 it has to start 
> it
> again for CPU0... But this is really not required as anyway we are going to
> suspend.
> 
> Can you try attached patch? I will then repost it formally...

I tried a equivalent of this for v3.12 tag:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 04548f7..9ec243c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct
device *dev,
return -EINVAL;
}

-   if (cpufreq_driver->target) {
+   if (cpufreq_driver->target && (!frozen ||
policy->governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err("%s: Failed to stop governor\n", __func__);
@@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
device *dev,

/* If cpu is last user of policy, free policy */
if (cpus == 1) {
-   if (cpufreq_driver->target) {
+   if (cpufreq_driver->target && !frozen) {
ret = __cpufreq_governor(policy,
CPUFREQ_GOV_POLICY_EXIT);
if (ret) {

And I see http://pastebin.mozilla.org/3528478

with a WARN patch for generating call stack.


Finally squelched warnings with a net diff (v3.12) of
http://pastebin.mozilla.org/3546062

However, ondemand is no longer functioning on resume (governor needs a
start after being unfrozen.. and obviously by avoiding that entirely
in frozen case.. not sure if I missed any other)..

> 
> commit 2aecab9be85ceafdbab5f824eec5d1f81f3fa803
> Author: Viresh Kumar 
> Date:   Tue Nov 12 11:26:36 2013 +0530
> 
> cpufreq: don't start governor in suspend path
> 
> When we suspend our system, we remove all non-boot CPUs one by one. At 
> this
> point we actually STOP/START governor for each non-boot cpu, which
> is a total
> waste of time as we aren't going to use governor until the time we are 
> back.
> 
> Also, this is causing problems for some platforms (like OMAP),
> where governor
> tries to change freq of core in suspend path which requires programming
> regulators via I2C which isn't possible then.
> 
> So, to make it better for everybody don't start the governor again
> in suspend
> path.
> 
> Reported-by: Nishanth Menon 
> Signed-off-by: Viresh Kumar 
> ---
>  drivers/cpufreq/cpufreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 02d534d..bec58cf 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1174,7 +1174,7 @@

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-12 Thread Nishanth Menon 
On 11/12/2013 12:03 AM, Viresh Kumar wrote:
 Cc'ing Shawn as well.
 
 Sorry for being really late.. I just forgot about it :(

Thanks for responding :)

 
 On 24 October 2013 23:38, Nishanth Menon n...@ti.com wrote:
 For platforms where regulators are used, regulator access tends to be
 disabled as part of the suspend path. In SMP systems such as OMAP,
 CPU1 is disabled as post suspend_noirq. This results in the following
 tail end sequence of actions:
 cpufreq_cpu_callback gets called with CPU_POST_DEAD
 __cpufreq_remove_dev_finish is invoked as a result
 __cpufreq_governor(policy, CPUFREQ_GOV_START) is
 triggered

 At this point, with ondemand governor, if the cpu entered suspend path
 at a lower OPP, this triggers a transition request. However, since
 irqs are disabled, typically, regulator control using I2C operations
 are not possible either, regulator operations will naturally fail
 (even though clk_set_rate might succeed depending on the platform).

 Unfortunately, cpufreq_driver-suspend|resume is too late as well, since
 they are invoked as part of syscore_ops (after CPU1 is hotplugged out).

 The proposal here is to use pm notifier suspend to disable any
 requests to target, as we may very well expect this to fail at a later
 suspend sequence.
 
 Yes the problem looks real but there are issues with this patch.
 - It doesn't solve your problem completely, because you returned -EBUSY,
 your suspend operation failed and we resumed immediately.

Seems like there was an error handling miss somewhere - for some
reason, it did suspend properly.

 - We can't make this solution true for everybody using cpu0 driver, it should
 be platform specific.

Agreed.

 - Its not a problem of cpu0 driver but all drivers. So, probably a better idea
 would be not calling -target() at all when drivers have marked them unusable
 in suspend path..

Ack.

 
 But I think the problem can/should be solved some other way.. Looking closely,
 we got to the problem because we called
 
 __cpufreq_governor(policy, CPUFREQ_GOV_START)
 
 at the first place. This happened because the policy structure had more than
 one cpu to take care of and after stopping goveronr for CPU1 it has to start 
 it
 again for CPU0... But this is really not required as anyway we are going to
 suspend.
 
 Can you try attached patch? I will then repost it formally...

I tried a equivalent of this for v3.12 tag:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 04548f7..9ec243c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct
device *dev,
return -EINVAL;
}

-   if (cpufreq_driver-target) {
+   if (cpufreq_driver-target  (!frozen ||
policy-governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err(%s: Failed to stop governor\n, __func__);
@@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
device *dev,

/* If cpu is last user of policy, free policy */
if (cpus == 1) {
-   if (cpufreq_driver-target) {
+   if (cpufreq_driver-target  !frozen) {
ret = __cpufreq_governor(policy,
CPUFREQ_GOV_POLICY_EXIT);
if (ret) {

And I see http://pastebin.mozilla.org/3528478

with a WARN patch for generating call stack.


Finally squelched warnings with a net diff (v3.12) of
http://pastebin.mozilla.org/3546062

However, ondemand is no longer functioning on resume (governor needs a
start after being unfrozen.. and obviously by avoiding that entirely
in frozen case.. not sure if I missed any other)..

 
 commit 2aecab9be85ceafdbab5f824eec5d1f81f3fa803
 Author: Viresh Kumar viresh.ku...@linaro.org
 Date:   Tue Nov 12 11:26:36 2013 +0530
 
 cpufreq: don't start governor in suspend path
 
 When we suspend our system, we remove all non-boot CPUs one by one. At 
 this
 point we actually STOP/START governor for each non-boot cpu, which
 is a total
 waste of time as we aren't going to use governor until the time we are 
 back.
 
 Also, this is causing problems for some platforms (like OMAP),
 where governor
 tries to change freq of core in suspend path which requires programming
 regulators via I2C which isn't possible then.
 
 So, to make it better for everybody don't start the governor again
 in suspend
 path.
 
 Reported-by: Nishanth Menon n...@ti.com
 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 ---
  drivers/cpufreq/cpufreq.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
 index 02d534d..bec58cf 100644
 --- a/drivers/cpufreq/cpufreq.c
 +++ b/drivers/cpufreq/cpufreq.c
 @@ -1174,7 +1174,7 @@ static int __cpufreq_remove_dev_prepare(struct

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-12 Thread Viresh Kumar 
On 12 November 2013 20:41, Nishanth Menon n...@ti.com wrote:
 On 11/12/2013 12:03 AM, Viresh Kumar wrote:

 Yes the problem looks real but there are issues with this patch.
 - It doesn't solve your problem completely, because you returned -EBUSY,
 your suspend operation failed and we resumed immediately.

 Seems like there was an error handling miss somewhere - for some
 reason, it did suspend properly.

Yeah, its missing in cpufreq_cpu_callback()..

 But I think the problem can/should be solved some other way.. Looking 
 closely,
 we got to the problem because we called

 __cpufreq_governor(policy, CPUFREQ_GOV_START)

 at the first place. This happened because the policy structure had more than
 one cpu to take care of and after stopping goveronr for CPU1 it has to start 
 it
 again for CPU0... But this is really not required as anyway we are going to
 suspend.

 Can you try attached patch? I will then repost it formally...

 I tried a equivalent of this for v3.12 tag:
 diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
 index 04548f7..9ec243c 100644
 --- a/drivers/cpufreq/cpufreq.c
 +++ b/drivers/cpufreq/cpufreq.c
 @@ -1186,7 +1186,7 @@ static int __cpufreq_remove_dev_prepare(struct
 device *dev,
 return -EINVAL;
 }

 -   if (cpufreq_driver-target) {
 +   if (cpufreq_driver-target  (!frozen ||
 policy-governor_enabled)) {
 ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 if (ret) {
 pr_err(%s: Failed to stop governor\n, __func__);
 @@ -1252,7 +1252,7 @@ static int __cpufreq_remove_dev_finish(struct
 device *dev,

 /* If cpu is last user of policy, free policy */
 if (cpus == 1) {
 -   if (cpufreq_driver-target) {
 +   if (cpufreq_driver-target  !frozen) {
 ret = __cpufreq_governor(policy,
 CPUFREQ_GOV_POLICY_EXIT);

This is not an equivalent of my patch :)

@@ -1282,7 +1282,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (has_target()) {
+   if (has_target()  !frozen) {
if ((ret = __cpufreq_governor(policy,
CPUFREQ_GOV_START)) ||
(ret =
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {


 And I see http://pastebin.mozilla.org/3528478

 with a WARN patch for generating call stack.

that's why you got it.. I was really surprised to see it just didn't
worked for you
and believe me it took me a lot of time understanding how isn't it
working for u.
Because I simply believed on your equivalent version and didn't looked at it
closely :)

 Finally squelched warnings with a net diff (v3.12) of
 http://pastebin.mozilla.org/3546062

we don't need that stuff in cpufreq_add_policy_cpu()

 However, ondemand is no longer functioning on resume (governor needs a
 start after being unfrozen.. and obviously by avoiding that entirely
 in frozen case.. not sure if I missed any other)..

It would be, try the right code once. :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-11 Thread Viresh Kumar 
Cc'ing Shawn as well.

Sorry for being really late.. I just forgot about it :(

On 24 October 2013 23:38, Nishanth Menon  wrote:
> For platforms where regulators are used, regulator access tends to be
> disabled as part of the suspend path. In SMP systems such as OMAP,
> CPU1 is disabled as post suspend_noirq. This results in the following
> tail end sequence of actions:
> cpufreq_cpu_callback gets called with CPU_POST_DEAD
> __cpufreq_remove_dev_finish is invoked as a result
> __cpufreq_governor(policy, CPUFREQ_GOV_START) is
> triggered
>
> At this point, with ondemand governor, if the cpu entered suspend path
> at a lower OPP, this triggers a transition request. However, since
> irqs are disabled, typically, regulator control using I2C operations
> are not possible either, regulator operations will naturally fail
> (even though clk_set_rate might succeed depending on the platform).
>
> Unfortunately, cpufreq_driver->suspend|resume is too late as well, since
> they are invoked as part of syscore_ops (after CPU1 is hotplugged out).
>
> The proposal here is to use pm notifier suspend to disable any
> requests to target, as we may very well expect this to fail at a later
> suspend sequence.

Yes the problem looks real but there are issues with this patch.
- It doesn't solve your problem completely, because you returned -EBUSY,
your suspend operation failed and we resumed immediately.
- We can't make this solution true for everybody using cpu0 driver, it should
be platform specific.
- Its not a problem of cpu0 driver but all drivers. So, probably a better idea
would be not calling ->target() at all when drivers have marked them unusable
in suspend path..

But I think the problem can/should be solved some other way.. Looking closely,
we got to the problem because we called

__cpufreq_governor(policy, CPUFREQ_GOV_START)

at the first place. This happened because the policy structure had more than
one cpu to take care of and after stopping goveronr for CPU1 it has to start it
again for CPU0... But this is really not required as anyway we are going to
suspend.

Can you try attached patch? I will then repost it formally...

commit 2aecab9be85ceafdbab5f824eec5d1f81f3fa803
Author: Viresh Kumar 
Date:   Tue Nov 12 11:26:36 2013 +0530

cpufreq: don't start governor in suspend path

When we suspend our system, we remove all non-boot CPUs one by one. At this
point we actually STOP/START governor for each non-boot cpu, which
is a total
waste of time as we aren't going to use governor until the time we are back.

Also, this is causing problems for some platforms (like OMAP),
where governor
tries to change freq of core in suspend path which requires programming
regulators via I2C which isn't possible then.

So, to make it better for everybody don't start the governor again
in suspend
path.

Reported-by: Nishanth Menon 
Signed-off-by: Viresh Kumar 
---
 drivers/cpufreq/cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 02d534d..bec58cf 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1174,7 +1174,7 @@ static int __cpufreq_remove_dev_prepare(struct
device *dev,
return -EINVAL;
}

-   if (has_target()) {
+   if (has_target() && (!frozen || policy->governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err("%s: Failed to stop governor\n", __func__);
@@ -1282,7 +1282,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (has_target()) {
+   if (has_target() && !frozen) {
if ((ret = __cpufreq_governor(policy,
CPUFREQ_GOV_START)) ||
(ret =
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {
pr_err("%s: Failed to start governor\n",


--
viresh

> Reported-by: J Keerthy 
> Signed-off-by: Nishanth Menon 
> ---
> based on: v3.12-rc6
> Tested on v3.12-rc6 vendor forked tree which supports suspend
> Platform: OMAP5uEVM
>
> Sample of the stack with i2c failure: http://pastebin.com/m5KxnB7a
> With i2c failure fixed: http://pastebin.com/8AfX4e7r
>
> I am open to alternative proposals if any - one option might be to
> introduce a similar behavior at cpufreq level as allowing cpufreq
> transitions in suspend path is probably not necessary.
>
> If folks think that respinning this patch such that there is no
> dependency on regulator to set is_suspended, it is fine with me as
> well.
>
>  drivers/cpufreq/cpufreq-cpu0.c |   47 
> +---
>  1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c

Re: [RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-11-11 Thread Viresh Kumar 
Cc'ing Shawn as well.

Sorry for being really late.. I just forgot about it :(

On 24 October 2013 23:38, Nishanth Menon n...@ti.com wrote:
 For platforms where regulators are used, regulator access tends to be
 disabled as part of the suspend path. In SMP systems such as OMAP,
 CPU1 is disabled as post suspend_noirq. This results in the following
 tail end sequence of actions:
 cpufreq_cpu_callback gets called with CPU_POST_DEAD
 __cpufreq_remove_dev_finish is invoked as a result
 __cpufreq_governor(policy, CPUFREQ_GOV_START) is
 triggered

 At this point, with ondemand governor, if the cpu entered suspend path
 at a lower OPP, this triggers a transition request. However, since
 irqs are disabled, typically, regulator control using I2C operations
 are not possible either, regulator operations will naturally fail
 (even though clk_set_rate might succeed depending on the platform).

 Unfortunately, cpufreq_driver-suspend|resume is too late as well, since
 they are invoked as part of syscore_ops (after CPU1 is hotplugged out).

 The proposal here is to use pm notifier suspend to disable any
 requests to target, as we may very well expect this to fail at a later
 suspend sequence.

Yes the problem looks real but there are issues with this patch.
- It doesn't solve your problem completely, because you returned -EBUSY,
your suspend operation failed and we resumed immediately.
- We can't make this solution true for everybody using cpu0 driver, it should
be platform specific.
- Its not a problem of cpu0 driver but all drivers. So, probably a better idea
would be not calling -target() at all when drivers have marked them unusable
in suspend path..

But I think the problem can/should be solved some other way.. Looking closely,
we got to the problem because we called

__cpufreq_governor(policy, CPUFREQ_GOV_START)

at the first place. This happened because the policy structure had more than
one cpu to take care of and after stopping goveronr for CPU1 it has to start it
again for CPU0... But this is really not required as anyway we are going to
suspend.

Can you try attached patch? I will then repost it formally...

commit 2aecab9be85ceafdbab5f824eec5d1f81f3fa803
Author: Viresh Kumar viresh.ku...@linaro.org
Date:   Tue Nov 12 11:26:36 2013 +0530

cpufreq: don't start governor in suspend path

When we suspend our system, we remove all non-boot CPUs one by one. At this
point we actually STOP/START governor for each non-boot cpu, which
is a total
waste of time as we aren't going to use governor until the time we are back.

Also, this is causing problems for some platforms (like OMAP),
where governor
tries to change freq of core in suspend path which requires programming
regulators via I2C which isn't possible then.

So, to make it better for everybody don't start the governor again
in suspend
path.

Reported-by: Nishanth Menon n...@ti.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
 drivers/cpufreq/cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 02d534d..bec58cf 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1174,7 +1174,7 @@ static int __cpufreq_remove_dev_prepare(struct
device *dev,
return -EINVAL;
}

-   if (has_target()) {
+   if (has_target()  (!frozen || policy-governor_enabled)) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err(%s: Failed to stop governor\n, __func__);
@@ -1282,7 +1282,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
if (!frozen)
cpufreq_policy_free(policy);
} else {
-   if (has_target()) {
+   if (has_target()  !frozen) {
if ((ret = __cpufreq_governor(policy,
CPUFREQ_GOV_START)) ||
(ret =
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {
pr_err(%s: Failed to start governor\n,


--
viresh

 Reported-by: J Keerthy j-keer...@ti.com
 Signed-off-by: Nishanth Menon n...@ti.com
 ---
 based on: v3.12-rc6
 Tested on v3.12-rc6 vendor forked tree which supports suspend
 Platform: OMAP5uEVM

 Sample of the stack with i2c failure: http://pastebin.com/m5KxnB7a
 With i2c failure fixed: http://pastebin.com/8AfX4e7r

 I am open to alternative proposals if any - one option might be to
 introduce a similar behavior at cpufreq level as allowing cpufreq
 transitions in suspend path is probably not necessary.

 If folks think that respinning this patch such that there is no
 dependency on regulator to set is_suspended, it is fine with me as
 well.

  drivers/cpufreq/cpufreq-cpu0.c |   47 
 +---
  1 file changed, 44 insertions(+), 3 deletions(-)

 diff --git

[RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-10-24 Thread Nishanth Menon 
For platforms where regulators are used, regulator access tends to be
disabled as part of the suspend path. In SMP systems such as OMAP,
CPU1 is disabled as post suspend_noirq. This results in the following
tail end sequence of actions:
cpufreq_cpu_callback gets called with CPU_POST_DEAD
__cpufreq_remove_dev_finish is invoked as a result
__cpufreq_governor(policy, CPUFREQ_GOV_START) is
triggered

At this point, with ondemand governor, if the cpu entered suspend path
at a lower OPP, this triggers a transition request. However, since
irqs are disabled, typically, regulator control using I2C operations
are not possible either, regulator operations will naturally fail
(even though clk_set_rate might succeed depending on the platform).

Unfortunately, cpufreq_driver->suspend|resume is too late as well, since
they are invoked as part of syscore_ops (after CPU1 is hotplugged out).

The proposal here is to use pm notifier suspend to disable any
requests to target, as we may very well expect this to fail at a later
suspend sequence.

Reported-by: J Keerthy 
Signed-off-by: Nishanth Menon 
---
based on: v3.12-rc6
Tested on v3.12-rc6 vendor forked tree which supports suspend
Platform: OMAP5uEVM

Sample of the stack with i2c failure: http://pastebin.com/m5KxnB7a
With i2c failure fixed: http://pastebin.com/8AfX4e7r

I am open to alternative proposals if any - one option might be to
introduce a similar behavior at cpufreq level as allowing cpufreq
transitions in suspend path is probably not necessary.

If folks think that respinning this patch such that there is no
dependency on regulator to set is_suspended, it is fine with me as
well.

 drivers/cpufreq/cpufreq-cpu0.c |   47 +---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index c522a95..401d975 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static unsigned int transition_latency;
 static unsigned int voltage_tolerance; /* in percentage */
@@ -29,6 +30,8 @@ static struct device *cpu_dev;
 static struct clk *cpu_clk;
 static struct regulator *cpu_reg;
 static struct cpufreq_frequency_table *freq_table;
+static DEFINE_MUTEX(cpu_lock);
+static bool is_suspended;
 
 static int cpu0_verify_speed(struct cpufreq_policy *policy)
 {
@@ -50,12 +53,19 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
unsigned int index;
int ret;
 
+   mutex_lock(_lock);
+
+   if (is_suspended) {
+   ret = -EBUSY;
+   goto out;
+   }
+
ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
 relation, );
if (ret) {
pr_err("failed to match target freqency %d: %d\n",
   target_freq, ret);
-   return ret;
+   goto out;
}
 
freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
@@ -65,8 +75,10 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
freqs.new = freq_Hz / 1000;
freqs.old = clk_get_rate(cpu_clk) / 1000;
 
-   if (freqs.old == freqs.new)
-   return 0;
+   if (freqs.old == freqs.new) {
+   ret = 0;
+   goto out;
+   }
 
cpufreq_notify_transition(policy, , CPUFREQ_PRECHANGE);
 
@@ -122,9 +134,32 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
 post_notify:
cpufreq_notify_transition(policy, , CPUFREQ_POSTCHANGE);
 
+out:
+   mutex_unlock(_lock);
return ret;
 }
 
+static int cpu0_pm_notify(struct notifier_block *nb, unsigned long event,
+   void *dummy)
+{
+   mutex_lock(_lock);
+   switch (event) {
+   case PM_SUSPEND_PREPARE:
+   is_suspended = true;
+   break;
+   case PM_POST_SUSPEND:
+   is_suspended = false;
+   break;
+   }
+   mutex_unlock(_lock);
+
+   return NOTIFY_OK;
+}
+
+static struct notifier_block cpu_pm_notifier = {
+   .notifier_call = cpu0_pm_notify,
+};
+
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 {
int ret;
@@ -147,11 +182,17 @@ static int cpu0_cpufreq_init(struct cpufreq_policy 
*policy)
 
cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
+   if (!IS_ERR(cpu_reg))
+   register_pm_notifier(_pm_notifier);
+
return 0;
 }
 
 static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
 {
+   if (!IS_ERR(cpu_reg))
+   unregister_pm_notifier(_pm_notifier);
+
cpufreq_frequency_table_put_attr(policy->cpu);
 
return 0;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at

[RFC PATCH] cpufreq: cpufreq-cpu0: do not allow transitions with regulators suspended

2013-10-24 Thread Nishanth Menon 
For platforms where regulators are used, regulator access tends to be
disabled as part of the suspend path. In SMP systems such as OMAP,
CPU1 is disabled as post suspend_noirq. This results in the following
tail end sequence of actions:
cpufreq_cpu_callback gets called with CPU_POST_DEAD
__cpufreq_remove_dev_finish is invoked as a result
__cpufreq_governor(policy, CPUFREQ_GOV_START) is
triggered

At this point, with ondemand governor, if the cpu entered suspend path
at a lower OPP, this triggers a transition request. However, since
irqs are disabled, typically, regulator control using I2C operations
are not possible either, regulator operations will naturally fail
(even though clk_set_rate might succeed depending on the platform).

Unfortunately, cpufreq_driver-suspend|resume is too late as well, since
they are invoked as part of syscore_ops (after CPU1 is hotplugged out).

The proposal here is to use pm notifier suspend to disable any
requests to target, as we may very well expect this to fail at a later
suspend sequence.

Reported-by: J Keerthy j-keer...@ti.com
Signed-off-by: Nishanth Menon n...@ti.com
---
based on: v3.12-rc6
Tested on v3.12-rc6 vendor forked tree which supports suspend
Platform: OMAP5uEVM

Sample of the stack with i2c failure: http://pastebin.com/m5KxnB7a
With i2c failure fixed: http://pastebin.com/8AfX4e7r

I am open to alternative proposals if any - one option might be to
introduce a similar behavior at cpufreq level as allowing cpufreq
transitions in suspend path is probably not necessary.

If folks think that respinning this patch such that there is no
dependency on regulator to set is_suspended, it is fine with me as
well.

 drivers/cpufreq/cpufreq-cpu0.c |   47 +---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index c522a95..401d975 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -21,6 +21,7 @@
 #include linux/platform_device.h
 #include linux/regulator/consumer.h
 #include linux/slab.h
+#include linux/suspend.h
 
 static unsigned int transition_latency;
 static unsigned int voltage_tolerance; /* in percentage */
@@ -29,6 +30,8 @@ static struct device *cpu_dev;
 static struct clk *cpu_clk;
 static struct regulator *cpu_reg;
 static struct cpufreq_frequency_table *freq_table;
+static DEFINE_MUTEX(cpu_lock);
+static bool is_suspended;
 
 static int cpu0_verify_speed(struct cpufreq_policy *policy)
 {
@@ -50,12 +53,19 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
unsigned int index;
int ret;
 
+   mutex_lock(cpu_lock);
+
+   if (is_suspended) {
+   ret = -EBUSY;
+   goto out;
+   }
+
ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
 relation, index);
if (ret) {
pr_err(failed to match target freqency %d: %d\n,
   target_freq, ret);
-   return ret;
+   goto out;
}
 
freq_Hz = clk_round_rate(cpu_clk, freq_table[index].frequency * 1000);
@@ -65,8 +75,10 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
freqs.new = freq_Hz / 1000;
freqs.old = clk_get_rate(cpu_clk) / 1000;
 
-   if (freqs.old == freqs.new)
-   return 0;
+   if (freqs.old == freqs.new) {
+   ret = 0;
+   goto out;
+   }
 
cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
 
@@ -122,9 +134,32 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
 post_notify:
cpufreq_notify_transition(policy, freqs, CPUFREQ_POSTCHANGE);
 
+out:
+   mutex_unlock(cpu_lock);
return ret;
 }
 
+static int cpu0_pm_notify(struct notifier_block *nb, unsigned long event,
+   void *dummy)
+{
+   mutex_lock(cpu_lock);
+   switch (event) {
+   case PM_SUSPEND_PREPARE:
+   is_suspended = true;
+   break;
+   case PM_POST_SUSPEND:
+   is_suspended = false;
+   break;
+   }
+   mutex_unlock(cpu_lock);
+
+   return NOTIFY_OK;
+}
+
+static struct notifier_block cpu_pm_notifier = {
+   .notifier_call = cpu0_pm_notify,
+};
+
 static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
 {
int ret;
@@ -147,11 +182,17 @@ static int cpu0_cpufreq_init(struct cpufreq_policy 
*policy)
 
cpufreq_frequency_table_get_attr(freq_table, policy-cpu);
 
+   if (!IS_ERR(cpu_reg))
+   register_pm_notifier(cpu_pm_notifier);
+
return 0;
 }
 
 static int cpu0_cpufreq_exit(struct cpufreq_policy *policy)
 {
+   if (!IS_ERR(cpu_reg))
+   unregister_pm_notifier(cpu_pm_notifier);
+
cpufreq_frequency_table_put_attr(policy-cpu);
 
return 0;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line