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 <[email protected]>

Author: bjarki-andreasen

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;

subsys/shell/Kconfig

@@ -8,7 +8,7 @@
 menuconfig SHELL
 	bool "Shell"
 	imply LOG_RUNTIME_FILTERING
-	select POLL
+	select EVENTS
 
 if SHELL
 

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;
 	}
 

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);

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)) {