Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31770
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
ec/google/chromeec: Synchronize AP RTC to the EC.
In google_chromeec_init(), if the AP is set as the RTC source for the system, then send current RTC timestamp over to the EC for synchronization.
BUG=b:111675966 BRANCH=none TEST=Tested with EC changes from b87515e21
Change-Id: Ib081aee5d10ec64dac79fc0ebf3fc39eab380a7f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/ec/google/chromeec/ec_commands.h 3 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31770/1
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 70b3f49..317cede 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -1076,6 +1076,35 @@
static int ec_image_type; /* Cached EC image type (ro or rw). */
+#if !IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC_RTC) +int google_chromeec_sync_rtc(void) +{ + struct chromeec_command cec_cmd; + struct ec_params_sync_rtc cmd_sync; + struct rtc_time tm; + int rtc_time_sec; + + if (rtc_get(&tm)) + { + return 1; + } + + rtc_time_sec = rtc_mktime(&tm); + + cec_cmd.cmd_code = EC_CMD_SYNC_RTC; + cec_cmd.cmd_version = 0; + cmd_sync.rtc_time_sec = (uint32_t)rtc_time_sec; + cec_cmd.cmd_data_in = &cmd_sync; + cec_cmd.cmd_data_out = NULL; + cec_cmd.cmd_size_in = sizeof(cmd_sync); + cec_cmd.cmd_size_out = 0; + cec_cmd.cmd_dev_index = 0; + google_chromeec_command(&cec_cmd); + printk(BIOS_WARNING, "Google Chrome synced RTC time; %lu\n", (unsigned long)rtc_time_sec); + return cec_cmd.cmd_code; +} +#endif /* !IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC_RTC) */ + void google_chromeec_init(void) { struct chromeec_command cec_cmd; @@ -1103,6 +1132,9 @@ ec_image_type = cec_resp.current_image; }
+#if !IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC_RTC) + google_chromeec_sync_rtc(); +#endif google_chromeec_log_uptimeinfo(); }
diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index 7a38336..75a441c 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -38,6 +38,7 @@ void google_chromeec_init(void); int google_chromeec_pd_get_amode(uint16_t svid); int google_chromeec_wait_for_displayport(long timeout); +int google_chromeec_sync_rtc(void);
/* Device events */ uint64_t google_chromeec_get_device_enabled_events(void); diff --git a/src/ec/google/chromeec/ec_commands.h b/src/ec/google/chromeec/ec_commands.h index 140ba5a..ffc3a90 100644 --- a/src/ec/google/chromeec/ec_commands.h +++ b/src/ec/google/chromeec/ec_commands.h @@ -5125,6 +5125,15 @@ uint8_t allow_charging; };
+/* + * Sync RTC information to the EC + */ +#define EC_CMD_SYNC_RTC 0x0603 + +struct __ec_align4 ec_params_sync_rtc { + uint32_t rtc_time_sec; +}; + /*****************************************************************************/ /* * Reserve a range of host commands for board-specific, experimental, or
build bot (Jenkins) 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 1:
(2 comments)
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c@1087 PS1, Line 1087: if (rtc_get(&tm)) that open brace { should be on the previous line
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c@1103 PS1, Line 1103: printk(BIOS_WARNING, "Google Chrome synced RTC time; %lu\n", (unsigned long)rtc_time_sec); line over 96 characters
Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/31770 )
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
ec/google/chromeec: Synchronize AP RTC to the EC.
In google_chromeec_init(), if the AP is set as the RTC source for the system, then send current RTC timestamp over to the EC for synchronization.
BUG=b:111675966 BRANCH=none TEST=Tested with EC changes from b87515e21
Change-Id: Ib081aee5d10ec64dac79fc0ebf3fc39eab380a7f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/ec/google/chromeec/ec_commands.h 3 files changed, 40 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31770/2
build bot (Jenkins) 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 2:
(1 comment)
https://review.coreboot.org/#/c/31770/2/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31770/2/src/ec/google/chromeec/ec.c@1087 PS2, Line 1087: if (rtc_get(&tm)) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31770
to look at the new patch set (#3).
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
ec/google/chromeec: Synchronize AP RTC to the EC.
In google_chromeec_init(), if the AP is set as the RTC source for the system, then send current RTC timestamp over to the EC for synchronization.
BUG=b:111675966 BRANCH=none TEST=Tested with EC changes from b87515e21
Change-Id: Ib081aee5d10ec64dac79fc0ebf3fc39eab380a7f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/ec/google/chromeec/ec_commands.h 3 files changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31770/3
Jett Rink 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: Code-Review+1
Tim Wawrzynczak 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:
(3 comments)
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c@1087 PS1, Line 1087: if (rtc_get(&tm))
that open brace { should be on the previous line
Done
https://review.coreboot.org/#/c/31770/1/src/ec/google/chromeec/ec.c@1103 PS1, Line 1103: printk(BIOS_WARNING, "Google Chrome synced RTC time; %lu\n", (unsigned long)rtc_time_sec);
line over 96 characters
Done
https://review.coreboot.org/#/c/31770/2/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31770/2/src/ec/google/chromeec/ec.c@1087 PS2, Line 1087: if (rtc_get(&tm)) {
braces {} are not necessary for single statement blocks
Done
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... ;)
Hello Julius Werner, Jett Rink, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31770
to look at the new patch set (#4).
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
ec/google/chromeec: Synchronize AP RTC to the EC.
In google_chromeec_init(), if the AP is set as the RTC source for the system, then send current RTC timestamp over to the EC for synchronization.
BUG=b:111675966 BRANCH=none TEST=Tested with EC changes from b87515e21
Change-Id: Ib081aee5d10ec64dac79fc0ebf3fc39eab380a7f Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/ec/google/chromeec/ec_commands.h 3 files changed, 37 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31770/4
Tim Wawrzynczak 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 4:
(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 […]
Done
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 […]
Future proof FTW!
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 4: Code-Review+1
LGTM but this needs to wait until the appropriate EC patch has landed. (Also, make sure that you copy the whole ec_commands.h file directly from the EC repository, and don't just manually add to the potentially stale coreboot copy.)
Tim Wawrzynczak has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31770 )
Change subject: ec/google/chromeec: Synchronize AP RTC to the EC. ......................................................................
Abandoned
-
build bot (Jenkins) (Code Review) -
Jett Rink (Code Review)
-
Julius Werner (Code Review)
-
Tim Wawrzynczak (Code Review)