[U-Boot] [PATCH][v0] RTC driver for PT7C4338 chip.

[Jain Priyanka-B32167]

Dear Wolfgang Denk,

Please find my answers to comments inline.

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de] 
Sent: Thursday, October 21, 2010 1:36 AM
To: Jain Priyanka-B32167
Cc: u-boot at lists.denx.de; Gala Kumar-B11780
Subject: Re: [U-Boot] [PATCH][v0] RTC driver for PT7C4338 chip.

Dear Priyanka Jain,

In message
<1287564032-32198-1-git-send-email-Priyanka.Jain at freescale.com> you
wrote:
> PT7C4338 chip is manufactured by Pericom Technology Inc.
> It is a serial real-time clock which provides:
> 1)Low-power clock/calendar.
> 2)Programmable square-wave output.
> It has 56 bytes of nonvolatile RAM.
> 
> Freescale P1010RDB uses PT7C4338 as RTC.
> 
> Signed-off-by: Priyanka Jain <Priyanka.Jain at freescale.com>
> Acked-by: Timur Tabi <timur at freescale.com>
> ---
>  drivers/rtc/Makefile   |    1 +
>  drivers/rtc/pt7c4338.c |  161 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 162 insertions(+), 0 deletions(-)  create mode 
> 100644 drivers/rtc/pt7c4338.c
...
> +/* Get the current time from the RTC */ int rtc_get(struct rtc_time 
> +*tmp) {
> +	int ret = 0;
> +	u8 sec, min, hour, mday, wday, mon, year, ctl_stat;
> +
> +	ctl_stat = rtc_read(RTC_CTL_STAT_REG_ADDR);
> +	sec = rtc_read(RTC_SEC_REG_ADDR);
> +	min = rtc_read(RTC_MIN_REG_ADDR);
> +	hour = rtc_read(RTC_HR_REG_ADDR);
> +	wday = rtc_read(RTC_DAY_REG_ADDR);
> +	mday = rtc_read(RTC_DATE_REG_ADDR);
> +	mon = rtc_read(RTC_MON_REG_ADDR);
> +	year = rtc_read(RTC_YR_REG_ADDR);
> +	debug("Get RTC year: %02x mon: %02x mday: %02x wday: %02x "
> +		"hr: %02x min: %02x sec: %02x control_status: %02x\n",
> +		year, mon, mday, wday, hour, min, sec, ctl_stat);
> +
> +	if (ctl_stat & RTC_CTL_STAT_BIT_OSF) {
> +		printf("### Warning: RTC oscillator has stopped\n");
> +		/* clear the OSF flag */
> +		rtc_write(RTC_CTL_STAT_REG_ADDR,
> +			rtc_read(RTC_CTL_STAT_REG_ADDR)\
> +			& ~RTC_CTL_STAT_BIT_OSF);
> +		ret = -1;

I guess you want to return here, not fall through?
[Priyanka] : It's a warning not an error. We can still continue with the
functionality.
I hope this answer your query.  

> +	}
> +
> +	if (sec & RTC_SEC_BIT_CH) {
> +		printf("### Warning: RTC oscillator has stopped\n");
> +		/* clear the CH flag */
> +		rtc_write(RTC_SEC_REG_ADDR,
> +			rtc_read(RTC_SEC_REG_ADDR) & ~RTC_SEC_BIT_CH);
> +		ret = -1;

I guess you want to return here, not fall through?
[Priyanka]: This check will be removed. 

Is it correct to have two different causes for the "Warning: RTC
oscillator has stopped" message?   I would be a bit connnnnnnnnnnfused
what it means when I see this twice on a system.
[Priyanka]: I will merge both the condition checks into one. 

Regards
Priyanka