From d6fbd393dfc1fa204d6c3f4afeae2cb705c5ce94 Mon Sep 17 00:00:00 2001 From: Fabio Baltieri Date: Wed, 13 Apr 2022 14:08:28 +0000 Subject: zephyr: shimmed_task: define shimmed tasks priority separately Decouple the shimmed task priorities from the actual task definition. This adds a place for defining extra priorities for tasks that are not meant to have the shim support structures. Also add the missing USB_CHG_P3 task. BRANCH=none BUG=none TEST=kernel threads shell command (brya) Signed-off-by: Fabio Baltieri Change-Id: I30c878b58369ea483b723f381f1ae5606a824a63 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3581776 Reviewed-by: Keith Short --- zephyr/app/ec/Kconfig | 2 +- zephyr/shim/include/shimmed_task_id.h | 118 ++++++++++++++++++++++++--------- zephyr/shim/src/tasks.c | 31 +++++---- zephyr/test/tasks/shimmed_test_tasks.h | 6 +- 4 files changed, 109 insertions(+), 48 deletions(-) diff --git a/zephyr/app/ec/Kconfig b/zephyr/app/ec/Kconfig index d90e22328d..ae126d4049 100644 --- a/zephyr/app/ec/Kconfig +++ b/zephyr/app/ec/Kconfig @@ -24,7 +24,7 @@ orsource "chip/$(ARCH)/*/Kconfig.*" # Override the maximum number of preemptive priority levels. # config NUM_PREEMPT_PRIORITIES - default 20 + default 24 config LTO bool "Link Time Optimization (LTO)" diff --git a/zephyr/shim/include/shimmed_task_id.h b/zephyr/shim/include/shimmed_task_id.h index 42fbf7bc28..62f3058a56 100644 --- a/zephyr/shim/include/shimmed_task_id.h +++ b/zephyr/shim/include/shimmed_task_id.h @@ -27,92 +27,150 @@ typedef uint8_t task_id_t; #define HAS_CONSOLE_STUB_TASK 1 #endif +/* Highest priority on bottom -- same as in platform/ec. */ +enum { + EC_TASK_PRIO_LOWEST = 0, + EC_TASK_CHG_RAMP_PRIO = EC_TASK_PRIO_LOWEST, + EC_TASK_USB_CHG_P0_PRIO, + EC_TASK_USB_CHG_P1_PRIO, + EC_TASK_USB_CHG_P2_PRIO, + EC_TASK_USB_CHG_P3_PRIO, + EC_TASK_DPS_PRIO, + EC_TASK_CHARGER_PRIO, + EC_TASK_CHIPSET_PRIO, + EC_TASK_MOTIONSENSE_PRIO, + EC_TASK_HOSTCMD_PRIO, + EC_TASK_SHELL_STUB_PRIO, + EC_TASK_KEYPROTO_PRIO, + EC_TASK_POWERBTN_PRIO, + EC_TASK_KEYSCAN_PRIO, + EC_TASK_PD_C0_PRIO, + EC_TASK_PD_C1_PRIO, + EC_TASK_PD_C2_PRIO, + EC_TASK_PD_C3_PRIO, + EC_TASK_PD_INT_SHARED_PRIO, + EC_TASK_PD_INT_C0_PRIO, + EC_TASK_PD_INT_C1_PRIO, + EC_TASK_PD_INT_C2_PRIO, + EC_TASK_PD_INT_C3_PRIO, + EC_TASK_USB_MUX_PRIO, + EC_TASK_PRIO_COUNT, +}; + +/* Helper macro to set tasks priorities */ +#define EC_TASK_PRIORITY(prio) K_PRIO_PREEMPT(EC_TASK_PRIO_COUNT - prio - 1) + /* - * Highest priority on bottom -- same as in platform/ec. List of CROS_EC_TASK - * items. See CONFIG_TASK_LIST in platform/ec's config.h for more information. - * For tests that want their own custom tasks, use CONFIG_HAS_TEST_TASKS and not - * CONFIG_SHIMMED_TASKS. + * List of CROS_EC_TASK items. See CONFIG_TASK_LIST in platform/ec's config.h + * for more information. For tests that want their own custom tasks, use + * CONFIG_HAS_TEST_TASKS and not CONFIG_SHIMMED_TASKS. */ #ifdef CONFIG_SHIMMED_TASKS #define CROS_EC_TASK_LIST \ COND_CODE_1(HAS_TASK_CHG_RAMP, \ (CROS_EC_TASK(CHG_RAMP, chg_ramp_task, 0, \ - CONFIG_TASK_CHG_RAMP_STACK_SIZE)), ()) \ + CONFIG_TASK_CHG_RAMP_STACK_SIZE, \ + EC_TASK_CHG_RAMP_PRIO)), ()) \ COND_CODE_1(HAS_TASK_USB_CHG_P0, \ (CROS_EC_TASK(USB_CHG_P0, usb_charger_task, 0, \ - CONFIG_TASK_USB_CHG_STACK_SIZE)), ()) \ + CONFIG_TASK_USB_CHG_STACK_SIZE, \ + EC_TASK_USB_CHG_P0_PRIO)), ()) \ COND_CODE_1(HAS_TASK_USB_CHG_P1, \ (CROS_EC_TASK(USB_CHG_P1, usb_charger_task, 0, \ - CONFIG_TASK_USB_CHG_STACK_SIZE)), ()) \ + CONFIG_TASK_USB_CHG_STACK_SIZE, \ + EC_TASK_USB_CHG_P1_PRIO)), ()) \ COND_CODE_1(HAS_TASK_USB_CHG_P2, \ (CROS_EC_TASK(USB_CHG_P2, usb_charger_task, 0, \ - CONFIG_TASK_USB_CHG_STACK_SIZE)), ()) \ + CONFIG_TASK_USB_CHG_STACK_SIZE, \ + EC_TASK_USB_CHG_P2_PRIO)), ()) \ + COND_CODE_1(HAS_TASK_USB_CHG_P3, \ + (CROS_EC_TASK(USB_CHG_P3, usb_charger_task, 0, \ + CONFIG_TASK_USB_CHG_STACK_SIZE, \ + EC_TASK_USB_CHG_P3_PRIO)), ()) \ COND_CODE_1(HAS_TASK_DPS, \ (CROS_EC_TASK(DPS, dps_task, 0, \ - CONFIG_TASK_DPS_STACK_SIZE)), ()) \ + CONFIG_TASK_DPS_STACK_SIZE, \ + EC_TASK_DPS_PRIO)), ()) \ COND_CODE_1(HAS_TASK_CHARGER, \ (CROS_EC_TASK(CHARGER, charger_task, 0, \ - CONFIG_TASK_CHARGER_STACK_SIZE)), ()) \ + CONFIG_TASK_CHARGER_STACK_SIZE, \ + EC_TASK_CHARGER_PRIO)), ()) \ COND_CODE_1(HAS_TASK_CHIPSET, \ (CROS_EC_TASK(CHIPSET, chipset_task, 0, \ - CONFIG_TASK_CHIPSET_STACK_SIZE)), ()) \ - COND_CODE_1(HAS_TASK_MOTIONSENSE, \ - (CROS_EC_TASK(MOTIONSENSE, motion_sense_task, 0, \ - CONFIG_TASK_MOTIONSENSE_STACK_SIZE)), ()) \ + CONFIG_TASK_CHIPSET_STACK_SIZE, \ + EC_TASK_CHIPSET_PRIO)), ()) \ + COND_CODE_1(HAS_TASK_MOTIONSENSE, \ + (CROS_EC_TASK(MOTIONSENSE, motion_sense_task, 0, \ + CONFIG_TASK_MOTIONSENSE_STACK_SIZE, \ + EC_TASK_MOTIONSENSE_PRIO)), ()) \ COND_CODE_1(HAS_TASK_HOSTCMD, \ (CROS_EC_TASK(HOSTCMD, host_command_task, 0, \ - CONFIG_TASK_HOSTCMD_STACK_SIZE)), ()) \ + CONFIG_TASK_HOSTCMD_STACK_SIZE, \ + EC_TASK_HOSTCMD_PRIO)), ()) \ /* Placeholder to set the shell task priority */ \ COND_CODE_1(HAS_CONSOLE_STUB_TASK, \ (CROS_EC_TASK(CONSOLE_STUB, console_task_nop, 0, \ - 0)), ()) \ + 0, EC_TASK_SHELL_STUB_PRIO)), ()) \ COND_CODE_1(HAS_TASK_KEYPROTO, \ (CROS_EC_TASK(KEYPROTO, keyboard_protocol_task, 0, \ - CONFIG_TASK_KEYPROTO_STACK_SIZE)), ()) \ + CONFIG_TASK_KEYPROTO_STACK_SIZE, \ + EC_TASK_KEYPROTO_PRIO)), ()) \ COND_CODE_1(HAS_TASK_POWERBTN, \ (CROS_EC_TASK(POWERBTN, power_button_task, 0, \ - CONFIG_TASK_POWERBTN_STACK_SIZE)), ()) \ + CONFIG_TASK_POWERBTN_STACK_SIZE, \ + EC_TASK_POWERBTN_PRIO)), ()) \ COND_CODE_1(HAS_TASK_KEYSCAN, \ (CROS_EC_TASK(KEYSCAN, keyboard_scan_task, 0, \ - CONFIG_TASK_KEYSCAN_STACK_SIZE)), ()) \ + CONFIG_TASK_KEYSCAN_STACK_SIZE, \ + EC_TASK_KEYSCAN_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_C0, \ (CROS_EC_TASK(PD_C0, pd_task, 0, \ - CONFIG_TASK_PD_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_STACK_SIZE, \ + EC_TASK_PD_C0_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_C1, \ (CROS_EC_TASK(PD_C1, pd_task, 0, \ - CONFIG_TASK_PD_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_STACK_SIZE, \ + EC_TASK_PD_C1_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_C2, \ (CROS_EC_TASK(PD_C2, pd_task, 0, \ - CONFIG_TASK_PD_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_STACK_SIZE, \ + EC_TASK_PD_C2_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_C3, \ (CROS_EC_TASK(PD_C3, pd_task, 0, \ - CONFIG_TASK_PD_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_STACK_SIZE, \ + EC_TASK_PD_C3_PRIO)), ()) \ IF_ENABLED(CONFIG_HAS_TASK_PD_INT_SHARED, \ (CROS_EC_TASK(PD_INT_SHARED, pd_shared_alert_task, \ PD_INT_SHARED_PORT_MASK, \ - CONFIG_TASK_PD_INT_STACK_SIZE))) \ + CONFIG_TASK_PD_INT_STACK_SIZE, \ + EC_TASK_PD_INT_SHARED_PRIO))) \ COND_CODE_1(HAS_TASK_PD_INT_C0, \ (CROS_EC_TASK(PD_INT_C0, pd_interrupt_handler_task, 0, \ - CONFIG_TASK_PD_INT_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_INT_STACK_SIZE, \ + EC_TASK_PD_INT_C0_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_INT_C1, \ (CROS_EC_TASK(PD_INT_C1, pd_interrupt_handler_task, 1, \ - CONFIG_TASK_PD_INT_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_INT_STACK_SIZE, \ + EC_TASK_PD_INT_C1_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_INT_C2, \ (CROS_EC_TASK(PD_INT_C2, pd_interrupt_handler_task, 2, \ - CONFIG_TASK_PD_INT_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_INT_STACK_SIZE, \ + EC_TASK_PD_INT_C2_PRIO)), ()) \ COND_CODE_1(HAS_TASK_PD_INT_C3, \ (CROS_EC_TASK(PD_INT_C3, pd_interrupt_handler_task, 3, \ - CONFIG_TASK_PD_INT_STACK_SIZE)), ()) \ + CONFIG_TASK_PD_INT_STACK_SIZE, \ + EC_TASK_PD_INT_C3_PRIO)), ()) \ IF_ENABLED(HAS_TASK_USB_MUX, \ (CROS_EC_TASK(USB_MUX, usb_mux_task, 0, \ - CONFIG_TASK_USB_MUX_STACK_SIZE))) + CONFIG_TASK_USB_MUX_STACK_SIZE, \ + EC_TASK_USB_MUX_PRIO))) #elif defined(CONFIG_HAS_TEST_TASKS) #include "shimmed_test_tasks.h" /* * There are two different ways to define a task list (because historical * reasons). Applications use CROS_EC_TASK_LIST to define their tasks, while * unit tests that need additional tasks use CONFIG_TEST_TASK_LIST. For - * shimming a unit test, define CROS_EC_TASk_LIST as whatever + * shimming a unit test, define CROS_EC_TASK_LIST as whatever * CONFIG_TEST_TASK_LIST expands to. */ #if defined(CONFIG_TEST_TASK_LIST) && !defined(CROS_EC_TASK_LIST) diff --git a/zephyr/shim/src/tasks.c b/zephyr/shim/src/tasks.c index 7c0d758d66..fc4ad01f02 100644 --- a/zephyr/shim/src/tasks.c +++ b/zephyr/shim/src/tasks.c @@ -13,12 +13,20 @@ #include "timer.h" #include "task.h" -/* We need to ensure that is one lower priority for the deferred task */ -BUILD_ASSERT(CONFIG_NUM_PREEMPT_PRIORITIES + 1 >= TASK_ID_COUNT, +/* Ensure that there are enough priorities for all the EC tasks plus the + * sysworkq, which is used to handle HOOK_DEFERRED calls. + */ +BUILD_ASSERT(CONFIG_NUM_PREEMPT_PRIORITIES + 1 >= EC_TASK_PRIO_COUNT, "Must increase number of available preempt priorities"); +/* Ensure that the idle task is at lower priority than lowest priority task. */ +BUILD_ASSERT(EC_TASK_PRIORITY(EC_TASK_PRIO_LOWEST) < K_IDLE_PRIO, + "CONFIG_NUM_PREEMPT_PRIORITIES too small, some tasks would run at " + "idle priority"); + + /* Declare all task stacks here */ -#define CROS_EC_TASK(name, e, p, size) \ +#define CROS_EC_TASK(name, e, p, size, pr) \ K_THREAD_STACK_DEFINE(name##_STACK, size); #define TASK_TEST(name, e, p, size) CROS_EC_TASK(name, e, p, size) CROS_EC_TASK_LIST @@ -46,6 +54,8 @@ struct task_ctx_cfg { void (*entry)(void *p); /** The parameter that is passed into the task entry point */ intptr_t parameter; + /** The task priority */ + int priority; }; struct task_ctx_base_data { @@ -64,12 +74,13 @@ struct task_ctx_data { struct task_ctx_base_data base; }; -#define CROS_EC_TASK(_name, _entry, _parameter, _size) \ +#define CROS_EC_TASK(_name, _entry, _parameter, _size, _prio) \ { \ .entry = _entry, \ .parameter = _parameter, \ .stack = _name##_STACK, \ .stack_size = _size, \ + .priority = EC_TASK_PRIORITY(_prio), \ COND_CODE_1(CONFIG_THREAD_NAME, (.name = #_name,), ()) \ }, #define TASK_TEST(_name, _entry, _parameter, _size) \ @@ -301,14 +312,8 @@ ZTEST_RULE(set_test_runner_tid, set_test_runner_tid_rule_before, NULL); #endif /* CONFIG_SET_TEST_RUNNER_TID_RULE */ #endif /* TEST_BUILD */ -BUILD_ASSERT((K_PRIO_PREEMPT(TASK_ID_COUNT - 1) < K_IDLE_PRIO), - "CONFIG_NUM_PREEMPT_PRIORITIES too small, some tasks would run at " - "idle priority"); - void start_ec_tasks(void) { - int priority; - for (size_t i = 0; i < TASK_ID_COUNT + 1; ++i) { k_timer_init(&shimmed_tasks_timers[i], timer_expire, NULL); } @@ -317,8 +322,6 @@ void start_ec_tasks(void) struct task_ctx_data *const data = &shimmed_tasks_data[i]; const struct task_ctx_cfg *const cfg = &shimmed_tasks_cfg[i]; - priority = K_PRIO_PREEMPT(TASK_ID_COUNT - i - 1); - #ifdef TEST_BUILD /* Do not create thread for test runner; it will be set later */ if (i == TASK_ID_TEST_RUNNER) { @@ -336,7 +339,7 @@ void start_ec_tasks(void) */ if (i == TASK_ID_CONSOLE_STUB) { data->zephyr_tid = NULL; - uart_shell_set_priority(priority); + uart_shell_set_priority(cfg->priority); continue; } #endif @@ -354,7 +357,7 @@ void start_ec_tasks(void) (void *)cfg, data, NULL, - priority, + cfg->priority, 0, K_NO_WAIT); diff --git a/zephyr/test/tasks/shimmed_test_tasks.h b/zephyr/test/tasks/shimmed_test_tasks.h index b7d72b59d5..c040ed1bad 100644 --- a/zephyr/test/tasks/shimmed_test_tasks.h +++ b/zephyr/test/tasks/shimmed_test_tasks.h @@ -15,8 +15,8 @@ /* Highest priority on bottom same as in platform/ec */ #define CROS_EC_TASK_LIST \ - CROS_EC_TASK(TASK_1, task1_entry, 0, 512) \ - CROS_EC_TASK(TASK_2, task2_entry, 0, 512) \ - CROS_EC_TASK(TASK_3, task3_entry, 0, 512) + CROS_EC_TASK(TASK_1, task1_entry, 0, 512, 2) \ + CROS_EC_TASK(TASK_2, task2_entry, 0, 512, 1) \ + CROS_EC_TASK(TASK_3, task3_entry, 0, 512, 0) #endif /* __CROS_EC_SHIMMED_TEST_TASKS_H */ -- cgit v1.2.1