From 4021d9e4d7ed08ab25d1515f4b77920f30195247 Mon Sep 17 00:00:00 2001 From: Bjarki Arge Andreasen Date: Tue, 15 Jul 2025 16:47:53 +0200 Subject: [PATCH] shell: exchange k_poll for k_event The shell subsystem currently uses k_poll for signalling. However, k_poll is only used for simple event signals, results are not used. Replacing the k_poll with k_event greatly simplifies the code, and saves 4 struct k_poll_signal and 4 struct k_poll_event (one of which was entirely unused) while costing a single struct k_event, for every shell instance. It also allows us to not select POLL, as we are using the simpler EVENTS instead. A quick test build of the shell test suite on an nrf54l15 produces the following build info: using EVENTS: FLASH: 71592 B 1428 KB 4.90% RAM: 9872 B 188 KB 5.13% IDT_LIST: 0 GB 32 KB 0.00% using POLL FLASH: 75524 B 1428 KB 5.16% RAM: 11224 B 188 KB 5.83% IDT_LIST: 0 GB 32 KB 0.00% Signed-off-by: Bjarki Arge Andreasen --- include/zephyr/shell/shell.h | 16 +++----- subsys/shell/Kconfig | 2 +- subsys/shell/shell.c | 68 +++++++++----------------------- subsys/shell/shell_log_backend.c | 5 +-- subsys/shell/shell_ops.c | 10 +---- 5 files changed, 28 insertions(+), 73 deletions(-) diff --git a/include/zephyr/shell/shell.h b/include/zephyr/shell/shell.h index ac0f9eaae3a0e..3d93dca841fef 100644 --- a/include/zephyr/shell/shell.h +++ b/include/zephyr/shell/shell.h @@ -900,11 +900,10 @@ union shell_backend_ctx { }; enum shell_signal { - SHELL_SIGNAL_RXRDY, - SHELL_SIGNAL_LOG_MSG, - SHELL_SIGNAL_KILL, - SHELL_SIGNAL_TXDONE, /* TXDONE must be last one before SHELL_SIGNALS */ - SHELL_SIGNALS + SHELL_SIGNAL_RXRDY = BIT(0), + SHELL_SIGNAL_LOG_MSG = BIT(1), + SHELL_SIGNAL_KILL = BIT(2), + SHELL_SIGNAL_TXDONE = BIT(3), }; /** @@ -962,12 +961,7 @@ struct shell_ctx { volatile union shell_backend_cfg cfg; volatile union shell_backend_ctx ctx; - struct k_poll_signal signals[SHELL_SIGNALS]; - - /** Events that should be used only internally by shell thread. - * Event for SHELL_SIGNAL_TXDONE is initialized but unused. - */ - struct k_poll_event events[SHELL_SIGNALS]; + struct k_event signal_event; struct k_mutex wr_mtx; k_tid_t tid; diff --git a/subsys/shell/Kconfig b/subsys/shell/Kconfig index bf2cd43264c36..54066fa2a32c1 100644 --- a/subsys/shell/Kconfig +++ b/subsys/shell/Kconfig @@ -8,7 +8,7 @@ menuconfig SHELL bool "Shell" imply LOG_RUNTIME_FILTERING - select POLL + select EVENTS if SHELL diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index eb38d5d01981e..63b3274e3bb37 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -1171,19 +1171,16 @@ static void state_collect(const struct shell *sh) static void transport_evt_handler(enum shell_transport_evt evt_type, void *ctx) { struct shell *sh = (struct shell *)ctx; - struct k_poll_signal *signal; + enum shell_signal sig = evt_type == SHELL_TRANSPORT_EVT_RX_RDY + ? SHELL_SIGNAL_RXRDY + : SHELL_SIGNAL_TXDONE; - signal = (evt_type == SHELL_TRANSPORT_EVT_RX_RDY) ? - &sh->ctx->signals[SHELL_SIGNAL_RXRDY] : - &sh->ctx->signals[SHELL_SIGNAL_TXDONE]; - k_poll_signal_raise(signal, 0); + k_event_post(&sh->ctx->signal_event, sig); } static void shell_log_process(const struct shell *sh) { bool processed = false; - int signaled = 0; - int result; do { if (!IS_ENABLED(CONFIG_LOG_MODE_IMMEDIATE)) { @@ -1193,9 +1190,6 @@ static void shell_log_process(const struct shell *sh) sh->log_backend); } - struct k_poll_signal *signal = - &sh->ctx->signals[SHELL_SIGNAL_RXRDY]; - z_shell_print_prompt_and_cmd(sh); /* Arbitrary delay added to ensure that prompt is @@ -1205,9 +1199,7 @@ static void shell_log_process(const struct shell *sh) k_sleep(K_MSEC(15)); } - k_poll_signal_check(signal, &signaled, &result); - - } while (processed && !signaled); + } while (processed && !k_event_test(&sh->ctx->signal_event, SHELL_SIGNAL_RXRDY)); } static int instance_init(const struct shell *sh, @@ -1224,16 +1216,9 @@ static int instance_init(const struct shell *sh, history_init(sh); + k_event_init(&sh->ctx->signal_event); k_mutex_init(&sh->ctx->wr_mtx); - for (int i = 0; i < SHELL_SIGNALS; i++) { - k_poll_signal_init(&sh->ctx->signals[i]); - k_poll_event_init(&sh->ctx->events[i], - K_POLL_TYPE_SIGNAL, - K_POLL_MODE_NOTIFY_ONLY, - &sh->ctx->signals[i]); - } - if (IS_ENABLED(CONFIG_SHELL_STATS)) { sh->stats->log_lost_cnt = 0; } @@ -1302,19 +1287,15 @@ static int instance_uninit(const struct shell *sh) typedef void (*shell_signal_handler_t)(const struct shell *sh); static void shell_signal_handle(const struct shell *sh, - enum shell_signal sig_idx, + enum shell_signal sig, shell_signal_handler_t handler) { - struct k_poll_signal *sig = &sh->ctx->signals[sig_idx]; - int set; - int res; - - k_poll_signal_check(sig, &set, &res); - - if (set) { - k_poll_signal_reset(sig); - handler(sh); + if (!k_event_test(&sh->ctx->signal_event, sig)) { + return; } + + k_event_clear(&sh->ctx->signal_event, sig); + handler(sh); } static void kill_handler(const struct shell *sh) @@ -1354,20 +1335,13 @@ void shell_thread(void *shell_handle, void *arg_log_backend, } while (true) { - /* waiting for all signals except SHELL_SIGNAL_TXDONE */ - err = k_poll(sh->ctx->events, SHELL_SIGNAL_TXDONE, + k_event_wait(&sh->ctx->signal_event, + SHELL_SIGNAL_RXRDY | + SHELL_SIGNAL_LOG_MSG | + SHELL_SIGNAL_KILL, + false, K_FOREVER); - if (err != 0) { - if (k_mutex_lock(&sh->ctx->wr_mtx, SHELL_TX_MTX_TIMEOUT) != 0) { - return; - } - z_shell_fprintf(sh, SHELL_ERROR, - "Shell thread error: %d", err); - k_mutex_unlock(&sh->ctx->wr_mtx); - return; - } - k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); shell_signal_handle(sh, SHELL_SIGNAL_KILL, kill_handler); @@ -1419,13 +1393,7 @@ void shell_uninit(const struct shell *sh, shell_uninit_cb_t cb) __ASSERT_NO_MSG(sh); if (IS_ENABLED(CONFIG_MULTITHREADING)) { - struct k_poll_signal *signal = - &sh->ctx->signals[SHELL_SIGNAL_KILL]; - - sh->ctx->uninit_cb = cb; - /* signal kill message */ - (void)k_poll_signal_raise(signal, 0); - + k_event_post(&sh->ctx->signal_event, SHELL_SIGNAL_KILL); return; } diff --git a/subsys/shell/shell_log_backend.c b/subsys/shell/shell_log_backend.c index 7e2a357da6e56..e46876a057c87 100644 --- a/subsys/shell/shell_log_backend.c +++ b/subsys/shell/shell_log_backend.c @@ -232,7 +232,6 @@ static void process(const struct log_backend *const backend, const struct log_output *log_output = log_backend->log_output; bool colors = IS_ENABLED(CONFIG_SHELL_VT100_COLORS) && z_flag_use_colors_get(sh); - struct k_poll_signal *signal; switch (sh->log_backend->control_block->state) { case SHELL_LOG_BACKEND_ENABLED: @@ -241,8 +240,8 @@ static void process(const struct log_backend *const backend, } else { if (copy_to_pbuffer(mpsc_buffer, msg, log_backend->timeout)) { if (IS_ENABLED(CONFIG_MULTITHREADING)) { - signal = &sh->ctx->signals[SHELL_SIGNAL_LOG_MSG]; - k_poll_signal_raise(signal, 0); + k_event_post(&sh->ctx->signal_event, + SHELL_SIGNAL_LOG_MSG); } } else { dropped(backend, 1); diff --git a/subsys/shell/shell_ops.c b/subsys/shell/shell_ops.c index af164d09f79d4..37715b7ec0e3e 100644 --- a/subsys/shell/shell_ops.c +++ b/subsys/shell/shell_ops.c @@ -432,14 +432,8 @@ static void shell_pend_on_txdone(const struct shell *sh) { if (IS_ENABLED(CONFIG_MULTITHREADING) && (sh->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE)) { - struct k_poll_event event; - - k_poll_event_init(&event, - K_POLL_TYPE_SIGNAL, - K_POLL_MODE_NOTIFY_ONLY, - &sh->ctx->signals[SHELL_SIGNAL_TXDONE]); - k_poll(&event, 1, K_FOREVER); - k_poll_signal_reset(&sh->ctx->signals[SHELL_SIGNAL_TXDONE]); + k_event_wait(&sh->ctx->signal_event, SHELL_SIGNAL_TXDONE, false, K_FOREVER); + k_event_clear(&sh->ctx->signal_event, SHELL_SIGNAL_TXDONE); } else { /* Blocking wait in case of bare metal. */ while (!z_flag_tx_rdy_get(sh)) {