All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.