From 28f4c1b703c5c7c6fae240938bdc94242219fad3 Mon Sep 17 00:00:00 2001
From: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Date: Mon, 11 May 2020 15:36:12 +0530
Subject: [PATCH] Bluetooth: controller: split: Rework with defines for adv set
 states

Review rework, added comments, TODOs, FIXMEs and converted
magic number use in advertising set state flags to defines.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
---
 subsys/bluetooth/controller/hci/hci.c         | 19 ++++++++++++++++++-
 subsys/bluetooth/controller/ll_sw/ll_addr.c   |  4 +++-
 subsys/bluetooth/controller/ll_sw/ull.c       |  3 +++
 subsys/bluetooth/controller/ll_sw/ull_adv.c   |  9 ++++++---
 .../bluetooth/controller/ll_sw/ull_adv_aux.c  |  5 ++---
 .../controller/ll_sw/ull_adv_internal.h       | 11 +++++++++++
 .../bluetooth/controller/ll_sw/ull_adv_sync.c |  5 ++++-
 subsys/bluetooth/controller/ll_sw/ull_chan.c  |  5 +++++
 .../bluetooth/controller/ll_sw/ull_filter.c   |  3 ++-
 .../bluetooth/controller/ll_sw/ull_master.c   |  6 +++++-
 subsys/bluetooth/controller/util/util.c       |  5 +++--
 subsys/bluetooth/controller/util/util.h       |  2 +-
 12 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c
index b493a80bebc..640a961d03c 100644
--- a/subsys/bluetooth/controller/hci/hci.c
+++ b/subsys/bluetooth/controller/hci/hci.c
@@ -1837,20 +1837,36 @@ static void le_set_ext_scan_param(struct net_buf *buf, struct net_buf **evt)
 	phys = cmd->phys;
 	p = cmd->p;
 
+	/* Number of bits set indicate scan sets to be configured by calling
+	 * ll_scan_params_set function.
+	 */
 	phys_bitmask = BT_HCI_LE_EXT_SCAN_PHY_1M;
 	if (IS_ENABLED(CONFIG_BT_CTLR_PHY_CODED)) {
 		phys_bitmask |= BT_HCI_LE_EXT_SCAN_PHY_CODED;
 	}
 
+	/* Irrespective of enabled PHYs to scan for, ll_scan_params_set needs
+	 * to be called to initialise the scan sets.
+	 * Passing interval and window as 0, disable the particular scan set
+	 * from being enabled.
+	 */
 	do {
 		uint16_t interval;
 		uint16_t window;
 		uint8_t type;
 		uint8_t phy;
 
+		/* Get single PHY bit from the loop bitmask */
 		phy = BIT(find_lsb_set(phys_bitmask) - 1);
+
+		/* Pass the PHY (1M or Coded) of scan set in MSbits of type
+		 * parameter
+		 */
 		type = (phy << 1);
 
+		/* If current PHY is one of the PHY in the Scanning_PHYs,
+		 * pick the supplied scan type, interval and window.
+		 */
 		if (phys & phy) {
 			type |= (p->type & 0x01);
 			interval = sys_le16_to_cpu(p->interval);
@@ -1890,6 +1906,7 @@ static void le_set_ext_scan_enable(struct net_buf *buf, struct net_buf **evt)
 	}
 #endif
 
+	/* FIXME: Add implementation to use duration and period parameters. */
 	status = ll_scan_enable(cmd->enable);
 
 	ccst = hci_cmd_complete(evt, sizeof(*ccst));
@@ -1965,7 +1982,7 @@ static void le_ext_create_connection(struct net_buf *buf, struct net_buf **evt)
 	*evt = cmd_status(status);
 }
 #endif /* CONFIG_BT_CENTRAL */
-#endif /* !CONFIG_BT_CTLR_ADV_EXT */
+#endif /* CONFIG_BT_CTLR_ADV_EXT */
 
 static int controller_cmd_handle(uint16_t  ocf, struct net_buf *cmd,
 				 struct net_buf **evt, void **node_rx)
diff --git a/subsys/bluetooth/controller/ll_sw/ll_addr.c b/subsys/bluetooth/controller/ll_sw/ll_addr.c
index 8dbe2dd3460..641b06fa146 100644
--- a/subsys/bluetooth/controller/ll_sw/ll_addr.c
+++ b/subsys/bluetooth/controller/ll_sw/ll_addr.c
@@ -55,7 +55,9 @@ uint32_t ll_addr_set(uint8_t addr_type, uint8_t const *const bdaddr)
 		uint32_t status = ull_adv_is_enabled(0);
 
 #if defined(CONFIG_BT_CTLR_ADV_EXT)
-		if ((status & 5U) == 1U) {
+		if ((status & (ULL_ADV_ENABLED_BITMASK_ENABLED |
+			       ULL_ADV_ENABLED_BITMASK_EXTENDED)) ==
+		     ULL_ADV_ENABLED_BITMASK_ENABLED) {
 #else /* !CONFIG_BT_CTLR_ADV_EXT */
 		if (status) {
 #endif /* !CONFIG_BT_CTLR_ADV_EXT */
diff --git a/subsys/bluetooth/controller/ll_sw/ull.c b/subsys/bluetooth/controller/ll_sw/ull.c
index 91f8c4c429c..07f27336e0d 100644
--- a/subsys/bluetooth/controller/ll_sw/ull.c
+++ b/subsys/bluetooth/controller/ll_sw/ull.c
@@ -642,6 +642,9 @@ void ll_rx_dequeue(void)
 		if (IS_ENABLED(CONFIG_BT_CTLR_PRIVACY)) {
 			uint8_t bm;
 
+			/* FIXME: use the correct adv and scan set to get
+			 * enabled status bitmask
+			 */
 			bm = (IS_ENABLED(CONFIG_BT_OBSERVER) &&
 			      ull_scan_is_enabled(0) << 1) |
 			     (IS_ENABLED(CONFIG_BT_BROADCASTER) &&
diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv.c b/subsys/bluetooth/controller/ll_sw/ull_adv.c
index d42b29d2abe..da20cc03f20 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_adv.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_adv.c
@@ -152,12 +152,13 @@ uint8_t ll_adv_params_set(uint16_t interval, uint8_t adv_type,
 		}
 
 		/* Mark the adv set as created by extended advertising cmd */
-		adv->is_created = 3U;
+		adv->is_created = ULL_ADV_CREATED_BITMASK_CREATED |
+				  ULL_ADV_CREATED_BITMASK_EXTENDED;
 	} else {
 		adv->lll.phy_p = BIT(0);
 
 		/* Mark the adv set as created by legacy advertising cmd */
-		adv->is_created = 1U;
+		adv->is_created = ULL_ADV_CREATED_BITMASK_CREATED;
 	}
 #endif /* CONFIG_BT_CTLR_ADV_EXT */
 
@@ -595,7 +596,9 @@ uint8_t ll_adv_enable(uint8_t enable)
 		if (!priv) {
 			uint8_t const *tx_addr;
 #if defined(CONFIG_BT_CTLR_ADV_EXT)
-			if ((adv->is_created & BIT(1)) && pdu_adv->tx_addr) {
+			if ((adv->is_created &
+			     ULL_ADV_CREATED_BITMASK_EXTENDED) &&
+			    pdu_adv->tx_addr) {
 				tx_addr = ll_adv_aux_random_addr_get(adv, NULL);
 			} else
 #endif /* CONFIG_BT_CTLR_ADV_EXT */
diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c b/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c
index 1c1dcccb5c8..ee730f62eb0 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c
@@ -80,7 +80,6 @@ uint8_t const *ll_adv_aux_random_addr_get(struct ll_adv_set const *const adv,
 	return adv->rnd_addr;
 }
 
-#if (CONFIG_BT_CTLR_ADV_AUX_SET > 0)
 uint8_t ll_adv_aux_ad_data_set(uint8_t handle, uint8_t op, uint8_t frag_pref, uint8_t len,
 			    uint8_t const *const data)
 {
@@ -105,7 +104,8 @@ uint8_t ll_adv_aux_ad_data_set(uint8_t handle, uint8_t op, uint8_t frag_pref, ui
 	 */
 
 	/* TODO: handle other op values */
-	if ((op != 0x03) && (op != 0x04)) {
+	if ((op != BT_HCI_LE_EXT_ADV_OP_COMPLETE_DATA) &&
+	    (op != BT_HCI_LE_EXT_ADV_OP_UNCHANGED_DATA)) {
 		/* FIXME: error code */
 		return BT_HCI_ERR_CMD_DISALLOWED;
 	}
@@ -496,7 +496,6 @@ uint16_t ll_adv_aux_max_data_length_get(void)
 {
 	return CONFIG_BT_CTLR_ADV_DATA_LEN_MAX;
 }
-#endif /* (CONFIG_BT_CTLR_ADV_AUX_SET > 0) */
 
 uint8_t ll_adv_aux_set_count_get(void)
 {
diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_internal.h b/subsys/bluetooth/controller/ll_sw/ull_adv_internal.h
index 88468ae66c2..c720a3d8e83 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_adv_internal.h
+++ b/subsys/bluetooth/controller/ll_sw/ull_adv_internal.h
@@ -6,6 +6,16 @@
 
 #define ULL_ADV_RANDOM_DELAY HAL_TICKER_US_TO_TICKS(10000)
 
+/* Bitmask values used in is_created field of adv set */
+#define ULL_ADV_CREATED_BITMASK_CREATED  BIT(0)
+#define ULL_ADV_CREATED_BITMASK_EXTENDED BIT(1)
+
+/* Bitmask value returned by ull_adv_is_enabled() */
+#define ULL_ADV_ENABLED_BITMASK_ENABLED  BIT(0)
+#define ULL_ADV_ENABLED_BITMASK_CREATED  (ULL_ADV_CREATED_BITMASK_CREATED << 1)
+#define ULL_ADV_ENABLED_BITMASK_EXTENDED (ULL_ADV_CREATED_BITMASK_EXTENDED << 1)
+
+/* Helper functions to initialise and reset ull_adv module */
 int ull_adv_init(void);
 int ull_adv_reset(void);
 
@@ -36,6 +46,7 @@ uint8_t scan_rsp_set(struct ll_adv_set *adv, uint8_t len,
 		     uint8_t const *const data);
 
 #if defined(CONFIG_BT_CTLR_ADV_EXT)
+/* Helper functions to initialise and reset ull_adv_aux module */
 int ull_adv_aux_init(void);
 int ull_adv_aux_reset(void);
 
diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c b/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c
index aae8210ca23..74055964c24 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c
@@ -76,6 +76,7 @@ uint8_t ll_adv_sync_param_set(uint8_t handle, uint16_t interval, uint16_t flags)
 		uint8_t *_pp, *pp, *ps, *_ps;
 		uint8_t ip, is, ad_len;
 		struct lll_adv *lll;
+		int err;
 
 		sync = sync_acquire();
 		if (!sync) {
@@ -94,7 +95,9 @@ uint8_t ll_adv_sync_param_set(uint8_t handle, uint16_t interval, uint16_t flags)
 		/* NOTE: ull_hdr_init(&sync->ull); is done on start */
 		lll_hdr_init(lll_sync, sync);
 
-		util_aa_to_le32(lll_sync->access_addr);
+		err = util_aa_le32(lll_sync->access_addr);
+		LL_ASSERT(!err);
+
 		util_rand(lll_sync->crc_init, sizeof(lll_sync->crc_init));
 
 		lll_sync->latency_prepare = 0;
diff --git a/subsys/bluetooth/controller/ll_sw/ull_chan.c b/subsys/bluetooth/controller/ll_sw/ull_chan.c
index df432ddb96c..d00e83ff4db 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_chan.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_chan.c
@@ -10,6 +10,11 @@
 
 #include "util/util.h"
 
+/* Initial channel map indicating Used and Unused data channels.
+ * The HCI LE Set Host Channel Classification command allows the Host to
+ * specify a channel classification for the data, secondary advertising,
+ * periodic, and isochronous physical channels based on its local information.
+ */
 static uint8_t map[5] = {0xFF, 0xFF, 0xFF, 0xFF, 0x1F};
 static uint8_t count = 37U;
 
diff --git a/subsys/bluetooth/controller/ll_sw/ull_filter.c b/subsys/bluetooth/controller/ll_sw/ull_filter.c
index cef7e68f2ce..05656c582d6 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_filter.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_filter.c
@@ -589,7 +589,8 @@ void ull_filter_adv_pdu_update(struct ll_adv_set *adv, struct pdu_adv *pdu)
 	} else {
 		pdu->tx_addr = adv->own_addr_type & 0x1;
 #if defined(CONFIG_BT_CTLR_ADV_EXT)
-		if ((adv->is_created & BIT(1)) && pdu->tx_addr) {
+		if ((adv->is_created & ULL_ADV_CREATED_BITMASK_EXTENDED) &&
+		    pdu->tx_addr) {
 			ll_adv_aux_random_addr_get(adv, adva);
 		} else
 #endif /* CONFIG_BT_CTLR_ADV_EXT */
diff --git a/subsys/bluetooth/controller/ll_sw/ull_master.c b/subsys/bluetooth/controller/ll_sw/ull_master.c
index 3cf19e2960a..c7203e3ad21 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_master.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_master.c
@@ -145,7 +145,7 @@ uint8_t ll_create_connection(uint16_t scan_interval, uint16_t scan_window,
 
 	conn_lll = &conn->lll;
 
-	err = util_aa_to_le32(conn_lll->access_addr);
+	err = util_aa_le32(conn_lll->access_addr);
 	LL_ASSERT(!err);
 
 	util_rand(conn_lll->crc_init, sizeof(conn_lll->crc_init));
@@ -414,6 +414,10 @@ uint8_t ll_connect_disable(void **rx)
 	return status;
 }
 
+/* FIXME: Refactor out this interface so that its usable by extended
+ * advertising channel classification, and also master role connections can
+ * perform channel map update control procedure.
+ */
 uint8_t ll_chm_update(uint8_t const *const chm)
 {
 	uint16_t handle;
diff --git a/subsys/bluetooth/controller/util/util.c b/subsys/bluetooth/controller/util/util.c
index 9f4871d2247..27b637f4fc8 100644
--- a/subsys/bluetooth/controller/util/util.c
+++ b/subsys/bluetooth/controller/util/util.c
@@ -76,7 +76,7 @@ int util_rand(void *buf, size_t len)
  * - It shall have no more than eleven transitions in the least significant 16
  *   bits.
  */
-int util_aa_to_le32(uint8_t *dst)
+int util_aa_le32(uint8_t *dst)
 {
 #if defined(CONFIG_BT_CTLR_PHY_CODED)
 	uint8_t transitions_lsb16;
@@ -97,7 +97,8 @@ again:
 	}
 	retry--;
 
-	util_rand((void *)&aa, sizeof(aa));
+	util_rand(dst, sizeof(uint32_t));
+	aa = sys_get_le32(dst);
 
 	bit_idx = 31U;
 	transitions = 0U;
diff --git a/subsys/bluetooth/controller/util/util.h b/subsys/bluetooth/controller/util/util.h
index 6dcf11cea39..73eabe68a44 100644
--- a/subsys/bluetooth/controller/util/util.h
+++ b/subsys/bluetooth/controller/util/util.h
@@ -17,4 +17,4 @@
 
 uint8_t util_ones_count_get(uint8_t *octets, uint8_t octets_len);
 int util_rand(void *buf, size_t len);
-int util_aa_to_le32(uint8_t *dst);
+int util_aa_le32(uint8_t *dst);