Hi Po-Yu Chuang,
> Dear Jean-Christophe PLAGNIOL-VILLARD,
>
> 2009/6/24 Jean-Christophe PLAGNIOL-VILLARD <plagn...@jcrosoft.com>:
>>> +
>>> +static volatile struct ftrtc010 *rtc = (struct ftrtc010
>>> *)CONFIG_SYS_RTC_BASE;
>>> +
>>> +static void ftrtc_enable (void)
>> you use it at one please only the reset
> Sorry, I don't understand what do you mean
I guess he means that he doesn't like functions which are only called
from one place - he'd rather like to see the code directly at the
callers site.
However I do not agree here - functions have also a documentation
effect, so I vote to keep the function. If neccessary, we could declare
the function inline, but this is really overkill here.
>>> +{
>>> + rtc->cr = cpu_to_le32 (FTRTC010_CR_ENABLE);
>> so please move this code there
>>> +}
>>> +
>>> +/*
>>> + * return current time in seconds
>>> + */
>>> +static unsigned long ftrtc_time (void)
>>> +{
>>> + unsigned long day;
>>> + unsigned long hour;
>>> + unsigned long minute;
>>> + unsigned long second;
>>> + unsigned long second2;
>>> +
>>> + do {
>>> + second = le32_to_cpu (rtc->sec);
>> please use proper accessor
>> readl/writel
> Should I use
> second = readl(&rtc->sec);
Yep.
Cheers
Detlev
--
BUGS:
It is not yet possible to change operating system by writing to
/proc/sys/kernel/ostype.
-- Linux sysctl(2) man page
--
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: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
