linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
To: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Cc: linux-pm@lists.linux-foundation.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org
Subject: Re: [linux-pm] [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off
Date: Wed, 30 Nov 2011 06:55:40 +0530	[thread overview]
Message-ID: <4ED58614.9090204@linux.vnet.ibm.com> (raw)
In-Reply-To: <4ED47F48.2070107@linux.vnet.ibm.com>

On 11/29/2011 12:14 PM, Deepthi Dharwar wrote:

> On 11/29/2011 02:09 AM, Benjamin Herrenschmidt wrote:
> 
>> On Mon, 2011-11-28 at 16:33 +0530, Deepthi Dharwar wrote:
>>
>>> On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to
>>> cpuidle_idle_call by default here. This would result in calling of
>>> cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The
>>> default idle is never executed.
>>> This would be a major design flaw. No fallback idle routine.
>>>
>>> We propose to fix this by checking the return value of
>>> ppc_md.power_save() call from void to int.
>>> Right now return value is void, but if we change this to int, this
>>> would solve two problems. One being removing the cast to a function
>>> pointer in the prev patch and this design flaw stated above.
kernel/idle.c:  ppc_md.power_save = NULL;
>>>
>>> So by checking the return value of ppc_md.power_save(), we can invoke
>>> the default idle on failure. But my only concern is about the effects of
>>> changing the ppc_md.power_save() to return int on other powerpc
>>> architectures. Would it be a good idea to change the return type to int
>>> which would help us flag an error and fallback to default idle?
>>
>> I would have preferred an approach where the cpuidle module sets
>> ppc_md.power_save when loaded and restores it when unloaded ... but that
>> would have to go into the cpuidle core as a powerpc specific tweak and
>> might not be generally well received.
>>
>> So go for it, add the return value, but you'll have to update all the
>> idle functions (grep for power_save in arch/powerpc to find them).
>>
> 
> 
> Thanks Ben. Yes, I will update all the idle functions under powerpc.
> I will re-work these patches with the discussed changes.
> 
> Regards,
> Deepthi
> 
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
> 
> 

Hi Ben,

I was trying to add a return value for power_save for all arch/powepc
idle functions but a few of them directly call *.S  routines, as they
are asm.

What would be a good way to change the return value  for asm routines ?
Do we make a change in asm only, put the return value in r3 or write a
wrapper function which would call these asm routines and return an int ?

Regards,
Deepthi

  reply	other threads:[~2011-11-30  1:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-17 11:28 [RFC PATCH v2 0/4] cpuidle: (POWER) cpuidle driver for pSeries Deepthi Dharwar
2011-11-17 11:28 ` [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines Deepthi Dharwar
2011-11-27 22:48   ` Benjamin Herrenschmidt
2011-11-28 11:02     ` Deepthi Dharwar
2011-11-28 20:35       ` Benjamin Herrenschmidt
2011-11-29  6:42         ` Deepthi Dharwar
2011-11-29  7:01           ` Benjamin Herrenschmidt
2011-11-29  7:15             ` Deepthi Dharwar
2011-11-17 11:28 ` [RFC PATCH v2 2/4] cpuidle: (POWER) cpuidle driver for pSeries Deepthi Dharwar
2011-11-27 23:03   ` Benjamin Herrenschmidt
2011-11-28 11:02     ` Deepthi Dharwar
2011-11-17 11:28 ` [RFC PATCH v2 3/4] cpuidle: (POWER) Enable cpuidle and directly call cpuidle_idle_call() " Deepthi Dharwar
2011-11-27 23:05   ` Benjamin Herrenschmidt
2011-11-28 11:03     ` Deepthi Dharwar
2011-11-17 11:29 ` [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off Deepthi Dharwar
2011-11-27 23:07   ` Benjamin Herrenschmidt
2011-11-28 11:03     ` Deepthi Dharwar
2011-11-28 20:39       ` Benjamin Herrenschmidt
2011-11-29  6:44         ` Deepthi Dharwar
2011-11-30  1:25           ` Deepthi Dharwar [this message]
2011-11-30  4:52             ` [linux-pm] " Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ED58614.9090204@linux.vnet.ibm.com \
    --to=deepthi@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).