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.