Bluetooth: Controller: Fix assert on aux LLL scheduled chain reception

Fix asserted in ULL due to incorrect resumption of scan
window when auxiliary channel chain PDU is LLL scheduled by
a ULL scheduled auxiliary channel PDU reception.

The issue is solved by having `is_chain_sched` flag in the
auxiliary channel scan context and using the already present
`is_aux_sched` in the primary channel scan context to
differentiate if the auxiliary PDU Rx ISR is to return back
to primary channel scan window or to close the auxiliary
chain PDU reception radio event.

Relates to #38146.

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

Author: cvinayak

subsys/bluetooth/controller/ll_sw/lll_scan.h

@@ -62,6 +62,7 @@ struct lll_scan_aux {
 
 	uint8_t chan:6;
 	uint8_t state:1;
+	uint8_t is_chain_sched:1;
 
 	uint8_t phy:3;
 

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

@@ -586,33 +586,36 @@ void lll_scan_aux_isr_aux_setup(void *param)
 
 static void isr_rx_ull_schedule(void *param)
 {
-	struct lll_scan_aux *lll;
-	struct lll_scan *scan_lll;
+	struct lll_scan_aux *lll_aux;
+	struct lll_scan *lll;
 
-	lll = param;
-	scan_lll = ull_scan_aux_lll_parent_get(lll, NULL);
+	lll_aux = param;
+	lll = ull_scan_aux_lll_parent_get(lll_aux, NULL);
 
-	isr_rx(scan_lll, lll, lll->phy);
+	isr_rx(lll, lll_aux, lll_aux->phy);
 }
 
 static void isr_rx_lll_schedule(void *param)
 {
-	struct lll_scan_aux *lll;
 	struct node_rx_pdu *node_rx;
-	struct lll_scan *scan_lll;
+	struct lll_scan *lll;
 	uint8_t phy_aux;
 
 	node_rx = param;
-	scan_lll = node_rx->hdr.rx_ftr.param;
-	lll = scan_lll->lll_aux;
+	lll = node_rx->hdr.rx_ftr.param;
+	phy_aux = node_rx->hdr.rx_ftr.aux_phy; /* PHY remembered in node rx */
 
-	if (lll) {
-		phy_aux = lll->phy;
+	/* scan context has used LLL scheduling for aux reception */
+	if (lll->is_aux_sched) {
+		isr_rx(lll, NULL, phy_aux);
 	} else {
-		phy_aux = node_rx->hdr.rx_ftr.aux_phy;
+		/* `lll->lll_aux` would be allocated in ULL for LLL scheduled
+		 * auxiliary PDU reception by scan context and for case
+		 * where LLL scheduled chain PDU reception by aux context, it
+		 * is assigned with the current aux context's LLL context.
+		 */
+		isr_rx(lll, lll->lll_aux, phy_aux);
 	}
-
-	isr_rx(scan_lll, NULL, phy_aux);
 }
 
 static void isr_rx(struct lll_scan *lll, struct lll_scan_aux *lll_aux,
@@ -710,8 +713,16 @@ static void isr_rx(struct lll_scan *lll, struct lll_scan_aux *lll_aux,
 			ull_rx_sched();
 		}
 
-		/* Resume scan if scanning ADV_AUX_IND chain */
-		radio_isr_set(lll_scan_isr_resume, lll);
+		/* Check if LLL scheduled auxiliary PDU reception by scan
+		 * context or auxiliary PDU reception by aux context
+		 */
+		if (lll->is_aux_sched) {
+			/* Go back to resuming primary channel scanning */
+			radio_isr_set(lll_scan_isr_resume, lll);
+		} else {
+			/* auxiliary channel radio event done */
+			radio_isr_set(isr_done, NULL);
+		}
 	}
 	radio_disable();
 }
@@ -1051,18 +1062,33 @@ static int isr_rx_pdu(struct lll_scan *lll, struct lll_scan_aux *lll_aux,
 	/* Passive scanner or scan responses */
 #if defined(CONFIG_BT_CENTRAL)
 	} else if (!lll->conn &&
-		   lll_scan_ext_tgta_check(lll, false, false, pdu, rl_idx)) {
+		   ((lll_aux && lll_aux->is_chain_sched) ||
+		    (lll->lll_aux && lll->lll_aux->is_chain_sched) ||
+		    lll_scan_ext_tgta_check(lll, false, false, pdu, rl_idx))) {
 #else /* !CONFIG_BT_CENTRAL */
-	} else if (lll_scan_ext_tgta_check(lll, false, false, pdu, rl_idx)) {
+	} else if ((lll_aux && lll_aux->is_chain_sched) ||
+		   (lll->lll_aux && lll->lll_aux->is_chain_sched) ||
+		   lll_scan_ext_tgta_check(lll, false, false, pdu, rl_idx)) {
 #endif /* !CONFIG_BT_CENTRAL */
 
 		ftr = &(node_rx->hdr.rx_ftr);
 		if (lll_aux) {
 			ftr->param = lll_aux;
 			ftr->scan_rsp = lll_aux->state;
+
+			/* Further auxiliary PDU reception will be chain PDUs */
+			lll_aux->is_chain_sched = 1U;
 		} else if (lll->lll_aux) {
 			ftr->param = lll;
 			ftr->scan_rsp = lll->lll_aux->state;
+
+			/* Auxiliary PDU received by LLL scheduling by scan
+			 * context.
+			 */
+			lll->is_aux_sched = 1U;
+
+			/* Further auxiliary PDU reception will be chain PDUs */
+			lll->lll_aux->is_chain_sched = 1U;
 		} else {
 			/* Return -ECHILD, as ULL execution has not yet assigned
 			 * an aux context. This can happen only under LLL
@@ -1113,7 +1139,6 @@ static int isr_rx_pdu(struct lll_scan *lll, struct lll_scan_aux *lll_aux,
 		 * disable so prevent caller from doing it again.
 		 */
 		if (ftr->aux_lll_sched) {
-			lll->is_aux_sched = 1U;
 			return 0;
 		}
 

subsys/bluetooth/controller/ll_sw/ull_scan_aux.c

@@ -137,7 +137,8 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx)
 			 */
 			lll_aux = ftr->param;
 			aux = HDR_LLL2ULL(lll_aux);
-			/* FIXME: pick the aux somehow */
+
+			/* aux parent will be NULL for periodic sync */
 			lll = aux->parent;
 		} else if (ull_scan_is_valid_get(HDR_LLL2ULL(ftr->param))) {
 			/* Node that does not have valid aux context but has
@@ -157,6 +158,7 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx)
 			 */
 			lll = NULL;
 			sync_lll = ftr->param;
+
 			lll_aux = sync_lll->lll_aux;
 			aux = HDR_LLL2ULL(lll_aux);
 		}
@@ -340,7 +342,7 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx)
 		ull_sync_chm_update(rx->handle, ptr, acad_len);
 	}
 
-	/* Do not ULL scheduling auxiliary PDU reception if not aux pointer
+	/* Do not ULL schedule auxiliary PDU reception if no aux pointer
 	 * or aux pointer is zero or scannable advertising has erroneous aux
 	 * pointer being present or PHY in the aux pointer is invalid.
 	 */
@@ -366,6 +368,7 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx)
 
 		aux->rx_head = aux->rx_last = NULL;
 		lll_aux = &aux->lll;
+		lll_aux->is_chain_sched = 0U;
 
 		ull_hdr_init(&aux->ull);
 		lll_hdr_init(lll_aux, aux);
@@ -409,6 +412,10 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx)
 		} else {
 			lll->lll_aux = lll_aux;
 		}
+
+		/* Reset auxiliary channel PDU scan state which otherwise is
+		 * done in the prepare_cb when ULL scheduling is used.
+		 */
 		lll_aux->state = 0U;
 
 		return;