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

[Nishanth Menon <nm@ti.com>]

On 05/09/2014 06:59 AM, jonghwan Choi wrote:
> On Thu, May 8, 2014 at 11:00 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
>> Why? So, as far as I got it your dividers are nothing but 0,1,2...
>> i.e.
>> Freqs: 400  500  600  700  800
>> div:      4      3      2      1      0
>>
>> right? That's what you are doing in exynos5440. So just add this in your
>> probe after doing: dev_pm_opp_init_cpufreq_table
>>
>> for(i = 0; all-available-freqs; i++)
>>     dvfs_info->freq_table[i].driver_data = dvfs_info->freq_count - i;
>>
>> And this will work with changes in dts files.
> 
> I am sorry
> I couldn’t provide detailed information about this suggestion.
> This suggestion is not for exynos5440. This is for exynos4210,
> exynos4x12 and exynos5250.
> (But this can be applied to exynos5440 also)
> I want to make exynos cpufreq driver simple.
> There are exynos-cpufreq.c, exynos4210-cpufreq.c exynos4x12-cpufreq.
> exynos5250-cpufreq.c for exynos soc.
> And exynos4210-cpufreq.c, exynos4x12 and exynos5250-cpufreq. c has a
> clk divider table for each frequency.
> 
> example) exynos4210-cpufreq.c
> static struct apll_freq apll_freq_4210[] = {
>         /*
>          * values:
>          * freq
>          * clock divider for CORE, COREM0, COREM1, PERIPH, ATB,
> PCLK_DBG, APLL, RESERVED
>          * clock divider for COPY, HPM, RESERVED
>          * PLL M, P, S
>          */
>         APLL_FREQ(1200, 0, 3, 7, 3, 4, 1, 7, 0, 5, 0, 0, 150, 3, 1),
>         APLL_FREQ(1000, 0, 3, 7, 3, 4, 1, 7, 0, 4, 0, 0, 250, 6, 1),
>         APLL_FREQ(800,  0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 200, 6, 1),
>         APLL_FREQ(500,  0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 250, 6, 2),
>         APLL_FREQ(200,  0, 1, 3, 1, 3, 1, 0, 0, 3, 0, 0, 200, 6, 3),
> };
> 
> If we can pass this clk divider value to exynos cpufreq driver through
> DT, we can remove most of exynosxxxx-cpufreq.c files/codes. And when
> new frequency is added/removed or new soc is released, for supporting
> dvfs we have only to describe frequency, voltage and divider value in
> dts file.

Have you considered the option of having a clock driver which can
decide the divider (based on dts OR index or whatever)?

example: you could do clk_set_rate(apll, rate);
and instead of implementing clock divider programmation inside cpufreq
driver, you let corresponding clock driver do it for you. that allows
you to reuse clock driver with various parameters needed for your SoC
variations. IMHO, we are trying to solve a problem meant to be solved
in clock framework instead of within cpufreq.

-- 
Regards,
Nishanth Menon