public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] MPC85xx: Question about Local Bus initialization
Date: Mon, 03 Dec 2007 16:39:31 +0100	[thread overview]
Message-ID: <m263zflrd8.fsf@ohwell.denx.de> (raw)
In-Reply-To: <474E8C99.90507@tqs.de> (Jens Gehrlein's message of "Thu\, 29 Nov 2007 10\:55\:37 +0100")

Hi Jens,

> I took a look into the files board/mpc8560ads.c and board/tqm85xx.c and 
> found something strange:
>
> 1. In the function local_bus_init() the current CLKDIV is read from the 
> register LCRR as was set by Hardreset. After that, the decision is made, 
> wether the DLL has to be enabled/disabled/overridden. Inside the if-else 
> blocks the new CLKDIV is changed. But IMO the CLKDIV has to be set 
> before the query.
>
> This is the current code:
> 	clkdiv = lbc->lcrr & 0x0f;
> 	lbc_hz = sysinfo.freqSystemBus / 1000000 / clkdiv;
>
> 	if (lbc_hz < 66) {
> 		lbc->lcrr = CFG_LBC_LCRR | 0x80000000;	/* DLL Bypass */
>
> 	} else if (lbc_hz >= 133) {
> 		lbc->lcrr = CFG_LBC_LCRR & (~0x80000000); /* DLL Enabled
> 	...
> This may be the situation on other 85xx boards, too. I didn't check them 
> all.
> What was the intention, DLL modification dependent on the clock set by
> the MPC at hardreset or dependent on the targeted frequency?

I am definitely not the specialist on these chips, but checking the
8555 and 8560 manuals, it seems that LCRR comes out of reset with
0x8000,0008.  I cannot find any reference to LCRR from the POR
description.

Keeping this in mind, "clkdiv" in the above code will always be 8 and
so it looks to me indeed like the tests really were intended to check
(CFG_LBC_LCRR & 0x0f).  Can anyone more knowledgable in this area
confirm this suspicion?

> 2. The variable is named lbc_hz, but it contains a value in units of 
> MHz. I suggest to use the name lbc_mhz or to use Hertz values by 
> removing the division by 1,000,000 and replacing 66 and 133 by 66666667 
> and 133333333.
> What's your opinion?

Personally I would second your reasoning.  But for others to comment,
it is really best to implement your proposed change and send a diff to
comment on.  Such a suggestion is more likely by several magnitudes to
be considered ;)

> 3. I assume, the function above is called while the MPC executes this 
> code from flash memory (array init_sequence[] in lib_ppc/board.c). Is 
> there a potential that the U-Boot would hang when it changed the CLKDIV?

If you get the timings wrong, of course things could get hairy, or am
I missing something?

Cheers
  Detlev

-- 
Man is a fool, and woman, for tolerating him, is a damned fool
                       -- Mark Twain
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

  reply	other threads:[~2007-12-03 15:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-29  9:55 [U-Boot-Users] MPC85xx: Question about Local Bus initialization Jens Gehrlein
2007-12-03 15:39 ` Detlev Zundel [this message]
2007-12-04  7:56   ` Jens Gehrlein
2007-12-03 16:30 ` Clemens Koller
2007-12-04  7:56   ` Jens Gehrlein

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=m263zflrd8.fsf@ohwell.denx.de \
    --to=dzu@denx.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