Bluetooth: controller: nRF5: Check preempt event on timeout

Check whether the preempt event matches with the head of the
pipeline before aborting the currently active event.

This is required to avoid preemption of events that became
active due to done event and there has been a race in
stopping the preempt ticker.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

subsys/bluetooth/controller/ll_sw/lll.h

@@ -413,11 +413,11 @@ int lll_csrand_isr_get(void *buf, size_t len);
 int lll_rand_get(void *buf, size_t len);
 int lll_rand_isr_get(void *buf, size_t len);
 
-int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
-			       lll_abort_cb_t abort_cb,
-			       struct lll_prepare_param *prepare_param,
-			       lll_prepare_cb_t prepare_cb,
-			       uint8_t is_resume);
+struct lll_event *ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
+				      lll_abort_cb_t abort_cb,
+				      struct lll_prepare_param *prepare_param,
+				      lll_prepare_cb_t prepare_cb,
+				      uint8_t is_resume);
 void *ull_prepare_dequeue_get(void);
 void *ull_prepare_dequeue_iter(uint8_t *idx);
 void ull_prepare_dequeue(uint8_t caller_id);

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

@@ -65,13 +65,15 @@ static int init_reset(void);
 #if defined(CONFIG_BT_CTLR_LOW_LAT_ULL_DONE)
 static inline void done_inc(void);
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
-static int resume_enqueue(lll_prepare_cb_t resume_cb);
+static struct lll_event *resume_enqueue(lll_prepare_cb_t resume_cb);
 static void isr_race(void *param);
 
 #if !defined(CONFIG_BT_CTLR_LOW_LAT)
 static void ticker_stop_op_cb(uint32_t status, void *param);
 static void ticker_start_op_cb(uint32_t status, void *param);
-static void preempt_ticker_start(struct lll_prepare_param *prepare_param);
+static void ticker_start_next_op_cb(uint32_t status, void *param);
+static uint32_t preempt_ticker_start(struct lll_event *event,
+				     ticker_op_func op_cb);
 static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
 			      uint16_t lazy, uint8_t force, void *param);
 static void preempt(void *param);
@@ -304,7 +306,6 @@ int lll_done(void *param)
 	struct lll_event *next;
 	struct ull_hdr *ull;
 	void *evdone;
-	int ret = 0;
 
 	/* Assert if param supplied without a pending prepare to cancel. */
 	next = ull_prepare_dequeue_get();
@@ -352,7 +353,7 @@ int lll_done(void *param)
 	evdone = ull_event_done(ull);
 	LL_ASSERT(evdone);
 
-	return ret;
+	return 0;
 }
 
 #if defined(CONFIG_BT_CTLR_LOW_LAT_ULL_DONE)
@@ -620,8 +621,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 	    (p && is_resume)) {
 #if defined(CONFIG_BT_CTLR_LOW_LAT)
 		lll_prepare_cb_t resume_cb;
-		struct lll_event *next;
 #endif /* CONFIG_BT_CTLR_LOW_LAT */
+		struct lll_event *next;
 
 		if (IS_ENABLED(CONFIG_BT_CTLR_LOW_LAT) && event.curr.param) {
 			/* early abort */
@@ -629,16 +630,21 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 		}
 
 		/* Store the next prepare for deferred call */
-		err = ull_prepare_enqueue(is_abort_cb, abort_cb, prepare_param,
-					  prepare_cb, is_resume);
-		LL_ASSERT(!err);
+		next = ull_prepare_enqueue(is_abort_cb, abort_cb, prepare_param,
+					   prepare_cb, is_resume);
+		LL_ASSERT(next);
 
 #if !defined(CONFIG_BT_CTLR_LOW_LAT)
 		if (is_resume) {
 			return -EINPROGRESS;
 		}
 
-		preempt_ticker_start(prepare_param);
+		/* Start the preempt timeout */
+		uint32_t ret;
+
+		ret  = preempt_ticker_start(next, ticker_start_op_cb);
+		LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
+			  (ret == TICKER_STATUS_BUSY));
 
 #else /* CONFIG_BT_CTLR_LOW_LAT */
 		next = NULL;
@@ -664,8 +670,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 			LL_ASSERT(err);
 
 			if (err == -EAGAIN) {
-				err = resume_enqueue(resume_cb);
-				LL_ASSERT(!err);
+				next = resume_enqueue(resume_cb);
+				LL_ASSERT(next);
 			} else {
 				LL_ASSERT(err == -ECANCELED);
 			}
@@ -675,6 +681,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 		return -EINPROGRESS;
 	}
 
+	LL_ASSERT(!p || &p->prepare_param == prepare_param);
+
 	event.curr.param = prepare_param->param;
 	event.curr.is_abort_cb = is_abort_cb;
 	event.curr.abort_cb = abort_cb;
@@ -701,13 +709,16 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 		}
 	} while (p->is_aborted || p->is_resume);
 
-	preempt_ticker_start(&p->prepare_param);
+	/* Start the preempt timeout */
+	ret = preempt_ticker_start(p, ticker_start_next_op_cb);
+	LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
+		  (ret == TICKER_STATUS_BUSY));
 #endif /* !CONFIG_BT_CTLR_LOW_LAT */
 
 	return err;
 }
 
-static int resume_enqueue(lll_prepare_cb_t resume_cb)
+static struct lll_event *resume_enqueue(lll_prepare_cb_t resume_cb)
 {
 	struct lll_prepare_param prepare_param = {0};
 
@@ -747,16 +758,26 @@ static void ticker_start_op_cb(uint32_t status, void *param)
 		  (status == TICKER_STATUS_FAILURE));
 }
 
-static void preempt_ticker_start(struct lll_prepare_param *prepare_param)
+static void ticker_start_next_op_cb(uint32_t status, void *param)
 {
+	ARG_UNUSED(param);
+
+	LL_ASSERT(status == TICKER_STATUS_SUCCESS);
+}
+
+static uint32_t preempt_ticker_start(struct lll_event *event,
+				     ticker_op_func op_cb)
+{
+	struct lll_prepare_param *p;
 	uint32_t preempt_anchor;
 	struct ull_hdr *ull;
 	uint32_t preempt_to;
 	uint32_t ret;
 
 	/* Calc the preempt timeout */
-	ull = HDR_LLL2ULL(prepare_param->param);
-	preempt_anchor = prepare_param->ticks_at_expire;
+	p = &event->prepare_param;
+	ull = HDR_LLL2ULL(p->param);
+	preempt_anchor = p->ticks_at_expire;
 	preempt_to = MAX(ull->ticks_active_to_start,
 			 ull->ticks_prepare_to_start) -
 		     ull->ticks_preempt_to_start;
@@ -771,11 +792,10 @@ static void preempt_ticker_start(struct lll_prepare_param *prepare_param)
 			   TICKER_NULL_REMAINDER,
 			   TICKER_NULL_LAZY,
 			   TICKER_NULL_SLOT,
-			   preempt_ticker_cb, NULL,
-			   ticker_start_op_cb, NULL);
-	LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
-		  (ret == TICKER_STATUS_FAILURE) ||
-		  (ret == TICKER_STATUS_BUSY));
+			   preempt_ticker_cb, event,
+			   op_cb, event);
+
+	return ret;
 }
 
 static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
@@ -785,6 +805,7 @@ static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
 	static struct mayfly mfy = {0, 0, &link, NULL, preempt};
 	uint32_t ret;
 
+	mfy.param = param;
 	ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH, TICKER_USER_ID_LLL,
 			     0, &mfy);
 	LL_ASSERT(!ret);
@@ -795,43 +816,63 @@ static void preempt(void *param)
 	lll_prepare_cb_t resume_cb;
 	struct lll_event *next;
 	uint8_t idx;
-	int ret;
+	int err;
 
+	/* No event to abort */
 	if (!event.curr.abort_cb || !event.curr.param) {
 		return;
 	}
 
+	/* Check if any prepare in pipeline */
 	idx = UINT8_MAX;
 	next = ull_prepare_dequeue_iter(&idx);
 	if (!next) {
 		return;
 	}
 
+	/* Find a prepare that is ready and not a resume */
 	while (next && (next->is_aborted || next->is_resume)) {
 		next = ull_prepare_dequeue_iter(&idx);
 	}
 
+	/* No ready prepare */
 	if (!next) {
 		return;
 	}
 
-	ret = event.curr.is_abort_cb(next->prepare_param.param,
+	/* Preemptor not in pipeline */
+	if (next != param) {
+		uint32_t ret;
+
+		/* Start the preempt timeout */
+		ret = preempt_ticker_start(next, ticker_start_next_op_cb);
+		LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
+			  (ret == TICKER_STATUS_BUSY));
+
+		return;
+	}
+
+	/* Check if current event want to continue */
+	err = event.curr.is_abort_cb(next->prepare_param.param,
 				     event.curr.param,
 				     &resume_cb);
-	if (!ret) {
-		/* Let LLL know about the cancelled prepare */
+	if (!err) {
+		/* Let preemptor LLL know about the cancelled prepare */
 		next->is_aborted = 1;
 		next->abort_cb(&next->prepare_param, next->prepare_param.param);
 
 		return;
 	}
 
+	/* Abort the current event */
 	event.curr.abort_cb(NULL, event.curr.param);
 
-	if (ret == -EAGAIN) {
+	/* Check if resume requested */
+	if (err == -EAGAIN) {
 		struct lll_event *iter;
 		uint8_t iter_idx;
 
+		/* Abort any duplicates so that they get dequeued */
 		iter_idx = UINT8_MAX;
 		iter = ull_prepare_dequeue_iter(&iter_idx);
 		while (iter) {
@@ -853,10 +894,11 @@ static void preempt(void *param)
 			iter = ull_prepare_dequeue_iter(&iter_idx);
 		}
 
-		ret = resume_enqueue(resume_cb);
-		LL_ASSERT(!ret);
+		/* Enqueue as resume event */
+		iter = resume_enqueue(resume_cb);
+		LL_ASSERT(iter);
 	} else {
-		LL_ASSERT(ret == -ECANCELED);
+		LL_ASSERT(err == -ECANCELED);
 	}
 }
 #else /* CONFIG_BT_CTLR_LOW_LAT */

subsys/bluetooth/controller/ll_sw/ull.c

@@ -1637,18 +1637,18 @@ void ull_rx_sched_done(void)
 }
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL */
 
-int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
-			lll_abort_cb_t abort_cb,
-			struct lll_prepare_param *prepare_param,
-			lll_prepare_cb_t prepare_cb,
-			uint8_t is_resume)
+struct lll_event *ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
+				      lll_abort_cb_t abort_cb,
+				      struct lll_prepare_param *prepare_param,
+				      lll_prepare_cb_t prepare_cb,
+				      uint8_t is_resume)
 {
 	struct lll_event *e;
 	uint8_t idx;
 
 	idx = MFIFO_ENQUEUE_GET(prep, (void **)&e);
 	if (!e) {
-		return -ENOBUFS;
+		return NULL;
 	}
 
 	memcpy(&e->prepare_param, prepare_param, sizeof(e->prepare_param));
@@ -1660,7 +1660,7 @@ int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
 
 	MFIFO_ENQUEUE(prep, idx);
 
-	return 0;
+	return e;
 }
 
 void *ull_prepare_dequeue_get(void)