Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31557
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 26 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/1
diff --git a/src/lib/rtc.c b/src/lib/rtc.c index c5c157f..1dd301c 100644 --- a/src/lib/rtc.c +++ b/src/lib/rtc.c @@ -27,51 +27,32 @@ #define DAYS_IN_YEAR(a) (LEAP_YEAR(a) ? 366 : 365) #define DAYS_IN_MONTH(a) (month_days[(a) - 1])
-static const int month_offset[] = { - 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334 -}; - static const char *const weekdays[] = { "Sun", "Mon", "Tues", "Wednes", "Thurs", "Fri", "Satur", };
-static int leaps_to_year(int year) -{ - return year / 4 - year / 100 + year / 400; -} - -/* This only works for the Gregorian calendar after Jan 1 1971. */ +/* Zeller's rule */ static int rtc_calc_weekday(struct rtc_time *tm) { - int leaps_to_date; - int day; - if (tm->year < 1971) return -1;
- day = 4; /* Jan 1 1970 was a Thursday. */ - - /* Number of leap corrections to apply up to end of last year */ - leaps_to_date = leaps_to_year(tm->year - 1) - leaps_to_year(1970); - - /* - * This year is a leap year if it is divisible by 4 except when it is - * divisible by 100 unless it is divisible by 400 - * - * e.g. 1904 was a leap year, 1900 was not, 1996 is, and 2000 is. - */ - if ((tm->year % 4) && - ((tm->year % 100 != 0) || (tm->year % 400 == 0)) && - (tm->mon > 2)) { - /* We are past Feb. 29 in a leap year */ - day++; - } - - day += (tm->year - 1970) * 365 + leaps_to_date + - month_offset[tm->mon-1] + tm->mday; - tm->wday = day % 7; - - return 0; + /* k: Day of month */ + const int k = tm->mday; + /* m: Month number (March is 1, April 2, ... January 11, February 12) */ + const int m = (tm->mon < 3) ? tm->mon + 10 : tm->mon - 2; + /* D: Last 2 digits of year; but treat Jan/Feb as "last year" */ + const int D = ((tm->mon < 3) ? tm->year - 1 : tm->year) % 100; + /* C: First two digits of the year */ + const int C = tm->year / 100; + const int f = k + + (13 * m - 1) / 5 + + D + + (D >> 2) + + (C >> 2) + - 2 * C; + tm->wday = f % 7; + return 0; }
int rtc_to_tm(int tim, struct rtc_time *tm) @@ -139,16 +120,18 @@ }
days = (unsigned long)(year / 4 - year / 100 + year / 400 + - 367 * mon / 12 + tm->mday) + - year * 365 - 719499; + 367 * mon / 12 + tm->mday) + + year * 365 - 719499; hours = days * 24 + tm->hour; return (hours * 60 + tm->min) * 60 + tm->sec; }
-void rtc_display(const struct rtc_time *tm) +void rtc_display(struct rtc_time *tm) { - printk(BIOS_INFO, "Date: %4d-%02d-%02d (%sday) Time: %2d:%02d:%02d\n", - tm->year, tm->mon, tm->mday, - (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], - tm->hour, tm->min, tm->sec); + (void)rtc_calc_weekday(tm); + printk(BIOS_INFO, "Current Date: %4d-%02d-%02d (%sday) " + "Time: %2d:%02d:%02d\n", + tm->year, tm->mon, tm->mday, + (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], + tm->hour, tm->min, tm->sec); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 1:
(22 comments)
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@49 PS1, Line 49: + (13 * m - 1) / 5 code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@49 PS1, Line 49: + (13 * m - 1) / 5 please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@50 PS1, Line 50: + D code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@50 PS1, Line 50: + D please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@51 PS1, Line 51: + (D >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@51 PS1, Line 51: + (D >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@52 PS1, Line 52: + (C >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@52 PS1, Line 52: + (C >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@53 PS1, Line 53: - 2 * C; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@53 PS1, Line 53: - 2 * C; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@54 PS1, Line 54: tm->wday = f % 7; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@54 PS1, Line 54: tm->wday = f % 7; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@55 PS1, Line 55: return 0; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@55 PS1, Line 55: return 0; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@133 PS1, Line 133: "Time: %2d:%02d:%02d\n", code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@133 PS1, Line 133: "Time: %2d:%02d:%02d\n", please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@134 PS1, Line 134: tm->year, tm->mon, tm->mday, code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@134 PS1, Line 134: tm->year, tm->mon, tm->mday, please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@135 PS1, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@135 PS1, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@136 PS1, Line 136: tm->hour, tm->min, tm->sec); code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/1/src/lib/rtc.c@136 PS1, Line 136: tm->hour, tm->min, tm->sec); please, no spaces at the start of a line
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#2).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 26 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 2:
(22 comments)
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@49 PS2, Line 49: + (13 * m - 1) / 5 code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@49 PS2, Line 49: + (13 * m - 1) / 5 please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@50 PS2, Line 50: + D code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@50 PS2, Line 50: + D please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@51 PS2, Line 51: + (D >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@51 PS2, Line 51: + (D >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@52 PS2, Line 52: + (C >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@52 PS2, Line 52: + (C >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@53 PS2, Line 53: - 2 * C; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@53 PS2, Line 53: - 2 * C; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@54 PS2, Line 54: tm->wday = f % 7; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@54 PS2, Line 54: tm->wday = f % 7; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@55 PS2, Line 55: return 0; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@55 PS2, Line 55: return 0; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@133 PS2, Line 133: "Time: %2d:%02d:%02d\n", code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@133 PS2, Line 133: "Time: %2d:%02d:%02d\n", please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@134 PS2, Line 134: tm->year, tm->mon, tm->mday, code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@134 PS2, Line 134: tm->year, tm->mon, tm->mday, please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@135 PS2, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@135 PS2, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@136 PS2, Line 136: tm->hour, tm->min, tm->sec); code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/2/src/lib/rtc.c@136 PS2, Line 136: tm->hour, tm->min, tm->sec); please, no spaces at the start of a line
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#3).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 26 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 3:
(22 comments)
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@49 PS3, Line 49: + (13 * m - 1) / 5 code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@49 PS3, Line 49: + (13 * m - 1) / 5 please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@50 PS3, Line 50: + D code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@50 PS3, Line 50: + D please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@51 PS3, Line 51: + (D >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@51 PS3, Line 51: + (D >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@52 PS3, Line 52: + (C >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@52 PS3, Line 52: + (C >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@53 PS3, Line 53: - 2 * C; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@53 PS3, Line 53: - 2 * C; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@54 PS3, Line 54: tm->wday = f % 7; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@54 PS3, Line 54: tm->wday = f % 7; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@55 PS3, Line 55: return 0; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@55 PS3, Line 55: return 0; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@133 PS3, Line 133: "Time: %2d:%02d:%02d\n", code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@133 PS3, Line 133: "Time: %2d:%02d:%02d\n", please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@134 PS3, Line 134: tm->year, tm->mon, tm->mday, code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@134 PS3, Line 134: tm->year, tm->mon, tm->mday, please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@135 PS3, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@135 PS3, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@136 PS3, Line 136: tm->hour, tm->min, tm->sec); code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/3/src/lib/rtc.c@136 PS3, Line 136: tm->hour, tm->min, tm->sec); please, no spaces at the start of a line
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#4).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 26 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 4:
(15 comments)
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@49 PS4, Line 49: + (13 * m - 1) / 5 code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@49 PS4, Line 49: + (13 * m - 1) / 5 please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@50 PS4, Line 50: + D code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@50 PS4, Line 50: + D please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@51 PS4, Line 51: + (D >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@51 PS4, Line 51: + (D >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@52 PS4, Line 52: + (C >> 2) code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@52 PS4, Line 52: + (C >> 2) please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@53 PS4, Line 53: - 2 * C; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@53 PS4, Line 53: - 2 * C; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@54 PS4, Line 54: tm->wday = f % 7; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@54 PS4, Line 54: tm->wday = f % 7; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@55 PS4, Line 55: return 0; code indent should use tabs where possible
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@55 PS4, Line 55: return 0; please, no spaces at the start of a line
https://review.coreboot.org/#/c/31557/4/src/lib/rtc.c@135 PS4, Line 135: (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], line over 80 characters
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#5).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 21 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/31557/5/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/5/src/lib/rtc.c@130 PS5, Line 130: (tm->wday < 0 || tm->wday > 6) trailing whitespace
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#6).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 21 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/6
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#7).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/include/rtc.h M src/lib/rtc.c 2 files changed, 22 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/7
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@47 PS7, Line 47: const int C = tm->year / 100; If I read the Wiki page right this also needs to be corrected for Jan/Feb? Maybe it would be best to define an "effective year" variable first and then use that to calculate D and C.
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@48 PS7, Line 48: D >> 2 Is there a good reason these are shifts? If the algorithm just requires you to integer divide by 4, just write / 4 to make it more readable.
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@126 PS7, Line 126: (void)rtc_calc_weekday(tm); I'm not sure we'd want to do this. This is just a display function, and struct rtc_time already has a wday member, so I think the expectation would be that this function displays whatever is in the struct (whether that's correct or not). Many RTCs actually keep a weekday counter in hardware (whether that's a good design or not is a different question, but it is what it is).
I think it would make more sense to just provide rtc_calc_weekday() as a function that RTC driver implementations can call if they know that their particular hardware doesn't track weekdays (or they don't want to rely on it).
If instead we really want to say that we don't want to rely on the weekdays tracked by hardware ever, then I think the correct change would be to remove the 'wday' member from struct rtc_time, so that this functions and all other users are forced to call rtc_calc_weekday() whenever they need the weekday for a particular date.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#8).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 27 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/8
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#9).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 27 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/9
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 9:
(3 comments)
Patch Set 1:
(22 comments)
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@47 PS7, Line 47: const int C = tm->year / 100;
If I read the Wiki page right this also needs to be corrected for Jan/Feb? Maybe it would be best to […]
Good catch! I did misread that (I guess that means I should put in some unit tests...).
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@48 PS7, Line 48: D >> 2
Is there a good reason these are shifts? If the algorithm just requires you to integer divide by 4, […]
Not particularly :) I guess this is my way of sussing out at what level the coreboot community cares about these sorts of low-level (potential) optimizations. Done.
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@126 PS7, Line 126: (void)rtc_calc_weekday(tm);
I'm not sure we'd want to do this. […]
Fair enough. My understanding was that many systems either a) incorrectly or b) don't bother to set the weekday register (source: https://wiki.osdev.org/CMOS#Weekday_Register). I noticed it was definitely set incorrectly on my system (2019-2-21 was marked as Saturday). It's completely redundant information to store in CMOS; but I suppose you're probably right that's the driver's issue to deal with. I'll correct that at least for the driver for the system I'm using.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/7/src/lib/rtc.c@48 PS7, Line 48: D >> 2
Not particularly :) I guess this is my way of sussing out at what level the coreboot community care […]
The compiler knows to turn / 4 into SHR $2... we're not in the '80s anymore. ;)
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@31 PS9, Line 31: "Satur", "Sun", "Mon", "Tues", "Wednes", "Thurs", "Fri" No, wait a second, you can't change this. It's a global convention that 0 is a Sunday. Other code accessing the RTC (e.g. Linux) will expect it that way. You can calculate with a Saturday-based week inside your function, but then you need to convert it when reading from or writing back to struct rtc_time.
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@99 PS9, Line 99: Is this tab supposed to get added here?
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@120 PS9, Line 120: No space after a cast, please.
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@134 PS9, Line 134: Current nit: Not sure if adding this here makes sense... someone could call the function with a struct rtc_time that wasn't just read from the RTC, after all.
Hello Simon Glass, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#10).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 24 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/10
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@31 PS9, Line 31: "Satur", "Sun", "Mon", "Tues", "Wednes", "Thurs", "Fri"
No, wait a second, you can't change this. It's a global convention that 0 is a Sunday. […]
Done
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@99 PS9, Line 99:
Is this tab supposed to get added here?
Done
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@120 PS9, Line 120:
No space after a cast, please.
Done
https://review.coreboot.org/#/c/31557/9/src/lib/rtc.c@134 PS9, Line 134: Current
nit: Not sure if adding this here makes sense... […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 10: Code-Review+2
(2 comments)
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c@120 PS10, Line 120: nit: Please no space after casts
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c@135 PS10, Line 135: "Time: %2d:%02d:%02d\n", nit: I don't think this line needs to be broken anymore?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/31557/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31557/10//COMMIT_MSG@12 PS10, Line 12: Please give the date, for which an incorrect weekday was calculated, so that both implementation can be tested.
Hello Simon Glass, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31557
to look at the new patch set (#11).
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/lib/rtc.c 1 file changed, 22 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31557/11
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 11: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/#/c/31557/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31557/10//COMMIT_MSG@12 PS10, Line 12:
Please give the date, for which an incorrect weekday was calculated, so that both implementation can […]
Sure! Today, February 26, 2019. I get Wednesday running the old algorithm, and Tuesday for Zeller's rule.
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c File src/lib/rtc.c:
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c@120 PS10, Line 120:
nit: Please no space after casts
Done
https://review.coreboot.org/#/c/31557/10/src/lib/rtc.c@135 PS10, Line 135: "Time: %2d:%02d:%02d\n",
nit: I don't think this line needs to be broken anymore?
Done
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31557 )
Change subject: rtc: Fix rtc_calc_weekday ......................................................................
rtc: Fix rtc_calc_weekday
This function appeared previously unused (called only from rtc_display, also unused), but it returned an incorrect weekday. Change the algorithm to use Zeller's Rule, a well-known algorithm for calculuating weekdays.
Change-Id: Ibce6822942f8d9d9f39c2b6065cd785dca9e8e09 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/31557 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/lib/rtc.c 1 file changed, 22 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/lib/rtc.c b/src/lib/rtc.c index c5c157f..3e4c3f7 100644 --- a/src/lib/rtc.c +++ b/src/lib/rtc.c @@ -27,57 +27,41 @@ #define DAYS_IN_YEAR(a) (LEAP_YEAR(a) ? 366 : 365) #define DAYS_IN_MONTH(a) (month_days[(a) - 1])
-static const int month_offset[] = { - 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334 -}; - static const char *const weekdays[] = { - "Sun", "Mon", "Tues", "Wednes", "Thurs", "Fri", "Satur", + "Sun", "Mon", "Tues", "Wednes", "Thurs", "Fri", "Satur" };
-static int leaps_to_year(int year) -{ - return year / 4 - year / 100 + year / 400; -} - -/* This only works for the Gregorian calendar after Jan 1 1971. */ +/* Zeller's rule */ static int rtc_calc_weekday(struct rtc_time *tm) { - int leaps_to_date; - int day; - if (tm->year < 1971) return -1;
- day = 4; /* Jan 1 1970 was a Thursday. */ - - /* Number of leap corrections to apply up to end of last year */ - leaps_to_date = leaps_to_year(tm->year - 1) - leaps_to_year(1970); + /* In Zeller's rule, January and February are treated as if they + are months 13 and 14 of the previous year (March is still month 3) */ + const int zyear = ((tm->mon < 3) ? tm->year - 1 : tm->year); + const int q = tm->mday; + const int m = (tm->mon < 3) ? tm->mon + 12 : tm->mon; + const int K = zyear % 100; + const int J = zyear / 100;
/* - * This year is a leap year if it is divisible by 4 except when it is - * divisible by 100 unless it is divisible by 400 - * - * e.g. 1904 was a leap year, 1900 was not, 1996 is, and 2000 is. + * Because of the way the modulo operator works with negative numbers, + * the traditional formulation of Zeller's rule must be modified + * slightly to make the numerator positive (i.e., add 5J instead of + * subtracting 2J). Also subtract 1 so that Sunday is day 0. */ - if ((tm->year % 4) && - ((tm->year % 100 != 0) || (tm->year % 400 == 0)) && - (tm->mon > 2)) { - /* We are past Feb. 29 in a leap year */ - day++; - } + const int h = (q + (13 * (m + 1)) / 5 + + K + (K / 4) + (J / 4) + (5 * J) - 1) % 7;
- day += (tm->year - 1970) * 365 + leaps_to_date + - month_offset[tm->mon-1] + tm->mday; - tm->wday = day % 7; - + tm->wday = h; return 0; }
int rtc_to_tm(int tim, struct rtc_time *tm) { int month_days[12] = { - 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 + 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }; register int i; register long hms, day; @@ -112,7 +96,7 @@
/* * Converts Gregorian date to seconds since 1970-01-01 00:00:00. - * Assumes input in normal date format, i.e. 1980-12-31 23:59:59 + * Assumes input in normal date format, i.e. 1980-12-31 23:59:59 * => year=1980, mon=12, day=31, hour=23, min=59, sec=59. * * [For the Julian calendar (which was used in Russia before 1917, @@ -130,7 +114,7 @@ { int mon = tm->mon; int year = tm->year; - int days, hours; + int days, hours;
mon -= 2; if (0 >= (int)mon) { /* 1..12 -> 11, 12, 1..10 */ @@ -139,15 +123,15 @@ }
days = (unsigned long)(year / 4 - year / 100 + year / 400 + - 367 * mon / 12 + tm->mday) + - year * 365 - 719499; + 367 * mon / 12 + tm->mday) + + year * 365 - 719499; hours = days * 24 + tm->hour; return (hours * 60 + tm->min) * 60 + tm->sec; }
void rtc_display(const struct rtc_time *tm) { - printk(BIOS_INFO, "Date: %4d-%02d-%02d (%sday) Time: %2d:%02d:%02d\n", + printk(BIOS_INFO, "Date: %5d-%02d-%02d (%sday) Time: %2d:%02d:%02d\n", tm->year, tm->mon, tm->mday, (tm->wday < 0 || tm->wday > 6) ? "unknown " : weekdays[tm->wday], tm->hour, tm->min, tm->sec);
-
build bot (Jenkins) (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)