Bluetooth: host: remove CONFIG_BT_RECV_BLOCKING

This config selects a variant of the HCI driver interface that spills
out host internals unto the drivers and even the Zephyr controller. It
will now be removed in favor of driver interfaces that hide the
internals of the host.

The new default is `CONFIG_BT_RECV_WORKQ_BT`.

Any references to the removed kconfig are refactored out.

Any out-of-tree driver using the removed interface can be easily adapted
by copying the following implementations into the driver as private
functions:

 - `hci_driver.h:BT_HCI_EVT_FLAG_RECV_PRIO`
 - `hci_driver.h:BT_HCI_EVT_FLAG_RECV`
 - `hci_driver.h:bt_hci_evt_get_flags`
 - `hci_raw.c:bt_recv_prio`

In combination these symbols function as a interface adapter. These
symbols will be removed in this PR in subsequent commits.

Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>

drivers/bluetooth/hci/h4.c

@@ -278,7 +278,6 @@ static size_t h4_discard(const struct device *uart, size_t len)
 static inline void read_payload(void)
 {
 	struct net_buf *buf;
-	uint8_t evt_flags;
 	int read;
 
 	if (!rx.buf) {
@@ -331,26 +330,15 @@ static inline void read_payload(void)
 	rx.buf = NULL;
 
 	if (rx.type == H4_EVT) {
-		evt_flags = bt_hci_evt_get_flags(rx.evt.evt);
 		bt_buf_set_type(buf, BT_BUF_EVT);
 	} else {
-		evt_flags = BT_HCI_EVT_FLAG_RECV;
 		bt_buf_set_type(buf, BT_BUF_ACL_IN);
 	}
 
 	reset_rx();
 
-	if (IS_ENABLED(CONFIG_BT_RECV_BLOCKING) &&
+	LOG_DBG("Putting buf %p to rx fifo", buf);
-	    (evt_flags & BT_HCI_EVT_FLAG_RECV_PRIO)) {
+	net_buf_put(&rx.fifo, buf);
-		LOG_DBG("Calling bt_recv_prio(%p)", buf);
-		bt_recv_prio(buf);
-	}
-
-	if ((evt_flags & BT_HCI_EVT_FLAG_RECV) ||
-	    !IS_ENABLED(CONFIG_BT_RECV_BLOCKING)) {
-		LOG_DBG("Putting buf %p to rx fifo", buf);
-		net_buf_put(&rx.fifo, buf);
-	}
 }
 
 static inline void read_header(void)

drivers/bluetooth/hci/slz_hci.c

@@ -55,7 +55,6 @@ uint32_t hci_common_transport_transmit(uint8_t *data, int16_t len)
 {
 	struct net_buf *buf;
 	uint8_t packet_type = data[0];
-	uint8_t flags;
 	uint8_t event_code;
 
 	LOG_HEXDUMP_DBG(data, len, "host packet data:");
@@ -67,7 +66,6 @@ uint32_t hci_common_transport_transmit(uint8_t *data, int16_t len)
 	switch (packet_type) {
 	case h4_event:
 		event_code = data[0];
-		flags = bt_hci_evt_get_flags(event_code);
 		buf = bt_buf_get_evt(event_code, false, K_FOREVER);
 		break;
 	case h4_acl:
@@ -79,12 +77,7 @@ uint32_t hci_common_transport_transmit(uint8_t *data, int16_t len)
 	}
 
 	net_buf_add_mem(buf, data, len);
-	if (IS_ENABLED(CONFIG_BT_RECV_BLOCKING) &&
+	bt_recv(buf);
-	    (packet_type == h4_event) && (flags & BT_HCI_EVT_FLAG_RECV_PRIO)) {
-		bt_recv_prio(buf);
-	} else {
-		bt_recv(buf);
-	}
 
 	sl_btctrl_hci_transmit_complete(0);
 

include/zephyr/drivers/bluetooth/hci_driver.h

@@ -49,10 +49,6 @@ enum {
  * Helper for the HCI driver to get HCI event flags that describes rules that.
  * must be followed.
  *
- * When @kconfig{CONFIG_BT_RECV_BLOCKING} is enabled the flags
- * BT_HCI_EVT_FLAG_RECV and BT_HCI_EVT_FLAG_RECV_PRIO indicates if the event
- * should be given to bt_recv or bt_recv_prio.
- *
  * @param evt HCI event code.
  *
  * @return HCI event flags for the specified event.
@@ -85,10 +81,6 @@ static inline uint8_t bt_hci_evt_get_flags(uint8_t evt)
  * host with data from the controller. The buffer needs to have its type
  * set with the help of bt_buf_set_type() before calling this API.
  *
- * When @kconfig{CONFIG_BT_RECV_BLOCKING} is defined then this API should not be used
- * for so-called high priority HCI events, which should instead be delivered to
- * the host stack through bt_recv_prio().
- *
  * @note This function must only be called from a cooperative thread.
  *
  * @param buf Network buffer containing data from the controller.
@@ -176,10 +168,6 @@ struct bt_hci_driver {
 	 * return until the transport is ready for operation, meaning it
 	 * is safe to start calling the send() handler.
 	 *
-	 * If the driver uses its own RX thread, i.e.
-	 * @kconfig{CONFIG_BT_RECV_BLOCKING} is set, then this
-	 * function is expected to start that thread.
-	 *
 	 * @return 0 on success or negative error number on failure.
 	 */
 	int (*open)(void);
@@ -190,9 +178,6 @@ struct bt_hci_driver {
 	 * Closes the HCI transport. This function must not return until the
 	 * transport is closed.
 	 *
-	 * If the driver uses its own RX thread, i.e.
-	 * @kconfig{CONFIG_BT_RECV_BLOCKING} is set, then this
-	 * function is expected to abort that thread.
 	 * @return 0 on success or negative error number on failure.
 	 */
 	int (*close)(void);

subsys/bluetooth/common/Kconfig

@@ -95,7 +95,6 @@ config BT_BUF_ACL_RX_SIZE
 config BT_BUF_ACL_RX_COUNT
 	int "Number of incoming ACL data buffers"
 	default NET_BUF_RX_COUNT if NET_L2_BT
-	default 3 if BT_RECV_BLOCKING
 	default 6
 	range 1 64
 	help
@@ -126,7 +125,6 @@ config BT_BUF_EVT_RX_SIZE
 
 config BT_BUF_EVT_RX_COUNT
 	int "Number of HCI Event buffers"
-	default 3 if BT_RECV_BLOCKING
 	default 20 if (BT_MESH && !(BT_BUF_EVT_DISCARDABLE_COUNT > 0))
 	default 10
 	range 2 255

subsys/bluetooth/controller/hci/hci_driver.c

@@ -73,7 +73,7 @@ struct k_thread prio_recv_thread_data;
 static K_KERNEL_STACK_DEFINE(prio_recv_thread_stack,
 			     CONFIG_BT_CTLR_RX_PRIO_STACK_SIZE);
 struct k_thread recv_thread_data;
-static K_KERNEL_STACK_DEFINE(recv_thread_stack, CONFIG_BT_RX_STACK_SIZE);
+static K_KERNEL_STACK_DEFINE(recv_thread_stack, CONFIG_BT_CTLR_RX_PRIO_STACK_SIZE);
 
 #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL)
 static struct k_poll_signal hbuf_signal;
@@ -81,11 +81,10 @@ static sys_slist_t hbuf_pend;
 static int32_t hbuf_count;
 #endif
 
-#if !defined(CONFIG_BT_RECV_BLOCKING)
 /* Copied here from `hci_raw.c`, which would be used in
  * conjunction with this driver when serializing HCI over wire.
- * This serves as a converter from the more complicated
+ * This serves as a converter from the historical (removed from
- * `CONFIG_BT_RECV_BLOCKING` API to the normal single-receiver
+ * tree) 'recv blocking' API to the normal single-receiver
  * `bt_recv` API.
  */
 int bt_recv_prio(struct net_buf *buf)
@@ -103,7 +102,6 @@ int bt_recv_prio(struct net_buf *buf)
 
 	return bt_recv(buf);
 }
-#endif /* CONFIG_BT_RECV_BLOCKING */
 
 #if defined(CONFIG_BT_CTLR_ISO)
 

subsys/bluetooth/host/Kconfig

@@ -95,7 +95,7 @@ config BT_HCI_RESERVE
 
 choice BT_RECV_CONTEXT
 	prompt "BT RX Thread Selection"
-	default BT_RECV_BLOCKING if BT_LL_SW_SPLIT || BT_H4
+	default BT_RECV_WORKQ_SYS if SOC_SERIES_NRF51X
 	default BT_RECV_WORKQ_BT
 	help
 	  Selects in which context incoming low priority HCI packets are processed.
@@ -104,13 +104,6 @@ choice BT_RECV_CONTEXT
 	  or bt_recv_prio(). The choice will influence RAM usage and how fast incoming HCI
 	  packets are processed.
 
-config BT_RECV_BLOCKING
-	bool "Process HCI packets in the context of bt_recv() and bt_recv_prio()"
-	help
-	  When this option is selected, the host will not have its own RX thread.
-	  With this option it is the responsibility of the HCI driver to call
-	  bt_recv_prio from a higher priority context than bt_recv() in order to avoid deadlocks.
-
 config BT_RECV_WORKQ_SYS
 	bool "Process low priority HCI packets in the system work queue"
 	help

subsys/bluetooth/host/hci_core.c

@@ -66,14 +66,12 @@ LOG_MODULE_REGISTER(bt_hci_core);
 #define HCI_CMD_TIMEOUT      K_SECONDS(10)
 
 /* Stacks for the threads */
-#if !defined(CONFIG_BT_RECV_BLOCKING)
 static void rx_work_handler(struct k_work *work);
 static K_WORK_DEFINE(rx_work, rx_work_handler);
 #if defined(CONFIG_BT_RECV_WORKQ_BT)
 static struct k_work_q bt_workq;
 static K_KERNEL_STACK_DEFINE(rx_thread_stack, CONFIG_BT_RX_STACK_SIZE);
 #endif /* CONFIG_BT_RECV_WORKQ_BT */
-#endif /* !CONFIG_BT_RECV_BLOCKING */
 static struct k_thread tx_thread_data;
 static K_KERNEL_STACK_DEFINE(tx_thread_stack, CONFIG_BT_HCI_TX_STACK_SIZE);
 
@@ -3854,7 +3852,6 @@ void hci_event_prio(struct net_buf *buf)
 	}
 }
 
-#if !defined(CONFIG_BT_RECV_BLOCKING)
 static void rx_queue_put(struct net_buf *buf)
 {
 	net_buf_slist_put(&bt_dev.rx_queue, buf);
@@ -3868,7 +3865,6 @@ static void rx_queue_put(struct net_buf *buf)
 		LOG_ERR("Could not submit rx_work: %d", err);
 	}
 }
-#endif /* !CONFIG_BT_RECV_BLOCKING */
 
 int bt_recv(struct net_buf *buf)
 {
@@ -3879,18 +3875,11 @@ int bt_recv(struct net_buf *buf)
 	switch (bt_buf_get_type(buf)) {
 #if defined(CONFIG_BT_CONN)
 	case BT_BUF_ACL_IN:
-#if defined(CONFIG_BT_RECV_BLOCKING)
-		hci_acl(buf);
-#else
 		rx_queue_put(buf);
-#endif
 		return 0;
 #endif /* BT_CONN */
 	case BT_BUF_EVT:
 	{
-#if defined(CONFIG_BT_RECV_BLOCKING)
-		hci_event(buf);
-#else
 		struct bt_hci_evt_hdr *hdr = (void *)buf->data;
 		uint8_t evt_flags = bt_hci_evt_get_flags(hdr->evt);
 
@@ -3901,17 +3890,12 @@ int bt_recv(struct net_buf *buf)
 		if (evt_flags & BT_HCI_EVT_FLAG_RECV) {
 			rx_queue_put(buf);
 		}
-#endif
-		return 0;
 
+		return 0;
 	}
 #if defined(CONFIG_BT_ISO)
 	case BT_BUF_ISO_IN:
-#if defined(CONFIG_BT_RECV_BLOCKING)
-		hci_iso(buf);
-#else
 		rx_queue_put(buf);
-#endif
 		return 0;
 #endif /* CONFIG_BT_ISO */
 	default:
@@ -3921,19 +3905,6 @@ int bt_recv(struct net_buf *buf)
 	}
 }
 
-#if defined(CONFIG_BT_RECV_BLOCKING)
-int bt_recv_prio(struct net_buf *buf)
-{
-	bt_monitor_send(bt_monitor_opcode(buf), buf->data, buf->len);
-
-	BT_ASSERT(bt_buf_get_type(buf) == BT_BUF_EVT);
-
-	hci_event_prio(buf);
-
-	return 0;
-}
-#endif /* CONFIG_BT_RECV_BLOCKING */
-
 int bt_hci_driver_register(const struct bt_hci_driver *drv)
 {
 	if (bt_dev.drv) {
@@ -4011,7 +3982,6 @@ static void init_work(struct k_work *work)
 	}
 }
 
-#if !defined(CONFIG_BT_RECV_BLOCKING)
 static void rx_work_handler(struct k_work *work)
 {
 	int err;
@@ -4063,7 +4033,6 @@ static void rx_work_handler(struct k_work *work)
 		}
 	}
 }
-#endif /* !CONFIG_BT_RECV_BLOCKING */
 
 #if defined(CONFIG_BT_TESTING)
 k_tid_t bt_testing_tx_tid_get(void)

subsys/bluetooth/host/hci_core.h

@@ -366,10 +366,8 @@ struct bt_dev {
 	/* Last sent HCI command */
 	struct net_buf		*sent_cmd;
 
-#if !defined(CONFIG_BT_RECV_BLOCKING)
 	/* Queue for incoming HCI events & ACL data */
 	sys_slist_t rx_queue;
-#endif
 
 	/* Queue for outgoing HCI commands */
 	struct k_fifo		cmd_tx_queue;

subsys/bluetooth/host/hci_ecc.c

@@ -92,11 +92,7 @@ static void send_cmd_status(uint16_t opcode, uint8_t status)
 	evt->opcode = sys_cpu_to_le16(opcode);
 	evt->status = status;
 
-	if (IS_ENABLED(CONFIG_BT_RECV_BLOCKING)) {
+	bt_recv(buf);
-		bt_recv_prio(buf);
-	} else {
-		bt_recv(buf);
-	}
 }
 
 static uint8_t generate_keys(void)

tests/bsim/bluetooth/host/misc/conn_stress/central/prj.conf

@@ -32,9 +32,6 @@ CONFIG_ASSERT_ON_ERRORS=y
 CONFIG_BT_RECV_WORKQ_BT=y
 # CONFIG_BT_RECV_WORKQ_SYS=y
 
-# errors out getting event flags in hci_core.c
-# CONFIG_BT_RECV_BLOCKING=y
-
 # TODO: remove when test is stable
 CONFIG_BT_AUTO_PHY_UPDATE=n
 CONFIG_BT_AUTO_DATA_LEN_UPDATE=n