Skip to content
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

modem: pipe: Add explicit timeout to sync APIs #74325

Merged
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
14 changes: 7 additions & 7 deletions drivers/gnss/gnss_luatos_air530z.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,21 +188,21 @@ static int gnss_luatos_air530z_init(const struct device *dev)

luatos_air530z_init_dynamic_script(dev);

ret = modem_pipe_open(data->uart_pipe);
ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10));
Copy link
Member

Choose a reason for hiding this comment

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

should the number be a kconfig option? (could be on a followup)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed could be nice to have a global default defined by a Kconfig option.

if (ret < 0) {
return ret;
}

ret = modem_chat_attach(&data->chat, data->uart_pipe);
if (ret < 0) {
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

ret = modem_chat_run_script(&data->chat, &init_script);
if (ret < 0) {
LOG_ERR("Failed to run init_script");
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

Expand All @@ -222,20 +222,20 @@ static int luatos_air530z_pm_resume(const struct device *dev)
struct gnss_luatos_air530z_data *data = dev->data;
int ret;

ret = modem_pipe_open(data->uart_pipe);
ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10));
if (ret < 0) {
return ret;
}

ret = modem_chat_attach(&data->chat, data->uart_pipe);
if (ret < 0) {
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

ret = modem_chat_run_script(&data->chat, &init_script);
if (ret < 0) {
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

Expand All @@ -251,7 +251,7 @@ static int luatos_air530z_pm_action(const struct device *dev, enum pm_device_act
switch (action) {
case PM_DEVICE_ACTION_SUSPEND:
gpio_pin_set_dt(&config->on_off_gpio, 0);
ret = modem_pipe_close(data->uart_pipe);
ret = modem_pipe_close(data->uart_pipe, K_SECONDS(10));
break;

case PM_DEVICE_ACTION_RESUME:
Expand Down
4 changes: 2 additions & 2 deletions drivers/gnss/gnss_nmea_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ static int gnss_nmea_generic_resume(const struct device *dev)
struct gnss_nmea_generic_data *data = dev->data;
int ret;

ret = modem_pipe_open(data->uart_pipe);
ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10));
if (ret < 0) {
return ret;
}
Expand All @@ -76,7 +76,7 @@ static int gnss_nmea_generic_resume(const struct device *dev)
}

if (ret < 0) {
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
}
return ret;
}
Expand Down
12 changes: 6 additions & 6 deletions drivers/gnss/gnss_quectel_lcx6g.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ static int quectel_lcx6g_resume(const struct device *dev)

quectel_lcx6g_await_pm_ready(dev);

ret = modem_pipe_open(data->uart_pipe);
ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10));
if (ret < 0) {
LOG_ERR("Failed to open pipe");
return ret;
Expand All @@ -214,21 +214,21 @@ static int quectel_lcx6g_resume(const struct device *dev)
ret = modem_chat_attach(&data->chat, data->uart_pipe);
if (ret < 0) {
LOG_ERR("Failed to attach chat");
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

ret = modem_chat_run_script(&data->chat, &resume_script);
if (ret < 0) {
LOG_ERR("Failed to initialize GNSS");
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

ret = quectel_lcx6g_configure_pps(dev);
if (ret < 0) {
LOG_ERR("Failed to configure PPS");
modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

Expand All @@ -253,7 +253,7 @@ static int quectel_lcx6g_suspend(const struct device *dev)
LOG_INF("Suspended");
}

modem_pipe_close(data->uart_pipe);
modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

Expand All @@ -268,7 +268,7 @@ static int quectel_lcx6g_turn_off(const struct device *dev)

LOG_INF("Powered off");

return modem_pipe_close(data->uart_pipe);
return modem_pipe_close(data->uart_pipe, K_SECONDS(10));
}

static int quectel_lcx6g_pm_action(const struct device *dev, enum pm_device_action action)
Expand Down
8 changes: 4 additions & 4 deletions drivers/gnss/gnss_u_blox_m10.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ static int ubx_m10_resume(const struct device *dev)
struct ubx_m10_data *data = dev->data;
int ret;

ret = modem_pipe_open(data->uart_pipe);
ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10));
if (ret < 0) {
return ret;
}

ret = modem_chat_attach(&data->chat, data->uart_pipe);
if (ret < 0) {
(void)modem_pipe_close(data->uart_pipe);
(void)modem_pipe_close(data->uart_pipe, K_SECONDS(10));
return ret;
}

Expand All @@ -111,7 +111,7 @@ static int ubx_m10_turn_off(const struct device *dev)
{
struct ubx_m10_data *data = dev->data;

return modem_pipe_close(data->uart_pipe);
return modem_pipe_close(data->uart_pipe, K_SECONDS(10));
}

static int ubx_m10_init_nmea0183_match(const struct device *dev)
Expand Down Expand Up @@ -204,7 +204,7 @@ static int ubx_m10_modem_module_change(const struct device *dev, bool change_fro
}

if (ret < 0) {
(void)modem_pipe_close(data->uart_pipe);
(void)modem_pipe_close(data->uart_pipe, K_SECONDS(10));
}

return ret;
Expand Down
6 changes: 4 additions & 2 deletions include/zephyr/modem/pipe.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ void modem_pipe_init(struct modem_pipe *pipe, void *data, struct modem_pipe_api
* @brief Open pipe
*
* @param pipe Pipe instance
* @param timeout Timeout waiting for pipe to open
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param timeout Timeout waiting for pipe to open
* @param timeout Time to wait for pipe to open

*
* @retval 0 if pipe was successfully opened or was already open
* @retval -errno code otherwise
Expand All @@ -95,7 +96,7 @@ void modem_pipe_init(struct modem_pipe *pipe, void *data, struct modem_pipe_api
* It may block the calling thread, which in the case of the system workqueue
* can result in a deadlock until this call times out waiting for the pipe to be open.
*/
int modem_pipe_open(struct modem_pipe *pipe);
int modem_pipe_open(struct modem_pipe *pipe, k_timeout_t timeout);

/**
* @brief Open pipe asynchronously
Expand Down Expand Up @@ -163,6 +164,7 @@ void modem_pipe_release(struct modem_pipe *pipe);
* @brief Close pipe
*
* @param pipe Pipe instance
* @param timeout Timeout waiting for pipe to close
*
* @retval 0 if pipe open was called closed or pipe was already closed
* @retval -errno code otherwise
Expand All @@ -171,7 +173,7 @@ void modem_pipe_release(struct modem_pipe *pipe);
* It may block the calling thread, which in the case of the system workqueue
* can result in a deadlock until this call times out waiting for the pipe to be closed.
*/
int modem_pipe_close(struct modem_pipe *pipe);
int modem_pipe_close(struct modem_pipe *pipe, k_timeout_t timeout);

/**
* @brief Close pipe asynchronously
Expand Down
14 changes: 8 additions & 6 deletions subsys/modem/modem_pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ static uint32_t pipe_test_events(struct modem_pipe *pipe, uint32_t events)
return k_event_test(&pipe->event, events);
}

static uint32_t pipe_await_events(struct modem_pipe *pipe, uint32_t events)
static uint32_t pipe_await_events(struct modem_pipe *pipe,
uint32_t events,
k_timeout_t timeout)
{
return k_event_wait(&pipe->event, events, false, K_MSEC(10000));
return k_event_wait(&pipe->event, events, false, timeout);
}

static void pipe_post_events(struct modem_pipe *pipe, uint32_t events)
Expand Down Expand Up @@ -88,7 +90,7 @@ void modem_pipe_init(struct modem_pipe *pipe, void *data, struct modem_pipe_api
k_event_init(&pipe->event);
}

int modem_pipe_open(struct modem_pipe *pipe)
int modem_pipe_open(struct modem_pipe *pipe, k_timeout_t timeout)
{
int ret;

Expand All @@ -101,7 +103,7 @@ int modem_pipe_open(struct modem_pipe *pipe)
return ret;
}

if (!pipe_await_events(pipe, PIPE_EVENT_OPENED_BIT)) {
if (!pipe_await_events(pipe, PIPE_EVENT_OPENED_BIT, timeout)) {
return -EAGAIN;
}

Expand Down Expand Up @@ -156,7 +158,7 @@ void modem_pipe_release(struct modem_pipe *pipe)
pipe_set_callback(pipe, NULL, NULL);
}

int modem_pipe_close(struct modem_pipe *pipe)
int modem_pipe_close(struct modem_pipe *pipe, k_timeout_t timeout)
{
int ret;

Expand All @@ -169,7 +171,7 @@ int modem_pipe_close(struct modem_pipe *pipe)
return ret;
}

if (!pipe_await_events(pipe, PIPE_EVENT_CLOSED_BIT)) {
if (!pipe_await_events(pipe, PIPE_EVENT_CLOSED_BIT, timeout)) {
return -EAGAIN;
}

Expand Down
12 changes: 6 additions & 6 deletions tests/subsys/modem/backends/tty/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ static void *test_modem_backend_tty_setup(void)

tty_pipe = modem_backend_tty_init(&tty_backend, &config);
modem_pipe_attach(tty_pipe, modem_pipe_callback_handler, NULL);
__ASSERT_NO_MSG(modem_pipe_open(tty_pipe) == 0);
__ASSERT_NO_MSG(modem_pipe_open(tty_pipe, K_SECONDS(10)) == 0);
return NULL;
}

Expand All @@ -119,7 +119,7 @@ static void test_modem_backend_tty_before(void *f)

static void test_modem_backend_tty_teardown(void *f)
{
modem_pipe_close(tty_pipe);
modem_pipe_close(tty_pipe, K_SECONDS(10));
}

/*************************************************************************************************/
Expand All @@ -129,12 +129,12 @@ ZTEST(modem_backend_tty_suite, test_close_open)
{
bool result;

zassert_ok(modem_pipe_close(tty_pipe), "Failed to close pipe");
zassert_ok(modem_pipe_close(tty_pipe), "Pipe should already be closed");
zassert_ok(modem_pipe_open(tty_pipe), "Failed to open pipe");
zassert_ok(modem_pipe_close(tty_pipe, K_SECONDS(10)), "Failed to close pipe");
zassert_ok(modem_pipe_close(tty_pipe, K_SECONDS(10)), "Pipe should already be closed");
zassert_ok(modem_pipe_open(tty_pipe, K_SECONDS(10)), "Failed to open pipe");
result = atomic_test_bit(&tty_pipe_events, TEST_MODEM_BACKEND_TTY_PIPE_EVENT_TIDLE_BIT);
zassert_true(result, "Transmit idle event should be set");
zassert_ok(modem_pipe_open(tty_pipe), "Pipe should already be open");
zassert_ok(modem_pipe_open(tty_pipe, K_SECONDS(10)), "Pipe should already be open");
}

ZTEST(modem_backend_tty_suite, test_receive_ready_event_not_raised)
Expand Down
4 changes: 2 additions & 2 deletions tests/subsys/modem/backends/uart/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ static void test_modem_backend_uart_before(void *f)
prng_reset();
ring_buf_reset(&transmit_ring_buf);
k_sem_reset(&receive_ready_sem);
__ASSERT_NO_MSG(modem_pipe_open(pipe) == 0);
__ASSERT_NO_MSG(modem_pipe_open(pipe, K_SECONDS(10)) == 0);
}

static void test_modem_backend_uart_after(void *f)
{
__ASSERT_NO_MSG(modem_pipe_close(pipe) == 0);
__ASSERT_NO_MSG(modem_pipe_close(pipe, K_SECONDS(10)) == 0);
}

/*************************************************************************************************/
Expand Down
2 changes: 1 addition & 1 deletion tests/subsys/modem/modem_chat/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ static void *test_modem_chat_setup(void)
};

mock_pipe = modem_backend_mock_init(&mock, &mock_config);
zassert(modem_pipe_open(mock_pipe) == 0, "Failed to open mock pipe");
zassert(modem_pipe_open(mock_pipe, K_SECONDS(10)) == 0, "Failed to open mock pipe");
zassert(modem_chat_attach(&cmd, mock_pipe) == 0, "Failed to attach pipe mock to modem CMD");
return NULL;
}
Expand Down
22 changes: 13 additions & 9 deletions tests/subsys/modem/modem_cmux/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ static void *test_modem_cmux_setup(void)
};

bus_mock_pipe = modem_backend_mock_init(&bus_mock, &bus_mock_config);
__ASSERT_NO_MSG(modem_pipe_open(bus_mock_pipe) == 0);
__ASSERT_NO_MSG(modem_pipe_open(bus_mock_pipe, K_SECONDS(10)) == 0);

/* Connect CMUX */
__ASSERT_NO_MSG(modem_cmux_attach(&cmux, bus_mock_pipe) == 0);
Expand Down Expand Up @@ -731,9 +731,9 @@ ZTEST(modem_cmux, test_modem_cmux_disconnect_connect)
ZTEST(modem_cmux, test_modem_cmux_disconnect_connect_sync)
{
modem_backend_mock_prime(&bus_mock, &transaction_dlci1_disc);
zassert_true(modem_pipe_close(dlci1_pipe) == 0, "Failed to close DLCI1");
zassert_true(modem_pipe_close(dlci1_pipe, K_SECONDS(10)) == 0, "Failed to close DLCI1");
modem_backend_mock_prime(&bus_mock, &transaction_dlci2_disc);
zassert_true(modem_pipe_close(dlci2_pipe) == 0, "Failed to close DLCI2");
zassert_true(modem_pipe_close(dlci2_pipe, K_SECONDS(10)) == 0, "Failed to close DLCI2");
modem_backend_mock_prime(&bus_mock, &transaction_control_cld);
zassert_true(modem_cmux_disconnect(&cmux) == 0, "Failed to disconnect CMUX");
zassert_true(modem_cmux_disconnect(&cmux) == -EALREADY,
Expand All @@ -745,21 +745,25 @@ ZTEST(modem_cmux, test_modem_cmux_disconnect_connect_sync)
"Should already be connected");

modem_backend_mock_prime(&bus_mock, &transaction_dlci1_sabm);
zassert_true(modem_pipe_open(dlci1_pipe) == 0, "Failed to open DLCI1 pipe");
zassert_true(modem_pipe_open(dlci1_pipe, K_SECONDS(10)) == 0,
"Failed to open DLCI1 pipe");
modem_backend_mock_prime(&bus_mock, &transaction_dlci2_sabm);
zassert_true(modem_pipe_open(dlci2_pipe) == 0, "Failed to open DLCI2 pipe");
zassert_true(modem_pipe_open(dlci2_pipe, K_SECONDS(10)) == 0,
"Failed to open DLCI2 pipe");
}

ZTEST(modem_cmux, test_modem_cmux_dlci_close_open_sync)
{
modem_backend_mock_prime(&bus_mock, &transaction_dlci1_disc);
zassert_true(modem_pipe_close(dlci1_pipe) == 0, "Failed to close DLCI1");
zassert_true(modem_pipe_close(dlci1_pipe, K_SECONDS(10)) == 0, "Failed to close DLCI1");
modem_backend_mock_prime(&bus_mock, &transaction_dlci2_disc);
zassert_true(modem_pipe_close(dlci2_pipe) == 0, "Failed to close DLCI2");
zassert_true(modem_pipe_close(dlci2_pipe, K_SECONDS(10)) == 0, "Failed to close DLCI2");
modem_backend_mock_prime(&bus_mock, &transaction_dlci1_sabm);
zassert_true(modem_pipe_open(dlci1_pipe) == 0, "Failed to open DLCI1 pipe");
zassert_true(modem_pipe_open(dlci1_pipe, K_SECONDS(10)) == 0,
"Failed to open DLCI1 pipe");
modem_backend_mock_prime(&bus_mock, &transaction_dlci2_sabm);
zassert_true(modem_pipe_open(dlci2_pipe) == 0, "Failed to open DLCI2 pipe");
zassert_true(modem_pipe_open(dlci2_pipe, K_SECONDS(10)) == 0,
"Failed to open DLCI2 pipe");
}

ZTEST_SUITE(modem_cmux, NULL, test_modem_cmux_setup, test_modem_cmux_before, NULL, NULL);
Loading
Loading