public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@avionic-design.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] tegra: Implement oscillator frequency detection
Date: Fri, 25 May 2012 06:54:09 +0200	[thread overview]
Message-ID: <20120525045409.GA15190@avionic-0098.mockup.avionic-design.de> (raw)
In-Reply-To: <4FBEB23B.4060202@wwwdotorg.org>

* 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120525/81466890/attachment.pgp>

      reply	other threads:[~2012-05-25  4:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-24  7:03 [U-Boot] [PATCH] tegra: Implement oscillator frequency detection Thierry Reding
2012-05-24 15:40 ` Stephen Warren
2012-05-24 21:03   ` Thierry Reding
2012-05-24 22:12     ` Stephen Warren
2012-05-25  4:54       ` Thierry Reding [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120525045409.GA15190@avionic-0098.mockup.avionic-design.de \
    --to=thierry.reding@avionic-design.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox