From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Gehrlein Date: Tue, 04 Dec 2007 08:56:02 +0100 Subject: [U-Boot-Users] MPC85xx: Question about Local Bus initialization In-Reply-To: References: <474E8C99.90507@tqs.de> Message-ID: <47550812.2020108@tqs.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Detlev, thanks for the reply. Detlev Zundel schrieb: > 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. Agreed. 0x80000008 is the hard reset value. > > 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? In the current implementation CLKDIV is not modified earlier. Therefore, lbc->lcrr & 0x0f == CFG_LBC_LCRR & 0x0f is in this code only true, if CFG_LBC_LCRR is defined equally to the MPC's hard reset value. But what if I desire another clock frequency and other timings? In the current code the decision to bypass or not to bypass the DLL is made, _before_ it is changed. Shouldn't it be "clkdiv = (CFG_LBC_LCRR & 0x0f);" in the first line? I'm not sure. And that's is the reason for my question. I hoped that the Freescale developers had something to say about it. > >> 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 ;) No problem. It was just a question. Regards, Jens