[U-Boot] [PATCH v2] Tegra114: SPL: Set SCLK source to CLK_M before PLLP init.

[Stephen Warren]

On 10/15/2013 03:18 PM, Tom Warren wrote:
> From: Jimmy Zhang <jimmzhang at nvidia.com>
> 
> Based on the Tegra114 TRM, SCLK (system clock) can run up to 275MHz.
> At POR, the default SCLK source is set to PLLP_OUT0. In function
> clock_early_init(), PLLP_OUT0 will be set to 480MHz which is beyond
> the SCLK's upper limit.
> 
> The fix is to set SCLK source to CLK_M before initializing PLLP.
> Tested on A02 dalmore board.

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

> +__weak void set_avp_clock_to_clkm(void)
> +{
> +}

Why do we need a weak symbol here? You always want to pick up the
implementation in arch/arm/cpu/arm720t/tegra114/cpu.c, or it won't work...

Sorry for not noticing this earlier.

> diff --git a/arch/arm/cpu/tegra114-common/clock.c b/arch/arm/cpu/tegra114-common/clock.c

> @@ -653,6 +654,18 @@ void clock_early_init(void)
>  	/* PLLD_MISC: Set CLKENABLE, CPCON 12, LFCON 1 */
>  	writel(0x40000C10, &clkrst->crc_pll[CLOCK_ID_DISPLAY].pll_misc);
>  	udelay(2);
> +
> +	/* Set PLLP_OUT3 and 4 freqs to 102MHz and 204MHz */
> +	/* Assert RSTN before enable */
> +	reg = PLLP_OUT4_RSTN_EN | PLLP_OUT3_RSTN_EN;
> +	writel(reg, &clkrst->crc_pll[CLOCK_ID_PERIPH].pll_out[1]);
> +
> +	/* set divisor and reenable */
> +	reg = (IN_408_OUT_204_DIVISOR << PLLP_OUT4_RATIO)
> +		| PLLP_OUT4_OVR | PLLP_OUT4_CLKEN | PLLP_OUT4_RSTN_DIS
> +		| (IN_408_OUT_102_DIVISOR << PLLP_OUT3_RATIO)
> +		| PLLP_OUT3_OVR | PLLP_OUT3_CLKEN | PLLP_OUT3_RSTN_DIS;
> +	writel(reg, &clkrst->crc_pll[CLOCK_ID_PERIPH].pll_out[1]);

Rather than calling set_avp_clock_to_clkm() from spl.c, wouldn't it make
sense to call it from inside clock_early_init() where the PLL_P rate
change actually  happens? That would absolutely minimize the time the
AVP spends running at a slow speed.

Also, shouldn't there be a matching set_avp_clock_to_pllpout4() or
similar after PLL_P is fully configured, or the AVP will continue to run
slowly for the rest of the time too. Admittedly it doesn't run a lot of
code so the different isn't likely to be large, but just dumping it down
to 12MHz forever seems like a bad idea.