Including Jimmy, since he wrote this patch originally. > -----Original Message----- > From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Friday, September 20, 2013 9:19 AM > To: Thierry Reding > Cc: Tom Warren; u-boot@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 <jimmzh...@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
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
