[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[Tom Warren]

From: Jimmy Zhang <jimmzhang@nvidia.com>

Based on Tegra114 TRM, system clock can run up to 275MHz. On power on,
the default sytem clock source is set to pllp_out0. In function
clock_early_init(), pllp_out0 will be set to 480MHz which is beyond system
clock's upper limit.

The fix is to set the system clock to CLK_M before initializing PLLP.
Tested on A02 dalmore board.

Signed-off-by: Jimmy Zhang <jimmzhang@nvidia.com>
---
 arch/arm/cpu/arm720t/tegra-common/spl.c   |  7 ++++++
 arch/arm/cpu/arm720t/tegra114/cpu.c       | 35 +++++++++++++++++---------
 arch/arm/cpu/tegra114-common/clock.c      | 13 ++++++++++
 arch/arm/include/asm/arch-tegra/clk_rst.h | 41 ++++++++++++++++++-------------
 4 files changed, 68 insertions(+), 28 deletions(-)

diff --git a/arch/arm/cpu/arm720t/tegra-common/spl.c b/arch/arm/cpu/arm720t/tegra-common/spl.c
index 5171a8f..8bb50b7 100644
--- a/arch/arm/cpu/arm720t/tegra-common/spl.c
+++ b/arch/arm/cpu/arm720t/tegra-common/spl.c
@@ -17,6 +17,10 @@
 #include <asm/arch/spl.h>
 #include "cpu.h"
 
+__weak void set_avp_clock_to_clkm(void)
+{
+}
+
 void spl_board_init(void)
 {
 	struct pmux_tri_ctlr *pmt = (struct pmux_tri_ctlr *)NV_PA_APB_MISC_BASE;
@@ -24,6 +28,9 @@ void spl_board_init(void)
 	/* enable JTAG */
 	writel(0xC0, &pmt->pmt_cfg_ctl);
 
+	/* use clk_m as avp clock source */
+	set_avp_clock_to_clkm();
+
 	board_init_uart_f();
 
 	/* Initialize periph GPIOs */
diff --git a/arch/arm/cpu/arm720t/tegra114/cpu.c b/arch/arm/cpu/arm720t/tegra114/cpu.c
index 51ecff7..844299b 100644
--- a/arch/arm/cpu/arm720t/tegra114/cpu.c
+++ b/arch/arm/cpu/arm720t/tegra114/cpu.c
@@ -127,8 +127,8 @@ void t114_init_clocks(void)
 	clrbits_le32(&flow->cluster_control, 1);
 
 	/*
-	 * Switch system clock to PLLP_OUT4 (108 MHz), AVP will now run
-	 * at 108 MHz. This is glitch free as only the source is changed, no
+	 * Switch system clock to PLLP_OUT4 (204 MHz), AVP will now run
+	 * at 204 MHz. This is glitch free as only the source is changed, no
 	 * special precaution needed.
 	 */
 	val = (SCLK_SOURCE_PLLP_OUT4 << SCLK_SWAKEUP_FIQ_SOURCE_SHIFT) |
@@ -152,18 +152,11 @@ void t114_init_clocks(void)
 	clock_set_enable(PERIPH_ID_CACHE2, 1);
 	clock_set_enable(PERIPH_ID_GPIO, 1);
 	clock_set_enable(PERIPH_ID_TMR, 1);
-	clock_set_enable(PERIPH_ID_RTC, 1);
 	clock_set_enable(PERIPH_ID_CPU, 1);
 	clock_set_enable(PERIPH_ID_EMC, 1);
 	clock_set_enable(PERIPH_ID_I2C5, 1);
-	clock_set_enable(PERIPH_ID_FUSE, 1);
-	clock_set_enable(PERIPH_ID_PMC, 1);
 	clock_set_enable(PERIPH_ID_APBDMA, 1);
 	clock_set_enable(PERIPH_ID_MEM, 1);
-	clock_set_enable(PERIPH_ID_IRAMA, 1);
-	clock_set_enable(PERIPH_ID_IRAMB, 1);
-	clock_set_enable(PERIPH_ID_IRAMC, 1);
-	clock_set_enable(PERIPH_ID_IRAMD, 1);
 	clock_set_enable(PERIPH_ID_CORESIGHT, 1);
 	clock_set_enable(PERIPH_ID_MSELECT, 1);
 	clock_set_enable(PERIPH_ID_EMC1, 1);
@@ -192,7 +185,6 @@ void t114_init_clocks(void)
 	reset_set_enable(PERIPH_ID_COP, 0);
 	reset_set_enable(PERIPH_ID_EMC, 0);
 	reset_set_enable(PERIPH_ID_I2C5, 0);
-	reset_set_enable(PERIPH_ID_FUSE, 0);
 	reset_set_enable(PERIPH_ID_APBDMA, 0);
 	reset_set_enable(PERIPH_ID_MEM, 0);
 	reset_set_enable(PERIPH_ID_CORESIGHT, 0);
@@ -220,7 +212,7 @@ static int is_clamp_enabled(u32 mask)
 	u32 reg;
 
 	/* Get clamp status. TODO: Add pmc_clamp_status alias to pmc.h */
-	reg = readl(&pmc->pmc_pwrgate_timer_on);
+	reg = readl(&pmc->pmc_clamp_status);
 	return (reg & mask) == mask;
 }
 
@@ -322,3 +314,24 @@ void start_cpu(u32 reset_vector)
 	/* If the CPU(s) don't already have power, power 'em up */
 	powerup_cpus();
 }
+
+/*
+ * On poweron, AVP clock source (also called system clock) is set to PLLP_out0
+ * with frequency set at 1MHz. Before initializing PLLP, we need to move the
+ * system clock's source to CLK_M temporarily. And then switch it to PLLP_out4
+ * (204MHz) at a later time.
+ */
+void set_avp_clock_to_clkm(void)
+{
+	struct clk_rst_ctlr *clkrst =
+			(struct clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
+	u32 val;
+
+	val = (SCLK_SOURCE_CLKM << SCLK_SWAKEUP_FIQ_SOURCE_SHIFT) |
+		(SCLK_SOURCE_CLKM << SCLK_SWAKEUP_IRQ_SOURCE_SHIFT) |
+		(SCLK_SOURCE_CLKM << SCLK_SWAKEUP_RUN_SOURCE_SHIFT) |
+		(SCLK_SOURCE_CLKM << SCLK_SWAKEUP_IDLE_SOURCE_SHIFT) |
+		(SCLK_SYS_STATE_RUN << SCLK_SYS_STATE_SHIFT);
+	writel(val, &clkrst->crc_sclk_brst_pol);
+	udelay(3);
+}
diff --git a/arch/arm/cpu/tegra114-common/clock.c b/arch/arm/cpu/tegra114-common/clock.c
index 9bb6529..0626bcd 100644
--- a/arch/arm/cpu/tegra114-common/clock.c
+++ b/arch/arm/cpu/tegra114-common/clock.c
@@ -697,6 +697,7 @@ void clock_early_init(void)
 {
 	struct clk_rst_ctlr *clkrst =
 		(struct clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
+	u32	reg;
 
 	/*
 	 * PLLP output frequency set to 408Mhz
@@ -741,6 +742,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]);
 }
 
 void arch_timer_init(void)
diff --git a/arch/arm/include/asm/arch-tegra/clk_rst.h b/arch/arm/include/asm/arch-tegra/clk_rst.h
index 5f52d60..7045266 100644
--- a/arch/arm/include/asm/arch-tegra/clk_rst.h
+++ b/arch/arm/include/asm/arch-tegra/clk_rst.h
@@ -209,6 +209,13 @@ enum {
 	IN_408_OUT_9_6_DIVISOR = 83,
 };
 
+#define PLLP_OUT3_RSTN_DIS	(1 << 0)
+#define PLLP_OUT3_RSTN_EN	(0 << 0)
+#define PLLP_OUT3_CLKEN		(1 << 1)
+#define PLLP_OUT4_RSTN_DIS	(1 << 16)
+#define PLLP_OUT4_RSTN_EN	(0 << 16)
+#define PLLP_OUT4_CLKEN		(1 << 17)
+
 /* CLK_RST_CONTROLLER_UTMIP_PLL_CFG1_0 */
 #define PLLU_POWERDOWN		(1 << 16)
 #define PLL_ENABLE_POWERDOWN	(1 << 14)
-- 
1.8.1.5

[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[Stephen Warren]

On 10/07/2013 03:17 PM, Tom Warren wrote:
> From: Jimmy Zhang <jimmzhang@nvidia.com>
> 
> Based on Tegra114 TRM, system clock can run up to 275MHz. On power on,

Which exact clock is "system clock"?

> the default sytem clock source is set to pllp_out0. In function
> clock_early_init(), pllp_out0 will be set to 480MHz which is beyond system
> clock's upper limit.
> 
> The fix is to set the system clock 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

> @@ -24,6 +28,9 @@ void spl_board_init(void)
>  	/* enable JTAG */
>  	writel(0xC0, &pmt->pmt_cfg_ctl);
>  
> +	/* use clk_m as avp clock source */
> +	set_avp_clock_to_clkm();

Doesn't clk_m run at the crystal frequency, so this is going to make the
AVP run pretty slow, at least for a while? Is that a good thing?

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

> @@ -127,8 +127,8 @@ void t114_init_clocks(void)
...
>  	/*
> -	 * Switch system clock to PLLP_OUT4 (108 MHz), AVP will now run
> -	 * at 108 MHz. This is glitch free as only the source is changed, no
> +	 * Switch system clock to PLLP_OUT4 (204 MHz), AVP will now run
> +	 * at 204 MHz. This is glitch free as only the source is changed, no
>  	 * special precaution needed.
>  	 */

... OK, so hopefully the AVP only runs very slowly for a very short
time. How much code runs between spl_board_init() and
t114_init_clocks()? Should spl_board_init() do all of the AVP clock
source setup to avoid too long a time running at crystal frequency?

> @@ -152,18 +152,11 @@ void t114_init_clocks(void)
>  	clock_set_enable(PERIPH_ID_CACHE2, 1);
>  	clock_set_enable(PERIPH_ID_GPIO, 1);
>  	clock_set_enable(PERIPH_ID_TMR, 1);
> -	clock_set_enable(PERIPH_ID_RTC, 1);

This seems entirely unrelated to the intended purpose of this patch. Why
remove all these clock_set_enable() calls? At the very least, this
should be explained in the commit description. Since it seems like a
different logical change, I would expect it to be a separate patch.

> @@ -220,7 +212,7 @@ static int is_clamp_enabled(u32 mask)
>  	u32 reg;
>  
>  	/* Get clamp status. TODO: Add pmc_clamp_status alias to pmc.h */
> -	reg = readl(&pmc->pmc_pwrgate_timer_on);
> +	reg = readl(&pmc->pmc_clamp_status);

Again, unrelated?

> +/*
> + * On poweron, AVP clock source (also called system clock) is set to PLLP_out0
> + * with frequency set at 1MHz. Before initializing PLLP, we need to move the
> + * system clock's source to CLK_M temporarily. And then switch it to PLLP_out4
> + * (204MHz) at a later time.
> + */
> +void set_avp_clock_to_clkm(void)

That comment would be better located in spl_board_init() in order to
explain why it calls this function. Any comment for this function would
be better explaining the function itself more than why other code might
call it.

[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[Tom Warren]

Jimmy - please answer Stephen's questions below, and/or correct my answers.


On Tue, Oct 8, 2013 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 10/07/2013 03:17 PM, Tom Warren wrote:
> > From: Jimmy Zhang <jimmzhang@nvidia.com>
> >
> > Based on Tegra114 TRM, system clock can run up to 275MHz. On power on,
>
> Which exact clock is "system clock"?
>

In the T1x4 TRM, SCLK is the system clock that drives the AVP/COP. It
appears to have 8 possible sources (PLLP, CLK_M, etc.). See the settings in
CLK_RST_CONTROLLER_SCLK_BURST_POLICY_0 (offset 0x28 in the CAR block).


>
> > the default sytem clock source is set to pllp_out0. In function
> > clock_early_init(), pllp_out0 will be set to 480MHz which is beyond
> system
> > clock's upper limit.
> >
> > The fix is to set the system clock 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
>
> > @@ -24,6 +28,9 @@ void spl_board_init(void)
> >       /* enable JTAG */
> >       writel(0xC0, &pmt->pmt_cfg_ctl);
> >
> > +     /* use clk_m as avp clock source */
> > +     set_avp_clock_to_clkm();
>
> Doesn't clk_m run at the crystal frequency, so this is going to make the
> AVP run pretty slow, at least for a while? Is that a good thing?
>

Another clock source that's closer to the 'limit' or 275MHz could probably
have been chosen, true. Jimmy?


>
> > diff --git a/arch/arm/cpu/arm720t/tegra114/cpu.c
> b/arch/arm/cpu/arm720t/tegra114/cpu.c
>
> > @@ -127,8 +127,8 @@ void t114_init_clocks(void)
> ...
> >       /*
> > -      * Switch system clock to PLLP_OUT4 (108 MHz), AVP will now run
> > -      * at 108 MHz. This is glitch free as only the source is changed,
> no
> > +      * Switch system clock to PLLP_OUT4 (204 MHz), AVP will now run
> > +      * at 204 MHz. This is glitch free as only the source is changed,
> no
> >        * special precaution needed.
> >        */
>
> ... OK, so hopefully the AVP only runs very slowly for a very short
> time. How much code runs between spl_board_init() and
> t114_init_clocks()? Should spl_board_init() do all of the AVP clock
> source setup to avoid too long a time running at crystal frequency?
>

Not sure if PLLP is set up with all of it's frequencies (OUT0-4) that early
in init. Jimmy?


>
> > @@ -152,18 +152,11 @@ void t114_init_clocks(void)
> >       clock_set_enable(PERIPH_ID_CACHE2, 1);
> >       clock_set_enable(PERIPH_ID_GPIO, 1);
> >       clock_set_enable(PERIPH_ID_TMR, 1);
> > -     clock_set_enable(PERIPH_ID_RTC, 1);
>
> This seems entirely unrelated to the intended purpose of this patch. Why
> remove all these clock_set_enable() calls? At the very least, this
> should be explained in the commit description. Since it seems like a
> different logical change, I would expect it to be a separate patch.
>

True, looks like I combined another change from the internal repo when
creating these patches. I'll move it to the correct patch in V2.


>
> > @@ -220,7 +212,7 @@ static int is_clamp_enabled(u32 mask)
> >       u32 reg;
> >
> >       /* Get clamp status. TODO: Add pmc_clamp_status alias to pmc.h */
> > -     reg = readl(&pmc->pmc_pwrgate_timer_on);
> > +     reg = readl(&pmc->pmc_clamp_status);
>
> Again, unrelated?
>

This register is called pwrgate_timer_on in one TRM and clamp_status in
another. I'll do a cleaner version of this update in V2.


>
> > +/*
> > + * On poweron, AVP clock source (also called system clock) is set to
> PLLP_out0
> > + * with frequency set at 1MHz. Before initializing PLLP, we need to
> move the
> > + * system clock's source to CLK_M temporarily. And then switch it to
> PLLP_out4
> > + * (204MHz) at a later time.
> > + */
> > +void set_avp_clock_to_clkm(void)
>
> That comment would be better located in spl_board_init() in order to
> explain why it calls this function. Any comment for this function would
> be better explaining the function itself more than why other code might
> call it.
>

I'll look at moving it to the actual set_avp_clock_to_clkm() call.

Thanks,

Tom

[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[Stephen Warren]

On 10/15/2013 01:37 PM, Tom Warren wrote:
> Jimmy - please answer Stephen's questions below, and/or correct my answers.
> 
> 
> On Tue, Oct 8, 2013 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     On 10/07/2013 03:17 PM, Tom Warren wrote:
>     > From: Jimmy Zhang <jimmzhang at nvidia.com <mailto:jimmzhang@nvidia.com>>
>     >
>     > Based on Tegra114 TRM, system clock can run up to 275MHz. On power on,
> 
>     Which exact clock is "system clock"?
> 
>  
> In the T1x4 TRM, SCLK is the system clock that drives the AVP/COP. It
> appears to have 8 possible sources (PLLP, CLK_M, etc.). See the settings
> in CLK_RST_CONTROLLER_SCLK_BURST_POLICY_0 (offset 0x28 in the CAR block).

My comment was really a request to ammend the commit description to use
the actual clock name from the TRM (sclk??) rather than a description of
the clock (system clock). Then, it'd be clear which clock it was talking
about.

[U-Boot] [PATCH] Tegra114: spl: Set system clock to clk_m

[Tom Warren]

Done, V2 inbound.


On Tue, Oct 15, 2013 at 12:47 PM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 10/15/2013 01:37 PM, Tom Warren wrote:
> > Jimmy - please answer Stephen's questions below, and/or correct my
> answers.
> >
> >
> > On Tue, Oct 8, 2013 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org
> > <mailto:swarren@wwwdotorg.org>> wrote:
> >
> >     On 10/07/2013 03:17 PM, Tom Warren wrote:
> >     > From: Jimmy Zhang <jimmzhang@nvidia.com <mailto:
> jimmzhang at nvidia.com>>
> >     >
> >     > Based on Tegra114 TRM, system clock can run up to 275MHz. On power
> on,
> >
> >     Which exact clock is "system clock"?
> >
> >
> > In the T1x4 TRM, SCLK is the system clock that drives the AVP/COP. It
> > appears to have 8 possible sources (PLLP, CLK_M, etc.). See the settings
> > in CLK_RST_CONTROLLER_SCLK_BURST_POLICY_0 (offset 0x28 in the CAR block).
>
> My comment was really a request to ammend the commit description to use
> the actual clock name from the TRM (sclk??) rather than a description of
> the clock (system clock). Then, it'd be clear which clock it was talking
> about.
>