Hi Wolfgang, hi Tom,
as I almost have the changes requested by Wolfgang in place, you two can choose, which version I should resubmit. ;)
Am 09.01.2013 um 20:34 schrieb Tom Rini:
+static void rtc32k_enable(void) +{
- struct rtc_regs *rtc = (struct rtc_regs *)AM335X_RTC_BASE;
- /*
* Unlock the RTC's registers. For more details please see the* RTC_SS section of the TRM. In order to unlock we need to* write these specific values (keys) in this order.*/- writel(0x83e70b13, &rtc->kick0r);
- writel(0x95a4f1e0, &rtc->kick1r);
These magic numbers should probbly be moved to some header file ?
This is a copy/paste from the am335x board.c file. I specifically did a long comment to explain what / where these values come from because that (to me) is more helpful than #define AM33X_RTC_KICK0R_KEY 0x... #define AM33X_RTC_KICK1R_KEY 0x...
davinci does it this way. I would prefer Tom's variant having the values right in place.
- if (!eth_getenv_enetaddr("ethaddr", mac_addr)) {
debug("<ethaddr> not set. Reading from E-fuse\n");This should be a printf(). Any such automatic changes are always worth to be told explicitly.
This is the normal case of asking the hardware what MAC it shipped with. Why do we want to tell the user the expected has happened?
Here I'd prefer the printf variant, because for a common user it is not obivous, that it is reading the MAC from efuse. So this message is helpful.
goto try_usbether;...
+#endif +try_usbether:
Did you ever try building without CONFIG_DRIVER_TI_CPSW set? I bet this causes a few compiler warnings. [Hint: You define a label, but don't use it anywhere.]
It's a valid question if this board uses the CPSW eth or not, yes.
Yes, the board uses CPSW, but here I would prefer having the try_usbether label inside the ifdef.
+#define CONFIG_ENV_IS_NOWHERE
Really?
Until they add NAND (which just now hit mainline), yes.
This is a very valuable information, thanks!
Regards, Lars