* 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
pgp6UiPIXjhOF.pgp
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
