[PATCH] omap2: fix DPLL_FREQSEL calculation

[PATCH] omap2: fix DPLL_FREQSEL calculation

[Felipe Balbi]

Hi,

On Thu, Jan 20, 2011 at 11:42:24AM +0100, John Ogness wrote:
> This patch fixes the calculation of the internal frequency. The value
> of "n" is one less than the actual divider. 0 is a valid value.
> 
> An example where the internal frequency is calculated correctly can be
> found here: clkt_dpll.c:_dpll_test_fint()
> 
> This patch is against linux-next-20110120.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

scripts/get_maintainer.pl would've helped you getting a better Cc list.
I'm adding linux-omap and lakml

> ---
>  arch/arm/mach-omap2/dpll3xxx.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/arch/arm/mach-omap2/dpll3xxx.c	2011-01-20 11:20:41.706860086 +0100
> +++ b/arch/arm/mach-omap2/dpll3xxx.c	2011-01-20 11:21:27.966860086 +0100
> @@ -93,7 +93,7 @@ static u16 _omap3_dpll_compute_freqsel(s
>  	unsigned long fint;
>  	u16 f = 0;
>  
> -	fint = clk->dpll_data->clk_ref->rate / n;
> +	fint = clk->dpll_data->clk_ref->rate / (n + 1);
>  
>  	pr_debug("clock: fint is %lu\n", fint);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
balbi

[PATCH v2] omap2: fix DPLL_FREQSEL calculation

[John Ogness]

On 2011-01-20, Felipe Balbi <balbi@ti.com> wrote:
> scripts/get_maintainer.pl would've helped you getting a better Cc
> list. I'm adding linux-omap and lakml

Thanks.

After investigating the issue further it seems that I had it
backwards. The value of "n" is the value to divide by, not the value
that goes in the register. This means that it is _dpll_test_fint()
that is doing it incorrectly. Here is a new version of the patch to
fix the right function.



This patch fixes the calculation of the internal frequency. The value
of "n" is the actual divider to use.

This patch is against linux-next-20110120.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 arch/arm/mach-omap2/clkt_dpll.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/clkt_dpll.c b/arch/arm/mach-omap2/clkt_dpll.c
index 337392c..acb7ae5 100644
--- a/arch/arm/mach-omap2/clkt_dpll.c
+++ b/arch/arm/mach-omap2/clkt_dpll.c
@@ -77,7 +77,7 @@ static int _dpll_test_fint(struct clk *clk, u8 n)
 	dd = clk->dpll_data;
 
 	/* DPLL divider must result in a valid jitter correction val */
-	fint = clk->parent->rate / (n + 1);
+	fint = clk->parent->rate / n;
 	if (fint < DPLL_FINT_BAND1_MIN) {
 
 		pr_debug("rejecting n=%d due to Fint failure, "

[PATCH v2] omap2: fix DPLL_FREQSEL calculation

[Felipe Balbi]

On Thu, Jan 20, 2011 at 03:29:50PM +0100, John Ogness wrote:
> On 2011-01-20, Felipe Balbi <balbi@ti.com> wrote:
> > scripts/get_maintainer.pl would've helped you getting a better Cc
> > list. I'm adding linux-omap and lakml
> 
> Thanks.
> 
> After investigating the issue further it seems that I had it
> backwards. The value of "n" is the value to divide by, not the value
> that goes in the register. This means that it is _dpll_test_fint()
> that is doing it incorrectly. Here is a new version of the patch to
> fix the right function.

you might want to look at Documentation/SubmittingPatches before sending
these patches. If anyone applies this patch of yours the commitlog will
have all the comments you've made above. Generally, patches are
"standalone" emails, without a discussion on them, if you want to add
comments which aren't supposed to go into commit log, then do so after
the tear line (---) and before the diff --git line; that way, git will
know it must drop that part of the comments.

-- 
balbi

[PATCH v3] OMAP2/3: clock: fix fint calculation for DPLL_FREQSEL

[John Ogness]

From: John Ogness <john.ogness@linutronix.de>

In OMAP35X TRM Rev 2010-05 Figure 7-18 "DPLL With EMI Reduction
Feature", it is shown that the internal frequency is calculated by
CLK_IN/(N+1). However, the value passed to _dpll_test_fint() is
already "N+1" since Linux is using the values to divide by. In the
technical reference manual, "N" is referring to the divider's register
value (0-127).

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
patch v3:
Patch against linux-next-20110121.
I have improved the commit message so that it is acceptable for the
git commit log.

 arch/arm/mach-omap2/clkt_dpll.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clkt_dpll.c b/arch/arm/mach-omap2/clkt_dpll.c
index 337392c..acb7ae5 100644
--- a/arch/arm/mach-omap2/clkt_dpll.c
+++ b/arch/arm/mach-omap2/clkt_dpll.c
@@ -77,7 +77,7 @@ static int _dpll_test_fint(struct clk *clk, u8 n)
 	dd = clk->dpll_data;
 
 	/* DPLL divider must result in a valid jitter correction val */
-	fint = clk->parent->rate / (n + 1);
+	fint = clk->parent->rate / n;
 	if (fint < DPLL_FINT_BAND1_MIN) {
 
 		pr_debug("rejecting n=%d due to Fint failure, "
-- 
1.5.6.5

[PATCH v3] OMAP2/3: clock: fix fint calculation for DPLL_FREQSEL

[Paul Walmsley]

On Fri, 21 Jan 2011, John Ogness wrote:

> From: John Ogness <john.ogness@linutronix.de>
> 
> In OMAP35X TRM Rev 2010-05 Figure 7-18 "DPLL With EMI Reduction
> Feature", it is shown that the internal frequency is calculated by
> CLK_IN/(N+1). However, the value passed to _dpll_test_fint() is
> already "N+1" since Linux is using the values to divide by. In the
> technical reference manual, "N" is referring to the divider's register
> value (0-127).
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Thanks for the patch, John.  Probably this should go into the -rc series 
since it could affect system stability, so will queue it there.


- Paul