From: Jonghwan Choi <jhbird.choi@samsung.com>
To: "'Viresh Kumar'" <viresh.kumar@linaro.org>,
"'Linux PM list'" <linux-pm@vger.kernel.org>
Cc: "'open list'" <linux-kernel@vger.kernel.org>,
"'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
"'Len Brown'" <len.brown@intel.com>,
"'Amit Daniel Kachhap'" <amit.daniel@samsung.com>
Subject: RE: [PATCH 1/3] PM / OPP: Add support for descending order for cpufreq table
Date: Sat, 03 May 2014 09:16:13 +0900 [thread overview]
Message-ID: <003901cf6664$e4e8d2a0$aeba77e0$@samsung.com> (raw)
In-Reply-To: <CAOh2x=m8DzmLOGqL0E_seOZMuzU=O9vQuu-JEp+E5454RmfEXw@mail.gmail.com>
Hi. Viresh Kumar
Your reply is so fast like Usain Bolt.
> So, create three flags:
> OPP_TABLE_ORDER_ASCENDING 0
> OPP_TABLE_ORDER_DESCENDING 1
> OPP_TABLE_ORDER_ORIGINAL 2 (And use this for your case.)
-> Actually, I want to use OPP_TABLE_ORDER_DESCENDING.(Not
OPP_TABLE_ORDER_ORIGINAL.)
I think that it is enough to support both descending and ascending ordering
only.
The meaning of "ORIGIANL" Amit, said, when he(and I) writes a frequency in
dts file with ordering(Ascending or Descending). He(and I) want the
frequency to be register according to ordering.(Ascending or Descending).
I concerned that if we use ORIGINAL ordering, opp_find_freq_ceil/foor can be
broken.
(example, 1GH - 500MH - 800MHz - 200MHz - 600MHz)
Thanks~
Best Regars
> -----Original Message-----
> From: viresh.linux@gmail.com [mailto:viresh.linux@gmail.com] On Behalf Of
> Viresh Kumar
> Sent: Wednesday, April 30, 2014 5:25 PM
> To: Jonghwan Choi; Linux PM list
> Cc: open list; Rafael J. Wysocki; Len Brown; Amit Daniel Kachhap
> Subject: Re: [PATCH 1/3] PM / OPP: Add support for descending order for
> cpufreq table
>
> Hi,
>
> This isn't a very big patchset and this patch is very much required to
> understand other patches and so please cc all people from other list here
> as well..
>
> On Wed, Apr 30, 2014 at 11:58 AM, Jonghwan Choi <jhbird.choi@samsung.com>
> wrote:
> > In the frequency table dts file, the frequencies are arranged in
>
> Improve your logs a bit. Which dts file are you talking about here ?
> How would anybody know that you are talking about exynos here?
>
> Also, you shouldn't mention that here, just tell the kind of requirement
> platforms may have. i.e. people may want to keep the opp list in the same
> order in which it came from DT.
>
> > descending order which maps 1 to 1 with other frequency parameter to
> > be calculated and programmed in some registers.
> > But the OPP library works by generating the frequencies in ascending
> > order which breaks the above logic.
> > So added OPP_TABLE_ORDER_DESCEND flag to consider descending order.
>
> So, create three flags:
> OPP_TABLE_ORDER_ASCENDING 0
> OPP_TABLE_ORDER_DESCENDING 1
> OPP_TABLE_ORDER_ORIGINAL 2 (And use this for your case.)
>
> > Cc: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
> > ---
> > drivers/base/power/opp.c | 17 ++++++++++++++++-
> > include/linux/pm_opp.h | 7 +++++--
> > 2 files changed, 21 insertions(+), 3 deletions(-)
>
> You are changing prototype of a function and so all other files which are
> using this routine will break after this patch and we can't afford it as
> we want git bisect to work properly.
>
> So, fix all platforms here in this patch only, i.e. part of 2/3 and
> complete 3/3 should have been merged into this one.
>
> > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index
> > 2553867..ec7d553 100644
> > --- a/drivers/base/power/opp.c
> > +++ b/drivers/base/power/opp.c
> > @@ -18,6 +18,7 @@
> > #include <linux/cpufreq.h>
> > #include <linux/device.h>
> > #include <linux/list.h>
> > +#include <linux/list_sort.h>
> > #include <linux/rculist.h>
> > #include <linux/rcupdate.h>
> > #include <linux/pm_opp.h>
> > @@ -597,10 +598,21 @@ int dev_pm_opp_disable(struct device *dev,
> > unsigned long freq) EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
> >
> > #ifdef CONFIG_CPU_FREQ
> > +
> > +static int opp_descend_cmp(void *priv, struct list_head *a,
> > + struct list_head *b) {
> > + struct dev_pm_opp *ra = list_entry(a, struct dev_pm_opp, node);
> > + struct dev_pm_opp *rb = list_entry(b, struct dev_pm_opp,
> > +node);
> > +
> > + return rb->rate - ra->rate;
> > +}
> > +
> > /**
> > * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a
> device
> > * @dev: device for which we do this operation
> > * @table: Cpufreq table returned back to caller
> > + * @flags: OPP_TABLE_ORDER_DESCEND or zero
> > *
> > * Generate a cpufreq table for a provided device- this assumes that
> the
> > * opp list is already initialized and ready for usage.
> > @@ -622,7 +634,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
> > * or in contexts where mutex locking cannot be used.
> > */
> > int dev_pm_opp_init_cpufreq_table(struct device *dev,
> > - struct cpufreq_frequency_table **table)
> > + struct cpufreq_frequency_table **table, unsigned char
> > + flags)
>
> You are targeting the wrong routine. Fix of_init_opp_table() instead and
> things would work automatically then..
>
> And please don't change prototype of dev_pm_opp_add() for now and just
> define __dev_pm_opp_add() which will be called from
> dev_pm_opp_add() and of_init_opp_table() with 'int order' parameter.
next prev parent reply other threads:[~2014-05-03 0:16 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-30 6:28 [PATCH 1/3] PM / OPP: Add support for descending order for cpufreq table Jonghwan Choi
2014-04-30 8:25 ` Viresh Kumar
2014-05-03 0:16 ` Jonghwan Choi [this message]
2014-05-05 5:54 ` Viresh Kumar
2014-05-05 13:38 ` Nishanth Menon
2014-05-05 14:14 ` Viresh Kumar
2014-05-05 14:23 ` Nishanth Menon
2014-05-05 14:38 ` Viresh Kumar
2014-05-05 14:46 ` Nishanth Menon
2014-05-06 23:43 ` Jonghwan Choi
2014-05-07 1:00 ` Nishanth Menon
2014-05-07 6:04 ` Viresh Kumar
2014-05-08 1:22 ` Jonghwan Choi
2014-05-08 1:55 ` Nishanth Menon
2014-05-08 2:07 ` Jonghwan Choi
2014-05-08 5:55 ` Viresh Kumar
2014-05-09 1:09 ` Jonghwan Choi
2014-05-09 6:00 ` Viresh Kumar
2014-05-09 11:59 ` jonghwan Choi
2014-05-09 13:23 ` Nishanth Menon
2014-05-11 11:38 ` jonghwan Choi
2014-05-12 6:18 ` Viresh Kumar
2014-05-08 5:50 ` Viresh Kumar
2014-05-06 17:25 ` Sudeep Holla
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='003901cf6664$e4e8d2a0$aeba77e0$@samsung.com' \
--to=jhbird.choi@samsung.com \
--cc=amit.daniel@samsung.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.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 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).