From a63d9b0134000bff51dd3eaeca6898f1f671d768 Mon Sep 17 00:00:00 2001 From: Diana Z Date: Wed, 15 Mar 2023 15:50:45 -0600 Subject: Host sleep: Consider a reboot after suspend to be a resume When we try to wake the host during a failed resume, the host may reboot rather than actually following the resume path. In this case, we will receive events of "0" indicating we're on a fresh boot and the chipset RESUME hooks never run. Instead, we should treat this new boot as a resume in order to ensure the previous SUSPEND hooks get a RESUME call to go with them. BRANCH=None BUG=b:273327518 TEST=on whiterun, run suspend with bad AP FW version and ensure the backlight turns on after we wake the host Change-Id: I9c8e7ad70dbca5245844a31772e99097256e592f Signed-off-by: Diana Z Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4344029 Reviewed-by: Wai-Hong Tam Commit-Queue: Wai-Hong Tam --- power/host_sleep.c | 7 ++ .../power_host_sleep/src/test_power_host_sleep.c | 91 ++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/power/host_sleep.c b/power/host_sleep.c index 7dc83ca8e9..b0b8b624e3 100644 --- a/power/host_sleep.c +++ b/power/host_sleep.c @@ -36,6 +36,13 @@ host_command_host_sleep_event(struct host_cmd_handler_args *args) struct host_sleep_event_context ctx; enum host_sleep_event state = p->sleep_event; + /* + * Treat a reboot after suspend as a resume for notification purposes + * (see b/273327518 for more details) + */ + if (host_sleep_state == HOST_SLEEP_EVENT_S0IX_SUSPEND && state == 0) + state = HOST_SLEEP_EVENT_S0IX_RESUME; + host_sleep_state = state; ctx.sleep_transitions = 0; switch (state) { diff --git a/zephyr/test/drivers/power_host_sleep/src/test_power_host_sleep.c b/zephyr/test/drivers/power_host_sleep/src/test_power_host_sleep.c index 616951c558..28617e2d59 100644 --- a/zephyr/test/drivers/power_host_sleep/src/test_power_host_sleep.c +++ b/zephyr/test/drivers/power_host_sleep/src/test_power_host_sleep.c @@ -256,6 +256,97 @@ ZTEST(power_host_sleep, test_suspend_then_resume_with_timeout) zassert_true(context.sleep_transitions & EC_HOST_RESUME_SLEEP_TIMEOUT); } +ZTEST(power_host_sleep, test_suspend_then_resume_with_reboot) +{ + struct ec_params_host_sleep_event_v1 p; + struct ec_response_host_sleep_event_v1 r; + struct host_cmd_handler_args args; + struct host_sleep_event_context context = { + .sleep_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT, + .sleep_transitions = 0. + }; + + /* Start then suspend process like the OS would */ + p.sleep_event = HOST_SLEEP_EVENT_S0IX_SUSPEND; + zassert_ok(ec_cmd_host_sleep_event_v1(&args, &p, &r)); + + /* Verify we notified the chipset */ + zassert_equal(power_chipset_handle_host_sleep_event_fake.call_count, 1); + zassert_equal(power_chipset_handle_host_sleep_event_fake.arg0_val, + p.sleep_event); + + /* Now kick the internals as if we suspend and then fail to resume */ + sleep_start_suspend(&context); + /* Register the suspend transition (cancels timeout hook) */ + sleep_suspend_transition(); + k_sleep(K_MSEC(CONFIG_SLEEP_TIMEOUT_MS * 2)); + + /* No timeout hooks should've fired */ + zassert_equal(power_chipset_handle_sleep_hang_fake.call_count, 0); + zassert_equal(power_board_handle_sleep_hang_fake.call_count, 0); + + /* Transition to resume state and wait for hang timeout */ + sleep_resume_transition(); + k_sleep(K_MSEC(CONFIG_SLEEP_TIMEOUT_MS * 2)); + + /* Resume state transition timeout hook should've fired */ + zassert_equal(power_chipset_handle_sleep_hang_fake.call_count, 1); + zassert_equal(power_board_handle_sleep_hang_fake.call_count, 1); + zassert_equal(power_chipset_handle_sleep_hang_fake.arg0_val, + SLEEP_HANG_S0IX_RESUME); + zassert_equal(power_board_handle_sleep_hang_fake.arg0_val, + SLEEP_HANG_S0IX_RESUME); + + /* But now the OS says it's actually rebooted */ + p.sleep_event = 0; + zassert_ok(ec_cmd_host_sleep_event_v1(&args, &p, &r)); + + /* Verify we alerted as if this was a resume */ + zassert_equal(power_chipset_handle_host_sleep_event_fake.call_count, 2); + zassert_equal(power_chipset_handle_host_sleep_event_fake.arg0_val, + HOST_SLEEP_EVENT_S0IX_RESUME); +} + +ZTEST(power_host_sleep, test_suspend_then_reboot) +{ + struct ec_params_host_sleep_event_v1 p; + struct ec_response_host_sleep_event_v1 r; + struct host_cmd_handler_args args; + struct host_sleep_event_context context = { + .sleep_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT, + .sleep_transitions = 0. + }; + + /* Start then suspend process like the OS would */ + p.sleep_event = HOST_SLEEP_EVENT_S0IX_SUSPEND; + zassert_ok(ec_cmd_host_sleep_event_v1(&args, &p, &r)); + + /* Verify we notified the chipset */ + zassert_equal(power_chipset_handle_host_sleep_event_fake.call_count, 1); + zassert_equal(power_chipset_handle_host_sleep_event_fake.arg0_val, + p.sleep_event); + + /* Now kick the internals as if we suspend and then fail to resume */ + sleep_start_suspend(&context); + /* Register the suspend transition (cancels timeout hook) */ + sleep_suspend_transition(); + k_sleep(K_MSEC(CONFIG_SLEEP_TIMEOUT_MS * 2)); + + /* No timeout hooks should've fired */ + zassert_equal(power_chipset_handle_sleep_hang_fake.call_count, 0); + zassert_equal(power_board_handle_sleep_hang_fake.call_count, 0); + + /* Transition to resume state and then send that we rebooted instead */ + sleep_resume_transition(); + p.sleep_event = 0; + zassert_ok(ec_cmd_host_sleep_event_v1(&args, &p, &r)); + + /* Verify we alerted as if this was a resume */ + zassert_equal(power_chipset_handle_host_sleep_event_fake.call_count, 2); + zassert_equal(power_chipset_handle_host_sleep_event_fake.arg0_val, + HOST_SLEEP_EVENT_S0IX_RESUME); +} + /* Only used in test_sleep_set_notify */ static bool _test_host_sleep_hook_called; -- cgit v1.2.1