[U-Boot] [PATCH] tegra: Implement oscillator frequency detection
Upon reset, the CRC_OSC_CTRL register defaults to a 13 MHz oscillator input frequency. With Lucas' recent commit b8cb519 ("tegra2: trivially enable 13 mhz crystal frequency) applied, this breaks on hardware that provides a different frequency.
The Tegra clock and reset controller provides a means to detect the input frequency by counting the number of oscillator periods within a given number of 32 kHz periods. This commit introduces a function, clock_detect_osc_freq(), which performs this calibration and programs the CRC_OSC_CTRL accordingly.
This code is loosely based on the Linux kernel Tegra2 clock code.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- arch/arm/cpu/armv7/tegra2/ap20.c | 2 ++ arch/arm/cpu/armv7/tegra2/clock.c | 42 ++++++++++++++++++++++++++++ arch/arm/include/asm/arch-tegra2/clk_rst.h | 9 ++++++ arch/arm/include/asm/arch-tegra2/clock.h | 3 ++ 4 files changed, 56 insertions(+)
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/armv7/tegra2/ap20.c index 698bfd0..150c713 100644 --- a/arch/arm/cpu/armv7/tegra2/ap20.c +++ b/arch/arm/cpu/armv7/tegra2/ap20.c @@ -351,6 +351,8 @@ void tegra2_start(void) /* not reached */ }
+ clock_detect_osc_freq(); + /* Init PMC scratch memory */ init_pmc_scratch();
diff --git a/arch/arm/cpu/armv7/tegra2/clock.c b/arch/arm/cpu/armv7/tegra2/clock.c index ccad351..77aefbc 100644 --- a/arch/arm/cpu/armv7/tegra2/clock.c +++ b/arch/arm/cpu/armv7/tegra2/clock.c @@ -396,6 +396,48 @@ static s8 periph_id_to_internal_id[PERIPH_ID_COUNT] = { NONE(CRAM2), };
+void clock_detect_osc_freq(void) +{ + struct clk_rst_ctlr *clkrst = + (struct clk_rst_ctlr *)NV_PA_CLK_RST_BASE; + enum clock_osc_freq frequency = CLOCK_OSC_FREQ_COUNT; + unsigned int periods; + u32 value; + + /* start OSC frequency detection */ + value = OSC_FREQ_DET_TRIGGER | REF_CLK_WIN_CFG(1); + writel(value, &clkrst->crc_osc_freq_det); + + /* wait for detection to complete */ + do { + value = readl(&clkrst->crc_osc_freq_det_stat); + if (!(value & OSC_FREQ_DET_BUSY)) + break; + } while (1); + + periods = (value & OSC_FREQ_DET_CNT_MASK) >> OSC_FREQ_DET_CNT_SHIFT; + + if (periods >= 732 - 3 && periods <= 732 + 3) + frequency = CLOCK_OSC_FREQ_12_0; + else if (periods >= 794 - 3 && periods <= 794 + 3) + frequency = CLOCK_OSC_FREQ_13_0; + else if (periods >= 1172 - 3 && periods <= 1172 + 3) + frequency = CLOCK_OSC_FREQ_19_2; + else if (periods >= 1587 - 3 && periods <= 1587 + 3) + frequency = CLOCK_OSC_FREQ_26_0; + + /* + * Configure oscillator frequency. If the measured frequency isn't + * among those supported, keep the default and hope for the best. + */ + if (frequency >= CLOCK_OSC_FREQ_COUNT) { + value = readl(&clkrst->crc_osc_ctrl); + value &= ~OSC_FREQ_MASK; + value |= frequency << OSC_FREQ_SHIFT; + writel(value, &clkrst->crc_osc_ctrl); + } +} + /* * Get the oscillator frequency, from the corresponding hardware configuration * field. diff --git a/arch/arm/include/asm/arch-tegra2/clk_rst.h b/arch/arm/include/asm/arch-tegra2/clk_rst.h index 8c3be91..66ca3ff 100644 --- a/arch/arm/include/asm/arch-tegra2/clk_rst.h +++ b/arch/arm/include/asm/arch-tegra2/clk_rst.h @@ -128,6 +128,15 @@ struct clk_rst_ctlr { #define OSC_XOBP_SHIFT 1 #define OSC_XOBP_MASK (1U << OSC_XOBP_SHIFT)
+/* CLK_RST_CONTROLLER_OSC_FREQ_DET_0 */ +#define OSC_FREQ_DET_TRIGGER (1 << 31) +#define REF_CLK_WIN_CFG(x) ((x) & 0xf) + +/* CLK_RST_CONTROLLER_OSC_FREQ_DET_STATUS_0 */ +#define OSC_FREQ_DET_BUSY (1 << 31) +#define OSC_FREQ_DET_CNT_SHIFT 0 +#define OSC_FREQ_DET_CNT_MASK (0xffff << OSC_FREQ_DET_CNT_SHIFT) + /* * CLK_RST_CONTROLLER_CLK_SOURCE_x_OUT_0 - the mask here is normally 8 bits * but can be 16. We could use knowledge we have to restrict the mask in diff --git a/arch/arm/include/asm/arch-tegra2/clock.h b/arch/arm/include/asm/arch-tegra2/clock.h index 1d3ae38..a86db42 100644 --- a/arch/arm/include/asm/arch-tegra2/clock.h +++ b/arch/arm/include/asm/arch-tegra2/clock.h @@ -192,6 +192,9 @@ enum periph_id { /* PLL stabilization delay in usec */ #define CLOCK_PLL_STABLE_DELAY_US 300
+/* detects the oscillator clock frequency */ +void clock_detect_osc_freq(void); + /* return the current oscillator clock frequency */ enum clock_osc_freq clock_get_osc_freq(void);
On 05/24/2012 01:03 AM, Thierry Reding wrote:
Upon reset, the CRC_OSC_CTRL register defaults to a 13 MHz oscillator input frequency. With Lucas' recent commit b8cb519 ("tegra2: trivially enable 13 mhz crystal frequency) applied, this breaks on hardware that provides a different frequency.
Can you expand upon "breaks"? Do you mean "detects the wrong value", or "causes U-Boot to fail to execute successfully", or...
For reference, I have this commit in my local branch, and have run U-Boot on at least a couple of our boards without any apparent issue.
But, I agree there is a problem that should be fixed; I'm just not sure what the current impact is.
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/armv7/tegra2/ap20.c
@@ -351,6 +351,8 @@ void tegra2_start(void) /* not reached */ }
- clock_detect_osc_freq();
Would this be better called from clock_early_init() in clock.c? That's called only very marginally later than tegra2_start(), and would keep all the clock-related code together. The patch would also edit fewer files:-)
diff --git a/arch/arm/cpu/armv7/tegra2/clock.c b/arch/arm/cpu/armv7/tegra2/clock.c
+void clock_detect_osc_freq(void)
...
- else if (periods >= 1587 - 3 && periods <= 1587 + 3)
frequency = CLOCK_OSC_FREQ_26_0;
Everything up to here looks good, and does indeed match the kernel.
- /*
* Configure oscillator frequency. If the measured frequency isn't* among those supported, keep the default and hope for the best.*/- if (frequency >= CLOCK_OSC_FREQ_COUNT) {
value = readl(&clkrst->crc_osc_ctrl);value &= ~OSC_FREQ_MASK;value |= frequency << OSC_FREQ_SHIFT;writel(value, &clkrst->crc_osc_ctrl);- }
+}
The above is quite different from what the kernel does, which is the following:
static unsigned long tegra2_clk_m_autodetect_rate(struct clk *c) { u32 auto_clock_control = clk_readl(OSC_CTRL) & ~OSC_CTRL_OSC_FREQ_MASK;
c->rate = clk_measure_input_freq(); switch (c->rate) { case 12000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_12MHZ; break; case 13000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_13MHZ; break; case 19200000: auto_clock_control |= OSC_CTRL_OSC_FREQ_19_2MHZ; break; case 26000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_26MHZ; break; default: pr_err("%s: Unexpected clock rate %ld", __func__, c->rate); BUG(); } clk_writel(auto_clock_control, OSC_CTRL); return c->rate; }
Is there a specific reason for U-Boot not to do the same thing here?
diff --git a/arch/arm/include/asm/arch-tegra2/clk_rst.h b/arch/arm/include/asm/arch-tegra2/clk_rst.h
+/* CLK_RST_CONTROLLER_OSC_FREQ_DET_0 */ +#define OSC_FREQ_DET_TRIGGER (1 << 31)
Nitpicky I know, but this is "TRIG" not "TRIGGER" in the TRM. It's probably a good idea to stay consistent where possible.
* Stephen Warren wrote:
On 05/24/2012 01:03 AM, Thierry Reding wrote:
Upon reset, the CRC_OSC_CTRL register defaults to a 13 MHz oscillator input frequency. With Lucas' recent commit b8cb519 ("tegra2: trivially enable 13 mhz crystal frequency) applied, this breaks on hardware that provides a different frequency.
Can you expand upon "breaks"? Do you mean "detects the wrong value", or "causes U-Boot to fail to execute successfully", or...
For reference, I have this commit in my local branch, and have run U-Boot on at least a couple of our boards without any apparent issue.
But, I agree there is a problem that should be fixed; I'm just not sure what the current impact is.
On Tamonten, U-Boot doesn't execute properly. Or at least I can't tell because it may just be that there is no output whatsoever on the serial port (perhaps due to the peripheral clock being configured wrongly?).
Strange thing is that if I don't do the frequency detection and without Lucas' patch things still work, even though CRC_OSC_CTRL contains the value for a 13 MHz clock.
Have you tested on Harmony? I believe that has a 12 MHz oscillator as well, so it should have the same problem than Tamonten.
diff --git a/arch/arm/cpu/armv7/tegra2/ap20.c b/arch/arm/cpu/armv7/tegra2/ap20.c
@@ -351,6 +351,8 @@ void tegra2_start(void) /* not reached */ }
- clock_detect_osc_freq();
Would this be better called from clock_early_init() in clock.c? That's called only very marginally later than tegra2_start(), and would keep all the clock-related code together. The patch would also edit fewer files:-)
On a second look that should be possible. I thought it was being used by the warmboot code, which is initialized in init_pmc_scratch(). But that's clock_get_osc_bypass(). I'll move the call to clock_early_init() and recheck if it works for me.
diff --git a/arch/arm/cpu/armv7/tegra2/clock.c b/arch/arm/cpu/armv7/tegra2/clock.c
+void clock_detect_osc_freq(void)
...
- else if (periods >= 1587 - 3 && periods <= 1587 + 3)
frequency = CLOCK_OSC_FREQ_26_0;Everything up to here looks good, and does indeed match the kernel.
- /*
* Configure oscillator frequency. If the measured frequency isn't* among those supported, keep the default and hope for the best.*/- if (frequency >= CLOCK_OSC_FREQ_COUNT) {
value = readl(&clkrst->crc_osc_ctrl);value &= ~OSC_FREQ_MASK;value |= frequency << OSC_FREQ_SHIFT;writel(value, &clkrst->crc_osc_ctrl);- }
+}
The above is quite different from what the kernel does, which is the following:
static unsigned long tegra2_clk_m_autodetect_rate(struct clk *c) { u32 auto_clock_control = clk_readl(OSC_CTRL) & ~OSC_CTRL_OSC_FREQ_MASK;
c->rate = clk_measure_input_freq(); switch (c->rate) { case 12000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_12MHZ; break; case 13000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_13MHZ; break; case 19200000: auto_clock_control |= OSC_CTRL_OSC_FREQ_19_2MHZ; break; case 26000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_26MHZ; break; default: pr_err("%s: Unexpected clock rate %ld", __func__, c->rate); BUG(); } clk_writel(auto_clock_control, OSC_CTRL); return c->rate; }
Is there a specific reason for U-Boot not to do the same thing here?
I can't see any difference between the two. Except that the U-Boot code doesn't BUG(), but instead continues hoping for the best.
diff --git a/arch/arm/include/asm/arch-tegra2/clk_rst.h b/arch/arm/include/asm/arch-tegra2/clk_rst.h
+/* CLK_RST_CONTROLLER_OSC_FREQ_DET_0 */ +#define OSC_FREQ_DET_TRIGGER (1 << 31)
Nitpicky I know, but this is "TRIG" not "TRIGGER" in the TRM. It's probably a good idea to stay consistent where possible.
Right, I'll fix that up.
Thierry
On 05/24/2012 03:03 PM, Thierry Reding wrote:
- Stephen Warren wrote:
On 05/24/2012 01:03 AM, Thierry Reding wrote:
Upon reset, the CRC_OSC_CTRL register defaults to a 13 MHz oscillator input frequency. With Lucas' recent commit b8cb519 ("tegra2: trivially enable 13 mhz crystal frequency) applied, this breaks on hardware that provides a different frequency.
Can you expand upon "breaks"? Do you mean "detects the wrong value", or "causes U-Boot to fail to execute successfully", or...
For reference, I have this commit in my local branch, and have run U-Boot on at least a couple of our boards without any apparent issue.
But, I agree there is a problem that should be fixed; I'm just not sure what the current impact is.
On Tamonten, U-Boot doesn't execute properly. Or at least I can't tell because it may just be that there is no output whatsoever on the serial port (perhaps due to the peripheral clock being configured wrongly?).
Strange thing is that if I don't do the frequency detection and without Lucas' patch things still work, even though CRC_OSC_CTRL contains the value for a 13 MHz clock.
Have you tested on Harmony? I believe that has a 12 MHz oscillator as well, so it should have the same problem than Tamonten.
Odd. Yes, I have tested on Harmony. I think all/most of the boards I have use a 12MHz clock.
I wonder if this is due to some incorrect setting in your BCT?
- /* + * Configure oscillator frequency. If the measured
frequency isn't + * among those supported, keep the default and hope for the best. + */ + if (frequency >= CLOCK_OSC_FREQ_COUNT) { + value = readl(&clkrst->crc_osc_ctrl); + value &= ~OSC_FREQ_MASK; + value |= frequency << OSC_FREQ_SHIFT; + writel(value, &clkrst->crc_osc_ctrl); + } +}
The above is quite different from what the kernel does, which is the following:
static unsigned long tegra2_clk_m_autodetect_rate(struct clk *c) { u32 auto_clock_control = clk_readl(OSC_CTRL) & ~OSC_CTRL_OSC_FREQ_MASK;
c->rate = clk_measure_input_freq(); switch (c->rate) { case 12000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_12MHZ; break; case 13000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_13MHZ; break; case 19200000: auto_clock_control |= OSC_CTRL_OSC_FREQ_19_2MHZ; break; case 26000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_26MHZ; break; default: pr_err("%s: Unexpected clock rate %ld", __func__, c->rate); BUG(); } clk_writel(auto_clock_control, OSC_CTRL); return c->rate; }
Is there a specific reason for U-Boot not to do the same thing here?
I can't see any difference between the two. Except that the U-Boot code doesn't BUG(), but instead continues hoping for the best.
The kernel code supports 4 explicit rates, and if the measured clock is any of those rates, it writes the appropriate enum to the OSC_CTRL register.
However, the U-Boot code above only writes to OSC_CTRL in the case where no known match was found. Perhaps it's just that:
- if (frequency >= CLOCK_OSC_FREQ_COUNT) {
should be:
- if (frequency < CLOCK_OSC_FREQ_COUNT) {
Given that though, I'm confused why this patch makes any difference, unless I'm just totally misreading it?
I think when I first read your patch, I thought there were other differences between kernel and U-Boot, but upon further inspection I think not.
* Stephen Warren wrote:
On 05/24/2012 03:03 PM, Thierry Reding wrote:
On Tamonten, U-Boot doesn't execute properly. Or at least I can't tell because it may just be that there is no output whatsoever on the serial port (perhaps due to the peripheral clock being configured wrongly?).
Strange thing is that if I don't do the frequency detection and without Lucas' patch things still work, even though CRC_OSC_CTRL contains the value for a 13 MHz clock.
Have you tested on Harmony? I believe that has a 12 MHz oscillator as well, so it should have the same problem than Tamonten.
Odd. Yes, I have tested on Harmony. I think all/most of the boards I have use a 12MHz clock.
I wonder if this is due to some incorrect setting in your BCT?
The BCT was actually the first thing I looked at, but none of the values seemed suspicious.
- /* + * Configure oscillator frequency. If the measured
frequency isn't + * among those supported, keep the default and hope for the best. + */ + if (frequency >= CLOCK_OSC_FREQ_COUNT) { + value = readl(&clkrst->crc_osc_ctrl); + value &= ~OSC_FREQ_MASK; + value |= frequency << OSC_FREQ_SHIFT; + writel(value, &clkrst->crc_osc_ctrl); + } +}
The above is quite different from what the kernel does, which is the following:
static unsigned long tegra2_clk_m_autodetect_rate(struct clk *c) { u32 auto_clock_control = clk_readl(OSC_CTRL) & ~OSC_CTRL_OSC_FREQ_MASK;
c->rate = clk_measure_input_freq(); switch (c->rate) { case 12000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_12MHZ; break; case 13000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_13MHZ; break; case 19200000: auto_clock_control |= OSC_CTRL_OSC_FREQ_19_2MHZ; break; case 26000000: auto_clock_control |= OSC_CTRL_OSC_FREQ_26MHZ; break; default: pr_err("%s: Unexpected clock rate %ld", __func__, c->rate); BUG(); } clk_writel(auto_clock_control, OSC_CTRL); return c->rate; }
Is there a specific reason for U-Boot not to do the same thing here?
I can't see any difference between the two. Except that the U-Boot code doesn't BUG(), but instead continues hoping for the best.
The kernel code supports 4 explicit rates, and if the measured clock is any of those rates, it writes the appropriate enum to the OSC_CTRL register.
However, the U-Boot code above only writes to OSC_CTRL in the case where no known match was found. Perhaps it's just that:
- if (frequency >= CLOCK_OSC_FREQ_COUNT) {
should be:
- if (frequency < CLOCK_OSC_FREQ_COUNT) {
Yes, correct.
Given that though, I'm confused why this patch makes any difference, unless I'm just totally misreading it?
I think when I first read your patch, I thought there were other differences between kernel and U-Boot, but upon further inspection I think not.
This also has me totally confused now. The code as it is now shouldn't write to the CRC_OSC_CTRL register in any case and therefore, as you said shouldn't make any difference. I'll have to double-check that I have the correct patch applied.
Thierry
participants (2)
-
Stephen Warren -
Thierry Reding