All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.