2014-04-30 06:28:38

by Jonghwan Choi

[permalink] [raw]
Subject: [PATCH 1/3] PM / OPP: Add support for descending order for cpufreq table

In the frequency table dts file, the frequencies are arranged in
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.

Cc: Amit Daniel Kachhap <[email protected]>
Signed-off-by: Jonghwan Choi <[email protected]>
---
drivers/base/power/opp.c | 17 ++++++++++++++++-
include/linux/pm_opp.h | 7 +++++--
2 files changed, 21 insertions(+), 3 deletions(-)

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)
{
struct device_opp *dev_opp;
struct dev_pm_opp *opp;
@@ -649,6 +661,9 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
return -ENOMEM;
}

+ if (flags & OPP_TABLE_ORDER_DESCEND)
+ list_sort(NULL, &dev_opp->opp_list, opp_descend_cmp);
+
list_for_each_entry(opp, &dev_opp->opp_list, node) {
if (opp->available) {
freq_table[i].driver_data = i;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5151b00..cf42770 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -118,13 +118,16 @@ static inline int of_init_opp_table(struct device
*dev)
#endif

#if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
+
+#define OPP_TABLE_ORDER_DESCEND (1 << 1)
+
int dev_pm_opp_init_cpufreq_table(struct device *dev,
- struct cpufreq_frequency_table **table);
+ struct cpufreq_frequency_table **table, unsigned char
flags);
void dev_pm_opp_free_cpufreq_table(struct device *dev,
struct cpufreq_frequency_table **table);
#else
static inline int dev_pm_opp_init_cpufreq_table(struct device *dev,
- struct cpufreq_frequency_table **table)
+ struct cpufreq_frequency_table **table, unsigned char flags)
{
return -EINVAL;
}
--
1.7.10.4


2014-04-30 08:25:25

by Viresh Kumar

[permalink] [raw]
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 <[email protected]> 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 <[email protected]>
> Signed-off-by: Jonghwan Choi <[email protected]>
> ---
> 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.