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.