Bluetooth: ISO: Make setting ISO data explicit

The stack will no longer implicitly set the data path
for ISO channel, and the responsibility for doing that is
now for the upper layers/applications.

This provides additional flexibility for the higher layers
as they can better control the values and timing of the data
path, as well as support removing and even reconfiguring the
data path at will.
This also removes some complexity from the stack.

This commit also fixed a inconsistency in the disconnected
handler. CIS for centrals as well as BIS were still valid
bt_iso_chan channels in the disconnected callback,
but CIS for peripherals were completely cleaned up at this
point. This issue is fixed by moving the disconnected callback
handling to before the code to cleanup the channel for
peripherals.

Since there is a difference in how you remove data paths
depending on the GAP role (central/peripheral), the
iso_info struct type has been expanded to be more
concise of which type of CIS it is.

Signed-off-by: Emil Gydesen <[email protected]>

Author: Thalley

doc/releases/migration-guide-4.1.rst

@@ -590,7 +590,7 @@ Bluetooth Host
   pairing, it should disable :kconfig:option:`CONFIG_BT_SMP_SC_PAIR_ONLY` manually.
 
 * The prompt for :kconfig:option:`CONFIG_BT_ECC` has been removed, since it only offers an internal
-  API, meaning internal users should explicitly select it in their respective Kconfig options.
+  API, meaning internal users should explicitly select it in their respecti
 
 Bluetooth Crypto
 ================

doc/releases/migration-guide-4.2.rst

@@ -35,6 +35,13 @@ Device Drivers and Devicetree
 Bluetooth
 *********
 
+Bluetooth Host
+==============
+
+* The ISO data paths are not longer setup automatically, and shall explicitly be setup and removed
+  by the application by calling :c:func:`bt_iso_setup_data_path` and
+  :c:func:`bt_iso_remove_data_path` respectively. (:github:`75549`)
+
 Networking
 **********
 

include/zephyr/bluetooth/iso.h

@@ -70,6 +70,14 @@ extern "C" {
 
 /** Unknown SDU interval */
 #define BT_ISO_SDU_INTERVAL_UNKNOWN 0x000000U
+/** The minimum value for vendor specific data path ID */
+#define BT_ISO_DATA_PATH_VS_ID_MIN  0x01
+/** The maximum value for vendor specific data path ID */
+#define BT_ISO_DATA_PATH_VS_ID_MAX  0xFE
+/** Minimum controller delay in microseconds (0 s) */
+#define BT_ISO_CONTROLLER_DELAY_MIN 0x000000
+/** Maximum controller delay in microseconds (4 s) */
+#define BT_ISO_CONTROLLER_DELAY_MAX 0x3D0900
 /** Minimum interval value in microseconds */
 #define BT_ISO_SDU_INTERVAL_MIN     0x0000FFU
 /** Maximum interval value in microseconds */
@@ -178,7 +186,8 @@ enum bt_iso_state {
  */
 enum bt_iso_chan_type {
 	BT_ISO_CHAN_TYPE_NONE,		/**< No channel type */
-	BT_ISO_CHAN_TYPE_CONNECTED,	/**< Connected */
+	BT_ISO_CHAN_TYPE_CENTRAL,       /**< Connected as central */
+	BT_ISO_CHAN_TYPE_PERIPHERAL,    /**< Connected as peripheral */
 	BT_ISO_CHAN_TYPE_BROADCASTER,	/**< Isochronous broadcaster */
 	BT_ISO_CHAN_TYPE_SYNC_RECEIVER	/**< Synchronized receiver */
 };
@@ -230,13 +239,6 @@ struct bt_iso_chan_io_qos {
 	 * This value is ignored if any advanced ISO parameters are set.
 	 */
 	uint8_t				rtn;
-	/**
-	 * @brief Channel data path reference
-	 *
-	 * Setting to NULL default to HCI data path (same as setting path.pid
-	 * to @ref BT_ISO_DATA_PATH_HCI).
-	 */
-	struct bt_iso_chan_path		*path;
 
 #if defined(CONFIG_BT_ISO_TEST_PARAMS) || defined(__DOXYGEN__)
 	/**
@@ -293,20 +295,37 @@ struct bt_iso_chan_qos {
 
 /** @brief ISO Channel Data Path structure. */
 struct bt_iso_chan_path {
-	/** Default path ID */
-	uint8_t				pid;
-	/** Coding Format */
-	uint8_t				format;
+	/**
+	 * @brief Default path ID
+	 *
+	 * @ref BT_ISO_DATA_PATH_HCI to use ISO over  HCI or between @ref BT_ISO_DATA_PATH_VS_ID_MIN
+	 * and @ref BT_ISO_DATA_PATH_VS_ID_MAX for vendor specific data paths.
+	 */
+	uint8_t pid;
+	/**
+	 * @brief Coding Format
+	 *
+	 * See the BT_HCI_CODING_FORMAT_* values for valid values.
+	 */
+	uint8_t format;
 	/** Company ID */
-	uint16_t			cid;
+	uint16_t cid;
 	/** Vendor-defined Codec ID */
-	uint16_t			vid;
-	/** Controller Delay */
-	uint32_t			delay;
-	/** Codec Configuration length*/
-	uint8_t				cc_len;
-	/** Pointer to an array containing the Codec Configuration */
-	uint8_t				*cc;
+	uint16_t vid;
+	/**
+	 * @brief Controller Delay in microseconds
+	 *
+	 * Value range from @ref BT_ISO_CONTROLLER_DELAY_MIN to @ref BT_ISO_CONTROLLER_DELAY_MAX.
+	 */
+	uint32_t delay;
+	/** Codec Configuration length */
+	uint8_t cc_len;
+	/**
+	 * @brief Pointer to an array containing the Codec Configuration
+	 *
+	 * Shall not be NULL if bt_iso_chan_path.cc_len is non-zero.
+	 */
+	uint8_t *cc;
 };
 
 /** ISO packet status flag bits */
@@ -691,6 +710,14 @@ struct bt_iso_chan_ops {
 	 * channel is disconnected, including when a connection gets
 	 * rejected or when setting security fails.
 	 *
+	 * If the channel was established then it still contain references to the BIS/CIS handles
+	 * and is not completely free'd at this point. The memory of the channel shall not be
+	 * memset to 0 or free'd.
+	 * To avoid any issue it is recommended to use a @ref k_work_submit or similar to not
+	 * overwrite any data while in the callback.
+	 *
+	 * For the above reason it is still possible to use bt_iso_chan_get_info() on the @p chan.
+	 *
 	 * @param chan   The channel that has been Disconnected
 	 * @param reason BT_HCI_ERR_* reason for the disconnection.
 	 */
@@ -957,6 +984,69 @@ int bt_iso_chan_send(struct bt_iso_chan *chan, struct net_buf *buf, uint16_t seq
 int bt_iso_chan_send_ts(struct bt_iso_chan *chan, struct net_buf *buf, uint16_t seq_num,
 			uint32_t ts);
 
+/**
+ * @brief Sets up the ISO data path for a ISO channel
+ *
+ * The channel must be associated with a BIS or CIS handle first which it is when the
+ * bt_iso_chan_ops.connected() callback is called.
+ *
+ * @param chan The channel to setup the ISO data path for
+ * @param dir The direction to setup, either @ref BT_HCI_DATAPATH_DIR_CTLR_TO_HOST or
+ *            @ref BT_HCI_DATAPATH_DIR_HOST_TO_CTLR. For ISO broadcast channels this can only be
+ *            @ref BT_HCI_DATAPATH_DIR_HOST_TO_CTLR, and for ISO sync receiver channels this can
+ *            only be @ref BT_HCI_DATAPATH_DIR_CTLR_TO_HOST.
+ * @param path The data path
+ *
+ * @retval 0 Success
+ * @retval -EINVAL Invalid parameters
+ * @retval -ENOBUFS No HCI command buffer could be allocated
+ * @retval -EIO The controller rejected the request or response contains invalid data
+ * @retval -ENODEV @p chan is not associated with a CIS or BIS handle
+ * @retval -EACCES The controller rejected the request as disallowed
+ * @retval -ENOEXEC Unexpected error occurred
+ */
+int bt_iso_setup_data_path(const struct bt_iso_chan *chan, uint8_t dir,
+			   const struct bt_iso_chan_path *path);
+
+/**
+ * @brief Removes the ISO data path for a ISO channel
+ *
+ * Removes the ISO data path configured by bt_iso_setup_data_path() for the provided @p dir.
+ *
+ * As per Bluetooth Core specification 5.4, Vol 4, Part E, Section 7.7.5, the data paths of CIS for
+ * Peripherals are deleted by the controller, and thus it is not necessary (or possible) to remove
+ * data paths of CIS after they have disconnected for a Peripheral.
+ * The data paths for CIS for a Central remain valid, even after a disconnection, and thus a Central
+ * device should call bt_iso_remove_data_path() on disconnect if it no longer wants to use that CIS.
+ * All data paths created by a Central are removed when the CIG is removed with
+ * bt_iso_cig_terminate().
+ *
+ * As per Bluetooth Core specification 5.4, Vol 4, Part E, Section 7.7.65.30 any data paths
+ * associated with an ISO Sync Receiver BIG are removed by the controller when the BIG sync is lost
+ * or terminated, and thus it is not necessary (or possible) to remove data paths of ISO channels
+ * associated with a BIG for a Sync Receiver.
+ *
+ * As per Bluetooth Core specification 5.4, Vol 4, Part E, Section 7.8.105 all data paths associated
+ * with an ISO Broadcaster BIG are removed when the BIG is terminated by bt_iso_big_terminate(),
+ * and thus it is not necessary (or possible) to remove data paths of ISO channels
+ * associated with a BIG for a Broadcaster.
+ *
+ * @param chan The channel to setup the ISO data path for
+ * @param dir The direction to setup, either @ref BT_HCI_DATAPATH_DIR_CTLR_TO_HOST or
+ *            @ref BT_HCI_DATAPATH_DIR_HOST_TO_CTLR. For ISO broadcast channels this can only be
+ *            @ref BT_HCI_DATAPATH_DIR_HOST_TO_CTLR, and for ISO sync receiver channels this can
+ *            only be @ref BT_HCI_DATAPATH_DIR_CTLR_TO_HOST.
+
+ * @retval 0 Success
+ * @retval -EINVAL Invalid parameters
+ * @retval -ENOBUFS No HCI command buffer could be allocated
+ * @retval -EIO The controller rejected the request or response contains invalid data
+ * @retval -ENODEV @p chan is not associated with a CIS or BIS handle
+ * @retval -EACCES The controller rejected the request as disallowed
+ * @retval -ENOEXEC Unexpected error occurred
+ */
+int bt_iso_remove_data_path(const struct bt_iso_chan *chan, uint8_t dir);
+
 /** @brief ISO Unicast TX Info Structure */
 struct bt_iso_unicast_tx_info {
 	/** The transport latency in us */
@@ -1082,20 +1172,30 @@ struct bt_iso_info {
 	/** Connection Type specific Info.*/
 	union {
 #if defined(CONFIG_BT_ISO_UNICAST) || defined(__DOXYGEN__)
-		/** Unicast specific Info.
+		/**
+		 * @brief Unicast specific Info.
+		 *
 		 * Only available when @kconfig{CONFIG_BT_ISO_UNICAST} is enabled.
+		 * Use this when the @ref bt_iso_info.type is @ref BT_ISO_CHAN_TYPE_CENTRAL or
+		 * @ref BT_ISO_CHAN_TYPE_PERIPHERAL.
 		 */
 		struct bt_iso_unicast_info unicast;
 #endif /* CONFIG_BT_ISO_UNICAST */
 #if defined(CONFIG_BT_ISO_BROADCASTER) || defined(__DOXYGEN__)
-		/** Broadcaster specific Info.
+		/**
+		 * @brief Broadcaster specific Info.
+		 *
 		 * Only available when @kconfig{CONFIG_BT_ISO_BROADCASTER} is enabled.
+		 * Use this when the @ref bt_iso_info.type is @ref BT_ISO_CHAN_TYPE_BROADCASTER.
 		 */
 		struct bt_iso_broadcaster_info broadcaster;
 #endif /* CONFIG_BT_ISO_BROADCASTER */
 #if defined(CONFIG_BT_ISO_SYNC_RECEIVER) || defined(__DOXYGEN__)
-		/** Sync receiver specific Info.
+		/**
+		 * @brief Sync receiver specific Info.
+		 *
 		 * Only available when @kconfig{CONFIG_BT_ISO_SYNC_RECEIVER} is enabled.
+		 * Use this when the @ref bt_iso_info.type is @ref BT_ISO_CHAN_TYPE_SYNC_RECEIVER.
 		 */
 		struct bt_iso_sync_receiver_info sync_receiver;
 #endif /* CONFIG_BT_ISO_SYNC_RECEIVER */

samples/bluetooth/iso_broadcast/src/main.c

@@ -8,6 +8,7 @@
 
 #include <zephyr/bluetooth/bluetooth.h>
 #include <zephyr/bluetooth/gap.h>
+#include <zephyr/bluetooth/hci_types.h>
 #include <zephyr/bluetooth/iso.h>
 #include <zephyr/sys/byteorder.h>
 
@@ -31,10 +32,21 @@ static uint16_t seq_num;
 
 static void iso_connected(struct bt_iso_chan *chan)
 {
+	const struct bt_iso_chan_path hci_path = {
+		.pid = BT_ISO_DATA_PATH_HCI,
+		.format = BT_HCI_CODING_FORMAT_TRANSPARENT,
+	};
+	int err;
+
 	printk("ISO Channel %p connected\n", chan);
 
 	seq_num = 0U;
 
+	err = bt_iso_setup_data_path(chan, BT_HCI_DATAPATH_DIR_HOST_TO_CTLR, &hci_path);
+	if (err != 0) {
+		printk("Failed to setup ISO TX data path: %d\n", err);
+	}
+
 	k_sem_give(&sem_big_cmplt);
 }
 

samples/bluetooth/iso_broadcast_benchmark/src/broadcaster.c

@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #include <zephyr/bluetooth/gap.h>
+#include <zephyr/bluetooth/hci_types.h>
 #include <zephyr/console/console.h>
 #include <zephyr/bluetooth/bluetooth.h>
 #include <zephyr/bluetooth/iso.h>
@@ -72,8 +73,19 @@ static const struct bt_data ad[] = {
 
 static void iso_connected(struct bt_iso_chan *chan)
 {
+	const struct bt_iso_chan_path hci_path = {
+		.pid = BT_ISO_DATA_PATH_HCI,
+		.format = BT_HCI_CODING_FORMAT_TRANSPARENT,
+	};
+	int err;
+
 	LOG_INF("ISO Channel %p connected", chan);
 
+	err = bt_iso_setup_data_path(chan, BT_HCI_DATAPATH_DIR_HOST_TO_CTLR, &hci_path);
+	if (err != 0) {
+		printk("Failed to setup ISO TX data path: %d\n", err);
+	}
+
 	connected_bis++;
 	if (connected_bis == big_create_param.num_bis) {
 		seq_num = 0U;

samples/bluetooth/iso_broadcast_benchmark/src/receiver.c

@@ -1,12 +1,13 @@
 /*
- * Copyright (c) 2021 Nordic Semiconductor ASA
+ * Copyright (c) 2021-2024 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #include <ctype.h>
 #include <zephyr/bluetooth/bluetooth.h>
 #include <zephyr/bluetooth/conn.h>
+#include <zephyr/bluetooth/hci_types.h>
 #include <zephyr/bluetooth/iso.h>
 #include <zephyr/sys/byteorder.h>
 #include <zephyr/console/console.h>
@@ -229,10 +230,21 @@ static void iso_recv(struct bt_iso_chan *chan,
 
 static void iso_connected(struct bt_iso_chan *chan)
 {
+	const struct bt_iso_chan_path hci_path = {
+		.pid = BT_ISO_DATA_PATH_HCI,
+		.format = BT_HCI_CODING_FORMAT_TRANSPARENT,
+	};
+	int err;
+
 	LOG_INF("ISO Channel %p connected", chan);
 
 	big_sync_start_time = k_uptime_get();
 
+	err = bt_iso_setup_data_path(chan, BT_HCI_DATAPATH_DIR_CTLR_TO_HOST, &hci_path);
+	if (err != 0) {
+		printk("Failed to setup ISO RX data path: %d\n", err);
+	}
+
 	k_sem_give(&sem_big_sync);
 }
 

samples/bluetooth/iso_central/src/main.c

@@ -16,6 +16,7 @@
 #include <zephyr/bluetooth/conn.h>
 #include <zephyr/bluetooth/uuid.h>
 #include <zephyr/bluetooth/gatt.h>
+#include <zephyr/bluetooth/hci_types.h>
 #include <zephyr/bluetooth/iso.h>
 #include <zephyr/settings/settings.h>
 #include <zephyr/sys/byteorder.h>
@@ -142,18 +143,36 @@ static void start_scan(void)
 
 static void iso_connected(struct bt_iso_chan *chan)
 {
+	const struct bt_iso_chan_path hci_path = {
+		.pid = BT_ISO_DATA_PATH_HCI,
+		.format = BT_HCI_CODING_FORMAT_TRANSPARENT,
+	};
+	int err;
+
 	printk("ISO Channel %p connected\n", chan);
 
 	seq_num = 0U;
 
-	/* Start send timer */
-	k_work_schedule(&iso_send_work, K_MSEC(0));
+	err = bt_iso_setup_data_path(chan, BT_HCI_DATAPATH_DIR_HOST_TO_CTLR, &hci_path);
+	if (err != 0) {
+		printk("Failed to setup ISO TX data path: %d\n", err);
+	} else {
+		/* Start send timer */
+		k_work_schedule(&iso_send_work, K_NO_WAIT);
+	}
 }
 
 static void iso_disconnected(struct bt_iso_chan *chan, uint8_t reason)
 {
+	int err;
+
 	printk("ISO Channel %p disconnected (reason 0x%02x)\n", chan, reason);
 	k_work_cancel_delayable(&iso_send_work);
+
+	err = bt_iso_remove_data_path(chan, BT_HCI_DATAPATH_DIR_HOST_TO_CTLR);
+	if (err != 0) {
+		printk("Failed to setup ISO TX data path: %d\n", err);
+	}
 }
 
 static struct bt_iso_chan_ops iso_ops = {
@@ -165,7 +184,6 @@ static struct bt_iso_chan_io_qos iso_tx = {
 	.sdu = CONFIG_BT_ISO_TX_MTU,
 	.phy = BT_GAP_LE_PHY_2M,
 	.rtn = 1,
-	.path = NULL,
 };
 
 static struct bt_iso_chan_qos iso_qos = {