| Message ID | 000e01cf6b23$58fda900$0af8fb00$@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On 9 May 2014 06:39, Jonghwan Choi <jhbird.choi@samsung.com> wrote: > -> In exynos cpufreq driver, if we want to support more frequency, then Don't add "->" to your replies, it doesn't make it more readable but less. > we have to describe frequency information in dts file and have to change exynos cpufreq > driver file also(For adding divider value). 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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Thanks Best Regards. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
On Fri, May 9, 2014 at 6:23 AM, Nishanth Menon <nm@ti.com> wrote: > 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. I already considered it. (But it only passes on what cpufreq driver has to do to clock framework. For changing clock rate, if changing operation just divides a rate of parent it can be solved easily But exycpufreq driver is more complicated. Previously, to change frequency, pll value and clk divider value were changed in cpufreq driver. Later someone moved the code which changes pll value to clock framework. In there, pll values are maintained as table per frequency. And if frequency is added/removed, values of pll table should be changed. when we change the pll value through clk_set_rate, internally to find proper pll value, pll table is searched. If proper pll value is found, that value is written into the register) My suggestion is that all these change details should be removed according to adding/removing frequency. I believe that cpufreq driver just writes a specific value per frequency into the register for dvfs(Maybe other work is also needed) If we just describe the specific value per frequency in dts file, the driver will get that information through DT, and use it for DVFS.) Then when a new chip is released(if the chip has the same h/w interface - register map), we only have to do as above. Thanks Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11 May 2014 17:08, jonghwan Choi <jhbird.choi@gmail.com> wrote: > I already considered it. > (But it only passes on what cpufreq driver has to do to clock framework. > For changing clock rate, if changing operation just divides a rate of > parent it can be solved easily > But exycpufreq driver is more complicated. > > Previously, to change frequency, pll value and clk divider value were > changed in cpufreq driver. > Later someone moved the code which changes pll value to clock framework. > In there, pll values are maintained as table per frequency. And if > frequency is added/removed, values of > pll table should be changed. > when we change the pll value through clk_set_rate, internally to find > proper pll value, pll table is searched. > If proper pll value is found, that value is written into the register) > > My suggestion is that all these change details should be removed > according to adding/removing frequency. > I believe that cpufreq driver just writes a specific value per > frequency into the register for dvfs(Maybe other work is also needed) > > If we just describe the specific value per frequency in dts file, the > driver will get that information through DT, and use it for DVFS.) > Then when a new chip is released(if the chip has the same h/w > interface - register map), we only have to do as above. We also want to make your life simple, but adding this field to OPP table isn't the right approach for sure. Can't you calculate the divider values at run time based on a frequency? I think it should work. That way you can just code these calculations in clock driver and things would work smoothly.. If there are problems, tell us what they are and we will try to find some solution for you. . -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -64,6 +64,7 @@ struct dev_pm_opp { bool available; unsigned long rate; unsigned long u_volt; + unsigned int ctl[2]; struct device_opp *dev_opp; struct rcu_head head; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index cacf614..007a411 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -81,6 +81,24 @@ interrupts = <2 2>, <3 2>; }; + cpufreq@160000 { + compatible = "samsung,exynos7842-cpufreq"; + reg = <0x160000 0x1000>; + interrupts = <0 57 0>; + operating-points = < + /* KHz uV clkdiv0 clkdiv1 */ + 1500000 1100000 0x11111 0x11111 + 1400000 1075000 0x22223 0x22222 + 1300000 1050000 0x33333 0x33333 + 1200000 1025000 0x44444 0x44444 + 1100000 1000000 0x55555 0x55555 + 1000000 975000 0x66666 0x66666 + 900000 950000 0x77777 0x77777 + 800000 925000 0x88888 0x88888 + >; + }; + + pinctrl_0: pinctrl@11400000 { compatible = "samsung,exynos4210-pinctrl"; reg = <0x11400000 0x1000>;