Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31770 )
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/31770/3/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31770/3/src/ec/google/chromeec/ec.c@1132 PS3, Line 1132: #if !IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC_RTC) You don't need to use preprocessor #if here, just use a normal C if() (and don't guard the function definition itself). The compiler will know how to eliminate it if it's not configured in.
https://review.coreboot.org/#/c/31770/3/src/ec/google/chromeec/ec_commands.h File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/#/c/31770/3/src/ec/google/chromeec/ec_commands.h... PS3, Line 5134: uint32_t rtc_time_sec; I assume this is seconds since epoch? Maybe call it epoch_sec to be a little more clear? Also maybe better to use a uint64_t right away, we don't want to have to do this all over again in 2038... ;)