All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dirk Brandewie <dirk.brandewie@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Lists linaro-kernel <linaro-kernel@lists.linaro.org>,
	Patch Tracking <patches@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>,
	Nishanth Menon <nm@ti.com>, Carlos Hernandez <ceh@ti.com>
Subject: Re: [PATCH V2] cpufreq: Make sure CPU is running on a freq from freq-table
Date: Mon, 25 Nov 2013 09:43:59 -0800	[thread overview]
Message-ID: <52938C5F.7000307@gmail.com> (raw)
In-Reply-To: <CAKohpok0QJ2xP5uoSaAsX76KwD5yi9Z54pgNyQ7CW3c4Fkbq2A@mail.gmail.com>

On 11/25/2013 09:01 AM, Viresh Kumar wrote:
> On 25 November 2013 22:08, Dirk Brandewie <dirk.brandewie@gmail.com> wrote:
>> IMHO this issue should be fixed in the scaling driver for the platform.
>>
>> The scaling driver sets policy->cur and fills in the frequency table and has
>
> Not anymore, policy->cur is set in the core for most of the drivers now.
> Drivers just provide ->get() callbacks.
>
>> the ability to adjust the frequency of the CPU.
>
> I believe this kind of decisions should stay with the core, drivers should
> just provide the backend instead of intelligence..
>


This is a platform specific bug fix AFAICT and belongs in a platform
specific piece of code


>> Letting this leak up into the core
>> is wrong IMHO and just widens the window where the CPU will be running at
>> the wrong frequency set by the bootloader.
>
> It doesn't stay there for long enough.. we get to this point in
> cpufreq core just
> after calling ->init(), and if the CPU is working without issues until
> now, it will
> stay stable for few more milliseconds.
>

And this is where the scaling driver should detect and fix the issue in ->init()
on platforms we know about today, What happens if platforms in the future are
more sensitive to the issue?

>> Shouldn't there be a check to see if the problem exists at all?  Otherwise
>> the core is setting a policy for ALL platform even those where the issue
>> does
>> not exist.
>
> That is taken care of by __cpufreq_driver_target(). It checks if we are
> already running at requested frequency or not (after getting the next
> higher frequency)... If current freq is present in table,
> cpufreq_frequency_table_target() will return current frequency only for
> policy->cur -1. And so we will not end up configuring hardware
> unnecessarily.
>

The core should not be working around bootloader bugs IMHO.  Silently
fixing it is evil IMHO at a minimum the core should complain LOUDLY
about this happening otherwise the bootloaders will have no incentive to
get their act together.


  reply	other threads:[~2013-11-25 17:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-25  4:23 [PATCH V2] cpufreq: Make sure CPU is running on a freq from freq-table Viresh Kumar
2013-11-25 16:38 ` Dirk Brandewie
2013-11-25 17:01   ` Viresh Kumar
2013-11-25 17:43     ` Dirk Brandewie [this message]
2013-11-25 21:13       ` Rafael J. Wysocki
2013-11-26  2:01         ` Viresh Kumar
2013-11-26  6:14           ` viresh kumar
2013-11-26 20:21           ` Rafael J. Wysocki
2013-11-27  3:01             ` Viresh Kumar
2013-11-27 14:22               ` Rafael J. Wysocki
2013-11-27 15:52                 ` Viresh Kumar
2013-11-27 20:21                   ` Rafael J. Wysocki
2013-11-28  3:20                     ` Viresh Kumar
2013-11-28 13:09                       ` Rafael J. Wysocki
2013-11-28 13:41                         ` Viresh Kumar
2013-11-28 14:12                           ` Rafael J. Wysocki
2013-11-28 14:14                             ` Viresh Kumar
2013-11-28 20:31                               ` Rafael J. Wysocki

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=52938C5F.7000307@gmail.com \
    --to=dirk.brandewie@gmail.com \
    --cc=ceh@ti.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=nm@ti.com \
    --cc=patches@linaro.org \
    --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.