Bluetooth: controller: refactor pdu encode/decode functions

Introducing common pdu struct declarations for conn param req/rsp and
data length req/rsp to utilize identicality for optimal pdu handling

Signed-off-by: Erik Brockhoff <erbr@oticon.com>

subsys/bluetooth/controller/ll_sw/pdu.h

@@ -710,6 +710,26 @@ struct pdu_data_llctrl_conn_param_rsp {
 	uint16_t offset5;
 } __packed;
 
+/*
+ * According to Spec Core v5.3, section 2.4.2.17
+ * LL_CONNECTION_PARAM_RSP and LL_CONNECTION_PARAM_REQ are identical
+ * This is utilized in pdu encode/decode, and for this is needed a common struct
+ */
+struct pdu_data_llctrl_conn_param_req_rsp_common {
+	uint16_t interval_min;
+	uint16_t interval_max;
+	uint16_t latency;
+	uint16_t timeout;
+	uint8_t  preferred_periodicity;
+	uint16_t reference_conn_event_count;
+	uint16_t offset0;
+	uint16_t offset1;
+	uint16_t offset2;
+	uint16_t offset3;
+	uint16_t offset4;
+	uint16_t offset5;
+} __packed;
+
 struct pdu_data_llctrl_reject_ext_ind {
 	uint8_t reject_opcode;
 	uint8_t error_code;
@@ -737,6 +757,18 @@ struct pdu_data_llctrl_length_rsp {
 	uint16_t max_tx_time;
 } __packed;
 
+/*
+ * According to Spec Core v5.3, section 2.4.2.21
+ * LL_LENGTH_REQ and LL_LENGTH_RSP are identical
+ * This is utilized in pdu encode/decode, and for this is needed a common struct
+ */
+struct pdu_data_llctrl_length_req_rsp_common {
+	uint16_t max_rx_octets;
+	uint16_t max_rx_time;
+	uint16_t max_tx_octets;
+	uint16_t max_tx_time;
+} __packed;
+
 struct pdu_data_llctrl_phy_req {
 	uint8_t tx_phys;
 	uint8_t rx_phys;

subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h

@@ -500,7 +500,6 @@ void llcp_rp_conn_param_req_apm_reply(struct ll_conn *conn, struct proc_ctx *ctx
  * Terminate Helper
  */
 void llcp_pdu_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu);
-void llcp_ntf_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu);
 void llcp_pdu_decode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu);
 
 /*

subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c

@@ -233,17 +233,6 @@ void llcp_pdu_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu)
 	p->error_code = ctx->data.term.error_code;
 }
 
-void llcp_ntf_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu)
-{
-	struct pdu_data_llctrl_terminate_ind *p;
-
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
-	pdu->len = PDU_DATA_LLCTRL_LEN(terminate_ind);
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_TERMINATE_IND;
-	p = &pdu->llctrl.terminate_ind;
-	p->error_code = ctx->data.term.error_code;
-}
-
 void llcp_pdu_decode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
 	ctx->data.term.error_code = pdu->llctrl.terminate_ind.error_code;
@@ -307,8 +296,8 @@ static int csrand_get(void *buf, size_t len)
 	}
 }
 
-#if defined(CONFIG_BT_CENTRAL)
+#if defined(CONFIG_BT_CENTRAL) || defined(CONFIG_BT_PERIPHERAL)
-void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
+static void encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
 	struct pdu_data_llctrl_enc_req *p;
 
@@ -320,7 +309,18 @@ void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 	memcpy(p->rand, ctx->data.enc.rand, sizeof(p->rand));
 	p->ediv[0] = ctx->data.enc.ediv[0];
 	p->ediv[1] = ctx->data.enc.ediv[1];
+}
+#endif /* CONFIG_BT_CENTRAL || CONFIG_BT_PERIPHERAL */
+
+#if defined(CONFIG_BT_CENTRAL)
+void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
+{
+	struct pdu_data_llctrl_enc_req *p;
+
+	encode_enc_req(ctx, pdu);
+
 	/* Optimal getting random data, p->ivm is packed right after p->skdm */
+	p = &pdu->llctrl.enc_req;
 	BUILD_ASSERT(offsetof(struct pdu_data_llctrl_enc_req, ivm) ==
 		     offsetof(struct pdu_data_llctrl_enc_req, skdm) + sizeof(p->skdm),
 		     "Member IVM must be after member SKDM");
@@ -332,16 +332,7 @@ void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 #if defined(CONFIG_BT_PERIPHERAL)
 void llcp_ntf_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_enc_req *p;
+	encode_enc_req(ctx, pdu);
-
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
-	pdu->len = PDU_DATA_LLCTRL_LEN(enc_req);
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_ENC_REQ;
-
-	p = &pdu->llctrl.enc_req;
-	memcpy(p->rand, ctx->data.enc.rand, sizeof(p->rand));
-	p->ediv[0] = ctx->data.enc.ediv[0];
-	p->ediv[1] = ctx->data.enc.ediv[1];
 }
 
 void llcp_pdu_encode_enc_rsp(struct pdu_data *pdu)
@@ -487,15 +478,15 @@ void llcp_pdu_decode_phy_rsp(struct proc_ctx *ctx, struct pdu_data *pdu)
 /*
  * Connection Update Procedure Helper
  */
-void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu)
+static void encode_conn_param_req_rsp_common(struct proc_ctx *ctx, struct pdu_data *pdu,
+					     struct pdu_data_llctrl_conn_param_req_rsp_common *p,
+					     uint8_t opcode)
 {
-	struct pdu_data_llctrl_conn_param_req *p;
-
 	pdu->ll_id = PDU_DATA_LLID_CTRL;
-	pdu->len = PDU_DATA_LLCTRL_LEN(conn_param_req);
+	/* The '+ 1U' is to count in opcode octet, the first member of struct pdu_data_llctrl */
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ;
+	pdu->len = sizeof(struct pdu_data_llctrl_conn_param_req_rsp_common) + 1U;
+	pdu->llctrl.opcode = opcode;
 
-	p = (void *)&pdu->llctrl.conn_param_req;
 	p->interval_min = sys_cpu_to_le16(ctx->data.cu.interval_min);
 	p->interval_max = sys_cpu_to_le16(ctx->data.cu.interval_max);
 	p->latency = sys_cpu_to_le16(ctx->data.cu.latency);
@@ -510,65 +501,48 @@ void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 	p->offset5 = sys_cpu_to_le16(ctx->data.cu.offsets[5]);
 }
 
+
+void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu)
+{
+	encode_conn_param_req_rsp_common(ctx, pdu,
+		(struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_req,
+		PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ);
+}
+
 void llcp_pdu_encode_conn_param_rsp(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_conn_param_req *p;
+	encode_conn_param_req_rsp_common(ctx, pdu,
+		(struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_rsp,
+		PDU_DATA_LLCTRL_TYPE_CONN_PARAM_RSP);
+}
 
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
+static void decode_conn_param_req_rsp_common(struct proc_ctx *ctx,
-	pdu->len = PDU_DATA_LLCTRL_LEN(conn_param_rsp);
+					     struct pdu_data_llctrl_conn_param_req_rsp_common *p)
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_RSP;
+{
-
+	ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min);
-	p = (void *)&pdu->llctrl.conn_param_rsp;
+	ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max);
-	p->interval_min = sys_cpu_to_le16(ctx->data.cu.interval_min);
+	ctx->data.cu.latency = sys_le16_to_cpu(p->latency);
-	p->interval_max = sys_cpu_to_le16(ctx->data.cu.interval_max);
+	ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout);
-	p->latency = sys_cpu_to_le16(ctx->data.cu.latency);
+	ctx->data.cu.preferred_periodicity = p->preferred_periodicity;
-	p->timeout = sys_cpu_to_le16(ctx->data.cu.timeout);
+	ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count);
-	p->preferred_periodicity = ctx->data.cu.preferred_periodicity;
+	ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0);
-	p->reference_conn_event_count = sys_cpu_to_le16(ctx->data.cu.reference_conn_event_count);
+	ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1);
-	p->offset0 = sys_cpu_to_le16(ctx->data.cu.offsets[0]);
+	ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2);
-	p->offset1 = sys_cpu_to_le16(ctx->data.cu.offsets[1]);
+	ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3);
-	p->offset2 = sys_cpu_to_le16(ctx->data.cu.offsets[2]);
+	ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4);
-	p->offset3 = sys_cpu_to_le16(ctx->data.cu.offsets[3]);
+	ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5);
-	p->offset4 = sys_cpu_to_le16(ctx->data.cu.offsets[4]);
-	p->offset5 = sys_cpu_to_le16(ctx->data.cu.offsets[5]);
 }
 
 void llcp_pdu_decode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_conn_param_req *p;
+	decode_conn_param_req_rsp_common(ctx,
-
+		(struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_req);
-	p = (void *)&pdu->llctrl.conn_param_req;
-	ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min);
-	ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max);
-	ctx->data.cu.latency = sys_le16_to_cpu(p->latency);
-	ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout);
-	ctx->data.cu.preferred_periodicity = p->preferred_periodicity;
-	ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count);
-	ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0);
-	ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1);
-	ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2);
-	ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3);
-	ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4);
-	ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5);
 }
 
 void llcp_pdu_decode_conn_param_rsp(struct proc_ctx *ctx, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_conn_param_rsp *p;
+	decode_conn_param_req_rsp_common(ctx,
-
+		(struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_rsp);
-	p = (void *)&pdu->llctrl.conn_param_req;
-	ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min);
-	ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max);
-	ctx->data.cu.latency = sys_le16_to_cpu(p->latency);
-	ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout);
-	ctx->data.cu.preferred_periodicity = p->preferred_periodicity;
-	ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count);
-	ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0);
-	ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1);
-	ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2);
-	ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3);
-	ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4);
-	ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5);
 }
 #endif /* defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) */
 
@@ -627,43 +601,43 @@ void llcp_pdu_decode_chan_map_update_ind(struct proc_ctx *ctx, struct pdu_data *
 /*
  * Data Length Update Procedure Helpers
  */
+static void encode_length_req_rsp_common(struct pdu_data *pdu,
+					 struct pdu_data_llctrl_length_req_rsp_common *p,
+					 const uint8_t opcode,
+					 const struct data_pdu_length *dle)
+{
+	pdu->ll_id = PDU_DATA_LLID_CTRL;
+	/* The '+ 1U' is to count in opcode octet, the first member of struct pdu_data_llctrl */
+	pdu->len = sizeof(struct pdu_data_llctrl_length_req_rsp_common) + 1U;
+	pdu->llctrl.opcode = opcode;
+	p->max_rx_octets = sys_cpu_to_le16(dle->max_rx_octets);
+	p->max_tx_octets = sys_cpu_to_le16(dle->max_tx_octets);
+	p->max_rx_time = sys_cpu_to_le16(dle->max_rx_time);
+	p->max_tx_time = sys_cpu_to_le16(dle->max_tx_time);
+}
+
 void llcp_pdu_encode_length_req(struct ll_conn *conn, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_length_req *p = &pdu->llctrl.length_req;
+	encode_length_req_rsp_common(pdu,
-
+		(struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_req,
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
+		PDU_DATA_LLCTRL_TYPE_LENGTH_REQ,
-	pdu->len = PDU_DATA_LLCTRL_LEN(length_req);
+		&conn->lll.dle.local);
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_REQ;
-	p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_rx_octets);
-	p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_tx_octets);
-	p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.local.max_rx_time);
-	p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.local.max_tx_time);
 }
 
 void llcp_pdu_encode_length_rsp(struct ll_conn *conn, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp;
+	encode_length_req_rsp_common(pdu,
-
+		(struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp,
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
+		PDU_DATA_LLCTRL_TYPE_LENGTH_RSP,
-	pdu->len = PDU_DATA_LLCTRL_LEN(length_rsp);
+		&conn->lll.dle.local);
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_RSP;
-	p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_rx_octets);
-	p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_tx_octets);
-	p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.local.max_rx_time);
-	p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.local.max_tx_time);
 }
 
 void llcp_ntf_encode_length_change(struct ll_conn *conn, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp;
+	encode_length_req_rsp_common(pdu,
-
+		(struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp,
-	pdu->ll_id = PDU_DATA_LLID_CTRL;
+		PDU_DATA_LLCTRL_TYPE_LENGTH_RSP,
-	pdu->len = PDU_DATA_LLCTRL_LEN(length_rsp);
+		&conn->lll.dle.eff);
-	pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_RSP;
-	p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.eff.max_rx_octets);
-	p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.eff.max_tx_octets);
-	p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.eff.max_rx_time);
-	p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.eff.max_tx_time);
 }
 
 static bool dle_remote_valid(const struct data_pdu_length *remote)
@@ -702,10 +676,9 @@ static bool dle_remote_valid(const struct data_pdu_length *remote)
 
 	return true;
 }
-
+static void decode_length_req_rsp_common(struct ll_conn *conn,
-void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu)
+					 struct pdu_data_llctrl_length_req_rsp_common *p)
 {
-	struct pdu_data_llctrl_length_req *p = &pdu->llctrl.length_req;
 	struct data_pdu_length remote;
 
 	remote.max_rx_octets = sys_le16_to_cpu(p->max_rx_octets);
@@ -720,21 +693,16 @@ void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu)
 	conn->lll.dle.remote = remote;
 }
 
+void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu)
+{
+	decode_length_req_rsp_common(conn,
+		(struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_req);
+}
+
 void llcp_pdu_decode_length_rsp(struct ll_conn *conn, struct pdu_data *pdu)
 {
-	struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp;
+	decode_length_req_rsp_common(conn,
-	struct data_pdu_length remote;
+		(struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp);
-
-	remote.max_rx_octets = sys_le16_to_cpu(p->max_rx_octets);
-	remote.max_tx_octets = sys_le16_to_cpu(p->max_tx_octets);
-	remote.max_rx_time = sys_le16_to_cpu(p->max_rx_time);
-	remote.max_tx_time = sys_le16_to_cpu(p->max_tx_time);
-
-	if (!dle_remote_valid(&remote)) {
-		return;
-	}
-
-	conn->lll.dle.remote = remote;
 }
 #endif /* CONFIG_BT_CTLR_DATA_LENGTH */