Bluetooth: Controller: Fix ticker to prefer ticker node started

Fix ticker implementation to prefer to keep ticker node
started in case of race condition to start and stop.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

subsys/bluetooth/controller/Kconfig.ll_sw_split

@@ -55,6 +55,8 @@ config BT_LLL_VENDOR_NORDIC
 	select BT_TICKER_REMAINDER_GET if BT_BROADCASTER && BT_CTLR_ADV_EXT
 	select BT_TICKER_LAZY_GET if BT_CTLR_ADV_PERIODIC || BT_CTLR_CENTRAL_ISO
 
+	select BT_TICKER_PREFER_START_BEFORE_STOP if BT_TICKER_SLOT_AGNOSTIC
+
 	default y
 	help
 	  Use Nordic Lower Link Layer implementation.
@@ -1036,6 +1038,21 @@ config BT_TICKER_SLOT_AGNOSTIC
 	  reservations and collision handling, and operates as a simple
 	  multi-instance programmable timer.
 
+config BT_TICKER_PREFER_START_BEFORE_STOP
+	bool "Ticker prefer start before stop request"
+	help
+	  Under race conditions wherein for a given ticker node if a number of
+	  start and stop operations are enqueued towards ticker_job by a said
+	  user execution context, then start operations is preferred to be
+	  processed before stop operations.
+
+	  Without this option, the default behavior is to defer all start
+	  requests after all stop requests enqueued by all user context having
+	  been processed. The rationale for default behavior being that under
+	  race conditions, start followed by stop requests, or start before stop
+	  requests, the said ticker node is always scheduled and at timeout the
+	  execution context can take decision based on its execution state.
+
 config BT_CTLR_JIT_SCHEDULING
 	bool "Just-in-Time Scheduling"
 	select BT_TICKER_SLOT_AGNOSTIC

subsys/bluetooth/controller/ticker/ticker.c

@@ -55,9 +55,11 @@ struct ticker_node {
 	uint8_t  force:1;		    /* If non-zero, node timeout should
 					     * be forced at next expiration
 					     */
+#if defined(CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP)
 	uint8_t  start_pending:1;	    /* If non-zero, start is pending for
 					     * bottom half of ticker_job.
 					     */
+#endif /* CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP */
 	uint32_t ticks_periodic;	    /* If non-zero, interval
 					     * between expirations
 					     */
@@ -1907,12 +1909,15 @@ static inline uint8_t ticker_job_list_manage(struct ticker_instance *instance,
 
 			/* if op is start, then skip update and stop ops */
 			if (user_op->op < TICKER_USER_OP_TYPE_UPDATE) {
+#if defined(CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP)
 				if (user_op->op == TICKER_USER_OP_TYPE_START) {
 					/* Set start pending to validate a
 					 * successive, inline stop operation.
 					 */
 					ticker->start_pending = 1U;
 				}
+#endif /* CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP */
+
 				continue;
 			}
 
@@ -1923,7 +1928,10 @@ static inline uint8_t ticker_job_list_manage(struct ticker_instance *instance,
 			 * set status and continue.
 			 */
 			if ((user_op->op > TICKER_USER_OP_TYPE_STOP_ABS) ||
-			    (((state == 0U) && !ticker->start_pending) &&
+			    ((state == 0U) &&
+#if defined(CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP)
+			     !ticker->start_pending &&
+#endif /* CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP */
 			     (user_op->op != TICKER_USER_OP_TYPE_YIELD_ABS)) ||
 			    ((user_op->op == TICKER_USER_OP_TYPE_UPDATE) &&
 			     (user_op->params.update.ticks_drift_plus == 0U) &&
@@ -2731,7 +2739,9 @@ static inline void ticker_job_list_insert(struct ticker_instance *instance,
 					continue;
 				}
 
+#if defined(CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP)
 				ticker->start_pending = 0U;
+#endif /* CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP */
 
 				if (((ticker->req -
 				      ticker->ack) & 0xff) != 0U) {

tests/bluetooth/init/prj_ctlr_ticker.conf

@@ -0,0 +1,65 @@
+CONFIG_BT=y
+CONFIG_BT_CTLR=y
+CONFIG_BT_LL_SW_SPLIT=y
+CONFIG_BT_CTLR_DUP_FILTER_LEN=16
+CONFIG_BT_CTLR_CONN_PARAM_REQ=y
+CONFIG_BT_CTLR_LE_PING=y
+CONFIG_BT_CTLR_PRIVACY=n
+CONFIG_BT_CTLR_EXT_SCAN_FP=n
+CONFIG_BT_DATA_LEN_UPDATE=n
+CONFIG_BT_PHY_UPDATE=y
+CONFIG_BT_CTLR_CHAN_SEL_2=y
+CONFIG_BT_CTLR_MIN_USED_CHAN=y
+CONFIG_BT_CTLR_ADV_EXT=y
+CONFIG_BT_CTLR_DTM_HCI=y
+CONFIG_BT_CTLR_ADVANCED_FEATURES=y
+CONFIG_BT_CTLR_PHY_2M=y
+CONFIG_BT_CTLR_PHY_2M_NRF=y
+CONFIG_BT_CTLR_PHY_CODED=y
+CONFIG_BT_CTLR_ADV_AUX_PDU_BACK2BACK=y
+CONFIG_BT_CTLR_LLL_PRIO=0
+CONFIG_BT_CTLR_ULL_HIGH_PRIO=1
+CONFIG_BT_CTLR_XTAL_ADVANCED=n
+CONFIG_BT_CTLR_SCHED_ADVANCED=n
+CONFIG_BT_CTLR_RADIO_ENABLE_FAST=y
+CONFIG_BT_CTLR_TIFS_HW=n
+CONFIG_BT_CTLR_FAST_ENC=y
+CONFIG_BT_CTLR_TX_RETRY_DISABLE=y
+CONFIG_BT_CTLR_CONN_RSSI=y
+CONFIG_BT_CTLR_ADV_INDICATION=y
+CONFIG_BT_CTLR_SCAN_REQ_NOTIFY=y
+CONFIG_BT_CTLR_SCAN_REQ_RSSI=y
+CONFIG_BT_CTLR_SCAN_INDICATION=y
+CONFIG_BT_CTLR_PROFILE_ISR=y
+CONFIG_BT_CTLR_DEBUG_PINS=y
+CONFIG_BT_CTLR_TEST=y
+CONFIG_BT_TICKER_EXT=n
+CONFIG_BT_TICKER_SLOT_AGNOSTIC=y
+CONFIG_BT_TICKER_PREFER_START_BEFORE_STOP=y
+CONFIG_BT_HCI_VS_EXT=y
+CONFIG_BT_HCI_MESH_EXT=n
+CONFIG_BT_PERIPHERAL=y
+CONFIG_BT_CENTRAL=y
+CONFIG_BT_SMP=y
+CONFIG_BT_SIGNING=y
+CONFIG_BT_SMP_SC_ONLY=y
+CONFIG_BT_TINYCRYPT_ECC=y
+CONFIG_BT_USE_DEBUG_KEYS=y
+CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
+CONFIG_BT_GATT_CLIENT=y
+CONFIG_BT_DEBUG_MONITOR_UART=y
+CONFIG_BT_HCI_CORE_LOG_LEVEL_DBG=y
+CONFIG_BT_CONN_LOG_LEVEL_DBG=y
+CONFIG_BT_KEYS_LOG_LEVEL_DBG=y
+CONFIG_BT_L2CAP_LOG_LEVEL_DBG=y
+CONFIG_BT_SMP_LOG_LEVEL_DBG=y
+CONFIG_BT_HCI_DRIVER_LOG_LEVEL_DBG=y
+CONFIG_BT_SMP_SELFTEST=y
+CONFIG_BT_ATT_LOG_LEVEL_DBG=y
+CONFIG_BT_GATT_LOG_LEVEL_DBG=y
+CONFIG_BT_BREDR=n
+CONFIG_DEBUG=y
+CONFIG_FLASH=y
+CONFIG_SOC_FLASH_NRF_RADIO_SYNC_TICKER=n
+CONFIG_ZTEST=y
+CONFIG_ZTEST_NEW_API=y

tests/bluetooth/init/testcase.yaml

@@ -122,6 +122,15 @@ tests:
     integration_platforms:
       - nrf52840dk_nrf52840
       - nrf52dk_nrf52832
+  bluetooth.init.test_ctlr_ticker:
+    extra_args:
+      - CONF_FILE=prj_ctlr_ticker.conf
+    platform_allow:
+      - nrf52840dk_nrf52840
+      - nrf52dk_nrf52832
+    integration_platforms:
+      - nrf52840dk_nrf52840
+      - nrf52dk_nrf52832
   bluetooth.init.test_ctlr_broadcaster:
     extra_args: CONF_FILE=prj_ctlr_broadcaster.conf
     platform_allow: