[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[Jonghwan Choi]

After applying "PM / OPP: Use list_for_each_entry_reverse instead of
list_for_each_entry",
cpufreq table is sorted int descending order. So
exynos_sort_descend_freq_table
doesn't need anymore.

Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
---
 drivers/cpufreq/exynos5440-cpufreq.c |   20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
b/drivers/cpufreq/exynos5440-cpufreq.c
index a6b8214..92db80b 100644
--- a/drivers/cpufreq/exynos5440-cpufreq.c
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -279,25 +279,6 @@ static irqreturn_t exynos_cpufreq_irq(int irq, void
*id)
 	return IRQ_HANDLED;
 }
 
-static void exynos_sort_descend_freq_table(void)
-{
-	struct cpufreq_frequency_table *freq_tbl = dvfs_info->freq_table;
-	int i = 0, index;
-	unsigned int tmp_freq;
-	/*
-	 * Exynos5440 clock controller state logic expects the cpufreq table
to
-	 * be in descending order. But the OPP library constructs the table
in
-	 * ascending order. So to make the table descending we just need to
-	 * swap the i element with the N - i element.
-	 */
-	for (i = 0; i < dvfs_info->freq_count / 2; i++) {
-		index = dvfs_info->freq_count - i - 1;
-		tmp_freq = freq_tbl[i].frequency;
-		freq_tbl[i].frequency = freq_tbl[index].frequency;
-		freq_tbl[index].frequency = tmp_freq;
-	}
-}
-
 static int exynos_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
 	policy->clk = dvfs_info->cpu_clk;
@@ -374,7 +355,6 @@ static int exynos_cpufreq_probe(struct platform_device
*pdev)
 		goto err_put_node;
 	}
 	dvfs_info->freq_count = dev_pm_opp_get_opp_count(dvfs_info->dev);
-	exynos_sort_descend_freq_table();
 
 	if (of_property_read_u32(np, "clock-latency", &dvfs_info->latency))
 		dvfs_info->latency = DEF_TRANS_LATENCY;
-- 
1.7.10.4

[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[Viresh Kumar]

On 29 April 2014 10:54, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
> diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
> -static void exynos_sort_descend_freq_table(void)
> -{
> -       struct cpufreq_frequency_table *freq_tbl = dvfs_info->freq_table;
> -       int i = 0, index;
> -       unsigned int tmp_freq;
> -       /*
> -        * Exynos5440 clock controller state logic expects the cpufreq table
> to
> -        * be in descending order. But the OPP library constructs the table
> in
> -        * ascending order. So to make the table descending we just need to
> -        * swap the i element with the N - i element.
> -        */

What I am more focused is on: Why do we need to worry about order at
all in the first place.

Okay, above comment says something about it but I couldn't understand
what's the logic behind that. Why do we need same order as of clock
controller. Please point out relevant code pieces as well..

@Amit: Your comments on this ?

[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[Amit Kachhap]

On 4/29/14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29 April 2014 10:54, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
>> diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
>> -static void exynos_sort_descend_freq_table(void)
>> -{
>> -       struct cpufreq_frequency_table *freq_tbl = dvfs_info->freq_table;
>> -       int i = 0, index;
>> -       unsigned int tmp_freq;
>> -       /*
>> -        * Exynos5440 clock controller state logic expects the cpufreq
>> table
>> to
>> -        * be in descending order. But the OPP library constructs the
>> table
>> in
>> -        * ascending order. So to make the table descending we just need
>> to
>> -        * swap the i element with the N - i element.
>> -        */
>
> What I am more focused is on: Why do we need to worry about order at
> all in the first place.
>
> Okay, above comment says something about it but I couldn't understand
> what's the logic behind that. Why do we need same order as of clock
> controller. Please point out relevant code pieces as well..
>
> @Amit: Your comments on this ?

Hi Viresh/Jonghwan,

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. Ideally i should expect frequency
values in same order as what is supplied.
So OPP library should not change the order or should take input flags
flags like,
dev_pm_opp_init_cpufreq_table(TABLE_ORDER_ASCEND|
TABLE_ORDER_DESCEND|TABLE_ORDER_ORIGINAL )

Thanks,
Amit Daniel
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[Viresh Kumar]

On 29 April 2014 11:44, Amit Kachhap <amit.kachhap@gmail.com> wrote:
> 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. Ideally i should expect frequency
> values in same order as what is supplied.
> So OPP library should not change the order or should take input flags
> flags like,
> dev_pm_opp_init_cpufreq_table(TABLE_ORDER_ASCEND|
> TABLE_ORDER_DESCEND|TABLE_ORDER_ORIGINAL )

Looks a good idea :)

This is what I wrote in another thread:

What I would recommend is, use .driver_data field to hold what has to
be written to hardware for any frequency. And then simply use
driver_data instead of index.

[PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in exynos5440-cpufreq.c

[Jonghwan Choi]

Thanks to Amit Kachhap & Viresh Kumar


> -----Original Message-----
> From: Viresh Kumar [mailto:viresh.kumar at linaro.org]
> Sent: Tuesday, April 29, 2014 3:21 PM
> To: Amit Kachhap
> Cc: Jonghwan Choi; Kukjin Kim; linux-samsung-soc; Rafael J. Wysocki;
> linux-arm-kernel at lists.infradead.org; open list:CPU FREQUENCY DRI...
> Subject: Re: [PATCH 2/2] cpufreq: Remove exynos_sort_descend_freq_table in
> exynos5440-cpufreq.c
> 
> On 29 April 2014 11:44, Amit Kachhap <amit.kachhap@gmail.com> wrote:
> > 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. Ideally i should expect frequency
> > values in same order as what is supplied.
> > So OPP library should not change the order or should take input flags
> > flags like, dev_pm_opp_init_cpufreq_table(TABLE_ORDER_ASCEND|
> > TABLE_ORDER_DESCEND|TABLE_ORDER_ORIGINAL )
> 
> Looks a good idea :)
> 
> This is what I wrote in another thread:
> 
> What I would recommend is, use .driver_data field to hold what has to be
> written to hardware for any frequency. And then simply use driver_data
> instead of index.