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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
