[PATCH] ignore invalid hw clock for non-interupt-based RTCs
Hi,
My previous comments for this bug report were not accurate : invalid
hardware clock (e.g. reset ones after battery ran out) are already
handled in hwclock, BUT _not_ for non-interrupt based RTCs, like the
ones on newworld PPCs, at least. It reads the clock in
synchronize_to_clock_tick_rtc() for the rtc-generic driver and errors
out from here (the "usual" invalid clock case is handled in
read_hardware_clock() later, but too late).
Before, hwclock ignored the error returned by the kernel for an invalid
hw clock (if returning EINVAL is the correct error code for an invalid
time -- I am not 100% sure of that, but Ben Hutchings' comment in
#542275 and looking at kernel drivers seem to imply that). It only used
the result from mktime() to check the date. But the kernel also has its
own routine. And this bug is also what cause a lot of drivers _not_ to
report any error when there is one, like I said in my previous comment.
I think this is quite serious.
The following patch fixes this bug (also including some error code enums
to be clearer) while trying not to be too intrusive.
This bug was introduced on powerpc when the default ppc RTC driver for
2.6.30 was changed to rtc-generic. Before that, RTC had been broken for
quite some time on PPC. And continues for people which have flacky
hardware (powerpc is mainly used on "old" machines today). This is why I
am CCing debian-powerpc : I think this is an important problem that
should really be fixed.
Regards,
benjamin
Signed-off-by: Benjamin Cama <benoar@free.fr>
---
diff --git a/hwclock/clock.h b/hwclock/clock.h
index e4eb7eb..a1d1bc9 100644
--- a/hwclock/clock.h
+++ b/hwclock/clock.h
@@ -23,6 +23,14 @@ extern struct clock_ops *probe_for_kd_clock(void);
typedef int bool;
+enum {
+ HWCLOCK_OK = 0,
+ HWCLOCK_ERROR,
+ HWCLOCK_SYNC_TIMEOUT,
+ HWCLOCK_KDGHWCLK_ERROR,
+ HWCLOCK_INVALID_TIME
+};
+
/* hwclock.c */
extern char *progname;
extern int debug;
diff --git a/hwclock/cmos.c b/hwclock/cmos.c
index 8b3495b..ecfdd53 100644
--- a/hwclock/cmos.c
+++ b/hwclock/cmos.c
@@ -431,13 +431,13 @@ synchronize_to_clock_tick_cmos(void) {
*/
for (i = 0; !cmos_clock_busy(); i++)
if (i >= 10000000)
- return 1;
+ return HWCLOCK_SYNC_TIMEOUT;
/* Wait for fall. Should be within 2.228 ms. */
for (i = 0; cmos_clock_busy(); i++)
if (i >= 1000000)
- return 1;
- return 0;
+ return HWCLOCK_SYNC_TIMEOUT;
+ return HWCLOCK_OK;
}
diff --git a/hwclock/hwclock.c b/hwclock/hwclock.c
index ac1f7c8..aa6d6ee 100644
--- a/hwclock/hwclock.c
+++ b/hwclock/hwclock.c
@@ -440,8 +440,11 @@ read_hardware_clock(const bool universal, bool *valid_p, time_t *systime_p){
int err;
err = ur->read_hardware_clock(&tm);
- if (err)
- return err;
+ if (err) {
+ if (err == HWCLOCK_INVALID_TIME)
+ *valid_p = FALSE;
+ return err;
+ }
if (badyear)
read_date_from_file(&tm);
@@ -1190,12 +1193,12 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
if (show || adjust || hctosys || (!noadjfile && !systz)) {
/* data from HW-clock are required */
rc = synchronize_to_clock_tick();
- if (rc && rc != 2) /* 2= synchronization timeout */
+ if (rc && rc != HWCLOCK_SYNC_TIMEOUT && rc != HWCLOCK_INVALID_TIME)
return EX_IOERR;
gettimeofday(&read_time, NULL);
rc = read_hardware_clock(universal, &hclock_valid, &hclocktime);
- if (rc)
- return EX_IOERR;
+ if (rc && rc != HWCLOCK_INVALID_TIME)
+ return EX_IOERR;
}
if (show) {
diff --git a/hwclock/kd.c b/hwclock/kd.c
index 3da87ca..9071843 100644
--- a/hwclock/kd.c
+++ b/hwclock/kd.c
@@ -55,7 +55,7 @@ synchronize_to_clock_tick_kd(void) {
if (ioctl(con_fd, KDGHWCLK, &start_time) == -1) {
outsyserr(_("KDGHWCLK ioctl to read time failed"));
- return 3;
+ return HWCLOCK_KDGHWCLK_ERROR;
}
/* Wait for change. Should be within a second, but in case something
@@ -73,18 +73,18 @@ synchronize_to_clock_tick_kd(void) {
if (ioctl(con_fd, KDGHWCLK, &nowtime) == -1) {
outsyserr(_("KDGHWCLK ioctl to read time failed in loop"));
- return 3;
+ return HWCLOCK_KDGHWCLK_ERROR;
}
if (start_time.tm_sec != nowtime.tm_sec)
break;
gettimeofday(&now, NULL);
if (time_diff(now, begin) > 1.5) {
fprintf(stderr, _("Timed out waiting for time change.\n"));
- return 2;
+ return HWCLOCK_SYNC_TIMEOUT;
}
} while(1);
- return 0;
+ return HWCLOCK_OK;
}
diff --git a/hwclock/rtc.c b/hwclock/rtc.c
index 2e05385..84d85aa 100644
--- a/hwclock/rtc.c
+++ b/hwclock/rtc.c
@@ -177,10 +177,14 @@ do_rtc_read_ioctl(int rtc_fd, struct tm *tm) {
rc = ioctl(rtc_fd, RTC_RD_TIME, tm);
}
if (rc == -1) {
+ int err = -1;
+ /* special error case for invalid RTC clock */
+ if (errno == EINVAL)
+ err = HWCLOCK_INVALID_TIME;
perror(ioctlname);
fprintf(stderr, _("ioctl() to %s to read the time failed.\n"),
rtc_dev_name);
- return -1;
+ return err;
}
tm->tm_isdst = -1; /* don't know whether it's dst */
@@ -204,8 +208,12 @@ busywait_for_rtc_clock_tick(const int rtc_fd) {
rtc_dev_name);
rc = do_rtc_read_ioctl(rtc_fd, &start_time);
- if (rc)
- return 1;
+ if (rc) {
+ if (rc == HWCLOCK_INVALID_TIME)
+ return HWCLOCK_INVALID_TIME;
+ else
+ return HWCLOCK_ERROR;
+ }
/* Wait for change. Should be within a second, but in case something
* weird happens, we have a time limit (1.5s) on this loop to reduce the
@@ -219,13 +227,13 @@ busywait_for_rtc_clock_tick(const int rtc_fd) {
gettimeofday(&now, NULL);
if (time_diff(now, begin) > 1.5) {
fprintf(stderr, _("Timed out waiting for time change.\n"));
- return 2;
+ return HWCLOCK_SYNC_TIMEOUT;
}
} while(1);
if (rc)
- return 3;
- return 0;
+ return HWCLOCK_ERROR;
+ return HWCLOCK_OK;
}
static int
@@ -239,7 +247,7 @@ int ret;
rtc_fd = open_rtc();
if (rtc_fd == -1) {
outsyserr(_("open() of %s failed"), rtc_dev_name);
- ret = 1;
+ ret = HWCLOCK_ERROR;
} else {
int rc; /* Return code from ioctl */
/* Turn on update interrupts (one per second) */
@@ -265,12 +273,12 @@ int ret;
/* this blocks until the next update interrupt */
rc = read(rtc_fd, &dummy, sizeof(dummy));
- ret = 1;
+ ret = HWCLOCK_ERROR;
if (rc == -1)
outsyserr(_("read() to %s to wait for clock tick failed"),
rtc_dev_name);
else
- ret = 0;
+ ret = HWCLOCK_OK;
#else
/* Just reading rtc_fd fails on broken hardware: no update
interrupt comes and a bootscript with a hwclock call hangs */
@@ -283,7 +291,7 @@ int ret;
tv.tv_sec = 5;
tv.tv_usec = 0;
rc = select(rtc_fd + 1, &rfds, NULL, NULL, &tv);
- ret = 1;
+ ret = HWCLOCK_ERROR;
if (rc == -1)
outsyserr(_("select() to %s to wait for clock tick failed"),
rtc_dev_name);
@@ -291,7 +299,7 @@ int ret;
fprintf(stderr, _("select() to %s to wait for clock tick timed out\n"),
rtc_dev_name);
else
- ret = 0;
+ ret = HWCLOCK_OK;
#endif
/* Turn off update interrupts */
@@ -302,7 +310,7 @@ int ret;
} else {
outsyserr(_("ioctl() to %s to turn on update interrupts "
"failed unexpectedly"), rtc_dev_name);
- ret = 1;
+ ret = HWCLOCK_ERROR;
}
}
return ret;
Reply to: