Bluetooth: Controller: Review rework flush timeout support

Review rework changed related to flush timeout support.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Author: cvinayak

subsys/bluetooth/controller/hci/hci.c

@@ -5762,6 +5762,18 @@ int hci_iso_handle(struct net_buf *buf, struct net_buf **evt)
 		/* Catch up local pkt_seq_num with internal pkt_seq_num */
 		event_count = cis->lll.event_count;
 		pkt_seq_num = event_count + 1U;
+		/* If pb_flag is BT_ISO_START (0b00) or BT_ISO_SINGLE (0b10)
+		 * then we simply check that the pb_flag is an even value, and
+		 * then  pkt_seq_num is a future sequence number value compare
+		 * to last recorded number in cis->pkt_seq_num.
+		 *
+		 * When (pkt_seq_num - stream->pkt_seq_num) is negative then
+		 * BIT64(39) will be set (2's compliment value). The diff value
+		 * less than or equal to BIT64_MASK(38) means the diff value is
+		 * positive and hence pkt_seq_num is greater than
+		 * stream->pkt_seq_num. This calculation is valid for when value
+		 * rollover too.
+		 */
 		if (!(pb_flag & 0x01) &&
 		    (((pkt_seq_num - cis->pkt_seq_num) &
 		      BIT64_MASK(39)) <= BIT64_MASK(38))) {
@@ -5770,12 +5782,17 @@ int hci_iso_handle(struct net_buf *buf, struct net_buf **evt)
 			pkt_seq_num = cis->pkt_seq_num;
 		}
 
-		/* Pre-increment, for next ISO data packet seq num comparison */
+		/* Pre-increment, when pg_flag is BT_ISO_SINGLE (0b10) or
+		 * BT_ISO_END (0b11) then we simple check if pb_flag has bit 1
+		 * is set, for next ISO data packet seq num comparison.
+		 */
 		if (pb_flag & 0x10) {
 			cis->pkt_seq_num++;
 		}
 
-		/* Target next event to avoid overlapping with current event */
+		/* Target next ISO event to avoid overlapping with, if any,
+		 * current ISO event
+		 */
 		pkt_seq_num++;
 		sdu_frag_tx.target_event = pkt_seq_num;
 		sdu_frag_tx.grp_ref_point =
@@ -5817,7 +5834,7 @@ int hci_iso_handle(struct net_buf *buf, struct net_buf **evt)
 #endif /* !CONFIG_BT_CTLR_ISOAL_PSN_IGNORE */
 
 		/* Get controller's input data path for CIS */
-		hdr = &(cis->hdr);
+		hdr = &cis->hdr;
 		dp_in = hdr->datapath_in;
 		if (!dp_in || dp_in->path_id != BT_HCI_DATAPATH_ID_HCI) {
 			LOG_ERR("Input data path not set for HCI");
@@ -5884,6 +5901,18 @@ int hci_iso_handle(struct net_buf *buf, struct net_buf **evt)
 		/* Catch up local pkt_seq_num with internal pkt_seq_num */
 		event_count = lll_iso->payload_count / lll_iso->bn;
 		pkt_seq_num = event_count;
+		/* If pb_flag is BT_ISO_START (0b00) or BT_ISO_SINGLE (0b10)
+		 * then we simply check that the pb_flag is an even value, and
+		 * then  pkt_seq_num is a future sequence number value compare
+		 * to last recorded number in cis->pkt_seq_num.
+		 *
+		 * When (pkt_seq_num - stream->pkt_seq_num) is negative then
+		 * BIT64(39) will be set (2's compliment value). The diff value
+		 * less than or equal to BIT64_MASK(38) means the diff value is
+		 * positive and hence pkt_seq_num is greater than
+		 * stream->pkt_seq_num. This calculation is valid for when value
+		 * rollover too.
+		 */
 		if (!(pb_flag & 0x01) &&
 		    (((pkt_seq_num - stream->pkt_seq_num) &
 		      BIT64_MASK(39)) <= BIT64_MASK(38))) {
@@ -5892,12 +5921,17 @@ int hci_iso_handle(struct net_buf *buf, struct net_buf **evt)
 			pkt_seq_num = stream->pkt_seq_num;
 		}
 
-		/* Pre-increment, for next ISO data packet seq num comparison */
+		/* Pre-increment, when pg_flag is BT_ISO_SINGLE (0b10) or
+		 * BT_ISO_END (0b11) then we simple check if pb_flag has bit 1
+		 * is set, for next ISO data packet seq num comparison.
+		 */
 		if (pb_flag & 0x10) {
 			stream->pkt_seq_num++;
 		}
 
-		/* Target next event to avoid overlapping with current event */
+		/* Target next ISO event to avoid overlapping with, if any,
+		 * current ISO event
+		 */
 		/* FIXME: Implement ISO Tx ack generation early in done compared
 		 *        to currently only in prepare. I.e. to ensure upper
 		 *        layer has the number of completed packet before the

subsys/bluetooth/controller/ll_sw/lll_conn_iso.h

@@ -15,8 +15,10 @@ struct lll_conn_iso_stream_rxtx {
 	uint64_t ft:8;             /* Flush timeout (FT) */
 	uint64_t bn:4;             /* Burst number (BN) */
 	uint64_t phy:3;            /* PHY */
-	uint64_t rfu:1;
-	uint8_t  bn_curr:4;        /* Current burst number */
+	uint64_t rfu0:1;
+
+	uint8_t bn_curr:4;        /* Current burst number */
+	uint8_t rfu1:4;
 
 #if defined(CONFIG_BT_CTLR_LE_ENC)
 	struct ccm ccm;
@@ -73,20 +75,26 @@ struct lll_conn_iso_group {
 	struct lll_hdr hdr;
 
 	uint16_t handle;      /* CIG handle (internal) */
-	uint8_t  num_cis:5;   /* Number of CISes in this CIG */
-	uint8_t  role:1;      /* 0: CENTRAL, 1: PERIPHERAL*/
-	uint8_t  paused:1;    /* 1: CIG is paused */
 
-	/* ISO interval to calculate timestamp under FT > 1 */
-	uint32_t iso_interval_us;
+	/* Resumption information */
+	uint16_t resume_cis;  /* CIS handle to schedule at resume */
+
+	/* ISO group information */
+	uint32_t num_cis:5;   /* Number of CISes in this CIG */
+	uint32_t role:1;      /* 0: CENTRAL, 1: PERIPHERAL*/
+	uint32_t paused:1;    /* 1: CIG is paused */
+	uint32_t rfu0:1;
+
+	/* ISO interval to calculate timestamp under FT > 1,
+	 * maximum ISO interval of 4 seconds can be represented in 22-bits.
+	 */
+	uint32_t iso_interval_us:22;
+	uint32_t rfu1:2;
 
 	/* Accumulates LLL prepare callback latencies */
 	uint16_t latency_prepare;
 	uint16_t latency_event;
 
-	/* Resumption information */
-	uint16_t resume_cis;  /* CIS handle to schedule at resume */
-
 #if defined(CONFIG_BT_CTLR_PERIPHERAL_ISO)
 	/* Window widening. Relies on vendor specific conversion macros, e.g.
 	 * EVENT_US_FRAC_TO_TICKS().

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c

@@ -48,7 +48,7 @@ static void isr_prepare_subevent(void *param);
 static void isr_done(void *param);
 static void payload_count_flush(struct lll_conn_iso_stream *cis_lll);
 static void payload_count_flush_or_inc_on_close(struct lll_conn_iso_stream *cis_lll);
-static void payload_count_lazy(struct lll_conn_iso_stream *cis_lll, uint16_t lazy);
+static void payload_count_lazy_update(struct lll_conn_iso_stream *cis_lll, uint16_t lazy);
 
 static uint16_t next_cis_chan_remap_idx;
 static uint16_t next_cis_chan_prn_s;
@@ -183,7 +183,7 @@ static int prepare_cb(struct lll_prepare_param *p)
 	se_curr = 1U;
 
 	/* Adjust the SN and NESN for skipped CIG events */
-	payload_count_lazy(cis_lll, lazy);
+	payload_count_lazy_update(cis_lll, lazy);
 
 	/* Start setting up of Radio h/w */
 	radio_reset();
@@ -370,7 +370,7 @@ static int prepare_cb(struct lll_prepare_param *p)
 		cis_lll = ull_conn_iso_lll_stream_get_by_group(cig_lll, &cis_handle);
 		if (cis_lll && cis_lll->active) {
 			/* Adjust sn and nesn for skipped CIG events */
-			payload_count_lazy(cis_lll, lazy);
+			payload_count_lazy_update(cis_lll, lazy);
 
 			/* Adjust sn and nesn for canceled events */
 			if (err) {
@@ -676,11 +676,20 @@ static void isr_rx(void *param)
 	/* No Rx */
 	if (!trx_done ||
 #if defined(CONFIG_TEST_FT_CEN_SKIP_SUBEVENTS)
+	    /* Used by test code,
+	     * to skip a number of events in every 3 event count when current subevent is less than
+	     * or equal to 2 or when current subevent has completed all its NSE number of subevents.
+	     * OR
+	     * to skip a (number + 1) of events in every 3 event count when current subevent is less
+	     * than or equal to 1 or when current subevent has completed all its NSE number of
+	     * subevents.
+	     */
 	    ((((cis_lll->event_count % 3U) < CONFIG_TEST_FT_CEN_SKIP_EVENTS_COUNT) &&
 	      ((se_curr > cis_lll->nse) || (se_curr <= 2U))) ||
+
 	     (((cis_lll->event_count % 3U) < (CONFIG_TEST_FT_CEN_SKIP_EVENTS_COUNT + 1U)) &&
 	      ((se_curr > cis_lll->nse) || (se_curr <= 1U)))) ||
-#endif
+#endif /* CONFIG_TEST_FT_CEN_SKIP_SUBEVENTS */
 	    false) {
 		payload_count_flush(cis_lll);
 
@@ -1230,7 +1239,7 @@ static void payload_count_flush_or_inc_on_close(struct lll_conn_iso_stream *cis_
 	}
 }
 
-static void payload_count_lazy(struct lll_conn_iso_stream *cis_lll, uint16_t lazy)
+static void payload_count_lazy_update(struct lll_conn_iso_stream *cis_lll, uint16_t lazy)
 {
 	if (cis_lll->tx.bn) {
 		uint16_t tx_lazy;

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_peripheral_iso.c

@@ -474,6 +474,14 @@ static void isr_rx(void *param)
 	/* No Rx */
 	if (!trx_done ||
 #if defined(CONFIG_TEST_FT_PER_SKIP_SUBEVENTS)
+	    /* Used by test code,
+	     * to skip a number of events in every 3 event count when current subevent is less than
+	     * or equal to 2 or when current subevent has completed all its NSE number of subevents.
+	     * OR
+	     * to skip a (number + 1) of events in every 3 event count when current subevent is less
+	     * than or equal to 1 or when current subevent has completed all its NSE number of
+	     * subevents.
+	     */
 	    ((((cis_lll->event_count % 3U) < CONFIG_TEST_FT_PER_SKIP_EVENTS_COUNT) &&
 	      ((se_curr > cis_lll->nse) || (se_curr <= 2U))) ||
 	     (((cis_lll->event_count % 3U) < (CONFIG_TEST_FT_PER_SKIP_EVENTS_COUNT + 1U)) &&