From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Fri, 20 Sep 2013 10:19:16 -0600
Subject: [U-Boot] [PATCH] Tegra114: Fix PLLX M, N, P init settings
In-Reply-To: <1379680859-24121-1-git-send-email-treding@nvidia.com>
References: <1379680859-24121-1-git-send-email-treding@nvidia.com>
Message-ID: <523C7584.2050902@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 09/20/2013 06:40 AM, Thierry Reding wrote:
> From: Jimmy Zhang <jimmzhang@nvidia.com>
> 
> The M, N and P width have been changed from Tegra30. The maximum value
> for N is limited to 255. So, the tegra_pll_x_table for Tegra114 should
> be set accordingly.

> diff --git a/arch/arm/cpu/arm720t/tegra-common/cpu.c b/arch/arm/cpu/arm720t/tegra-common/cpu.c

>  struct clk_pll_table tegra_pll_x_table[TEGRA_SOC_CNT][CLOCK_OSC_FREQ_COUNT] = {
>  	/* T20: 1 GHz */
> -	/*  n,  m, p, cpcon */
> +	/*
> +	 * Field Bits Width
> +	 *  n    17:8  10
> +	 *  m     4:0   5
> +	 *  p    22:20  3

It'd be nice to document the cpcon field size/position here too.

Nit: The various comments aren't consistent in their alignment (i.e.
width left/right justified, some bits values don't line up with others,
etc.)

> +	 */
> +	 /*  n,  m, p, cpcon */
>  	{{ 1000, 13, 0, 12},	/* OSC 13M */
>  	 { 625,  12, 0, 8},	/* OSC 19.2M */
>  	 { 1000, 12, 0, 12},	/* OSC 12M */

It might be useful to label all the table entries with the OSC frequency
that they apply to. But, perhaps that could be a different patch.

> -	/* T114: 1.4 GHz */
> -	{{ 862, 8, 0, 8},
> -	 { 583, 8, 0, 4},
> -	 { 696, 12, 0, 8},
> -	 { 700, 13, 0, 8},
> +	/* T114: 1.9 GHz    */

What does "1.9 GHz" mean here? Is that the/a max frequency the chip
supports? If so, it seems like a single value can't be accurate, since
max frequency is SKU-specific. As such, I wonder if it's even worth
including a number here?

I assume that the new table entries set up the same frequencies as what
was intended before; it's just that the old values were calculated
incorrectly. In other words, this patch is a bug-fix, not a change.

> +	/*
> +	 * Field Bits Width
> +	 *  n    15:8   8
> +	 *  m     7:0   8
> +	 *  p    23:20  4
> +	 */
> +	{{ 108, 1, 1, 8},	/* actual: 702.0 MHz */
> +	 { 73, 1, 1, 4},	/* actual: 700.8 MHz */
> +	 { 116, 1, 1, 8},	/* actual: 696.0 MHz */
> +	 { 108, 2, 1, 8},	/* actual: 702.0 MHz */