Hi Marek,

On 09/12/2012 10:03 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,

+static inline void rtc_access_disable(void)
+{
+       s3c24xx_rtc *const rtc = s3c24xx_get_base_rtc();
+       uchar rtccon;
+
+       rtccon = readb(&rtc->rtccon);
+       rtccon &= ~0x01;
Magic numbers, fix globally in the patchset

OK.

+       writeb(rtccon, &rtc->rtccon);
+}
+
+/*
-------------------------------------------------------------------------
*/ +
+int rtc_get(struct rtc_time *tmp)
+{
+       s3c24xx_rtc *const rtc = s3c24xx_get_base_rtc();
+       uchar sec, min, hour, mday, wday, mon, year;
+       int have_retried = 0;
+
+       rtc_access_enable();
+
+       /* Read RTC registers */
+retry_get_time:
+       min = readb(&rtc->bcdmin);
+       hour = readb(&rtc->bcdhour);
+       mday = readb(&rtc->bcddate);
+       wday = readb(&rtc->bcdday);
+       mon = readb(&rtc->bcdmon);
+       year = readb(&rtc->bcdyear);
+       sec = readb(&rtc->bcdsec);
+
+       /* The only way to work out whether the RTC was mid-update
+        * when we read it is to check the seconds counter.
+        * If it's zero, then we re-try the entire read.
+        */
Wrong multiline comment style ... use ./tools/checkpatch.pl before resending

OK.


+       if (rtc->bcdsec == 0 && !have_retried) {
I'm sure you can avoid the goto here ...besides, this rtc->bcdsec doesn't make
much sense

I could do it with a do..while loop instead... this part was copied from the linux driver.
And there is a bug... instead of 'rtc->bcdsec' it should be 'sec'!


Regards,
José Gonçalves
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to