Bluetooth: HCI: Use H:4 encoding for buffers

Encode the packet type as a H:4 payload prefix for buffers passing to &
from HCI drivers. The existing bt_buf_set/get_type functions are
deprecated, but kept compatible with the change, except that they can only
be called once, since they modify the buffer payload.

Signed-off-by: Johan Hedberg <[email protected]>

Author: jhedberg

include/zephyr/bluetooth/buf.h

@@ -36,6 +36,8 @@ extern "C" {
 
 /** Possible types of buffers passed around the Bluetooth stack in a form of bitmask. */
 enum bt_buf_type {
+	/** Invalid value, used for error checking */
+	BT_BUF_TYPE_INVALID = 0,
 	/** HCI command */
 	BT_BUF_CMD = BIT(0),
 	/** HCI event */
@@ -48,15 +50,60 @@ enum bt_buf_type {
 	BT_BUF_ISO_OUT = BIT(4),
 	/** Incoming ISO data */
 	BT_BUF_ISO_IN = BIT(5),
-	/** H:4 data */
-	BT_BUF_H4 = BIT(6),
 };
 
-/** @brief This is a base type for bt_buf user data. */
-struct bt_buf_data {
-	uint8_t type;
+/** Direction of HCI packets. Only used for mapping H:4 to BT_BUF_* values. */
+enum bt_buf_dir {
+	BT_BUF_IN,
+	BT_BUF_OUT,
 };
 
+/** Convert from bt_buf_type to H:4 type.
+ *
+ *  @param type The bt_buf_type to convert
+ *  @return The H:4 type
+ */
+static inline uint8_t bt_buf_type_to_h4(enum bt_buf_type type)
+{
+	switch (type) {
+	case BT_BUF_CMD:
+		return BT_HCI_H4_CMD;
+	case BT_BUF_ACL_IN:
+	case BT_BUF_ACL_OUT:
+		return BT_HCI_H4_ACL;
+	case BT_BUF_ISO_IN:
+	case BT_BUF_ISO_OUT:
+		return BT_HCI_H4_ISO;
+	case BT_BUF_EVT:
+		return BT_HCI_H4_EVT;
+	default:
+		__ASSERT_NO_MSG(false);
+		return 0;
+	}
+}
+
+/** Convert from H:4 type to bt_buf_type.
+ *
+ *  @param h4_type The H:4 type to convert
+ *  @param dir     The direction of the packet
+ *  @return The bt_buf_type
+ */
+static inline enum bt_buf_type bt_buf_type_from_h4(uint8_t h4_type, enum bt_buf_dir dir)
+{
+	switch (h4_type) {
+	case BT_HCI_H4_CMD:
+		return BT_BUF_CMD;
+	case BT_HCI_H4_ACL:
+		return dir == BT_BUF_OUT ? BT_BUF_ACL_OUT : BT_BUF_ACL_IN;
+	case BT_HCI_H4_EVT:
+		return BT_BUF_EVT;
+	case BT_HCI_H4_ISO:
+		return dir == BT_BUF_OUT ? BT_BUF_ISO_OUT : BT_BUF_ISO_IN;
+	default:
+		return BT_BUF_TYPE_INVALID;
+	}
+}
+
 /* Headroom reserved in buffers, primarily for HCI transport encoding purposes */
 #define BT_BUF_RESERVE 1
 
@@ -138,8 +185,7 @@ BUILD_ASSERT(CONFIG_BT_BUF_EVT_RX_COUNT > CONFIG_BT_BUF_ACL_TX_COUNT,
 
 /** Allocate a buffer for incoming data
  *
- *  This will set the buffer type so bt_buf_set_type() does not need to
- *  be explicitly called.
+ *  This will set the buffer type so it doesn't need to be explicitly encoded into the buffer.
  *
  *  @param type    Type of buffer. Only BT_BUF_EVT, BT_BUF_ACL_IN and BT_BUF_ISO_IN
  *                 are allowed.
@@ -172,14 +218,12 @@ void bt_buf_rx_freed_cb_set(bt_buf_rx_freed_cb_t cb);
 
 /** Allocate a buffer for outgoing data
  *
- *  This will set the buffer type so bt_buf_set_type() does not need to
- *  be explicitly called.
+ *  This will set the buffer type so it doesn't need to be explicitly encoded into the buffer.
  *
- *  @param type    Type of buffer. Only BT_BUF_CMD, BT_BUF_ACL_OUT or
- *                 BT_BUF_H4, when operating on H:4 mode, are allowed.
+ *  @param type    Type of buffer. BT_BUF_CMD or BT_BUF_ACL_OUT.
  *  @param timeout Non-negative waiting period to obtain a buffer or one of the
  *                 special values K_NO_WAIT and K_FOREVER.
- *  @param data    Initial data to append to buffer.
+ *  @param data    Initial data to append to buffer. This is optional and can be NULL.
  *  @param size    Initial data size.
  *  @return A new buffer.
  */
@@ -188,8 +232,7 @@ struct net_buf *bt_buf_get_tx(enum bt_buf_type type, k_timeout_t timeout,
 
 /** Allocate a buffer for an HCI Event
  *
- *  This will set the buffer type so bt_buf_set_type() does not need to
- *  be explicitly called.
+ *  This will set the buffer type so it doesn't need to be explicitly encoded into the buffer.
  *
  *  @param evt          HCI event code
  *  @param discardable  Whether the driver considers the event discardable.
@@ -199,26 +242,33 @@ struct net_buf *bt_buf_get_tx(enum bt_buf_type type, k_timeout_t timeout,
  */
 struct net_buf *bt_buf_get_evt(uint8_t evt, bool discardable, k_timeout_t timeout);
 
-/** Set the buffer type
+/** Set the buffer type. The type is encoded as an H:4 byte prefix as part of
+ *  the payload itself.
  *
  *  @param buf   Bluetooth buffer
  *  @param type  The BT_* type to set the buffer to
  */
-static inline void bt_buf_set_type(struct net_buf *buf, enum bt_buf_type type)
+static inline void __deprecated bt_buf_set_type(struct net_buf *buf, enum bt_buf_type type)
 {
-	((struct bt_buf_data *)net_buf_user_data(buf))->type = type;
+	__ASSERT_NO_MSG(net_buf_headroom(buf) >= 1);
+	net_buf_push_u8(buf, bt_buf_type_to_h4(type));
 }
 
-/** Get the buffer type
+
+/** Get the buffer type. This pulls the H:4 byte prefix from the payload, which means
+ *  that the call can be done only once per buffer.
  *
  *  @param buf   Bluetooth buffer
  *
  *  @return The BT_* type to of the buffer
  */
-static inline enum bt_buf_type bt_buf_get_type(struct net_buf *buf)
+static inline enum bt_buf_type __deprecated bt_buf_get_type(struct net_buf *buf)
 {
-	return (enum bt_buf_type)((struct bt_buf_data *)net_buf_user_data(buf))
-		->type;
+	/* We have to assume the direction since the H:4 type doesn't tell us
+	 * if the buffer is incoming or outgoing. The common use case of this API is for outgoing
+	 * buffers, so we assume that.
+	 */
+	return bt_buf_type_from_h4(net_buf_pull_u8(buf), BT_BUF_OUT);
 }
 
 /**

include/zephyr/bluetooth/hci_raw.h

@@ -38,87 +38,6 @@ extern "C" {
  */
 int bt_send(struct net_buf *buf);
 
-enum {
-	/** Passthrough mode
-	 *
-	 *  While in this mode the buffers are passed as is between the stack
-	 *  and the driver.
-	 */
-	BT_HCI_RAW_MODE_PASSTHROUGH = 0x00,
-
-	/** H:4 mode
-	 *
-	 *  While in this mode H:4 headers will added into the buffers
-	 *  according to the buffer type when coming from the stack and will be
-	 *  removed and used to set the buffer type.
-	 */
-	BT_HCI_RAW_MODE_H4 = 0x01,
-};
-
-/** @brief Set Bluetooth RAW channel mode
- *
- *  Set access mode of Bluetooth RAW channel.
- *
- *  @param mode Access mode.
- *
- *  @return Zero on success or (negative) error code otherwise.
- */
-int bt_hci_raw_set_mode(uint8_t mode);
-
-/** @brief Get Bluetooth RAW channel mode
- *
- *  Get access mode of Bluetooth RAW channel.
- *
- *  @return Access mode.
- */
-uint8_t bt_hci_raw_get_mode(void);
-
-#define BT_HCI_ERR_EXT_HANDLED  0xff
-
-/** Helper macro to define a command extension
- *
- *  @param _op Opcode of the command.
- *  @param _min_len Minimal length of the command.
- *  @param _func Handler function to be called.
- */
-#define BT_HCI_RAW_CMD_EXT(_op, _min_len, _func) \
-	{ \
-		.op = _op, \
-		.min_len = _min_len, \
-		.func = _func, \
-	}
-
-struct bt_hci_raw_cmd_ext {
-	/** Opcode of the command */
-	uint16_t  op;
-
-	/** Minimal length of the command */
-	size_t min_len;
-
-	/** Handler function.
-	 *
-	 *  Handler function to be called when a command is intercepted.
-	 *
-	 *  @param buf Buffer containing the command.
-	 *
-	 *  @return HCI Status code or BT_HCI_ERR_EXT_HANDLED if command has
-	 *  been handled already and a response has been sent as oppose to
-	 *  BT_HCI_ERR_SUCCESS which just indicates that the command can be
-	 *  sent to the controller to be processed.
-	 */
-	uint8_t   (*func)(struct net_buf *buf);
-};
-
-/** @brief Register Bluetooth RAW command extension table
- *
- *  Register Bluetooth RAW channel command extension table, opcodes in this
- *  table are intercepted to sent to the handler function.
- *
- *  @param cmds Pointer to the command extension table.
- *  @param size Size of the command extension table.
- */
-void bt_hci_raw_cmd_ext_register(struct bt_hci_raw_cmd_ext *cmds, size_t size);
-
 /** @brief Enable Bluetooth RAW channel:
  *
  *  Enable Bluetooth RAW HCI channel.

include/zephyr/drivers/bluetooth.h

@@ -114,9 +114,8 @@ __subsystem struct bt_hci_driver_api {
  *
  * @param dev  HCI device
  * @param recv This is callback through which the HCI driver provides the
- *             host with data from the controller. The buffer passed to
- *             the callback will have its type set with bt_buf_set_type().
- *             The callback is expected to be called from thread context.
+ *             host with data from the controller. The callback is expected
+ *             to be called from thread context.
  *
  * @return 0 on success or negative POSIX error number on failure.
  */
@@ -151,8 +150,10 @@ static inline int bt_hci_close(const struct device *dev)
 /**
  * @brief Send HCI buffer to controller.
  *
- * Send an HCI packet to the controller. The packet type of the buffer
- * must be set using bt_buf_set_type().
+ * Send an HCI packet to the controller. The packet type is encoded as H:4,
+ * i.e. the UART transport encoding, as a prefix to the actual payload. This means
+ * that HCI drivers that use H:4 as their native encoding don't need to do any
+ * special handling of the packet type.
  *
  * @note This function must only be called from a cooperative thread.
  *

samples/bluetooth/hci_ipc/src/main.c

@@ -225,30 +225,10 @@ static void tx_thread(void *p1, void *p2, void *p3)
 
 static void hci_ipc_send(struct net_buf *buf, bool is_fatal_err)
 {
-	uint8_t pkt_indicator;
 	uint8_t retries = 0;
 	int ret;
 
-	LOG_DBG("buf %p type %u len %u", buf, bt_buf_get_type(buf), buf->len);
-
-	LOG_HEXDUMP_DBG(buf->data, buf->len, "Controller buffer:");
-
-	switch (bt_buf_get_type(buf)) {
-	case BT_BUF_ACL_IN:
-		pkt_indicator = HCI_IPC_ACL;
-		break;
-	case BT_BUF_EVT:
-		pkt_indicator = HCI_IPC_EVT;
-		break;
-	case BT_BUF_ISO_IN:
-		pkt_indicator = HCI_IPC_ISO;
-		break;
-	default:
-		LOG_ERR("Unknown type %u", bt_buf_get_type(buf));
-		net_buf_unref(buf);
-		return;
-	}
-	net_buf_push_u8(buf, pkt_indicator);
+	LOG_DBG("buf %p type %u len %u", buf, buf->data[0], buf->len);
 
 	LOG_HEXDUMP_DBG(buf->data, buf->len, "Final HCI buffer:");
 

samples/bluetooth/hci_spi/src/main.c

@@ -107,20 +107,7 @@ static inline int spi_send(struct net_buf *buf)
 				    0x00, 0x00, 0x00 };
 	int ret;
 
-	LOG_DBG("buf %p type %u len %u", buf, bt_buf_get_type(buf), buf->len);
-
-	switch (bt_buf_get_type(buf)) {
-	case BT_BUF_ACL_IN:
-		net_buf_push_u8(buf, HCI_ACL);
-		break;
-	case BT_BUF_EVT:
-		net_buf_push_u8(buf, HCI_EVT);
-		break;
-	default:
-		LOG_ERR("Unknown type %u", bt_buf_get_type(buf));
-		net_buf_unref(buf);
-		return -EINVAL;
-	}
+	LOG_DBG("buf %p type %u len %u", buf, buf->data[0], buf->len);
 
 	if (buf->len > SPI_MAX_MSG_LEN) {
 		LOG_ERR("TX message too long");
@@ -245,8 +232,7 @@ static void bt_tx_thread(void *p1, void *p2, void *p3)
 			continue;
 		}
 
-		LOG_DBG("buf %p type %u len %u",
-			buf, bt_buf_get_type(buf), buf->len);
+		LOG_DBG("buf %p type %u len %u", buf, buf->data[0], buf->len);
 
 		ret = bt_send(buf);
 		if (ret) {

samples/bluetooth/hci_uart/src/main.c

@@ -150,8 +150,8 @@ static void rx_isr(void)
 				 * interrupt. On failed allocation state machine
 				 * is reset.
 				 */
-				buf = bt_buf_get_tx(BT_BUF_H4, K_NO_WAIT,
-						    &type, sizeof(type));
+				buf = bt_buf_get_tx(bt_buf_type_from_h4(type, BT_BUF_OUT),
+						    K_NO_WAIT, NULL, 0);
 				if (!buf) {
 					LOG_ERR("No available command buffers!");
 					state = ST_IDLE;
@@ -272,8 +272,7 @@ static void tx_thread(void *p1, void *p2, void *p3)
 
 static int h4_send(struct net_buf *buf)
 {
-	LOG_DBG("buf %p type %u len %u", buf, bt_buf_get_type(buf),
-		    buf->len);
+	LOG_DBG("buf %p type %u len %u", buf, buf->data[0], buf->len);
 
 	k_fifo_put(&uart_tx_queue, buf);
 	uart_irq_tx_enable(hci_uart_dev);

samples/bluetooth/hci_uart_3wire/src/main.c

@@ -479,8 +479,8 @@ static void bt_uart_isr(const struct device *unused, void *user_data)
 			case HCI_COMMAND_PKT:
 			case HCI_ACLDATA_PKT:
 			case HCI_ISODATA_PKT:
-				h5.rx_buf = bt_buf_get_tx(BT_BUF_H4, K_NO_WAIT,
-							  &type, sizeof(type));
+				h5.rx_buf = bt_buf_get_tx(bt_buf_type_from_h4(type, BT_BUF_OUT),
+							  K_NO_WAIT, NULL, 0);
 				if (!h5.rx_buf) {
 					LOG_WRN("No available data buffers");
 					h5_reset_rx();
@@ -565,7 +565,7 @@ static void bt_uart_isr(const struct device *unused, void *user_data)
 
 static int h5_queue(struct net_buf *buf)
 {
-	LOG_DBG("buf %p type %u len %u", buf, bt_buf_get_type(buf), buf->len);
+	LOG_DBG("buf %p type %u len %u", buf, buf->data[0], buf->len);
 
 	k_fifo_put(&h5.tx_queue, buf);
 

samples/bluetooth/hci_uart_async/src/hci_uart_async.c

@@ -150,7 +150,6 @@ static void send_hw_error(void)
 
 	struct net_buf *buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER);
 
-	net_buf_add_u8(buf, BT_HCI_H4_EVT);
 	net_buf_add_mem(buf, hci_evt_hw_err, sizeof(hci_evt_hw_err));
 
 	/* Inject the message into the c2h queue. */
@@ -181,7 +180,8 @@ static void recover_sync_by_reset_pattern(void)
 	}
 
 	LOG_DBG("Pattern found");
-	h2c_cmd_reset = bt_buf_get_tx(BT_BUF_H4, K_FOREVER, h4_cmd_reset, sizeof(h4_cmd_reset));
+	h2c_cmd_reset = bt_buf_get_tx(BT_BUF_CMD, K_FOREVER,
+				      &h4_cmd_reset[1], sizeof(h4_cmd_reset) - 1);
 	LOG_DBG("Fowarding reset");
 
 	err = bt_send(h2c_cmd_reset);
@@ -219,7 +219,7 @@ static void h2c_h4_transport(void)
 		LOG_DBG("h2c: h4_type %d", h4_type);
 
 		/* Allocate buf. */
-		buf = bt_buf_get_tx(BT_BUF_H4, K_FOREVER, &h4_type, sizeof(h4_type));
+		buf = bt_buf_get_tx(bt_buf_type_from_h4(h4_type, BT_BUF_OUT), K_FOREVER, NULL, 0);
 		LOG_DBG("h2c: buf %p", buf);
 
 		if (!buf) {