linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Lists linaro-kernel <linaro-kernel@lists.linaro.org>,
	"cpufreq@vger.kernel.org" <cpufreq@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Pierre Ossman <pierre-list@ossman.eu>
Subject: Re: [PATCH 1/2] cpufreq: Return error if ->get() failed in cpufreq_update_policy()
Date: Tue, 25 Feb 2014 11:23:27 +0530	[thread overview]
Message-ID: <530C2FD7.4000105@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAKohpokk5-6vGV93fZDob8Q=xZXsd7E4OkMUAC1M6Z71G=d1Ow@mail.gmail.com>

On 02/25/2014 10:11 AM, Viresh Kumar wrote:
> On 18 February 2014 07:49, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 18 February 2014 03:30, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> On Monday, February 17, 2014 02:25:34 PM Srivatsa S. Bhat wrote:
>>>> Why go to no_policy when we can actually set things right?
>>>>
>>>> Anyway, I am not arguing against this strongly. I just wanted to share my
>>>> thoughts, since this is the approach that made more sense to me.
>>>
>>> And I agree with that.  In particular, since we're going to set the new
>>> policy *anyway* at this point, we can adjust the current frequency just fine
>>> in the process, can't we?
>>
>> Though I still feel that it wouldn't be the right thing to do as get()
>> just can't
>> return zero. Actually I was planning to place a WARN() over its return value
>> of zero.

A WARN() would definitely be good.

>>
>> Anyway, as two of the three are in favor of this we can get that in.. But how
>> would that work?
>>
>> - What frequency should we call cpufreq_driver_target ?
>> - Remember that it wouldn't do anything if policy->cur is same as new freq.
>> - Also remember that drivers need special attention if new freq is > old
>> freq or vice versa. As they will change voltage before or after change here.
>> And because we actually don't know what frequency we are at currently, how
>> will we decide that?
> 

Hmm, that's a good point. However, lets first think about the simple scenario
that the driver _is_ able to detect the current frequency from the hardware
(a non-zero, sane value) say X KHz, and that frequency is different from what
the cpufreq subsystem thinks it is (Y KHz).

In the current code, when we observe this, we send out a notification and try
to adjust to X KHz. Instead, what I'm suggesting is to invoke the driver to
set the frequency to Y KHz, since that's what the cpufreq subsystems wants the
frequency to be at.

As for the case where the driver reports the frequency to be 0 KHz, we can
print a WARN() and try to force set the frequency to Y KHz. But if that turns
out to be too tricky to handle, we can just put a WARN() and error out, as you
proposed earlier.
 
Regards,
Srivatsa S. Bhat


  reply	other threads:[~2014-02-25  5:53 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-14 11:00 [PATCH 1/2] cpufreq: Return error if ->get() failed in cpufreq_update_policy() Viresh Kumar
2014-02-14 11:00 ` [PATCH 2/2] cpufreq: don't call cpufreq_update_policy() on CPU addition Viresh Kumar
2014-02-17  0:21   ` Rafael J. Wysocki
2014-02-17  5:15     ` Viresh Kumar
2014-02-17 22:22       ` Rafael J. Wysocki
2014-02-17  8:43   ` Srivatsa S. Bhat
2014-02-17  8:54     ` Viresh Kumar
2014-02-17  8:59       ` Srivatsa S. Bhat
2014-02-17  0:28 ` [PATCH 1/2] cpufreq: Return error if ->get() failed in cpufreq_update_policy() Rafael J. Wysocki
2014-02-17  5:14   ` Viresh Kumar
2014-02-17  8:19     ` Srivatsa S. Bhat
2014-02-17  8:39       ` Viresh Kumar
2014-02-17  8:55         ` Srivatsa S. Bhat
2014-02-17  9:10           ` Viresh Kumar
2014-02-17 22:00           ` Rafael J. Wysocki
2014-02-18  2:19             ` Viresh Kumar
2014-02-25  4:41               ` Viresh Kumar
2014-02-25  5:53                 ` Srivatsa S. Bhat [this message]
2014-02-25  6:08                   ` Viresh Kumar
2014-02-25 13:10                     ` Rafael J. Wysocki
2014-02-25 14:42                       ` Viresh Kumar
2014-02-25 22:29                         ` Rafael J. Wysocki
2014-02-26  5:15                           ` Viresh Kumar
2014-03-10  5:37                             ` Viresh Kumar

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=530C2FD7.4000105@linux.vnet.ibm.com \
    --to=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=cpufreq@vger.kernel.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pierre-list@ossman.eu \
    --cc=rjw@rjwysocki.net \
    --cc=viresh.kumar@linaro.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).