Bluetooth: Controller: Fix missing radio timer comp and range delay

Fix missing PPI to timer start compensation and missing inclusion of range delay in the calculation of packet header receive timeout value. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
zephyrproject-rtos · Feb 29, 2024 · 8573d36 · 8573d36
1 parent 9fda7a3
commit 8573d36
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 31 deletions.
diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c
@@ -874,8 +874,9 @@ void sw_switch(uint8_t dir_curr, uint8_t dir_next, uint8_t phy_curr, uint8_t fla
 	}
 
 	if (delay < SW_SWITCH_TIMER->CC[cc]) {
-		nrf_timer_cc_set(SW_SWITCH_TIMER, cc,
-				 (SW_SWITCH_TIMER->CC[cc] - delay));
+		nrf_timer_cc_set(SW_SWITCH_TIMER,
+				 cc,
+				 (SW_SWITCH_TIMER->CC[cc] - delay - HAL_RADIO_TMR_START_DELAY_US));
 	} else {
 		nrf_timer_cc_set(SW_SWITCH_TIMER, cc, 1);
 	}

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h
@@ -10,7 +10,9 @@
 /* Common radio resources */
 #include "radio_nrf5_resources.h"
 
-/* Helpers for radio timing conversions */
+/* Helpers for radio timing conversions.
+ * These has to come before the radio_*.h include below.
+ */
 #define HAL_RADIO_NS2US_CEIL(ns)  ((ns + 999)/1000)
 #define HAL_RADIO_NS2US_ROUND(ns) ((ns + 500)/1000)
 
@@ -42,7 +44,9 @@
 #error "Unsupported SoC."
 #endif
 
-/* Define to reset PPI registration */
+/* Define to reset PPI registration.
+ * This has to come before the ppi/dppi includes below.
+ */
 #define NRF_PPI_NONE 0
 
 /* This has to come before the ppi/dppi includes below. */
@@ -66,17 +70,20 @@
 
 #include "radio_nrf5_txp.h"
 
-/* SoC specific Radio PDU length field maximum value */
-#if defined(CONFIG_SOC_SERIES_NRF51X)
-#define HAL_RADIO_PDU_LEN_MAX (BIT(5) - 1)
-#else
-#define HAL_RADIO_PDU_LEN_MAX (BIT(8) - 1)
-#endif
-
 /* Common NRF_RADIO power-on reset value. Refer to Product Specification,
  * RADIO Registers section for the documented reset values.
  *
  * NOTE: Only implementation used values defined here.
  *       In the future if MDK or nRFx header include these, use them instead.
  */
 #define HAL_RADIO_RESET_VALUE_PCNF1 0x00000000UL
+
+/* SoC specific Radio PDU length field maximum value */
+#if defined(CONFIG_SOC_SERIES_NRF51X)
+#define HAL_RADIO_PDU_LEN_MAX (BIT(5) - 1)
+#else
+#define HAL_RADIO_PDU_LEN_MAX (BIT(8) - 1)
+#endif
+
+/* This is delay between PPI task START and timer actual start counting. */
+#define HAL_RADIO_TMR_START_DELAY_US 1U
diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c
@@ -1196,8 +1196,10 @@ static void isr_tx(void *param)
 	}
 #endif /* CONFIG_BT_CTLR_PRIVACY */
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
-	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + 4 + 1;
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
+	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
+	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
+	       HAL_RADIO_TMR_START_DELAY_US;
 	hcto += radio_rx_chain_delay_get(phy_p, 0);
 	hcto += addr_us_get(phy_p);
 	hcto -= radio_tx_chain_delay_get(phy_p, 0);

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c
@@ -509,9 +509,10 @@ static void isr_tx_rx(void *param)
 	}
 #endif /* CONFIG_BT_CTLR_PRIVACY */
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
 	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
-	       (EVENT_CLOCK_JITTER_US << 1U) + 1U;
+	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
+	       HAL_RADIO_TMR_START_DELAY_US;
 	hcto += radio_rx_chain_delay_get(lll->phy_s, PHY_FLAGS_S8);
 	hcto += addr_us_get(lll->phy_s);
 	hcto -= radio_tx_chain_delay_get(lll->phy_s, PHY_FLAGS_S8);

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c
@@ -500,10 +500,10 @@ static void isr_tx(void *param)
 	/* assert if radio packet ptr is not set and radio started rx */
 	LL_ASSERT(!radio_is_ready());
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
 	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
 	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
-	       HCTO_START_DELAY_US;
+	       HAL_RADIO_TMR_START_DELAY_US;
 
 #if defined(CONFIG_BT_CTLR_PHY)
 	hcto += radio_rx_chain_delay_get(cis_lll->rx.phy, PHY_FLAGS_S8);

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c
@@ -579,9 +579,10 @@ void lll_conn_isr_tx(void *param)
 	}
 #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_TX */
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
-	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + (EVENT_CLOCK_JITTER_US << 1) +
-	       RANGE_DELAY_US + HCTO_START_DELAY_US;
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
+	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
+	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
+	       HAL_RADIO_TMR_START_DELAY_US;
 #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_TX)
 	hcto += cte_len;
 #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_TX */

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c
@@ -805,8 +805,10 @@ static void isr_tx(void *param)
 	}
 #endif /* CONFIG_BT_CTLR_PRIVACY */
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
-	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + 4 + 1;
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
+	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
+	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
+	       HAL_RADIO_TMR_START_DELAY_US;
 	hcto += radio_rx_chain_delay_get(0, 0);
 	hcto += addr_us_get(0);
 	hcto -= radio_tx_chain_delay_get(0, 0);

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c
@@ -1350,9 +1350,10 @@ static void isr_tx(struct lll_scan_aux *lll_aux, void *pdu_rx,
 	}
 #endif /* CONFIG_BT_CTLR_PRIVACY */
 
-	/* +/- 2us active clock jitter, +1 us hcto compensation */
+	/* +/- 2us active clock jitter, +1 us PPI to timer start compensation */
 	hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US +
-	       (EVENT_CLOCK_JITTER_US << 1U) + RANGE_DELAY_US + 1U;
+	       (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US +
+	       HAL_RADIO_TMR_START_DELAY_US;
 	hcto += radio_rx_chain_delay_get(lll_aux->phy, PHY_FLAGS_S8);
 	hcto += addr_us_get(lll_aux->phy);
 	hcto -= radio_tx_chain_delay_get(lll_aux->phy, PHY_FLAGS_S8);

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c
@@ -1004,7 +1004,7 @@ static void isr_rx(void *param)
 		 * the current subevent we are listening.
 		 */
 		hcto += (((EVENT_CLOCK_JITTER_US << 1) * nse) << 1) +
-			RANGE_DELAY_US + HCTO_START_DELAY_US;
+			RANGE_DELAY_US + HAL_RADIO_TMR_START_DELAY_US;
 	} else {
 		/* First subevent PDU was not received, hence setup radio packet
 		 * timer header complete timeout from where the first subevent

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h
@@ -12,12 +12,6 @@
 #define RANGE_DISTANCE 1000 /* meters */
 #define RANGE_DELAY_US (2 * RANGE_DISTANCE * 4 / 1000)
 
-/* This is a compensation of delay between PPI task START and timer start counting.
- * HCTO is a timer used to stop Radio peripheral in receive mode if packet address was not
- * received.
- */
-#define HCTO_START_DELAY_US 1U
-
 static inline uint32_t addr_us_get(uint8_t phy)
 {
 	switch (phy) {