Re: [RFC PATCH] OMAP3:PM: Fix OPP scale logic
ext Nishanth Menon wrote:
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 25535a3..1ceaed8 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -240,13 +240,23 @@ static int program_opp_freq(int res, int target_level,
int current_level)
lock_scratchpad_sem();
if (res == VDD1_OPP) {
curr_opp = curr_vdd1_opp;
- clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
- clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+ ret = clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
+ if (unlikely(ret))
+ return ret;
if we return here we're not calling unlock_scratchpad_sem(). if you remove the
return statement the expected functionality will be achieved by the next if(ret)
statement.
looks like you are changing the return behaviour from opp level to 0/1. You
should explain this in a function header comment.
+
+ ret = clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+ if (unlikely(ret))
+ /* reset the dpll1 if failed */
+ clk_set_rate(dpll1_clk, mpu_opps[current_level].rate);
#ifndef CONFIG_CPU_FREQ
- /*Update loops_per_jiffy if processor speed is being changed*/
- loops_per_jiffy = compute_lpj(loops_per_jiffy,
- mpu_opps[current_level].rate/1000,
- mpu_opps[target_level].rate/1000);
+ else
+ /*
+* Update loops_per_jiffy if processor speed
+* is being changed
+*/
+ loops_per_jiffy = compute_lpj(loops_per_jiffy,
+ mpu_opps[current_level].rate/1000,
+ mpu_opps[target_level].rate/1000);
#endif
} else {
curr_opp = curr_vdd2_opp;
@@ -257,7 +267,7 @@ static int program_opp_freq(int res, int target_level, int
current_level)
}
if (ret) {
unlock_scratchpad_sem();
- return current_level;
+ return ret;
}
#ifdef CONFIG_PM
omap3_save_scratchpad_contents();
@@ -265,7 +275,7 @@ static int program_opp_freq(int res, int target_level, int
current_level)
unlock_scratchpad_sem();
*curr_opp = target_level;
- return target_level;
+ return ret;
}
static int program_opp(int res, struct omap_opp *opp, int target_level,
@@ -289,13 +299,35 @@ static int program_opp(int res, struct omap_opp *opp, int
target_level,
current_level);
#ifdef CONFIG_OMAP_SMARTREFLEX
else
- sr_voltagescale_vcbypass(t_opp, c_opp,
+ ret = sr_voltagescale_vcbypass(t_opp, c_opp,
opp[target_level].vsel,
opp[current_level].vsel);
+ if (ret) {
+ int ret1 = 0;
+ /*
+* If something did not work, put me back to old state.
+* Recover the other guy if at least 1 prev iteration
+* had run
+*/
+ if (i raise)
+ ret1 = sr_voltagescale_vcbypass(c_opp, t_opp,
+ opp[current_level].vsel,
+ opp[target_level].vsel);
+ else if (i)
+ ret1 = program_opp_freq(res, current_level,
+ target_level);
+ /*
+* If I could not reset my old state back.. system
+* is no longer in a controlled state.. bug me
+*/
+ if (unlikely(ret1))
+ BUG();
+ break;
+ }
#endif
}
- return ret;
+ return (res == PRCM_VDD1) ? curr_vdd1_opp : curr_vdd2_opp;
}
int resource_set_opp_level(int res, u32 target_level, int flags)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] OMAP3:PM: Fix OPP scale logic
Roger Quadros had written, on 08/04/2009 03:00 AM, the following:
ext Nishanth Menon wrote:
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 25535a3..1ceaed8 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -240,13 +240,23 @@ static int program_opp_freq(int res, int target_level,
int current_level)
lock_scratchpad_sem();
if (res == VDD1_OPP) {
curr_opp = curr_vdd1_opp;
- clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
- clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+ ret = clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
+ if (unlikely(ret))
+ return ret;
if we return here we're not calling unlock_scratchpad_sem(). if you remove the
return statement the expected functionality will be achieved by the next if(ret)
statement.
yep. my bad.. thanks.. will fix and resend.
looks like you are changing the return behaviour from opp level to 0/1. You
should explain this in a function header comment.
This function does not have a function header comment. this information
is already part of the patch header. Quote:
This changes program_freq_opp return type in the process
for program_opp to handle error in a consistent
manner.
Will send out a rev2 based on comment 1 (unlock_scratchpad_sem).
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] OMAP3:PM: Fix OPP scale logic
While switching from higher OPP to lower OPP,
current scale logic can fail by switching to lower
voltage while frequency remains at old value.
This patch adds a cleaner recovery logic and
additional freq dpll checks. This changes
program_freq_opp return type in the process
for program_opp to handle error in a consistent
manner.
Tested on:rx-51, ported to pm branch - untested linux-omap
Patch generated on linux-omap pm branch, commit:
7e7377395d6b4576341a6939bf2179f3946f2ea0
Signed-off-by: Nishanth Menon n...@ti.com
---
arch/arm/mach-omap2/resource34xx.c | 52 +---
1 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 25535a3..1ceaed8 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -240,13 +240,23 @@ static int program_opp_freq(int res, int target_level,
int current_level)
lock_scratchpad_sem();
if (res == VDD1_OPP) {
curr_opp = curr_vdd1_opp;
- clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
- clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+ ret = clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
+ if (unlikely(ret))
+ return ret;
+
+ ret = clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+ if (unlikely(ret))
+ /* reset the dpll1 if failed */
+ clk_set_rate(dpll1_clk, mpu_opps[current_level].rate);
#ifndef CONFIG_CPU_FREQ
- /*Update loops_per_jiffy if processor speed is being changed*/
- loops_per_jiffy = compute_lpj(loops_per_jiffy,
- mpu_opps[current_level].rate/1000,
- mpu_opps[target_level].rate/1000);
+ else
+ /*
+* Update loops_per_jiffy if processor speed
+* is being changed
+*/
+ loops_per_jiffy = compute_lpj(loops_per_jiffy,
+ mpu_opps[current_level].rate/1000,
+ mpu_opps[target_level].rate/1000);
#endif
} else {
curr_opp = curr_vdd2_opp;
@@ -257,7 +267,7 @@ static int program_opp_freq(int res, int target_level, int
current_level)
}
if (ret) {
unlock_scratchpad_sem();
- return current_level;
+ return ret;
}
#ifdef CONFIG_PM
omap3_save_scratchpad_contents();
@@ -265,7 +275,7 @@ static int program_opp_freq(int res, int target_level, int
current_level)
unlock_scratchpad_sem();
*curr_opp = target_level;
- return target_level;
+ return ret;
}
static int program_opp(int res, struct omap_opp *opp, int target_level,
@@ -289,13 +299,35 @@ static int program_opp(int res, struct omap_opp *opp, int
target_level,
current_level);
#ifdef CONFIG_OMAP_SMARTREFLEX
else
- sr_voltagescale_vcbypass(t_opp, c_opp,
+ ret = sr_voltagescale_vcbypass(t_opp, c_opp,
opp[target_level].vsel,
opp[current_level].vsel);
+ if (ret) {
+ int ret1 = 0;
+ /*
+* If something did not work, put me back to old state.
+* Recover the other guy if at least 1 prev iteration
+* had run
+*/
+ if (i raise)
+ ret1 = sr_voltagescale_vcbypass(c_opp, t_opp,
+ opp[current_level].vsel,
+ opp[target_level].vsel);
+ else if (i)
+ ret1 = program_opp_freq(res, current_level,
+ target_level);
+ /*
+* If I could not reset my old state back.. system
+* is no longer in a controlled state.. bug me
+*/
+ if (unlikely(ret1))
+ BUG();
+ break;
+ }
#endif
}
- return ret;
+ return (res == PRCM_VDD1) ? curr_vdd1_opp : curr_vdd2_opp;
}
int resource_set_opp_level(int res, u32 target_level, int flags)
--
1.5.4.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
