[PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Paul Walmsley]

Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
caused non-CORE DPLL rates to be incorrectly set on boot in
omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
<tomi.valkeinen@nokia.com> - thanks Tomi.

Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
DPLL reprogram.

Tested on 3430SDP.


Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Rick Bronson <rick@efn.org>
Cc: Timo Kokkonen <timo.t.kokkonen@nokia.com>
Cc: Sakari Poussa <sakari.poussa@nokia.com>
---
 arch/arm/mach-omap2/clock34xx.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index df258f7..cc43f4f 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -271,7 +271,6 @@ static int _omap3_noncore_dpll_stop(struct clk *clk)
 static int omap3_noncore_dpll_enable(struct clk *clk)
 {
 	int r;
-	long rate;
 	struct dpll_data *dd;
 
 	if (clk == &dpll3_ck)
@@ -287,7 +286,7 @@ static int omap3_noncore_dpll_enable(struct clk *clk)
 		r = _omap3_noncore_dpll_lock(clk);
 
 	if (!r)
-		clk->rate = rate;
+		clk->rate = omap2_get_dpll_rate(clk);
 
 	return r;
 }
@@ -430,6 +429,9 @@ static int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
 		ret = omap3_noncore_dpll_program(clk, dd->last_rounded_m,
 						 dd->last_rounded_n, freqsel);
 
+		if (!ret)
+			clk->rate = rate;
+
 	}
 
 	omap3_dpll_recalc(clk);

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [081017 15:19]:
> 
> Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
> caused non-CORE DPLL rates to be incorrectly set on boot in
> omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
> <tomi.valkeinen@nokia.com> - thanks Tomi.
> 
> Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
> DPLL reprogram.
> 
> Tested on 3430SDP.

Great, pushing today.

Tony

> 
> 
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> Cc: Rick Bronson <rick@efn.org>
> Cc: Timo Kokkonen <timo.t.kokkonen@nokia.com>
> Cc: Sakari Poussa <sakari.poussa@nokia.com>
> ---
>  arch/arm/mach-omap2/clock34xx.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index df258f7..cc43f4f 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -271,7 +271,6 @@ static int _omap3_noncore_dpll_stop(struct clk *clk)
>  static int omap3_noncore_dpll_enable(struct clk *clk)
>  {
>  	int r;
> -	long rate;
>  	struct dpll_data *dd;
>  
>  	if (clk == &dpll3_ck)
> @@ -287,7 +286,7 @@ static int omap3_noncore_dpll_enable(struct clk *clk)
>  		r = _omap3_noncore_dpll_lock(clk);
>  
>  	if (!r)
> -		clk->rate = rate;
> +		clk->rate = omap2_get_dpll_rate(clk);
>  
>  	return r;
>  }
> @@ -430,6 +429,9 @@ static int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
>  		ret = omap3_noncore_dpll_program(clk, dd->last_rounded_m,
>  						 dd->last_rounded_n, freqsel);
>  
> +		if (!ret)
> +			clk->rate = rate;
> +
>  	}
>  
>  	omap3_dpll_recalc(clk);
> --
> 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

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Kevin Hilman]

Paul Walmsley wrote:
> Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
> caused non-CORE DPLL rates to be incorrectly set on boot in
> omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
> <tomi.valkeinen@nokia.com> - thanks Tomi.
> 
> Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
> DPLL reprogram.
> 
> Tested on 3430SDP.

FYI, This patch breaks the ability to come out of retention in dynamic 
idle, but I haven't yet discovered why.

Kevin


> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> Cc: Rick Bronson <rick@efn.org>
> Cc: Timo Kokkonen <timo.t.kokkonen@nokia.com>
> Cc: Sakari Poussa <sakari.poussa@nokia.com>
> ---
>  arch/arm/mach-omap2/clock34xx.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index df258f7..cc43f4f 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -271,7 +271,6 @@ static int _omap3_noncore_dpll_stop(struct clk *clk)
>  static int omap3_noncore_dpll_enable(struct clk *clk)
>  {
>  	int r;
> -	long rate;
>  	struct dpll_data *dd;
>  
>  	if (clk == &dpll3_ck)
> @@ -287,7 +286,7 @@ static int omap3_noncore_dpll_enable(struct clk *clk)
>  		r = _omap3_noncore_dpll_lock(clk);
>  
>  	if (!r)
> -		clk->rate = rate;
> +		clk->rate = omap2_get_dpll_rate(clk);
>  
>  	return r;
>  }
> @@ -430,6 +429,9 @@ static int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
>  		ret = omap3_noncore_dpll_program(clk, dd->last_rounded_m,
>  						 dd->last_rounded_n, freqsel);
>  
> +		if (!ret)
> +			clk->rate = rate;
> +
>  	}
>  
>  	omap3_dpll_recalc(clk);
> --
> 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

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Paul Walmsley]

On Tue, 28 Oct 2008, Kevin Hilman wrote:

> Paul Walmsley wrote:
> > Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
> > caused non-CORE DPLL rates to be incorrectly set on boot in
> > omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
> > <tomi.valkeinen@nokia.com> - thanks Tomi.
> > 
> > Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
> > DPLL reprogram.
> > 
> > Tested on 3430SDP.
> 
> FYI, This patch breaks the ability to come out of retention in dynamic idle,
> but I haven't yet discovered why.

Hi Kevin,

would you be willing to test this patch for me?  It's not for merging, but 
I would be interested to know if it fixes the problem.


- Paul

Author: Paul Walmsley <paul@pwsan.com>
Date:   Wed Oct 29 08:16:09 2008 -0600

    OMAP3 clock: reprogram DPLL when coming out of bypass
    
    The OMAP3 DPLL enable code has assumed that the contents of M, N,
    etc. are preserved across disables and enables.  This assumption
    appears to be wrong.  Test to see whether reprogramming the DPLL on
    each re-enable fixes the problem that Kevin reported.
    
    This patch is not for merging.  It is intended to test a hypothesis.  A
    mergeable version of this patch would preserve M, N, etc. across
    DPLL disable/enable to avoid the time-consuming rate rounding step
    upon return from dynamic idle.

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index cc43f4f..bfbd966 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -283,7 +283,7 @@ static int omap3_noncore_dpll_enable(struct clk *clk)
 	if (clk->rate == dd->bypass_clk->rate)
 		r = _omap3_noncore_dpll_bypass(clk);
 	else
-		r = _omap3_noncore_dpll_lock(clk);
+		r = omap3_noncore_dpll_set_rate(clk, clk->rate);
 
 	if (!r)
 		clk->rate = omap2_get_dpll_rate(clk);

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> From: Paul Walmsley <paul@pwsan.com>
> Subject: Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs
> To: Kevin Hilman <khilman@deeprootsystems.com>
> cc: linux-omap@vger.kernel.org, tomi.valkeinen@nokia.com, rick@efn.org,
>   timo.t.kokkonen@nokia.com, sakari.poussa@nokia.com
> Date: Wed, 29 Oct 2008 08:24:13 -0600 (MDT)
>
> On Tue, 28 Oct 2008, Kevin Hilman wrote:
>
>> Paul Walmsley wrote:
>> > Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
>> > caused non-CORE DPLL rates to be incorrectly set on boot in
>> > omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
>> > <tomi.valkeinen@nokia.com> - thanks Tomi.
>> > 
>> > Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
>> > DPLL reprogram.
>> > 
>> > Tested on 3430SDP.
>> 
>> FYI, This patch breaks the ability to come out of retention in dynamic idle,
>> but I haven't yet discovered why.
>
> Hi Kevin,
>
> would you be willing to test this patch for me?  It's not for merging, but 
> I would be interested to know if it fixes the problem.

Hi Paul,

This patch doesn't fix the problem. :(

Kevin

>
> Author: Paul Walmsley <paul@pwsan.com>
> Date:   Wed Oct 29 08:16:09 2008 -0600
>
>     OMAP3 clock: reprogram DPLL when coming out of bypass
>     
>     The OMAP3 DPLL enable code has assumed that the contents of M, N,
>     etc. are preserved across disables and enables.  This assumption
>     appears to be wrong.  Test to see whether reprogramming the DPLL on
>     each re-enable fixes the problem that Kevin reported.
>     
>     This patch is not for merging.  It is intended to test a hypothesis.  A
>     mergeable version of this patch would preserve M, N, etc. across
>     DPLL disable/enable to avoid the time-consuming rate rounding step
>     upon return from dynamic idle.
>
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index cc43f4f..bfbd966 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -283,7 +283,7 @@ static int omap3_noncore_dpll_enable(struct clk *clk)
>  	if (clk->rate == dd->bypass_clk->rate)
>  		r = _omap3_noncore_dpll_bypass(clk);
>  	else
> -		r = _omap3_noncore_dpll_lock(clk);
> +		r = omap3_noncore_dpll_set_rate(clk, clk->rate);
>  
>  	if (!r)
>  		clk->rate = omap2_get_dpll_rate(clk);

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Paul Walmsley]

yOn Wed, 29 Oct 2008, Kevin Hilman wrote:

> Hi Paul,
> 
> This patch doesn't fix the problem. :(
> 
> Kevin
> 

OK, could you private E-mail me the patches you are using to test?  
thanks for testing 

- Paul

Re: [PATCH] OMAP3 clock: fix non-CORE DPLL rate assignment bugs

[Kevin Hilman]

Kevin Hilman wrote:
> Paul Walmsley wrote:
>> Commit 8b1f0bd44fe490ec631230c8c040753a2bda8caa introduced a bug that
>> caused non-CORE DPLL rates to be incorrectly set on boot in
>> omap3_noncore_dpll_enable().  Debugged by Tomi Valkeinen
>> <tomi.valkeinen@nokia.com> - thanks Tomi.
>>
>> Also fix omap3_noncore_dpll_set_rate() to assign clk->rate after a
>> DPLL reprogram.
>>
>> Tested on 3430SDP.
> 
> FYI, This patch breaks the ability to come out of retention in dynamic 
> idle, but I haven't yet discovered why.
> 

It appears this is related to the UART patches being used in the PM 
branch, and not this patch.

I have a (forthcoming) set of UART updates that will allow the UART to 
disable its clocks and enable the chip to hit retention.

Kevin