[U-Boot] [PATCH] Tegra114: Fix PLLX M, N, P init settings

[Tom Warren]

Including Jimmy, since he wrote this patch originally.

> -----Original Message-----
> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
> Sent: Friday, September 20, 2013 9:19 AM
> To: Thierry Reding
> Cc: Tom Warren; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] Tegra114: Fix PLLX M, N, P init settings
> 
> On 09/20/2013 06:40 AM, Thierry Reding wrote:
> > From: Jimmy Zhang <jimmzhang at 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.
CPCON is always bits 11:8. Unfortunately, it appears CPCON is gone from PLLX_MISC on T114, or at least it's not documented in the TRM. It was there in T30.
> 
> 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 */
--
nvpublic