[nrf fromtree] Bluetooth: Controller: Fix scanner window close by using lll_disable

Fix race condition in setting up ISR callback and parameter
caused between ULL_HIGH and LLL context. As LLL IRQ is not
disabled the parameter and ISR callback would get out of
sync causing incorrect parameter supplied to callback and
hence leading to development assert in ull_scan_done().

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
(cherry picked from commit d367447)
(cherry picked from commit 073483a)

Author: cvinayak

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

@@ -375,9 +375,14 @@ void lll_done_ull_inc(void)
 }
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
 
-bool lll_is_done(void *param)
+bool lll_is_done(void *param, bool *is_resume)
 {
-	/* FIXME: use param to check */
+	/* NOTE: Current radio event when preempted could put itself in resume
+	 *       into the prepare pipeline in which case event.curr.param would
+	 *       be set to NULL.
+	 */
+	*is_resume = (param != event.curr.param);
+
 	return !event.curr.abort_cb;
 }
 
@@ -731,6 +736,10 @@ static struct lll_event *resume_enqueue(lll_prepare_cb_t resume_cb)
 {
 	struct lll_prepare_param prepare_param = {0};
 
+	/* Enqueue into prepare pipeline as resume radio event, and remove
+	 * parameter assignment from currently active radio event so that
+	 * done event is not generated.
+	 */
 	prepare_param.param = event.curr.param;
 	event.curr.param = NULL;
 

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_internal.h

@@ -6,7 +6,7 @@
 
 int lll_prepare_done(void *param);
 int lll_done(void *param);
-bool lll_is_done(void *param);
+bool lll_is_done(void *param, bool *is_resume);
 int lll_is_abort_cb(void *next, void *curr, lll_prepare_cb_t *resume_cb);
 void lll_abort_cb(struct lll_prepare_param *prepare_param, void *param);
 

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

@@ -79,7 +79,6 @@ static void isr_abort(void *param);
 	* (EVENT_OVERHEAD_PREEMPT_US <= EVENT_OVERHEAD_PREEMPT_MIN_US)
 	*/
 static void isr_done_cleanup(void *param);
-static void isr_cleanup(void *param);
 
 static inline int isr_rx_pdu(struct lll_scan *lll, struct pdu_adv *pdu_adv_rx,
 			     uint8_t devmatch_ok, uint8_t devmatch_id,
@@ -620,11 +619,8 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
 				cpu_sleep();
 			}
 #endif /* CONFIG_BT_CENTRAL */
-		} else if (IS_ENABLED(CONFIG_BT_CTLR_LOW_LAT)) {
-			radio_isr_set(isr_done_cleanup, param);
-			radio_disable();
 		} else {
-			radio_isr_set(isr_cleanup, param);
+			radio_isr_set(isr_done_cleanup, param);
 			radio_disable();
 		}
 		return;
@@ -640,10 +636,17 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
 }
 
 static void ticker_stop_cb(uint32_t ticks_at_expire, uint32_t ticks_drift,
-			   uint32_t remainder, uint16_t lazy, uint8_t force, void *param)
+			   uint32_t remainder, uint16_t lazy, uint8_t force,
+			   void *param)
 {
-	radio_isr_set(isr_done_cleanup, param);
-	radio_disable();
+	static memq_link_t link;
+	static struct mayfly mfy = {0, 0, &link, NULL, lll_disable};
+	uint32_t ret;
+
+	mfy.param = param;
+	ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH, TICKER_USER_ID_LLL, 0,
+			     &mfy);
+	LL_ASSERT(!ret);
 }
 
 static void ticker_op_start_cb(uint32_t status, void *param)
@@ -982,14 +985,15 @@ static void isr_abort(void *param)
 static void isr_done_cleanup(void *param)
 {
 	struct lll_scan *lll;
+	bool is_resume;
 
 	/* Clear radio status and events */
 	lll_isr_status_reset();
 
 	/* Under race between duration expire, is_stop is set in this function,
 	 * and event preemption, prevent generating duplicate scan done events.
 	 */
-	if (lll_is_done(param)) {
+	if (lll_is_done(param, &is_resume)) {
 		return;
 	}
 
@@ -1009,67 +1013,26 @@ static void isr_done_cleanup(void *param)
 	ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_LLL,
 		    TICKER_ID_SCAN_STOP, NULL, NULL);
 
-#if defined(CONFIG_BT_CTLR_ADV_EXT)
-	struct event_done_extra *extra;
-
-	/* Generate Scan done events so that duration and max expiry is
-	 * detected in ULL.
-	 */
-	extra = ull_done_extra_type_set(EVENT_DONE_EXTRA_TYPE_SCAN);
-	LL_ASSERT(extra);
+#if defined(CONFIG_BT_CTLR_SCAN_INDICATION)
+	struct node_rx_hdr *node_rx;
 
-	/* Prevent scan events in pipeline from being scheduled if duration has
-	 * expired.
+	/* Check if there are enough free node rx available:
+	 * 1. For generating this scan indication
+	 * 2. Keep one available free for reception of ACL connection Rx data
+	 * 3. Keep one available free for reception on ACL connection to NACK
+	 *    the PDU
 	 */
-	if (unlikely(lll->duration_reload && !lll->duration_expire)) {
-		lll->is_stop = 1U;
-	}
-
-	if (lll->is_aux_sched) {
-		struct node_rx_pdu *node_rx;
-
-		lll->is_aux_sched = 0U;
-
-		node_rx = ull_pdu_rx_alloc();
-		LL_ASSERT(node_rx);
-
-		node_rx->hdr.type = NODE_RX_TYPE_EXT_AUX_RELEASE;
+	node_rx = ull_pdu_rx_alloc_peek(3);
+	if (node_rx) {
+		ull_pdu_rx_alloc();
 
-		node_rx->hdr.rx_ftr.param = lll;
+		/* TODO: add other info by defining a payload struct */
+		node_rx->type = NODE_RX_TYPE_SCAN_INDICATION;
 
-		ull_rx_put(node_rx->hdr.link, node_rx);
+		ull_rx_put(node_rx->link, node_rx);
 		ull_rx_sched();
 	}
-#endif  /* CONFIG_BT_CTLR_ADV_EXT */
-
-	lll_isr_cleanup(param);
-}
-
-static void isr_cleanup(void *param)
-{
-	/* Clear radio status and events */
-	lll_isr_status_reset();
-
-	/* Do not generate done event for connection initiation, ULL will
-	 * disable the event when establishing/setting up the connection.
-	 * Also, do not generate done event when duration expire, as ULL
-	 * will disable the event.
-	 * As it was transmission of CONNECT_IND, there is not filters to
-	 * disable either.
-	 */
-	if (lll_is_done(param)) {
-		return;
-	}
-
-	/* Disable Rx filters, if cleanup after being in Rx state */
-	radio_filter_disable();
-
-	/* Scanner stop can expire while here in this ISR.
-	 * Deferred attempt to stop can fail as it would have
-	 * expired, hence ignore failure.
-	 */
-	ticker_stop(TICKER_INSTANCE_ID_CTLR, TICKER_USER_ID_LLL,
-		    TICKER_ID_SCAN_STOP, NULL, NULL);
+#endif /* CONFIG_BT_CTLR_SCAN_INDICATION */
 
 #if defined(CONFIG_BT_HCI_MESH_EXT)
 	if (_radio.advertiser.is_enabled && _radio.advertiser.is_mesh &&
@@ -1078,22 +1041,26 @@ static void isr_cleanup(void *param)
 	}
 #endif /* CONFIG_BT_HCI_MESH_EXT */
 
-#if defined(CONFIG_BT_CTLR_SCAN_INDICATION)
-	struct node_rx_hdr *node_rx = ull_pdu_rx_alloc_peek(3);
-
-	if (node_rx) {
-		ull_pdu_rx_alloc();
-
-		/* TODO: add other info by defining a payload struct */
-		node_rx->type = NODE_RX_TYPE_SCAN_INDICATION;
+#if defined(CONFIG_BT_CTLR_ADV_EXT)
+	/* If continuous scan then do not generate scan done when radio event
+	 * has been placed into prepare pipeline as a resume radio event.
+	 */
+	if (!is_resume) {
+		struct event_done_extra *extra;
 
-		ull_rx_put(node_rx->link, node_rx);
-		ull_rx_sched();
+		/* Generate Scan done events so that duration and max expiry is
+		 * detected in ULL.
+		 */
+		extra = ull_done_extra_type_set(EVENT_DONE_EXTRA_TYPE_SCAN);
+		LL_ASSERT(extra);
 	}
-#endif /* CONFIG_BT_CTLR_SCAN_INDICATION */
 
-#if defined(CONFIG_BT_CTLR_ADV_EXT)
-	struct lll_scan *lll = param;
+	/* Prevent scan events in pipeline from being scheduled if duration has
+	 * expired.
+	 */
+	if (unlikely(lll->duration_reload && !lll->duration_expire)) {
+		lll->is_stop = 1U;
+	}
 
 	if (lll->is_aux_sched) {
 		struct node_rx_pdu *node_rx;
@@ -1112,7 +1079,6 @@ static void isr_cleanup(void *param)
 	}
 #endif  /* CONFIG_BT_CTLR_ADV_EXT */
 
-
 	lll_isr_cleanup(param);
 }
 
@@ -1206,7 +1172,7 @@ static inline int isr_rx_pdu(struct lll_scan *lll, struct pdu_adv *pdu_adv_rx,
 			lll_prof_cputime_capture();
 		}
 
-		radio_isr_set(isr_cleanup, lll);
+		radio_isr_set(isr_done_cleanup, lll);
 
 #if defined(HAL_RADIO_GPIO_HAVE_PA_PIN)
 		if (IS_ENABLED(CONFIG_BT_CTLR_PROFILE_ISR)) {