Bluetooth: Controller: Review rework of jitter in aux offset

Review rework of the fix to remove the jitter in aux offset
and sync offset values.

Force select BT_TICKER_REMAINDER_GET and BT_TICKER_LAZY_GET
features when Extended Advertising and Periodic Advertising
is supported.

Rename ticks and microsecond offset value struct members
for primary PDU event offset (to auxiliary PDU event).

Converted HAL_TICKER_REMOVE_JITTER and HAL_TICKER_ADD_JITTER
macro to functions.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Author: cvinayak

subsys/bluetooth/controller/Kconfig.ll_sw_split

@@ -43,6 +43,9 @@ config BT_LLL_VENDOR_NORDIC
 	select BT_CTLR_TIFS_HW_SUPPORT
 	select BT_CTLR_ULL_LLL_PRIO_SUPPORT
 
+	select BT_TICKER_REMAINDER_GET if BT_BROADCASTER && BT_CTLR_ADV_EXT
+	select BT_TICKER_LAZY_GET if BT_CTLR_ADV_PERIODIC
+
 	default y
 	help
 	  Use Nordic Lower Link Layer implementation.
@@ -709,15 +712,13 @@ config BT_TICKER_LOW_LAT
 
 config BT_TICKER_REMAINDER_GET
 	bool "Ticker Next Slot Get with Remainder"
-	default y if BT_BROADCASTER && BT_CTLR_ADV_EXT
 	help
 	  This option enables ticker interface to iterate through active
 	  ticker nodes, returning tick to expire and remainder from a reference
 	  tick.
 
 config BT_TICKER_LAZY_GET
 	bool "Ticker Next Slot Get with Lazy"
-	default y if BT_CTLR_ADV_PERIODIC
 	help
 	  This option enables ticker interface to iterate through active
 	  ticker nodes, returning tick to expire and lazy count from a reference

subsys/bluetooth/controller/ll_sw/lll_adv.h

@@ -130,8 +130,8 @@ struct lll_adv_aux {
 	/* Store used by primary channel PDU event to fill the
 	 * auxiliary offset to this auxiliary PDU event.
 	 */
-	uint32_t ticks_offset;
-	uint32_t us_offset;
+	uint32_t ticks_pri_pdu_offset;
+	uint32_t us_pri_pdu_offset;
 
 	struct lll_adv_pdu data;
 #if defined(CONFIG_BT_CTLR_ADV_PDU_LINK)

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c

@@ -1050,7 +1050,7 @@ void radio_tmr_tifs_set(uint32_t tifs)
 
 uint32_t radio_tmr_start(uint8_t trx, uint32_t ticks_start, uint32_t remainder)
 {
-	HAL_TICKER_REMOVE_JITTER(ticks_start, remainder);
+	hal_ticker_remove_jitter(&ticks_start, &remainder);
 
 	nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_CLEAR);
 	EVENT_TIMER->MODE = 0;

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/ticker.h

@@ -6,6 +6,7 @@
  */
 
 #define HAL_TICKER_CNTR_CLK_FREQ_HZ 32768U
+#define HAL_TICKER_CNTR_CLK_UNIT_FS 30517578125UL
 
 /* Macro defining the minimum counter compare offset */
 #define HAL_TICKER_CNTR_CMP_OFFSET_MIN 3
@@ -24,44 +25,28 @@
  */
 #define HAL_TICKER_US_TO_TICKS(x) \
 	( \
-		((uint32_t)(((uint64_t) (x) * 1000000000UL) / 30517578125UL)) \
-		& HAL_TICKER_CNTR_MASK \
+		((uint32_t)(((uint64_t) (x) * 1000000000UL) / \
+		 HAL_TICKER_CNTR_CLK_UNIT_FS)) & HAL_TICKER_CNTR_MASK \
 	)
 
-/* Macro returning remainder in picoseconds */
+/* Macro to translate tick units to microseconds. */
+#define HAL_TICKER_TICKS_TO_US(x) \
+	( \
+		((uint32_t)(((uint64_t)(x) * HAL_TICKER_CNTR_CLK_UNIT_FS) / \
+		 1000000000UL)) \
+	)
+
+/* Macro returning remainder in picoseconds (to fit in 32-bits) */
 #define HAL_TICKER_REMAINDER(x) \
 	( \
 		( \
 			((uint64_t) (x) * 1000000000UL) \
-			- ((uint64_t)HAL_TICKER_US_TO_TICKS(x) * 30517578125UL) \
+			- ((uint64_t)HAL_TICKER_US_TO_TICKS(x) * \
+			   HAL_TICKER_CNTR_CLK_UNIT_FS) \
 		) \
 		/ 1000UL \
 	)
 
-/* Macro to remove ticks and return positive remainder value in microseconds */
-#define HAL_TICKER_REMOVE_JITTER(t, r) \
-	{ \
-		if ((!(r / 1000000UL)) || (r & BIT(31))) { \
-			t--; \
-			r += 30517578UL; \
-		} \
-		r /= 1000000UL; \
-	}
-
-/* Macro to add ticks and return positive remainder value in microseconds */
-#define HAL_TICKER_ADD_JITTER(t, r) \
-	{ \
-		if ((!(r / 1000000UL)) || (r & BIT(31))) { \
-			t++; \
-			r += 30517578UL; \
-		} \
-		r /= 1000000UL; \
-	}
-
-/* Macro to translate tick units to microseconds. */
-#define HAL_TICKER_TICKS_TO_US(x) \
-	((uint32_t)(((uint64_t)(x) * 30517578125UL) / 1000000000UL))
-
 /* Macro defining the remainder resolution/range
  * ~ 1000000 * HAL_TICKER_TICKS_TO_US(1)
  */
@@ -71,3 +56,30 @@
 /* Macro defining the margin for positioning re-scheduled nodes */
 #define HAL_TICKER_RESCHEDULE_MARGIN \
 	HAL_TICKER_US_TO_TICKS(150)
+
+/* Remove ticks and return positive remainder value in microseconds */
+static inline void hal_ticker_remove_jitter(uint32_t *ticks,
+					    uint32_t *remainder)
+{
+	/* Is remainder less than 1 us */
+	if ((*remainder & BIT(31)) || !(*remainder / 1000000UL)) {
+		*ticks -= 1U;
+		*remainder += HAL_TICKER_CNTR_CLK_UNIT_FS / 1000UL;
+	}
+
+	/* pico seconds to micro seconds unit */
+	*remainder /= 1000000UL;
+}
+
+/* Add ticks and return positive remainder value in microseconds */
+static inline void hal_ticker_add_jitter(uint32_t *ticks, uint32_t *remainder)
+{
+	/* Is remainder less than 1 us */
+	if ((*remainder & BIT(31)) || !(*remainder / 1000000UL)) {
+		*ticks += 1U;
+		*remainder += HAL_TICKER_CNTR_CLK_UNIT_FS / 1000UL;
+	}
+
+	/* pico seconds to micro seconds unit */
+	*remainder /= 1000000UL;
+}

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

@@ -1316,8 +1316,8 @@ static void isr_done(void *param)
 		lll_aux = lll->aux;
 		if (lll_aux) {
 			(void)ull_adv_aux_lll_offset_fill(pdu,
-							  lll_aux->ticks_offset,
-							  lll_aux->us_offset,
+							  lll_aux->ticks_pri_pdu_offset,
+							  lll_aux->us_pri_pdu_offset,
 							  start_us);
 		}
 #else /* !CONFIG_BT_CTLR_ADV_EXT */

subsys/bluetooth/controller/ll_sw/ull_adv_aux.c

@@ -1503,24 +1503,24 @@ static void mfy_aux_offset_get(void *param)
 	} while (id != ticker_id);
 
 	/* Adjust ticks to expire based on remainder value */
-	HAL_TICKER_REMOVE_JITTER(ticks_to_expire, remainder);
+	hal_ticker_remove_jitter(&ticks_to_expire, &remainder);
 
 	/* Store the ticks offset for population in other advertising primary
 	 * channel PDUs.
 	 */
-	lll_aux->ticks_offset =	ticks_to_expire;
+	lll_aux->ticks_pri_pdu_offset = ticks_to_expire;
 
 	/* NOTE: as first primary channel PDU does not use remainder, the packet
 	 * timer is started one tick in advance to start the radio with
 	 * microsecond precision, hence compensate for the higher start_us value
 	 * captured at radio start of the first primary channel PDU.
 	 */
-	lll_aux->ticks_offset += 1U;
+	lll_aux->ticks_pri_pdu_offset += 1U;
 
 	/* Store the microsecond remainder offset for population in other
 	 * advertising primary channel PDUs.
 	 */
-	lll_aux->us_offset = remainder;
+	lll_aux->us_pri_pdu_offset = remainder;
 
 	/* Fill the aux offset in the first Primary channel PDU */
 	/* FIXME: we are in ULL_LOW context, fill offset in LLL context? */

subsys/bluetooth/controller/ll_sw/ull_adv_sync.c

@@ -1831,11 +1831,17 @@ static void mfy_sync_offset_get(void *param)
 		LL_ASSERT(id != TICKER_NULL);
 	} while (id != ticker_id);
 
-	HAL_TICKER_REMOVE_JITTER(ticks_to_expire, remainder);
+	/* Reduced a tick for negative remainder and return positive remainder
+	 * value.
+	 */
+	hal_ticker_remove_jitter(&ticks_to_expire, &remainder);
 	sync_remainder_us = remainder;
 
+	/* Add a tick for negative remainder and return positive remainder
+	 * value.
+	 */
 	remainder = sync->aux_remainder;
-	HAL_TICKER_ADD_JITTER(ticks_to_expire, remainder);
+	hal_ticker_add_jitter(&ticks_to_expire, &remainder);
 	aux_remainder_us = remainder;
 
 	pdu = lll_adv_aux_data_latest_peek(adv->lll.aux);