Bluetooth: controller: Rework TX data pause

Change LLCP TX data pause into using mask to allow for pausing from
multiple different sources - but only once per source
(the underlying TX queue still just has an integer counter).

Signed-off-by: Erik Brockhoff <erbr@oticon.com>
Signed-off-by: Thomas Ebert Hansen <thoh@oticon.com>

subsys/bluetooth/controller/ll_sw/ull_conn_types.h

@@ -449,6 +449,7 @@ struct llcp_struct {
 
 	uint8_t tx_buffer_alloc;
 #endif /* (CONFIG_BT_CTLR_LLCP_PER_CONN_TX_CTRL_BUF_NUM > 0) */
+	uint8_t tx_q_pause_data_mask;
 
 }; /* struct llcp_struct */
 

subsys/bluetooth/controller/ll_sw/ull_llcp.c

@@ -237,15 +237,22 @@ void llcp_tx_enqueue(struct ll_conn *conn, struct node_tx *tx)
 	ull_tx_q_enqueue_ctrl(&conn->tx_q, tx);
 }
 
-void llcp_tx_pause_data(struct ll_conn *conn)
+void llcp_tx_pause_data(struct ll_conn *conn, enum llcp_tx_q_pause_data_mask pause_mask)
 {
+	if ((conn->llcp.tx_q_pause_data_mask & pause_mask) == 0) {
+		conn->llcp.tx_q_pause_data_mask |= pause_mask;
 		ull_tx_q_pause_data(&conn->tx_q);
 	}
+}
 
-void llcp_tx_resume_data(struct ll_conn *conn)
+void llcp_tx_resume_data(struct ll_conn *conn, enum llcp_tx_q_pause_data_mask resume_mask)
 {
+	conn->llcp.tx_q_pause_data_mask &= ~resume_mask;
+
+	if (conn->llcp.tx_q_pause_data_mask == 0) {
 		ull_tx_q_resume_data(&conn->tx_q);
 	}
+}
 
 void llcp_tx_flush(struct ll_conn *conn)
 {
@@ -494,6 +501,7 @@ void ull_llcp_init(struct ll_conn *conn)
 #endif /* (CONFIG_BT_CTLR_LLCP_PER_CONN_TX_CTRL_BUF_NUM > 0) */
 #endif /* LLCP_TX_CTRL_BUF_QUEUE_ENABLE */
 
+	conn->llcp.tx_q_pause_data_mask = 0;
 	conn->lll.event_counter = 0;
 
 	llcp_lr_init(conn);

subsys/bluetooth/controller/ll_sw/ull_llcp_common.c

@@ -359,7 +359,7 @@ static void lp_comm_complete(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
 			/* Resume data, but only if there is no remote procedure pending RSP
 			 * in which case, the RSP tx-ACK will resume data
 			 */
-			llcp_tx_resume_data(conn);
+			llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH);
 		}
 		break;
 #endif /* CONFIG_BT_CTLR_DATA_LENGTH */
@@ -476,7 +476,7 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t
 				 * update DLE without conflicting with out-going LL Data PDUs
 				 * See BT Core 5.2 Vol6: B-4.5.10 & B-5.1.9
 				 */
-				llcp_tx_pause_data(conn);
+				llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH);
 				lp_comm_tx(conn, ctx);
 				ctx->state = LP_COMMON_STATE_WAIT_RX;
 			}
@@ -760,7 +760,7 @@ static void rp_comm_rx_decode(struct ll_conn *conn, struct proc_ctx *ctx, struct
 		 * conflicting with out-going LL Data PDUs
 		 * See BT Core 5.2 Vol6: B-4.5.10 & B-5.1.9
 		 */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH);
 		break;
 #endif /* CONFIG_BT_CTLR_DATA_LENGTH */
 #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP)
@@ -1052,7 +1052,7 @@ static void rp_comm_st_wait_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, u
 			/* Apply changes in data lengths/times */
 			uint8_t dle_changed = ull_dle_update_eff(conn);
 
-			llcp_tx_resume_data(conn);
+			llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH);
 
 			if (dle_changed && !llcp_ntf_alloc_is_available()) {
 				ctx->state = RP_COMMON_STATE_WAIT_NTF;

subsys/bluetooth/controller/ll_sw/ull_llcp_enc.c

@@ -342,7 +342,7 @@ static void lp_enc_st_unencrypted(struct ll_conn *conn, struct proc_ctx *ctx, ui
 	switch (evt) {
 	case LP_ENC_EVT_RUN:
 		/* Pause Tx data */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		llcp_tx_flush(conn);
 		lp_enc_send_enc_req(conn, ctx, evt, param);
 		break;
@@ -407,7 +407,7 @@ static void lp_enc_st_wait_rx_start_enc_req(struct ll_conn *conn, struct proc_ct
 		break;
 	case LP_ENC_EVT_REJECT:
 		/* Resume Tx data */
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		/* Resume Rx data */
 		ull_conn_resume_rx_data(conn);
 		ctx->data.enc.error = (pdu->llctrl.opcode == PDU_DATA_LLCTRL_TYPE_REJECT_IND) ?
@@ -443,7 +443,7 @@ static void lp_enc_st_wait_rx_start_enc_rsp(struct ll_conn *conn, struct proc_ct
 	switch (evt) {
 	case LP_ENC_EVT_START_ENC_RSP:
 		/* Resume Tx data */
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		/* Resume Rx data */
 		ull_conn_resume_rx_data(conn);
 		ctx->data.enc.error = BT_HCI_ERR_SUCCESS;
@@ -477,7 +477,7 @@ static void lp_enc_state_encrypted(struct ll_conn *conn, struct proc_ctx *ctx, u
 	switch (evt) {
 	case LP_ENC_EVT_RUN:
 		/* Pause Tx data */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		llcp_tx_flush(conn);
 		lp_enc_send_pause_enc_req(conn, ctx, evt, param);
 		break;
@@ -822,7 +822,7 @@ static void rp_enc_send_reject_ind(struct ll_conn *conn, struct proc_ctx *ctx, u
 		ctx->state = RP_ENC_STATE_UNENCRYPTED;
 
 		/* Resume Tx data */
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		/* Resume Rx data */
 		ull_conn_resume_rx_data(conn);
 		/* Resume possibly paused local procedure */
@@ -841,7 +841,7 @@ static void rp_enc_send_start_enc_rsp(struct ll_conn *conn, struct proc_ctx *ctx
 		ctx->state = RP_ENC_STATE_UNENCRYPTED;
 
 		/* Resume Tx data */
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		/* Resume Rx data */
 		ull_conn_resume_rx_data(conn);
 
@@ -907,7 +907,7 @@ static void rp_enc_state_wait_rx_enc_req(struct ll_conn *conn, struct proc_ctx *
 	switch (evt) {
 	case RP_ENC_EVT_ENC_REQ:
 		/* Pause Tx data */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		llcp_tx_flush(conn);
 		/* Pause Rx data */
 		ull_conn_pause_rx_data(conn);
@@ -1050,7 +1050,7 @@ static void rp_enc_state_wait_rx_pause_enc_req(struct ll_conn *conn, struct proc
 	switch (evt) {
 	case RP_ENC_EVT_PAUSE_ENC_REQ:
 		/* Pause Tx data */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION);
 		llcp_tx_flush(conn);
 		/*
 		 * Pause Rx data; will be resumed when the encapsulated

subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h

@@ -29,6 +29,12 @@ enum llcp_proc {
 	/* A helper enum entry, to use in pause procedure context */
 	PROC_NONE = 0x0,
 };
+enum llcp_tx_q_pause_data_mask {
+	LLCP_TX_QUEUE_PAUSE_DATA_ENCRYPTION = 0x01,
+	LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE = 0x02,
+	LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH = 0x04,
+};
+
 #if ((CONFIG_BT_CTLR_LLCP_COMMON_TX_CTRL_BUF_NUM <\
 			(CONFIG_BT_CTLR_LLCP_TX_PER_CONN_TX_CTRL_BUF_NUM_MAX *\
 			CONFIG_BT_CTLR_LLCP_CONN)) &&\
@@ -344,8 +350,8 @@ void llcp_proc_ctx_release(struct proc_ctx *ctx);
  * ULL -> LLL Interface
  */
 void llcp_tx_enqueue(struct ll_conn *conn, struct node_tx *tx);
-void llcp_tx_pause_data(struct ll_conn *conn);
+void llcp_tx_pause_data(struct ll_conn *conn, enum llcp_tx_q_pause_data_mask pause_mask);
-void llcp_tx_resume_data(struct ll_conn *conn);
+void llcp_tx_resume_data(struct ll_conn *conn, enum llcp_tx_q_pause_data_mask resume_mask);
 void llcp_tx_flush(struct ll_conn *conn);
 
 /*

subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c

@@ -446,7 +446,7 @@ static void lp_pu_send_phy_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8
 		llcp_rr_set_incompat(conn, INCOMPAT_RESOLVABLE);
 		llcp_rr_set_paused_cmd(conn, PROC_CTE_REQ);
 		lp_pu_tx(conn, ctx, PDU_DATA_LLCTRL_TYPE_PHY_REQ);
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		ctx->state = LP_PU_STATE_WAIT_TX_ACK_PHY_REQ;
 	}
 }
@@ -504,7 +504,7 @@ static void lp_pu_st_wait_rx_phy_rsp(struct ll_conn *conn, struct proc_ctx *ctx,
 
 		llcp_pdu_decode_phy_rsp(ctx, (struct pdu_data *)param);
 		/* Pause data tx */
-		llcp_tx_pause_data(conn);
+		llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		/* Combine with the 'Preferred' phys */
 		pu_combine_phys(conn, ctx, tx_pref, rx_pref);
 		lp_pu_send_phy_update_ind(conn, ctx, evt, param);
@@ -551,7 +551,7 @@ static void lp_pu_st_wait_tx_ack_phy_req(struct ll_conn *conn, struct proc_ctx *
 			/* Unknown role */
 			LL_ASSERT(0);
 		}
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		break;
 	default:
 		/* Ignore other evts */
@@ -597,7 +597,7 @@ static void lp_pu_st_wait_tx_ack_phy_update_ind(struct ll_conn *conn, struct pro
 			ctx->data.pu.ntf_pu = ctx->data.pu.host_initiated;
 			lp_pu_complete(conn, ctx, evt, param);
 		}
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		break;
 	default:
 		/* Ignore other evts */
@@ -901,7 +901,7 @@ static void rp_pu_st_wait_rx_phy_req(struct ll_conn *conn, struct proc_ctx *ctx,
 	llcp_pdu_decode_phy_req(ctx, (struct pdu_data *)param);
 	/* Combine with the 'Preferred' the phys in conn->phy_pref_?x */
 	pu_combine_phys(conn, ctx, conn->phy_pref_tx, conn->phy_pref_rx);
-	llcp_tx_pause_data(conn);
+	llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 	switch (evt) {
 	case RP_PU_EVT_PHY_REQ:
 		switch (conn->lll.role) {
@@ -976,7 +976,7 @@ static void rp_pu_st_wait_tx_ack_phy(struct ll_conn *conn, struct proc_ctx *ctx,
 		} else {
 			/* empty clause */
 		}
-		llcp_tx_resume_data(conn);
+		llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_PHY_UPDATE);
 		break;
 	default:
 		/* Ignore other evts */

subsys/bluetooth/controller/ll_sw/ull_tx_queue.c

@@ -8,7 +8,7 @@
 
 void ull_tx_q_init(struct ull_tx_q *queue)
 {
-	queue->pause_data = 0;
+	queue->pause_data = 0U;
 	sys_slist_init(&queue->tx_list);
 	sys_slist_init(&queue->data_list);
 }

tests/bluetooth/controller/ctrl_tx_queue/src/main.c

@@ -385,6 +385,71 @@ void test_ctrl_and_data_5(void)
 	zassert_equal_ptr(node, NULL, "");
 }
 
+/*
+ * (1) Enqueue data nodes.
+ * Pause Tx Queue TWICE.
+ * (2) Enqueue data nodes.
+ * Dequeue and verify order of data nodes from (1).
+ * Verify Tx Queue is empty.
+ * Resume Tx Queue.
+ * Verify Tx Queue is empty.
+ * Resume Tx Queue.
+ * Dequeue and verify order of data nodes from (2).
+ */
+void test_multiple_pause_resume(void)
+{
+	struct ull_tx_q tx_q;
+	struct node_tx *node;
+	struct node_tx data_nodes1[SIZE] = { 0 };
+	struct node_tx data_nodes2[SIZE] = { 0 };
+
+	ull_tx_q_init(&tx_q);
+
+	/* Enqueue data 1 nodes */
+	for (int i = 0U; i < SIZE; i++) {
+		ull_tx_q_enqueue_data(&tx_q, &data_nodes1[i]);
+	}
+
+	/* Pause Tx Queue Twice */
+	ull_tx_q_pause_data(&tx_q);
+	ull_tx_q_pause_data(&tx_q);
+
+	/* Enqueue data 2 nodes */
+	for (int i = 0U; i < SIZE; i++) {
+		ull_tx_q_enqueue_data(&tx_q, &data_nodes2[i]);
+	}
+
+	/* Dequeue data 1 nodes */
+	for (int i = 0U; i < SIZE; i++) {
+		node = ull_tx_q_dequeue(&tx_q);
+		zassert_equal_ptr(node, &data_nodes1[i], NULL);
+	}
+
+	/* Tx Queue shall be empty */
+	node = ull_tx_q_dequeue(&tx_q);
+	zassert_equal_ptr(node, NULL, "");
+
+	/* Resume Tx Queue */
+	ull_tx_q_resume_data(&tx_q);
+
+	/* Tx Queue shall be empty */
+	node = ull_tx_q_dequeue(&tx_q);
+	zassert_equal_ptr(node, NULL, "");
+
+	/* Resume Tx Queue */
+	ull_tx_q_resume_data(&tx_q);
+
+	/* Dequeue data 2 nodes */
+	for (int i = 0U; i < SIZE; i++) {
+		node = ull_tx_q_dequeue(&tx_q);
+		zassert_equal_ptr(node, &data_nodes2[i], NULL);
+	}
+
+	/* Tx Queue shall be empty */
+	node = ull_tx_q_dequeue(&tx_q);
+	zassert_equal_ptr(node, NULL, "");
+}
+
 void test_main(void)
 {
 	ztest_test_suite(test, ztest_unit_test(test_init), ztest_unit_test(test_ctrl),
@@ -392,6 +457,7 @@ void test_main(void)
 			 ztest_unit_test(test_ctrl_and_data_2),
 			 ztest_unit_test(test_ctrl_and_data_3),
 			 ztest_unit_test(test_ctrl_and_data_4),
-			 ztest_unit_test(test_ctrl_and_data_5));
+			 ztest_unit_test(test_ctrl_and_data_5),
+			 ztest_unit_test(test_multiple_pause_resume));
 	ztest_run_test_suite(test);
 }