From 44f3d20339921bed75eb2880c21ebdd85fed1a80 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 26 Mar 2020 18:30:59 +0530 Subject: [PATCH 1/6] Bluetooth: controller: Fix ticker catch up on ISR latency Fix ticker to avoid catch up of periodic timeouts in case of large ISR latencies like in case of flash erase scenarios. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ticker/ticker.c | 59 +++++++++++++++++---- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/subsys/bluetooth/controller/ticker/ticker.c b/subsys/bluetooth/controller/ticker/ticker.c index ef68fa77789e..60380d17729e 100644 --- a/subsys/bluetooth/controller/ticker/ticker.c +++ b/subsys/bluetooth/controller/ticker/ticker.c @@ -1007,7 +1007,6 @@ static u8_t ticker_remainder_inc(struct ticker_node *ticker) #endif } -#if defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) /** * @brief Decrement remainder * @@ -1037,7 +1036,6 @@ static u8_t ticker_remainder_dec(struct ticker_node *ticker) return 0; #endif } -#endif /* CONFIG_BT_TICKER_COMPATIBILITY_MODE */ /** * @brief Invoke user operation callback @@ -1093,7 +1091,6 @@ static inline void ticker_job_node_update(struct ticker_node *ticker, if ((ticker->ticks_periodic != 0U) && (user_op->params.update.lazy != 0U)) { user_op->params.update.lazy--; -#if defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) while ((ticks_to_expire > ticker->ticks_periodic) && (ticker->lazy_current > user_op->params.update.lazy)) { ticks_to_expire -= ticker->ticks_periodic + @@ -1106,7 +1103,6 @@ static inline void ticker_job_node_update(struct ticker_node *ticker, ticker_remainder_inc(ticker); ticker->lazy_current++; } -#endif /* CONFIG_BT_TICKER_COMPATIBILITY_MODE */ ticker->lazy_periodic = user_op->params.update.lazy; } @@ -1365,6 +1361,14 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, struct ticker_node *node; u32_t ticks_expired; +#if !defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) + u32_t ticks_latency; + u32_t ticks_now; + + ticks_now = cntr_cnt_get(); + ticks_latency = ticker_ticks_diff_get(ticks_now, ticks_previous); +#endif /* !CONFIG_BT_TICKER_COMPATIBILITY_MODE */ + node = &instance->nodes[0]; ticks_expired = 0U; while (instance->ticker_id_head != TICKER_NULL) { @@ -1388,6 +1392,8 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, ticks_expired += ticks_to_expire; #if !defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) + ticks_latency -= ticks_to_expire; + if (ticker->lazy_current != 0U && !TICKER_RESCHEDULE_PENDING(ticker)) { instance->ticker_id_slot_previous = TICKER_NULL; @@ -1433,11 +1439,46 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, */ ticker->ticks_to_expire = ticks_elapsed; } else { - /* Reload ticks_to_expire with one period */ - ticker->ticks_to_expire = - ticker->ticks_periodic; - ticker->ticks_to_expire += - ticker_remainder_inc(ticker); + u16_t lazy_periodic; + u32_t count; + u16_t lazy; + + /* If not skipped apply lazy_periodic */ + if (!ticker->lazy_current) { + lazy_periodic = ticker->lazy_periodic; + } else { + lazy_periodic = 0U; + } + + /* Reload ticks_to_expire with atleast one + * period. + */ + ticks_to_expire = 0U; + count = 1 + lazy_periodic; + while (count--) { + ticks_to_expire += + ticker->ticks_periodic; + ticks_to_expire += + ticker_remainder_inc(ticker); + } + + /* Skip intervals that have elapsed w.r.t. + * current ticks. + */ + lazy = 0U; + while (ticks_to_expire < ticks_latency) { + ticks_to_expire += + ticker->ticks_periodic; + ticks_to_expire += + ticker_remainder_inc(ticker); + lazy++; + } + + /* Use the calculated ticks to expire and + * laziness. + */ + ticker->ticks_to_expire = ticks_to_expire; + ticker->lazy_current += (lazy_periodic + lazy); } ticks_to_expire_prep(ticker, instance->ticks_current, From b3001e473115532aaed916834533a2c7e8d69689 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 27 Mar 2020 14:54:49 +0530 Subject: [PATCH 2/6] Bluetooth: controller: Remove lazy handling in ticker_worker Removed lazy handling from ticker_worker as it is now taken care in ticker_job. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ticker/ticker.c | 23 ++++++++------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/subsys/bluetooth/controller/ticker/ticker.c b/subsys/bluetooth/controller/ticker/ticker.c index 60380d17729e..fb784589d22d 100644 --- a/subsys/bluetooth/controller/ticker/ticker.c +++ b/subsys/bluetooth/controller/ticker/ticker.c @@ -638,15 +638,10 @@ static u32_t ticker_dequeue(struct ticker_instance *instance, u8_t id) static u8_t ticker_resolve_collision(struct ticker_node *nodes, struct ticker_node *ticker) { - u8_t skipped = 0; - s32_t lazy_current = ticker->lazy_current; + if ((ticker->priority != TICKER_PRIORITY_CRITICAL) && + (ticker->next != TICKER_NULL)) { + s32_t lazy_current = ticker->lazy_current; - if (ticker->lazy_periodic > lazy_current) { - /* Programmed latency must be respected */ - skipped = 1; - - } else if ((ticker->priority != TICKER_PRIORITY_CRITICAL) && - (ticker->next != TICKER_NULL)) { /* Check if this ticker node will starve next node which has * latency or higher priority */ @@ -654,7 +649,7 @@ static u8_t ticker_resolve_collision(struct ticker_node *nodes, lazy_current -= ticker->lazy_periodic; } u8_t id_head = ticker->next; - u32_t acc_ticks_to_expire = 0; + u32_t acc_ticks_to_expire = 0U; /* Age is time since last expiry */ u32_t current_age = ticker->ticks_periodic + @@ -670,7 +665,7 @@ static u8_t ticker_resolve_collision(struct ticker_node *nodes, } /* We only care about nodes with slot reservation */ - if (ticker_next->ticks_slot == 0) { + if (ticker_next->ticks_slot == 0U) { id_head = ticker_next->next; continue; } @@ -725,13 +720,13 @@ static u8_t ticker_resolve_collision(struct ticker_node *nodes, (next_has_priority && !current_is_older) || (equal_priority && next_is_older))) { /* This node must be skipped - check window */ - skipped = 1; - break; + return 1U; } id_head = ticker_next->next; } } - return skipped; + + return 0U; } #endif /* !CONFIG_BT_TICKER_COMPATIBILITY_MODE */ @@ -1443,7 +1438,7 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, u32_t count; u16_t lazy; - /* If not skipped apply lazy_periodic */ + /* If not skipped, apply lazy_periodic */ if (!ticker->lazy_current) { lazy_periodic = ticker->lazy_periodic; } else { From 7e5765bdf4e3e0c7e36d35ccb0edf6fb14709d07 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 27 Mar 2020 14:59:23 +0530 Subject: [PATCH 3/6] Bluetooth: controller: Fix ticker state on skipped Reset ticker state in ticker_job for ticker instances that have been skipped in the ticker_worker. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ticker/ticker.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/subsys/bluetooth/controller/ticker/ticker.c b/subsys/bluetooth/controller/ticker/ticker.c index fb784589d22d..af9617b1a653 100644 --- a/subsys/bluetooth/controller/ticker/ticker.c +++ b/subsys/bluetooth/controller/ticker/ticker.c @@ -1433,6 +1433,12 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, * restarted */ ticker->ticks_to_expire = ticks_elapsed; + + /* Reset ticker state, so that its put + * back in requested state later down + * in the code. + */ + ticker->req = ticker->ack; } else { u16_t lazy_periodic; u32_t count; @@ -1443,6 +1449,12 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, lazy_periodic = ticker->lazy_periodic; } else { lazy_periodic = 0U; + + /* Reset ticker state, so that its put + * back in requested state later down + * in the code. + */ + ticker->req = ticker->ack; } /* Reload ticks_to_expire with atleast one From 6db710e0113c840e73dd4b45b81fe4607fbf7b95 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 27 Mar 2020 15:04:00 +0530 Subject: [PATCH 4/6] Bluetooth: controller: Fix ticks_slot_previous calculation Fix ticks_slot_previous calculation in the ticker_job when tickers are skipped. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ticker/ticker.c | 37 ++++++++------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/subsys/bluetooth/controller/ticker/ticker.c b/subsys/bluetooth/controller/ticker/ticker.c index af9617b1a653..a1ada4e4b0bb 100644 --- a/subsys/bluetooth/controller/ticker/ticker.c +++ b/subsys/bluetooth/controller/ticker/ticker.c @@ -1388,33 +1388,24 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, #if !defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) ticks_latency -= ticks_to_expire; +#endif /* !CONFIG_BT_TICKER_COMPATIBILITY_MODE */ - if (ticker->lazy_current != 0U && - !TICKER_RESCHEDULE_PENDING(ticker)) { + /* decrement ticks_slot_previous */ + if (instance->ticks_slot_previous > ticks_to_expire) { + instance->ticks_slot_previous -= ticks_to_expire; + } else { instance->ticker_id_slot_previous = TICKER_NULL; instance->ticks_slot_previous = 0U; - } else -#endif /* !CONFIG_BT_TICKER_COMPATIBILITY_MODE */ - { - /* decrement ticks_slot_previous */ - if (instance->ticks_slot_previous > ticks_to_expire) { - instance->ticks_slot_previous -= - ticks_to_expire; - } else { - instance->ticker_id_slot_previous = TICKER_NULL; - instance->ticks_slot_previous = 0U; - } + } - /* If a reschedule is set pending, we will need to keep - * the slot_previous information - */ - if (!TICKER_RESCHEDULE_PENDING(ticker) && - ticker->ticks_slot != 0U) { - instance->ticker_id_slot_previous = - id_expired; - instance->ticks_slot_previous = - ticker->ticks_slot; - } + /* If a reschedule is set pending, we will need to keep + * the slot_previous information + */ + if ((ticker->ticks_slot != 0U) && + (((ticker->req - ticker->ack) & 0xff) == 2U) && + !TICKER_RESCHEDULE_PENDING(ticker)) { + instance->ticker_id_slot_previous = id_expired; + instance->ticks_slot_previous = ticker->ticks_slot; } /* ticker expired, set ticks_to_expire zero */ From 1c63b8ee569227c51815398468a313b4d9b7805d Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 27 Mar 2020 14:57:02 +0530 Subject: [PATCH 5/6] Bluetooth: controller: Consider must_expire while avoiding catchup If must_expire is set for a ticker instance, then ticker expiry shall still perform catchup. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ticker/ticker.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/subsys/bluetooth/controller/ticker/ticker.c b/subsys/bluetooth/controller/ticker/ticker.c index a1ada4e4b0bb..f18c53d22da6 100644 --- a/subsys/bluetooth/controller/ticker/ticker.c +++ b/subsys/bluetooth/controller/ticker/ticker.c @@ -1367,6 +1367,7 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, node = &instance->nodes[0]; ticks_expired = 0U; while (instance->ticker_id_head != TICKER_NULL) { + u8_t is_must_expire_skip = 0U; struct ticker_node *ticker; u32_t ticks_to_expire; u8_t id_expired; @@ -1388,6 +1389,9 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, #if !defined(CONFIG_BT_TICKER_COMPATIBILITY_MODE) ticks_latency -= ticks_to_expire; + + is_must_expire_skip = (ticker->must_expire && + (ticker->lazy_current != 0U)); #endif /* !CONFIG_BT_TICKER_COMPATIBILITY_MODE */ /* decrement ticks_slot_previous */ @@ -1403,6 +1407,7 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, */ if ((ticker->ticks_slot != 0U) && (((ticker->req - ticker->ack) & 0xff) == 2U) && + !is_must_expire_skip && !TICKER_RESCHEDULE_PENDING(ticker)) { instance->ticker_id_slot_previous = id_expired; instance->ticks_slot_previous = ticker->ticks_slot; @@ -1464,12 +1469,15 @@ static inline void ticker_job_worker_bh(struct ticker_instance *instance, * current ticks. */ lazy = 0U; - while (ticks_to_expire < ticks_latency) { - ticks_to_expire += - ticker->ticks_periodic; - ticks_to_expire += - ticker_remainder_inc(ticker); - lazy++; + if (!ticker->must_expire) { + while (ticks_to_expire < + ticks_latency) { + ticks_to_expire += + ticker->ticks_periodic; + ticks_to_expire += + ticker_remainder_inc(ticker); + lazy++; + } } /* Use the calculated ticks to expire and From 32a77ef8ef00c572737c054fbfe6bfae5211ca4c Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 26 Mar 2020 18:34:13 +0530 Subject: [PATCH 6/6] Bluetooth: controller: nRF: Revert use of ticker compat mode as default Ticker is now fixed to avoid catch up of periodic timeout under large ISR latencies. Revert commit a749e28d986c ("Bluetooth: controller: split: nRF: Use ticker compat mode as default"). Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/Kconfig | 6 ------ 1 file changed, 6 deletions(-) diff --git a/subsys/bluetooth/controller/Kconfig b/subsys/bluetooth/controller/Kconfig index 267d9724ca2f..b318083b06e2 100644 --- a/subsys/bluetooth/controller/Kconfig +++ b/subsys/bluetooth/controller/Kconfig @@ -117,7 +117,6 @@ config BT_LLL_VENDOR_NORDIC select BT_HAS_HCI_VS select ENTROPY_NRF5_RNG select ENTROPY_NRF5_BIAS_CORRECTION - select BT_CTLR_LE_ENC_SUPPORT if !BT_CTLR_DATA_LENGTH_CLEAR && \ !BT_CTLR_PHY_2M_NRF select BT_CTLR_CONN_PARAM_REQ_SUPPORT @@ -137,11 +136,6 @@ config BT_LLL_VENDOR_NORDIC select BT_CTLR_XTAL_ADVANCED_SUPPORT select BT_CTLR_SCHED_ADVANCED_SUPPORT select BT_CTLR_TIFS_HW_SUPPORT - - # Until ticker resolve collision is fixed for skipped ticker catch-up - # issue, use the old compatibility mode - select BT_TICKER_COMPATIBILITY_MODE - default y help Use Nordic Lower Link Layer implementation.