Suspending UART Async using PM_DEVICE ACTION_SUSPEND causes crash.

Question

Hello, 
 I am currently using the BLE NUS example from the Nordic Dev Academy course for BLE ( https://github.com/NordicDeveloperAcademy/bt-fund/blob/main/lesson4/blefund_less4_exer3_solution/src/main.c ). 
 However, the current consumption was around 3.5 mA, which is too high for my application. 
 Thus, I used the commands "pm_device_action_run(uart, PM_DEVICE_ACTION_SUSPEND);" on the ble connection callback and "pm_action_run(uart, PM_DEVICE_ACTION_RESUME);" to suspend the UART driver and hopefully reduce current consumption. After the device is suspended, the current consumption does go down to around 300 uA. The idea is that when the device is not connected on bluetooth, UART would be suspended to save power, and UART could resume as soon as the device is reconnected on bluetooth. I implemented the code as shown below: 
 static void connected(struct bt_conn *conn, uint8_t err)
{
	char addr[BT_ADDR_LE_STR_LEN];

	if (err) {
 LOG_ERR("Connection failed (err %u)", err);
 return;
	}

	bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr));
	LOG_INF("Connected %s", addr);

	current_conn = bt_conn_ref(conn);

	dk_set_led_on(CON_STATUS_LED);
	
	pm_device_action_run(uart, PM_DEVICE_ACTION_RESUME);
	struct uart_data_t *buf;
	uart_rx_enable(uart, buf->data, sizeof(buf->data), UART_WAIT_FOR_RX);
}

static void disconnected(struct bt_conn *conn, uint8_t reason)
{
	char addr[BT_ADDR_LE_STR_LEN];

	bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr));

	LOG_INF("Disconnected: %s (reason %u)", addr, reason);

	if (auth_conn) {
 bt_conn_unref(auth_conn);
 auth_conn = NULL;
	}

	if (current_conn) {
 bt_conn_unref(current_conn);
 current_conn = NULL;
 dk_set_led_off(CON_STATUS_LED);
	}
	
	uart_rx_disable(uart);
	pm_device_action_run(uart, PM_DEVICE_ACTION_SUSPEND);
}
 
 However, whenever PM_DEVICE_ACTION_SUSPEND is executed, the device crashes and the RGB LED starts to flicker. The same occurs if I put the "pm_device_action_run(uart, PM_DEVICE_ACTION_SUSPEND);" statement in the main function after all initializations. 
 For context, I am using the EVK-ANNA-B112 development board that uses nrf52832. When I flash the device, the UART TX and RX pins are connected to a separate device (EmStat Pico Development Board) for communication. 
 Is there anything I am getting wrong about suspending UART Async?

Vidar Berg · Accepted Answer

Hello, Sorry for the delayed response. I have now had a chance to test this with the peripheral_uart sample from nRF Connect version 2.6.1. The changes I made to suspend UART while not connected diff --git a/prj.conf b/prj.conf index 25deae6..c267e7e 100644 --- a/prj.conf +++ b/prj.conf @@ -7,6 +7,8 @@ # Enable the UART driver CONFIG_UART_ASYNC_API=y CONFIG_NRFX_UARTE0=y +CONFIG_UART_0_NRF_HW_ASYNC=y +CONFIG_UART_0_NRF_HW_ASYNC_TIMER=1 CONFIG_SERIAL=y CONFIG_GPIO=y @@ -49,3 +51,7 @@ CONFIG_LOG_BACKEND_UART=n CONFIG_LOG_PRINTK=n CONFIG_ASSERT=y + +CONFIG_PM_DEVICE=y + + diff --git a/src/main.c b/src/main.c index 7526fd6..fe5bc0b 100644 --- a/src/main.c +++ b/src/main.c @@ -13,6 +13,10 @@ #include #include #include +#include + +#include +#include #include #include @@ -54,6 +58,10 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME); #define UART_WAIT_FOR_BUF_DELAY K_MSEC(50) #define UART_WAIT_FOR_RX CONFIG_BT_NUS_UART_RX_WAIT_TIME +#define UART_SUSPENDED 1 + +static ATOMIC_DEFINE(uart_suspended, 1); + static K_SEM_DEFINE(ble_init_ok, 0, 1); static struct bt_conn *current_conn; @@ -61,6 +69,8 @@ static struct bt_conn *auth_conn; static const struct device *uart = DEVICE_DT_GET(DT_CHOSEN(nordic_nus_uart)); static struct k_work_delayable uart_work; +struct onoff_client uart_clk_request; + struct uart_data_t { void *fifo_reserved; @@ -86,6 +96,98 @@ UART_ASYNC_ADAPTER_INST_DEFINE(async_adapter); static const struct device *const async_adapter; #endif +static void rx_hfclk_callback(struct onoff_manager *mgr, + struct onoff_client *cli, + uint32_t state, int res) +{ + int err; + struct uart_data_t *rx; + + if (res) { + LOG_ERR("HFCLK request error %d", err); + } else { + LOG_INF("HFCLK started"); + } + + rx = k_malloc(sizeof(*rx)); + if (rx) { + rx->len = 0; + } else { + LOG_ERR("Failed to allocate UART RX buffer"); + return; + } + + err = uart_rx_enable(uart, rx->data, sizeof(rx->data), UART_WAIT_FOR_RX); + if (err) { + LOG_ERR("Cannot enable uart reception (err: %d)", err); + /* Free the rx buffer only because the tx buffer will be handled in the callback */ + k_free(rx); + } + + atomic_clear_bit(uart_suspended, UART_SUSPENDED); + +} + +static void rx_hfclk_request(const struct device *dev) +{ + struct onoff_manager *mgr = + z_nrf_clock_control_get_onoff(CLOCK_CONTROL_NRF_SUBSYS_HF); + int err; + + sys_notify_init_callback(&uart_clk_request.notify, rx_hfclk_callback); + err = onoff_request(mgr, &uart_clk_request); + __ASSERT_NO_MSG(err >= 0); +} + +static void rx_hfclk_release(void) +{ + int err; + struct onoff_manager *mgr = + z_nrf_clock_control_get_onoff(CLOCK_CONTROL_NRF_SUBSYS_HF); + + (void)onoff_cancel_or_release(mgr, &uart_clk_request); + +} + +static void uart_suspend(void) +{ + int err; + + atomic_set_bit(uart_suspended, UART_SUSPENDED); + + (void)uart_rx_disable(uart); + (void)uart_tx_abort(uart); + + /* Delay to allow pending UART events to be processed */ + k_msleep(100); + + err = pm_device_action_run(uart, PM_DEVICE_ACTION_SUSPEND); + if (err) { + LOG_ERR("Failed to suspend uart (err: %d)", err); + return; + } + /* Release request for HF crystal oscillator to reduce idle current*/ + rx_hfclk_release(); + + LOG_INF("UART suspended.."); +} + +static void uart_resume(void) +{ + int err; + + err = pm_device_action_run(uart, PM_DEVICE_ACTION_RESUME); + if (err) { + LOG_ERR("Failed to re-enable uart (err: %d)", err); + } + + /* + * Request HF crystal oscillator before enabling UART to ensure + * sufficient clock accuracy for stable communication. + */ + rx_hfclk_request(uart); +} + static void uart_cb(const struct device *dev, struct uart_event *evt, void *user_data) { ARG_UNUSED(dev); @@ -147,6 +249,10 @@ static void uart_cb(const struct device *dev, struct uart_event *evt, void *user LOG_DBG("UART_RX_DISABLED"); disable_req = false; + if (atomic_test_bit(uart_suspended, UART_SUSPENDED)) { + return; + } + buf = k_malloc(sizeof(*buf)); if (buf) { buf->len = 0; @@ -233,9 +339,6 @@ static bool uart_test_async_api(const struct device *dev) static int uart_init(void) { int err; - int pos; - struct uart_data_t *rx; - struct uart_data_t *tx; if (!device_is_ready(uart)) { return -ENODEV; @@ -249,16 +352,8 @@ static int uart_init(void) } } - rx = k_malloc(sizeof(*rx)); - if (rx) { - rx->len = 0; - } else { - return -ENOMEM; - } - k_work_init_delayable(&uart_work, uart_work_handler); - if (IS_ENABLED(CONFIG_BT_NUS_UART_ASYNC_ADAPTER) && !uart_test_async_api(uart)) { /* Implement API adapter */ uart_async_adapter_init(async_adapter, uart); @@ -267,7 +362,6 @@ static int uart_init(void) err = uart_callback_set(uart, uart_cb, NULL); if (err) { - k_free(rx); LOG_ERR("Cannot initialize UART callback"); return err; } @@ -295,40 +389,6 @@ static int uart_init(void) } } - tx = k_malloc(sizeof(*tx)); - - if (tx) { - pos = snprintf(tx->data, sizeof(tx->data), - "Starting Nordic UART service example "); - - if ((pos < 0) || (pos >= sizeof(tx->data))) { - k_free(rx); - k_free(tx); - LOG_ERR("snprintf returned %d", pos); - return -ENOMEM; - } - - tx->len = pos; - } else { - k_free(rx); - return -ENOMEM; - } - - err = uart_tx(uart, tx->data, tx->len, SYS_FOREVER_MS); - if (err) { - k_free(rx); - k_free(tx); - LOG_ERR("Cannot display welcome message (err: %d)", err); - return err; - } - - err = uart_rx_enable(uart, rx->data, sizeof(rx->data), UART_WAIT_FOR_RX); - if (err) { - LOG_ERR("Cannot enable uart reception (err: %d)", err); - /* Free the rx buffer only because the tx buffer will be handled in the callback */ - k_free(rx); - } - return err; } @@ -347,6 +407,7 @@ static void connected(struct bt_conn *conn, uint8_t err) current_conn = bt_conn_ref(conn); dk_set_led_on(CON_STATUS_LED); + uart_resume(); } static void disconnected(struct bt_conn *conn, uint8_t reason) @@ -367,6 +428,8 @@ static void disconnected(struct bt_conn *conn, uint8_t reason) current_conn = NULL; dk_set_led_off(CON_STATUS_LED); } + + uart_suspend(); } #ifdef CONFIG_BT_NUS_SECURITY_ENABLED @@ -581,6 +644,8 @@ int main(void) if (err) { error(); } + + uart_suspend(); if (IS_ENABLED(CONFIG_BT_NUS_SECURITY_ENABLED)) { err = bt_conn_auth_cb_register(&conn_auth_callbacks); Measurement results Sample project 3513.peripheral_uart_lp.zip To further reduce the current consumption, you may consider extending the connection and advertising interval. We have an online power profiler at https://devzone.nordicsemi.com/power/w/opp/2/online-power-profiler-for-bluetooth-le , which can be used to estimate the expected current consumption based on paramaters such as the advertising interval. Please note that I have not done any extensive testing with this modification apart from verifying the basic functionality. Best regards, Vidar

Suspending UART Async using PM_DEVICE ACTION_SUSPEND causes crash.

Top Replies