| Message ID | 1381180679-29524-1-git-send-email-twarren@nvidia.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
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.
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
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@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.
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@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. >
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)