Bluetooth: controller: Fixes and improvements for PTO implementation

- Add next pointer to node_rx_iso_meta for pre-transmission linked list (does not increase memory usage as union is larger). - Fix broadcaster not setting PTC with test configuration - Refactor stopping of sync ISO tickers to separate function Signed-off-by: Morten Priess <mtpr@oticon.com>
zephyrproject-rtos · Sep 27, 2024 · fe83a9d · fe83a9d
1 parent 541b516
commit fe83a9d
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 30 deletions.
diff --git a/subsys/bluetooth/controller/ll_sw/lll.h b/subsys/bluetooth/controller/ll_sw/lll.h
@@ -396,6 +396,7 @@ struct node_rx_iso_meta {
 	uint64_t payload_number:39; /* cisPayloadNumber */
 	uint64_t status:8;          /* Status of reception (OK/not OK) */
 	uint32_t timestamp;         /* Time of reception */
+	void     *next;             /* Pointer to next pre-transmission rx_node (BIS) */
 };
 
 /* Define invalid/unassigned Controller state/role instance handle */

diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_iso.c b/subsys/bluetooth/controller/ll_sw/ull_adv_iso.c
@@ -444,13 +444,15 @@ static uint8_t big_create(uint8_t big_handle, uint8_t adv_handle, uint8_t num_bi
 		return BT_HCI_ERR_INVALID_PARAM;
 	}
 
+	lll_adv_iso->ptc = ptc_calc(lll_adv_iso, event_spacing, event_spacing_max);
+
 	if (test_config) {
 		lll_adv_iso->pto = pto;
 
+		if (pto && !lll_adv_iso->ptc) {
+			return BT_HCI_ERR_INVALID_PARAM;
+		}
 	} else {
-		lll_adv_iso->ptc = ptc_calc(lll_adv_iso, event_spacing,
-						event_spacing_max);
-
 		/* Pre-Transmission Offset (PTO) */
 		if (lll_adv_iso->ptc) {
 			lll_adv_iso->pto = bn / lll_adv_iso->bn;

diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c b/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c
@@ -67,6 +67,7 @@ static void ticker_stop_op_cb(uint32_t status, void *param);
 static void sync_iso_disable(void *param);
 static void disabled_cb(void *param);
 static void lll_flush(void *param);
+static void stop_ticker(struct ll_sync_iso_set *sync_iso, ticker_op_func fp_op_func);
 
 static memq_link_t link_lll_prepare;
 static struct mayfly mfy_lll_prepare = {0U, 0U, &link_lll_prepare, NULL, NULL};
@@ -741,18 +742,10 @@ void ull_sync_iso_done(struct node_rx_event_done *done)
 
 	/* Check for establishmet failure */
 	if (done->extra.estab_failed) {
-		uint8_t handle;
-		uint32_t ret;
-
 		/* Stop Sync ISO Ticker directly. Establishment failure has been
 		 * notified.
 		 */
-		handle = sync_iso_handle_get(sync_iso);
-		ret = ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
-				  (TICKER_ID_SCAN_SYNC_ISO_BASE +
-				  sync_iso_handle_to_index(handle)), NULL, NULL);
-		LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
-			  (ret == TICKER_STATUS_BUSY));
+		stop_ticker(sync_iso, NULL);
 		return;
 	}
 
@@ -835,8 +828,6 @@ void ull_sync_iso_done_terminate(struct node_rx_event_done *done)
 	struct ll_sync_iso_set *sync_iso;
 	struct lll_sync_iso *lll;
 	struct node_rx_pdu *rx;
-	uint8_t handle;
-	uint32_t ret;
 
 	/* Get reference to ULL context */
 	sync_iso = CONTAINER_OF(done->param, struct ll_sync_iso_set, ull);
@@ -850,13 +841,7 @@ void ull_sync_iso_done_terminate(struct node_rx_event_done *done)
 	*((uint8_t *)rx->pdu) = lll->term_reason;
 
 	/* Stop Sync ISO Ticker */
-	handle = sync_iso_handle_get(sync_iso);
-	ret = ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
-			  (TICKER_ID_SCAN_SYNC_ISO_BASE +
-			   sync_iso_handle_to_index(handle)),
-			  ticker_stop_op_cb, (void *)sync_iso);
-	LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
-		  (ret == TICKER_STATUS_BUSY));
+	stop_ticker(sync_iso, ticker_stop_op_cb);
 }
 
 static void disable(uint8_t sync_idx)
@@ -866,6 +851,11 @@ static void disable(uint8_t sync_idx)
 
 	sync_iso = &ll_sync_iso[sync_idx];
 
+	/* Stop any active resume ticker */
+	(void)ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
+			  TICKER_ID_SCAN_SYNC_ISO_RESUME_BASE + sync_idx,
+			  NULL, NULL);
+
 	err = ull_ticker_stop_with_mark(TICKER_ID_SCAN_SYNC_ISO_BASE +
 					sync_idx, sync_iso, &sync_iso->lll);
 	LL_ASSERT_INFO2(err == 0 || err == -EALREADY, sync_idx, err);
@@ -935,8 +925,6 @@ static uint16_t sync_iso_stream_handle_get(struct lll_sync_iso_stream *stream)
 static void timeout_cleanup(struct ll_sync_iso_set *sync_iso)
 {
 	struct node_rx_pdu *rx;
-	uint8_t handle;
-	uint32_t ret;
 
 	/* Populate the Sync Lost which will be enqueued in disabled_cb */
 	rx = (void *)&sync_iso->node_rx_lost;
@@ -952,13 +940,7 @@ static void timeout_cleanup(struct ll_sync_iso_set *sync_iso)
 	}
 
 	/* Stop Sync ISO Ticker */
-	handle = sync_iso_handle_get(sync_iso);
-	ret = ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
-			  (TICKER_ID_SCAN_SYNC_ISO_BASE +
-			   sync_iso_handle_to_index(handle)),
-			  ticker_stop_op_cb, (void *)sync_iso);
-	LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
-		  (ret == TICKER_STATUS_BUSY));
+	stop_ticker(sync_iso, ticker_stop_op_cb);
 }
 
 static void ticker_cb(uint32_t ticks_at_expire, uint32_t ticks_drift,
@@ -1107,3 +1089,24 @@ static void disabled_cb(void *param)
 			     TICKER_USER_ID_LLL, 0U, &mfy);
 	LL_ASSERT(!ret);
 }
+
+static void stop_ticker(struct ll_sync_iso_set *sync_iso, ticker_op_func fp_op_func)
+{
+
+	uint8_t handle;
+	uint32_t ret;
+
+	handle = sync_iso_handle_get(sync_iso);
+
+	(void)ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
+			  TICKER_ID_SCAN_SYNC_ISO_RESUME_BASE +
+			  sync_iso_handle_to_index(handle), NULL, NULL);
+
+	ret = ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_ULL_HIGH,
+			  TICKER_ID_SCAN_SYNC_ISO_BASE +
+			  sync_iso_handle_to_index(handle),
+			  fp_op_func, fp_op_func ? (void *)sync_iso : NULL);
+
+	LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
+		  (ret == TICKER_STATUS_BUSY));
+}