| Message ID | 000701cf636b$3d1d13c0$b7573b40$@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 ? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks to Amit Kachhap & Viresh Kumar > -----Original Message----- > From: Viresh Kumar [mailto:viresh.kumar@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@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. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; - } -} -
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(-) 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;