| Message ID | 1305704266-17623-5-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Nishanth Menon <nm@ti.com> writes: > OMAP2 does not use OPP tables at the moment for DVFS. Currently, > we depend on opp table initialization to give us the freq_table, > which makes sense for OMAP3+. for OMAP2, we should be using > clk_init_cpufreq_table - so if the opp based frequency table > initilization fails, fall back to clk_init_cpufreq_table to give > us the table. > > Signed-off-by: Nishanth Menon <nm@ti.com> This is a good approach, but for readability, the OPP version and the clk version should probably be separated into separate functions, along with their error handling. Minor: please capitalize acronyms: OPP, CPU, OMAP, etc... Kevin > --- > arch/arm/mach-omap2/omap2plus-cpufreq.c | 9 ++++++++- > 1 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c > index 45f1e9e..854f4b3 100644 > --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c > +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c > @@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) > pr_warning("%s: unable to get the mpu device\n", __func__); > return -EINVAL; > } > - opp_init_cpufreq_table(mpu_dev, &freq_table); > + > + /* > + * if we dont get cpufreq table using opp, use traditional omap2 lookup > + * as a fallback > + */ > + if (opp_init_cpufreq_table(mpu_dev, &freq_table)) > + clk_init_cpufreq_table(&freq_table); > > if (freq_table) { > result = cpufreq_frequency_table_cpuinfo(policy, freq_table); > @@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) > cpufreq_frequency_table_get_attr(freq_table, > policy->cpu); > } else { > + clk_exit_cpufreq_table(&freq_table); > WARN(true, "%s: fallback to clk_round(freq_table=%d)\n", > __func__, result); > kfree(freq_table); -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote: > Nishanth Menon <nm@ti.com> writes: > >> OMAP2 does not use OPP tables at the moment for DVFS. Currently, >> we depend on opp table initialization to give us the freq_table, >> which makes sense for OMAP3+. for OMAP2, we should be using >> clk_init_cpufreq_table - so if the opp based frequency table >> initilization fails, fall back to clk_init_cpufreq_table to give >> us the table. >> >> Signed-off-by: Nishanth Menon <nm@ti.com> > > This is a good approach, but for readability, the OPP version and the > clk version should probably be separated into separate functions, along > with their error handling. Was thinking more of the lines of splitting the file up. OMAP3+ all have OPPs defined. only one pending is OMAP2 Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP based stuff on ARCH_HAS_OPP and CPUFREQ > > Minor: please capitalize acronyms: OPP, CPU, OMAP, etc... will do. Regards, Nishanth Menon > > Kevin > >> --- >> arch/arm/mach-omap2/omap2plus-cpufreq.c | 9 ++++++++- >> 1 files changed, 8 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c >> index 45f1e9e..854f4b3 100644 >> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c >> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c >> @@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) >> pr_warning("%s: unable to get the mpu device\n", __func__); >> return -EINVAL; >> } >> - opp_init_cpufreq_table(mpu_dev, &freq_table); >> + >> + /* >> + * if we dont get cpufreq table using opp, use traditional omap2 lookup >> + * as a fallback >> + */ >> + if (opp_init_cpufreq_table(mpu_dev, &freq_table)) >> + clk_init_cpufreq_table(&freq_table); >> >> if (freq_table) { >> result = cpufreq_frequency_table_cpuinfo(policy, freq_table); >> @@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) >> cpufreq_frequency_table_get_attr(freq_table, >> policy->cpu); >> } else { >> + clk_exit_cpufreq_table(&freq_table); >> WARN(true, "%s: fallback to clk_round(freq_table=%d)\n", >> __func__, result); >> kfree(freq_table); > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"Menon, Nishanth" <nm@ti.com> writes: > On Thu, May 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote: >> Nishanth Menon <nm@ti.com> writes: >> >>> OMAP2 does not use OPP tables at the moment for DVFS. Currently, >>> we depend on opp table initialization to give us the freq_table, >>> which makes sense for OMAP3+. for OMAP2, we should be using >>> clk_init_cpufreq_table - so if the opp based frequency table >>> initilization fails, fall back to clk_init_cpufreq_table to give >>> us the table. >>> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >> >> This is a good approach, but for readability, the OPP version and the >> clk version should probably be separated into separate functions, along >> with their error handling. > > Was thinking more of the lines of splitting the file up. OMAP3+ all > have OPPs defined. only one pending is OMAP2 > Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP > based stuff on ARCH_HAS_OPP and CPUFREQ Let's take the latter approach, and just focus on a single OPP-based driver. When someone wants to add DVFS for OMAP2, all that's necessary is to add the OPPs. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 24, 2011 at 17:01, Kevin Hilman <khilman@ti.com> wrote: > "Menon, Nishanth" <nm@ti.com> writes: > >> On Thu, May 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote: >>> Nishanth Menon <nm@ti.com> writes: >>> >>>> OMAP2 does not use OPP tables at the moment for DVFS. Currently, >>>> we depend on opp table initialization to give us the freq_table, >>>> which makes sense for OMAP3+. for OMAP2, we should be using >>>> clk_init_cpufreq_table - so if the opp based frequency table >>>> initilization fails, fall back to clk_init_cpufreq_table to give >>>> us the table. >>>> >>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> >>> This is a good approach, but for readability, the OPP version and the >>> clk version should probably be separated into separate functions, along >>> with their error handling. >> >> Was thinking more of the lines of splitting the file up. OMAP3+ all >> have OPPs defined. only one pending is OMAP2 >> Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP >> based stuff on ARCH_HAS_OPP and CPUFREQ > > Let's take the latter approach, and just focus on a single OPP-based driver. > > When someone wants to add DVFS for OMAP2, all that's necessary is to add > the OPPs. yes, I have isolated the code to do that earlier today.. hopefully I should get time to post this out asap. Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c index 45f1e9e..854f4b3 100644 --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c @@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) pr_warning("%s: unable to get the mpu device\n", __func__); return -EINVAL; } - opp_init_cpufreq_table(mpu_dev, &freq_table); + + /* + * if we dont get cpufreq table using opp, use traditional omap2 lookup + * as a fallback + */ + if (opp_init_cpufreq_table(mpu_dev, &freq_table)) + clk_init_cpufreq_table(&freq_table); if (freq_table) { result = cpufreq_frequency_table_cpuinfo(policy, freq_table); @@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) cpufreq_frequency_table_get_attr(freq_table, policy->cpu); } else { + clk_exit_cpufreq_table(&freq_table); WARN(true, "%s: fallback to clk_round(freq_table=%d)\n", __func__, result); kfree(freq_table);
OMAP2 does not use OPP tables at the moment for DVFS. Currently, we depend on opp table initialization to give us the freq_table, which makes sense for OMAP3+. for OMAP2, we should be using clk_init_cpufreq_table - so if the opp based frequency table initilization fails, fall back to clk_init_cpufreq_table to give us the table. Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm/mach-omap2/omap2plus-cpufreq.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)