From mboxrd@z Thu Jan  1 00:00:00 1970
From: Detlev Zundel <dzu@denx.de>
Date: Wed, 24 Jun 2009 10:47:27 +0200
Subject: [U-Boot] [PATCH 2/3] A320: driver for FTRTC010 real time clock
In-Reply-To: <ec1aee9f0906232205x33819620q770d150bf2d133eb@mail.gmail.com>
	(Po-Yu Chuang's message of "Wed, 24 Jun 2009 13:05:37 +0800")
References: <ec1aee9f0906230635o7e706fecs507d18c8feae49a3@mail.gmail.com>
	<20090623234706.GA6875@game.jcrosoft.org>
	<ec1aee9f0906232205x33819620q770d150bf2d133eb@mail.gmail.com>
Message-ID: <m2iqimkogw.fsf@ohwell.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Po-Yu Chuang,

> Dear Jean-Christophe PLAGNIOL-VILLARD,
>
> 2009/6/24 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@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: dzu at denx.de