Skip to content

shell: exchange k_poll for k_event #93154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions include/zephyr/shell/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};

/**
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion subsys/shell/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
menuconfig SHELL
bool "Shell"
imply LOG_RUNTIME_FILTERING
select POLL
select EVENTS

if SHELL

Expand Down
68 changes: 18 additions & 50 deletions subsys/shell/shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened to this callback? It was lost.

/* signal kill message */
(void)k_poll_signal_raise(signal, 0);

k_event_post(&sh->ctx->signal_event, SHELL_SIGNAL_KILL);
return;
}

Expand Down
5 changes: 2 additions & 3 deletions subsys/shell/shell_log_backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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);
Expand Down
10 changes: 2 additions & 8 deletions subsys/shell/shell_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down