[U-Boot] [PATCH] Add support for Faraday RTC IP FTRTC

[Wolfgang Denk]

Dear PoYu_Chuang,

In message <200903310858.n2V8wZnB021884 at ftcpcw82.faraday.com.tw> you wrote:
> This patch adds support for Faraday Technology RTC IP - FTRTC
> 
> Signed-off-by: Po-Yu Chuang <ratbert at faraday-tech.com>
> ---
> diff -ruN u-boot-2009.03/drivers/rtc/ftrtc.c FA5A320LINUX26_u-boot/drivers/rtc/ftrtc.c
> --- u-boot-2009.03/drivers/rtc/ftrtc.c	1970-01-01 08:00:00.000000000 +0800
> +++ FA5A320LINUX26_u-boot/drivers/rtc/ftrtc.c	2009-03-31 16:28:40.000000000 +0800
> @@ -0,0 +1,110 @@
>
> +static unsigned int	rtc	= CONFIG_SYS_RTC_BASE;

Please use a proper C structure to describe the RTC; do not use base
address plus offsets.

> +static void
> +ftrtc_enable(void)
> +{
> +	outl(FTRTC_CR_ENABLE, rtc + FTRTC_OFFSET_CR);
> +}

outl() is not portable across architectures?

> +	do {
> +		second	= inl(rtc + FTRTC_OFFSET_SEC);
> +		day	= inl(rtc + FTRTC_OFFSET_DAY);
> +		hour	= inl(rtc + FTRTC_OFFSET_HOUR);
> +		minute	= inl(rtc + FTRTC_OFFSET_MIN);
> +		second2	= inl(rtc + FTRTC_OFFSET_SEC);

inl() is not portable across architectures?

> +{
> +	unsigned long	now;
> +
> +	debug("rtc_get(): record register: %x\n", inl(rtc + FTRTC_OFFSET_RECORD));

Line too long.

> diff -ruN u-boot-2009.03/drivers/rtc/ftrtc.h FA5A320LINUX26_u-boot/drivers/rtc/ftrtc.h
> --- u-boot-2009.03/drivers/rtc/ftrtc.h	1970-01-01 08:00:00.000000000 +0800
> +++ FA5A320LINUX26_u-boot/drivers/rtc/ftrtc.h	2009-03-31 15:19:32.000000000 +0800
> @@ -0,0 +1,44 @@
...
> +#ifndef __FTRTC_H
> +#define __FTRTC_H
> +
> +#define	FTRTC_OFFSET_SEC	0x00
> +#define	FTRTC_OFFSET_MIN	0x04
> +#define	FTRTC_OFFSET_HOUR	0x08
> +#define	FTRTC_OFFSET_DAY	0x0c
> +#define	FTRTC_OFFSET_ALRM_SEC	0x10
> +#define	FTRTC_OFFSET_ALRM_MIN	0x14
> +#define	FTRTC_OFFSET_ALRM_HOUR	0x18
> +#define	FTRTC_OFFSET_RECORD	0x1c
> +#define	FTRTC_OFFSET_CR		0x20

Please use a proper C structure to describe the RTC; do not use base
address plus offsets.

> +/*
> + * RTC Control Register
> + */
> +#define	FTRTC_CR_ENABLE		(1 << 0)
> +#define	FTRTC_CR_INTERRUPT_SEC	(1 << 1)	/* enable interrupt per second */
> +#define	FTRTC_CR_INTERRUPT_MIN	(1 << 2)	/* enable interrupt per minute */
> +#define	FTRTC_CR_INTERRUPT_HR	(1 << 3)	/* enable interrupt per hour */
> +#define	FTRTC_CR_INTERRUPT_DAY	(1 << 4)	/* enable interrupt per day */

Lines too long.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is a theory which states that if ever anyone discovers  exactly
what  the  Universe is for and why it is here, it will instantly dis-
appear and be replaced by something even more  bizarre  and  inexpli-
cable.  There  is  another  theory which states that this has already
happened.    -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"