From a5b4709c1bead5918d72e19e30ead2b8c09d6a11 Mon Sep 17 00:00:00 2001 From: Jeremy Bettis Date: Wed, 12 Jan 2022 12:08:45 -0700 Subject: zephyr: Change power_common tests to avoid tasks Change test_power_exit_hard_off and test_power_reboot_ap_at_g3 to run before ec_app_main, to avoid background tasks. Remove force_power_state from stubs. I plan to remove more code from stubs and replace it with real code from qcom.c, and force_power_state is one of the things that needs to be deleted. BRANCH=None BUG=b:214256453 TEST=zmake testall Change-Id: I022344b1352e0daebf3d12ba623bc78b79f6f2ff Signed-off-by: Jeremy Bettis Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3384478 Commit-Queue: Jeremy Bettis Tested-by: Jeremy Bettis Auto-Submit: Jeremy Bettis Reviewed-by: Aaron Massey Commit-Queue: Aaron Massey --- include/power.h | 8 +++ power/common.c | 12 ++++ zephyr/test/drivers/include/stubs.h | 11 ---- zephyr/test/drivers/src/main.c | 2 + zephyr/test/drivers/src/power_common.c | 107 +++++++++++++++------------------ zephyr/test/drivers/src/stubs.c | 23 ------- 6 files changed, 71 insertions(+), 92 deletions(-) diff --git a/include/power.h b/include/power.h index e12f97cdb2..7605a33a2c 100644 --- a/include/power.h +++ b/include/power.h @@ -379,4 +379,12 @@ __override_proto void board_power_5v_enable(int enable); */ void power_5v_enable(task_id_t tid, int enable); +#ifdef CONFIG_ZTEST +/** + * @brief Perform one state transition with power_common_state() as + * chipset_task() would. + */ +void test_power_common_state(void); +#endif + #endif /* __CROS_EC_POWER_H */ diff --git a/power/common.c b/power/common.c index 0285ca75a7..e63b324bf5 100644 --- a/power/common.c +++ b/power/common.c @@ -677,6 +677,18 @@ void chipset_exit_hard_off(void) task_wake(TASK_ID_CHIPSET); } +#ifdef CONFIG_ZTEST +void test_power_common_state(void) +{ + enum power_state new_state; + + task_wake(task_get_current()); + new_state = power_common_state(state); + if (new_state != state) + power_set_state(new_state); +} +#endif + /*****************************************************************************/ /* Task function */ diff --git a/zephyr/test/drivers/include/stubs.h b/zephyr/test/drivers/include/stubs.h index af32a988c6..fdbe13b864 100644 --- a/zephyr/test/drivers/include/stubs.h +++ b/zephyr/test/drivers/include/stubs.h @@ -14,17 +14,6 @@ enum usbc_port { USBC_PORT_C0 = 0, USBC_PORT_C1, USBC_PORT_COUNT }; /* Structure used by usb_mux test. It is part of usb_muxes chain. */ extern struct usb_mux usbc1_virtual_usb_mux; -/** - * @brief Set state which should be returned by power_handle_state() and wake - * chipset task to immediately change state - * - * @param force If true @p state will be used as return for power_handle_state() - * and will wake up chipset task. If false argument of - * power_handle_state() will be used as return value - * @param state Power state to use when @p force is true - */ -void force_power_state(bool force, enum power_state state); - /** * @brief Set product ID that should be returned by board_get_ps8xxx_product_id * diff --git a/zephyr/test/drivers/src/main.c b/zephyr/test/drivers/src/main.c index 687ea0785e..360079dfa9 100644 --- a/zephyr/test/drivers/src/main.c +++ b/zephyr/test/drivers/src/main.c @@ -32,10 +32,12 @@ extern void test_suite_tcpci(void); extern void test_suite_ps8xxx(void); extern void test_suite_integration_usb(void); extern void test_suite_power_common(void); +extern void test_suite_power_common_no_tasks(void); void test_main(void) { /* Test suites to run before ec_app_main.*/ + test_suite_power_common_no_tasks(); ec_app_main(); diff --git a/zephyr/test/drivers/src/power_common.c b/zephyr/test/drivers/src/power_common.c index 86e03ea38c..d355623fc6 100644 --- a/zephyr/test/drivers/src/power_common.c +++ b/zephyr/test/drivers/src/power_common.c @@ -17,6 +17,7 @@ #include "power.h" #include "stubs.h" #include "task.h" +#include "ec_tasks.h" #include "emul/emul_common_i2c.h" #include "emul/emul_smart_battery.h" @@ -174,67 +175,64 @@ static void test_power_chipset_in_or_transitioning_to_state(void) } } -/** Test using chipset_exit_hard_off() in different power states */ +/* Test using chipset_exit_hard_off() in different power states. The only + * way to test the value of want_g3_exit is to set the power state to G3 + * and then to see if test_power_common_state() transitions to G3S5 or not. + */ static void test_power_exit_hard_off(void) { /* Force initial state */ - force_power_state(true, POWER_G3); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3, power_get_state(), NULL); - /* Stop forcing state */ - force_power_state(false, 0); - /* Test after exit hard off, we reach G3S5 */ chipset_exit_hard_off(); - /* - * TODO(b/201420132) - chipset_exit_hard_off() is waking up - * TASK_ID_CHIPSET Sleep is required to run chipset task before - * continuing with test - */ - k_msleep(1); + test_power_common_state(); zassert_equal(POWER_G3S5, power_get_state(), NULL); /* Go back to G3 and check we stay there */ - force_power_state(true, POWER_G3); - force_power_state(false, 0); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3, power_get_state(), NULL); /* Exit G3 again */ chipset_exit_hard_off(); - /* TODO(b/201420132) - see comment above */ - k_msleep(1); + test_power_common_state(); zassert_equal(POWER_G3S5, power_get_state(), NULL); /* Go to S5G3 */ - force_power_state(true, POWER_S5G3); + power_set_state(POWER_S5G3); + test_power_common_state(); zassert_equal(POWER_S5G3, power_get_state(), NULL); - /* Test exit hard off in S5G3 -- should immedietly exit G3 */ + /* Test exit hard off in S5G3 -- should set want_g3_exit */ chipset_exit_hard_off(); /* Go back to G3 and check we exit it to G3S5 */ - force_power_state(true, POWER_G3); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3S5, power_get_state(), NULL); /* Test exit hard off is cleared on entering S5 */ chipset_exit_hard_off(); - force_power_state(true, POWER_S5); + power_set_state(POWER_S5); zassert_equal(POWER_S5, power_get_state(), NULL); + /* Go back to G3 and check we stay in G3 */ - force_power_state(true, POWER_G3); - force_power_state(false, 0); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3, power_get_state(), NULL); /* Test exit hard off doesn't work on other states */ - force_power_state(true, POWER_S5S3); - force_power_state(false, 0); + power_set_state(POWER_S5S3); + test_power_common_state(); zassert_equal(POWER_S5S3, power_get_state(), NULL); chipset_exit_hard_off(); - /* TODO(b/201420132) - see comment above */ - k_msleep(1); + test_power_common_state(); /* Go back to G3 and check we stay in G3 */ - force_power_state(true, POWER_G3); - force_power_state(false, 0); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3, power_get_state(), NULL); } @@ -249,18 +247,20 @@ static void test_power_reboot_ap_at_g3(void) .params = ¶ms, .params_size = sizeof(params), }; - int offset_for_still_in_g3_test; int delay_ms; + int64_t before_time; /* Force initial state S0 */ - force_power_state(true, POWER_S0); + power_set_state(POWER_S0); + test_power_common_state(); zassert_equal(POWER_S0, power_get_state(), NULL); /* Test version 0 (no delay argument) */ zassert_equal(EC_RES_SUCCESS, host_command_process(&args), NULL); /* Go to G3 and check if reboot is triggered */ - force_power_state(true, POWER_G3); + power_set_state(POWER_G3); + test_power_common_state(); zassert_equal(POWER_G3S5, power_get_state(), NULL); /* Test version 1 (with delay argument) */ @@ -270,24 +270,10 @@ static void test_power_reboot_ap_at_g3(void) zassert_equal(EC_RES_SUCCESS, host_command_process(&args), NULL); /* Go to G3 and check if reboot is triggered after delay */ - force_power_state(true, POWER_G3); - force_power_state(false, 0); - zassert_equal(POWER_G3, power_get_state(), NULL); - /* - * Arbitrary chosen offset before end of reboot delay to check if G3 - * state wasn't left too soon - */ - offset_for_still_in_g3_test = 50; - k_msleep(delay_ms - offset_for_still_in_g3_test); - /* Test if still in G3 */ - zassert_equal(POWER_G3, power_get_state(), NULL); - /* - * power_common_state() use for loop with 100ms sleeps. msleep() wait at - * least specified time, so wait 10% longer than specified delay to take - * this into account. - */ - k_msleep(offset_for_still_in_g3_test + delay_ms / 10); - /* Test if reboot is triggered */ + power_set_state(POWER_G3); + before_time = k_uptime_get(); + test_power_common_state(); + zassert_true(k_uptime_delta(&before_time) > 3000, NULL); zassert_equal(POWER_G3S5, power_get_state(), NULL); } @@ -481,12 +467,9 @@ static void setup_hibernation_delay(void) bat->volt = battery_get_info()->voltage_normal; /* Force initial state */ - force_power_state(true, POWER_G3); + power_set_state(POWER_G3); zassert_equal(POWER_G3, power_get_state(), NULL); - /* Stop forcing state */ - force_power_state(false, 0); - /* Disable AC */ zassert_ok(gpio_emul_input_set(acok_dev, GPIO_ACOK_OD_PIN, 0), NULL); msleep(CONFIG_EXTPOWER_DEBOUNCE_MS + 1); @@ -603,12 +586,9 @@ static void test_power_hc_hibernation_delay(void) zassert_equal(0, extpower_is_present(), NULL); /* Go to different state */ - force_power_state(true, POWER_G3S5); + power_set_state(POWER_G3S5); zassert_equal(POWER_G3S5, power_get_state(), NULL); - /* Stop forcing state */ - force_power_state(false, 0); - /* Get hibernate delay */ params.seconds = 0; zassert_equal(EC_RES_SUCCESS, host_command_process(&args), NULL); @@ -657,14 +637,25 @@ static void test_power_cmd_hibernation_delay(void) system_hibernate_fake.call_count); } +void test_suite_power_common_no_tasks(void) +{ + ztest_test_suite( + power_common_no_tasks, + ztest_unit_test_setup_teardown(test_power_exit_hard_off, + set_test_runner_tid, + unit_test_noop), + ztest_unit_test_setup_teardown(test_power_reboot_ap_at_g3, + set_test_runner_tid, + unit_test_noop)); + ztest_run_test_suite(power_common_no_tasks); +} + void test_suite_power_common(void) { ztest_test_suite(power_common, ztest_unit_test(test_power_chipset_in_state), ztest_unit_test( test_power_chipset_in_or_transitioning_to_state), - ztest_unit_test(test_power_exit_hard_off), - ztest_unit_test(test_power_reboot_ap_at_g3), ztest_unit_test(test_power_hc_smart_discharge), ztest_unit_test(test_power_board_system_is_idle), ztest_unit_test_setup_teardown( diff --git a/zephyr/test/drivers/src/stubs.c b/zephyr/test/drivers/src/stubs.c index 85c528a5bd..9ebf768098 100644 --- a/zephyr/test/drivers/src/stubs.c +++ b/zephyr/test/drivers/src/stubs.c @@ -330,25 +330,6 @@ enum power_state power_chipset_init(void) return POWER_G3; } -static enum power_state forced_state; -static bool force_state; - -void force_power_state(bool force, enum power_state state) -{ - forced_state = state; - force_state = force; - - if (force) { - task_wake(TASK_ID_CHIPSET); - /* - * TODO(b/201420132) - setting power state requires to wake up - * TASK_ID_CHIPSET Sleep is required to run chipset task before - * continuing with test - */ - k_msleep(1); - } -} - enum power_state power_handle_state(enum power_state state) { switch (state) { @@ -372,10 +353,6 @@ enum power_state power_handle_state(enum power_state state) break; } - if (force_state) { - state = forced_state; - } - return state; } -- cgit v1.2.1