From 71ef6c6979698b514c52f088d5d996122e287464 Mon Sep 17 00:00:00 2001 From: Lyle Zhu Date: Wed, 14 May 2025 12:24:32 +0800 Subject: [PATCH] Drivers: Bluetooth: H4: Use a semaphore to wake up HCI RX thread There is an issue that the buffer cannot be allocated by the function `read_payload()` in UART ISR context. Then the UART RX will be disabled. The H4 driver hopes to get the receive buffer in the HCI RX thread and then open the UART RX again. However, there is a situation where the HCI RX thread is blocked in getting the received data buffer. However, since the UARt RX has been disabled, the HCI RX thread cannot get the received data buffer. Therefore, the RX thread is always blocked here, causing the Bluetooth host to not work properly. Add a semaphore `rx.ready` to notify new received data buffer has been added to H4 RX queue. Wait for the semaphore `rx.ready` instead of H4 RX queue in HCI RX thread. Wake up the HCI RX thread when failing to allocate the RX buffer. Fixes #89879. Signed-off-by: Lyle Zhu --- drivers/bluetooth/hci/h4.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c index a673aac5f52..21c25420beb 100644 --- a/drivers/bluetooth/hci/h4.c +++ b/drivers/bluetooth/hci/h4.c @@ -37,6 +37,8 @@ struct h4_data { struct net_buf *buf; struct k_fifo fifo; + struct k_sem ready; + uint16_t remaining; uint16_t discard; @@ -257,8 +259,10 @@ static void rx_thread(void *p1, void *p2, void *p3) /* Let the ISR continue receiving new packets */ uart_irq_rx_enable(cfg->uart); - buf = k_fifo_get(&h4->rx.fifo, K_FOREVER); - do { + k_sem_take(&h4->rx.ready, K_FOREVER); + + buf = k_fifo_get(&h4->rx.fifo, K_NO_WAIT); + while (buf != NULL) { uart_irq_rx_enable(cfg->uart); LOG_DBG("Calling bt_recv(%p)", buf); @@ -272,7 +276,7 @@ static void rx_thread(void *p1, void *p2, void *p3) uart_irq_rx_disable(cfg->uart); buf = k_fifo_get(&h4->rx.fifo, K_NO_WAIT); - } while (buf); + } } } @@ -311,6 +315,18 @@ static inline void read_payload(const struct device *dev) LOG_WRN("Failed to allocate, deferring to rx_thread"); uart_irq_rx_disable(cfg->uart); + /* + * At this time, HCI UART RX is turned off, which means that no new + * received data buffer will be put into the RX FIFO. This will cause + * `rx.ready` to not be modified. It will probably remain unchanged and + * the count of `rx.ready` will probably be 0. + * + * Since it is uncertain whether the RX thread is blocked waiting for + * `rx.ready`, give a semaphore to try to wake up the RX thread. + * Then there will be a renewed attempt at allocating an RX buffer in + * the RX thread. + */ + k_sem_give(&h4->rx.ready); return; } @@ -352,6 +368,7 @@ static inline void read_payload(const struct device *dev) LOG_DBG("Putting buf %p to rx fifo", buf); k_fifo_put(&h4->rx.fifo, buf); + k_sem_give(&h4->rx.ready); } static inline void read_header(const struct device *dev) @@ -513,6 +530,9 @@ static int h4_open(const struct device *dev, bt_hci_recv_t recv) 0, K_NO_WAIT); k_thread_name_set(tid, "bt_rx_thread"); + /* Active rx_thread at first time */ + k_sem_give(&h4->rx.ready); + return 0; } @@ -585,6 +605,7 @@ static DEVICE_API(bt_hci, h4_driver_api) = { static struct h4_data h4_data_##inst = { \ .rx = { \ .fifo = Z_FIFO_INITIALIZER(h4_data_##inst.rx.fifo), \ + .ready = Z_SEM_INITIALIZER(h4_data_##inst.rx.ready, 0, 1), \ }, \ .tx = { \ .fifo = Z_FIFO_INITIALIZER(h4_data_##inst.tx.fifo), \