* [RFC PATCH] OMAP3:PM: Fix OPP scale logic
@ 2009-08-03 13:41 Nishanth Menon
2009-08-04 8:00 ` Roger Quadros
0 siblings, 1 reply; 3+ messages in thread
From: Nishanth Menon @ 2009-08-03 13:41 UTC (permalink / raw)
To: Kevin; +Cc: Paul, linux-omap, Nishanth Menon
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 <nm@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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH] OMAP3:PM: Fix OPP scale logic
2009-08-03 13:41 [RFC PATCH] OMAP3:PM: Fix OPP scale logic Nishanth Menon
@ 2009-08-04 8:00 ` Roger Quadros
2009-08-04 13:11 ` Nishanth Menon
0 siblings, 1 reply; 3+ messages in thread
From: Roger Quadros @ 2009-08-04 8:00 UTC (permalink / raw)
To: ext Nishanth Menon; +Cc: Kevin, Paul, linux-omap
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)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] OMAP3:PM: Fix OPP scale logic
2009-08-04 8:00 ` Roger Quadros
@ 2009-08-04 13:11 ` Nishanth Menon
0 siblings, 0 replies; 3+ messages in thread
From: Nishanth Menon @ 2009-08-04 13:11 UTC (permalink / raw)
To: Roger Quadros; +Cc: Kevin, Paul, linux-omap
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-08-04 13:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-03 13:41 [RFC PATCH] OMAP3:PM: Fix OPP scale logic Nishanth Menon
2009-08-04 8:00 ` Roger Quadros
2009-08-04 13:11 ` Nishanth Menon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox