You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2022/04/07 12:18:56 UTC

[mynewt-nimble] branch master updated (2a0379d5 -> 44f119a7)

This is an automated email from the ASF dual-hosted git repository.

andk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git


    from 2a0379d5 nimble/transport/h4: Assert on H4 failure
     new d2539b11 nimble/ll: Add helpers to check/add peer features
     new d95165ce nimble/ll: Add calculations for subrated conn events
     new 7a797992 nimble/ll: Add Connection Subrate Request/Update procedures
     new 44f119a7 nimble/ll: Add HCI for connection subrating

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 nimble/controller/include/controller/ble_ll_conn.h |  74 ++++-
 nimble/controller/include/controller/ble_ll_ctrl.h |   9 +-
 nimble/controller/include/controller/ble_ll_hci.h  |   2 +-
 .../controller/include/controller/ble_ll_utils.h   |   4 +
 nimble/controller/src/ble_ll.c                     |   4 +
 nimble/controller/src/ble_ll_adv.c                 |   2 +-
 nimble/controller/src/ble_ll_conn.c                | 338 +++++++++++++++++++--
 nimble/controller/src/ble_ll_conn_hci.c            | 101 +++++-
 nimble/controller/src/ble_ll_conn_priv.h           |  13 +
 nimble/controller/src/ble_ll_ctrl.c                | 182 ++++++++++-
 nimble/controller/src/ble_ll_hci.c                 |   9 +
 nimble/controller/src/ble_ll_hci_ev.c              |  32 ++
 nimble/controller/src/ble_ll_supp_cmd.c            |  16 +
 nimble/controller/src/ble_ll_sync.c                |   8 +-
 nimble/controller/syscfg.yml                       |   9 +
 nimble/include/nimble/hci_common.h                 |  30 ++
 16 files changed, 796 insertions(+), 37 deletions(-)


[mynewt-nimble] 03/04: nimble/ll: Add Connection Subrate Request/Update procedures

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git

commit 7a79799238e9d9c9d2cc57130f08276b5642e3d1
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Tue Feb 8 16:01:55 2022 +0100

    nimble/ll: Add Connection Subrate Request/Update procedures
---
 nimble/controller/include/controller/ble_ll_conn.h |  36 +++++
 nimble/controller/include/controller/ble_ll_ctrl.h |   5 +-
 .../controller/include/controller/ble_ll_utils.h   |   4 +
 nimble/controller/src/ble_ll.c                     |   4 +
 nimble/controller/src/ble_ll_conn.c                | 166 ++++++++++++++++++-
 nimble/controller/src/ble_ll_conn_priv.h           |   7 +
 nimble/controller/src/ble_ll_ctrl.c                | 179 +++++++++++++++++++++
 7 files changed, 395 insertions(+), 6 deletions(-)

diff --git a/nimble/controller/include/controller/ble_ll_conn.h b/nimble/controller/include/controller/ble_ll_conn.h
index 85f59bcf..a50e6e69 100644
--- a/nimble/controller/include/controller/ble_ll_conn.h
+++ b/nimble/controller/include/controller/ble_ll_conn.h
@@ -132,6 +132,8 @@ union ble_ll_conn_sm_flags {
         uint32_t rxd_features:1;
         uint32_t pending_hci_rd_features:1;
         uint32_t pending_initiate_dle:1;
+        uint32_t subrate_trans:1;
+        uint32_t subrate_ind_txd:1;
     } cfbit;
     uint32_t conn_flags;
 } __attribute__((packed));
@@ -183,6 +185,22 @@ struct hci_conn_update
     uint16_t max_ce_len;
 };
 
+struct ble_ll_conn_subrate_params {
+    uint16_t subrate_factor;
+    uint16_t subrate_base_event;
+    uint16_t periph_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+};
+
+struct ble_ll_conn_subrate_req_params {
+    uint16_t subrate_min;
+    uint16_t subrate_max;
+    uint16_t max_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+};
+
 /* Connection state machine */
 struct ble_ll_conn_sm
 {
@@ -284,10 +302,21 @@ struct ble_ll_conn_sm
 
     uint16_t periph_latency;
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    uint16_t acc_subrate_min;
+    uint16_t acc_subrate_max;
+    uint16_t acc_max_latency;
+    uint16_t acc_cont_num;
+    uint16_t acc_supervision_tmo;
+
     uint16_t subrate_base_event;
     uint16_t subrate_factor;
     uint16_t cont_num;
     uint16_t last_pdu_event;
+
+    union {
+        struct ble_ll_conn_subrate_params subrate_trans;
+        struct ble_ll_conn_subrate_req_params subrate_req;
+    };
 #endif
 
     /*
@@ -464,6 +493,13 @@ void ble_ll_conn_created_on_aux(struct os_mbuf *rxpdu,
                                 uint8_t *targeta);
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int ble_ll_conn_subrate_req_llcp(struct ble_ll_conn_sm *connsm,
+                                 struct ble_ll_conn_subrate_req_params *srp);
+void ble_ll_conn_subrate_set(struct ble_ll_conn_sm *connsm,
+                             struct ble_ll_conn_subrate_params *sp);
+#endif
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/nimble/controller/include/controller/ble_ll_ctrl.h b/nimble/controller/include/controller/ble_ll_ctrl.h
index 423d13af..17300dc2 100644
--- a/nimble/controller/include/controller/ble_ll_ctrl.h
+++ b/nimble/controller/include/controller/ble_ll_ctrl.h
@@ -41,7 +41,9 @@ extern "C" {
 #define BLE_LL_CTRL_PROC_PHY_UPDATE     (9)
 #define BLE_LL_CTRL_PROC_SCA_UPDATE     (10)
 #define BLE_LL_CTRL_PROC_CIS_CREATE     (11)
-#define BLE_LL_CTRL_PROC_NUM            (12)
+#define BLE_LL_CTRL_PROC_SUBRATE_REQ    (12)
+#define BLE_LL_CTRL_PROC_SUBRATE_UPDATE (13)
+#define BLE_LL_CTRL_PROC_NUM            (14)
 #define BLE_LL_CTRL_PROC_IDLE           (255)
 
 /* Checks if a particular control procedure is running */
@@ -307,6 +309,7 @@ int ble_ll_ctrl_reject_ind_send(struct ble_ll_conn_sm *connsm,
 int ble_ll_ctrl_start_enc_send(struct ble_ll_conn_sm *connsm);
 int ble_ll_ctrl_enc_allowed_pdu_rx(struct os_mbuf *rxpdu);
 int ble_ll_ctrl_enc_allowed_pdu_tx(struct os_mbuf_pkthdr *pkthdr);
+int ble_ll_ctrl_tx_start(struct ble_ll_conn_sm *connsm, struct os_mbuf *txpdu);
 int ble_ll_ctrl_tx_done(struct os_mbuf *txpdu, struct ble_ll_conn_sm *connsm);
 int ble_ll_ctrl_is_start_enc_rsp(struct os_mbuf *txpdu);
 
diff --git a/nimble/controller/include/controller/ble_ll_utils.h b/nimble/controller/include/controller/ble_ll_utils.h
index ae8abfb3..16b91a0c 100644
--- a/nimble/controller/include/controller/ble_ll_utils.h
+++ b/nimble/controller/include/controller/ble_ll_utils.h
@@ -19,6 +19,10 @@
 
 #include <stdint.h>
 
+#define INT16_GT(_a, _b) ((int16_t)((_a) - (_b)) > 0)
+#define INT16_LT(_a, _b) ((int16_t)((_a) - (_b)) < 0)
+#define INT16_LTE(_a, _b) ((int16_t)((_a) - (_b)) <= 0)
+
 uint32_t ble_ll_utils_calc_access_addr(void);
 uint8_t ble_ll_utils_remapped_channel(uint8_t remap_index, const uint8_t *chanmap);
 uint8_t ble_ll_utils_dci_csa2(uint16_t counter, uint16_t chan_id,
diff --git a/nimble/controller/src/ble_ll.c b/nimble/controller/src/ble_ll.c
index f7ab7751..b9d9d7b1 100644
--- a/nimble/controller/src/ble_ll.c
+++ b/nimble/controller/src/ble_ll.c
@@ -1943,6 +1943,10 @@ ble_ll_init(void)
     features |= BLE_LL_FEAT_ISO_HOST_SUPPORT;
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    features |= BLE_LL_FEAT_CONN_SUBRATING;
+#endif
+
     lldata->ll_supp_features = features;
 
     /* Initialize random number generation */
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index b328296d..4c60c461 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 #include "syscfg/syscfg.h"
 #include "os/os.h"
 #include "nimble/ble.h"
@@ -948,6 +949,17 @@ ble_ll_conn_chk_csm_flags(struct ble_ll_conn_sm *connsm)
         }
     }
 #endif
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    if (connsm->csmflags.cfbit.subrate_ind_txd) {
+        ble_ll_conn_subrate_set(connsm, &connsm->subrate_trans);
+        connsm->subrate_trans.subrate_factor = 0;
+        ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE);
+        connsm->csmflags.cfbit.subrate_ind_txd = 0;
+    }
+#endif /* BLE_LL_CTRL_SUBRATE_IND */
+#endif /* BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE */
 }
 
 /**
@@ -1109,6 +1121,7 @@ ble_ll_conn_tx_pdu(struct ble_ll_conn_sm *connsm)
         pktlen = pkthdr->omp_len;
         if (llid == BLE_LL_LLID_CTRL) {
             cur_txlen = pktlen;
+            ble_ll_ctrl_tx_start(connsm, m);
         } else {
             cur_txlen = ble_ll_conn_adjust_pyld_len(connsm, pktlen);
         }
@@ -1689,6 +1702,14 @@ ble_ll_conn_central_common_init(struct ble_ll_conn_sm *connsm)
     memcpy(connsm->chanmap, g_ble_ll_conn_params.central_chan_map,
            BLE_LL_CONN_CHMAP_LEN);
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    connsm->acc_subrate_min = g_ble_ll_conn_params.acc_subrate_min;
+    connsm->acc_subrate_max = g_ble_ll_conn_params.acc_subrate_max;
+    connsm->acc_max_latency = g_ble_ll_conn_params.acc_max_latency;
+    connsm->acc_cont_num = g_ble_ll_conn_params.acc_cont_num;
+    connsm->acc_supervision_tmo = g_ble_ll_conn_params.acc_supervision_tmo;
+#endif
+
     /*  Calculate random access address and crc initialization value */
     connsm->access_addr = ble_ll_utils_calc_access_addr();
     connsm->crcinit = ble_ll_rand() & 0xffffff;
@@ -2213,6 +2234,12 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
     uint8_t next_is_subrated;
     uint16_t subrate_factor;
     uint16_t event_cntr_diff;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    struct ble_ll_conn_subrate_params *cstp;
+    uint16_t trans_next_event_cntr;
+    uint16_t subrate_conn_upd_event_cntr;
+#endif
+
 
     /* XXX: deal with connection request procedure here as well */
     ble_ll_conn_chk_csm_flags(connsm);
@@ -2272,18 +2299,63 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
         if (use_periph_latency) {
             next_event_cntr += subrate_factor * connsm->periph_latency;
         }
+
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
-        /* Make this our next base event. This is technically incorrect, because
-         * subrate base event is determined by LL_SUBRATE_IND and shall only be
-         * changed if counter wrapped, but that does not really matter as once
-         * set it's only used internally.
+        /* If we are in subrate transition mode, we should also listen on
+         * subrated connection events based on new parameters.
          */
-        connsm->subrate_base_event = next_event_cntr;
+        if (connsm->csmflags.cfbit.subrate_trans) {
+            BLE_LL_ASSERT(CONN_IS_CENTRAL(connsm));
+
+            cstp = &connsm->subrate_trans;
+            trans_next_event_cntr = cstp->subrate_base_event;
+            while (INT16_LTE(trans_next_event_cntr, connsm->event_cntr)) {
+                trans_next_event_cntr += cstp->subrate_factor;
+            }
+            cstp->subrate_base_event = trans_next_event_cntr;
+
+            if (INT16_LT(trans_next_event_cntr, next_event_cntr)) {
+                next_event_cntr = trans_next_event_cntr;
+                next_is_subrated = 0;
+            }
+        }
 #endif
     } else {
         next_event_cntr = connsm->event_cntr + 1;
     }
 
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    /* If connection update is scheduled, peripheral shall listen at instant
+     * and one connection event before instant regardless of subrating.
+     */
+    if (CONN_IS_PERIPHERAL(connsm) &&
+        connsm->csmflags.cfbit.conn_update_sched &&
+        (connsm->subrate_factor > 1)) {
+        subrate_conn_upd_event_cntr = connsm->conn_update_req.instant - 1;
+        if (connsm->event_cntr == subrate_conn_upd_event_cntr) {
+            subrate_conn_upd_event_cntr++;
+        }
+
+        if (INT16_GT(next_event_cntr, subrate_conn_upd_event_cntr)) {
+            next_event_cntr = subrate_conn_upd_event_cntr;
+            next_is_subrated = 0;
+        }
+    }
+
+    /* Set next connection event as a subrate base event if that connection
+     * event is a subrated event, this simplifies calculations later.
+     * Note that according to spec base event should only be changed on
+     * wrap-around, but since we only use this value internally we can use any
+     * valid value.
+     */
+    if (next_is_subrated ||
+        (connsm->subrate_base_event +
+         connsm->subrate_factor == next_event_cntr)) {
+        connsm->subrate_base_event = next_event_cntr;
+    }
+#endif
+
     event_cntr_diff = next_event_cntr - connsm->event_cntr;
     BLE_LL_ASSERT(event_cntr_diff > 0);
 
@@ -2339,6 +2411,14 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
             connsm->csmflags.cfbit.host_expects_upd_event = 1;
         }
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+        if (connsm->conn_itvl != upd->interval) {
+            connsm->subrate_base_event = connsm->event_cntr;
+            connsm->subrate_factor = 1;
+            connsm->cont_num = 0;
+        }
+#endif
+
         connsm->supervision_tmo = upd->timeout;
         connsm->periph_latency = upd->latency;
         connsm->tx_win_size = upd->winsize;
@@ -3959,6 +4039,74 @@ err_periph_start:
 }
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int
+ble_ll_conn_subrate_req_llcp(struct ble_ll_conn_sm *connsm,
+                             struct ble_ll_conn_subrate_req_params *srp)
+{
+    BLE_LL_ASSERT(connsm->conn_role == BLE_LL_CONN_ROLE_CENTRAL);
+
+    if ((srp->subrate_min < 0x0001) || (srp->subrate_min > 0x01f4) ||
+        (srp->subrate_max < 0x0001) || (srp->subrate_max > 0x01f4) ||
+        (srp->max_latency > 0x01f3) || (srp->cont_num > 0x01f3) ||
+        (srp->supervision_tmo < 0x000a) || (srp->supervision_tmo > 0x0c80)) {
+        return -EINVAL;
+    }
+
+    if (connsm->cur_ctrl_proc == BLE_LL_CTRL_PROC_CONN_PARAM_REQ) {
+        return -EBUSY;
+    }
+
+    if ((srp->max_latency > connsm->acc_max_latency) ||
+        (srp->supervision_tmo > connsm->acc_supervision_tmo) ||
+        (srp->subrate_max < connsm->acc_subrate_min) ||
+        (srp->subrate_min > connsm->acc_subrate_max) ||
+        ((connsm->conn_itvl * BLE_LL_CONN_ITVL_USECS * srp->subrate_min *
+          (srp->max_latency + 1)) * 2 >= srp->supervision_tmo *
+                                         BLE_HCI_CONN_SPVN_TMO_UNITS * 1000)) {
+        return -EINVAL;
+    }
+
+    connsm->subrate_trans.subrate_factor = min(connsm->acc_subrate_max,
+                                               srp->subrate_max);
+    connsm->subrate_trans.subrate_base_event = connsm->event_cntr;
+    connsm->subrate_trans.periph_latency = min(connsm->acc_max_latency,
+                                               srp->max_latency);
+    connsm->subrate_trans.cont_num = min(max(connsm->acc_cont_num,
+                                             srp->cont_num),
+                                         connsm->subrate_trans.subrate_factor - 1);
+    connsm->subrate_trans.supervision_tmo = min(connsm->supervision_tmo,
+                                                srp->supervision_tmo);
+
+    ble_ll_ctrl_proc_start(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE, NULL);
+
+    return 0;
+}
+
+void
+ble_ll_conn_subrate_set(struct ble_ll_conn_sm *connsm,
+                        struct ble_ll_conn_subrate_params *sp)
+{
+    int16_t event_cntr_diff;
+    int16_t subrate_events_diff;
+
+    /* Assume parameters were checked by caller */
+
+    connsm->subrate_factor = sp->subrate_factor;
+    connsm->subrate_base_event = sp->subrate_base_event;
+    connsm->periph_latency = sp->periph_latency;
+    connsm->cont_num = sp->cont_num;
+    connsm->supervision_tmo = sp->supervision_tmo;
+
+    /* Let's update subrate base event to "latest" one */
+    event_cntr_diff = connsm->event_cntr - connsm->subrate_base_event;
+    subrate_events_diff = event_cntr_diff / connsm->subrate_factor;
+    connsm->subrate_base_event += connsm->subrate_factor * subrate_events_diff;
+
+    /* TODO send hci event */
+}
+#endif
+
 #define MAX_TIME_UNCODED(_maxbytes) \
         ble_ll_pdu_tx_time_get(_maxbytes + BLE_LL_DATA_MIC_LEN, \
                                BLE_PHY_MODE_1M);
@@ -4044,6 +4192,14 @@ ble_ll_conn_module_reset(void)
     memset(conn_params->central_chan_map, 0xff, BLE_LL_CONN_CHMAP_LEN - 1);
     conn_params->central_chan_map[4] = 0x1f;
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    conn_params->acc_subrate_min = 0x0001;
+    conn_params->acc_subrate_max = 0x0001;
+    conn_params->acc_max_latency = 0x0000;
+    conn_params->acc_cont_num = 0x0000;
+    conn_params->acc_supervision_tmo = 0x0c80;
+#endif
+
     /* Reset statistics */
     STATS_RESET(ble_ll_conn_stats);
 
diff --git a/nimble/controller/src/ble_ll_conn_priv.h b/nimble/controller/src/ble_ll_conn_priv.h
index 7541f642..c05e1955 100644
--- a/nimble/controller/src/ble_ll_conn_priv.h
+++ b/nimble/controller/src/ble_ll_conn_priv.h
@@ -79,6 +79,13 @@ struct ble_ll_conn_global_params
     uint16_t conn_init_max_tx_time_coded;
     uint16_t supp_max_tx_time;
     uint16_t supp_max_rx_time;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    uint16_t acc_subrate_min;
+    uint16_t acc_subrate_max;
+    uint16_t acc_max_latency;
+    uint16_t acc_cont_num;
+    uint16_t acc_supervision_tmo;
+#endif
 #endif
 };
 extern struct ble_ll_conn_global_params g_ble_ll_conn_params;
diff --git a/nimble/controller/src/ble_ll_ctrl.c b/nimble/controller/src/ble_ll_ctrl.c
index 04b93543..f29d3154 100644
--- a/nimble/controller/src/ble_ll_ctrl.c
+++ b/nimble/controller/src/ble_ll_ctrl.c
@@ -19,6 +19,7 @@
 #include <stdint.h>
 #include <assert.h>
 #include <string.h>
+#include <errno.h>
 #include "syscfg/syscfg.h"
 #include "nimble/ble.h"
 #include "nimble/nimble_opt.h"
@@ -408,7 +409,12 @@ ble_ll_ctrl_conn_upd_make(struct ble_ll_conn_sm *connsm, uint8_t *pyld,
      * request will actually get sent. We add one more event plus the
      * minimum as per the spec of 6 connection events.
      */
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    instant = connsm->subrate_base_event + 6 * connsm->subrate_factor *
+                                           (connsm->periph_latency + 1);
+#else
     instant = connsm->event_cntr + connsm->periph_latency + 6 + 1;
+#endif
 
     /*
      * XXX: This should change in the future, but for now we will just
@@ -1144,6 +1150,126 @@ ble_ll_ctrl_rx_sca_rsp(struct ble_ll_conn_sm *connsm, uint8_t *dptr)
 
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+static void
+ble_ll_ctrl_subrate_req_make(struct ble_ll_conn_sm *connsm, uint8_t *pyld,
+                             struct ble_ll_conn_subrate_req_params *srp)
+{
+    put_le16(pyld + 0, srp->subrate_min);
+    put_le16(pyld + 2, srp->subrate_max);
+    put_le16(pyld + 4, srp->max_latency);
+    put_le16(pyld + 6, srp->cont_num);
+    put_le16(pyld + 8, srp->supervision_tmo);
+}
+
+static void
+ble_ll_ctrl_subrate_ind_make(struct ble_ll_conn_sm *connsm, uint8_t *pyld,
+                             struct ble_ll_conn_subrate_params *sp)
+{
+    put_le16(pyld + 0, sp->subrate_factor);
+    put_le16(pyld + 2, sp->subrate_base_event);
+    put_le16(pyld + 4, sp->periph_latency);
+    put_le16(pyld + 6, sp->cont_num);
+    put_le16(pyld + 8, sp->supervision_tmo);
+}
+
+static uint8_t
+ble_ll_ctrl_rx_subrate_req(struct ble_ll_conn_sm *connsm, uint8_t *req,
+                           uint8_t *rsp)
+{
+    struct ble_ll_conn_subrate_req_params params;
+    struct ble_ll_conn_subrate_req_params *srp = &params;
+    uint8_t err;
+    int rc;
+
+#if MYNEWT_VAL(BLE_LL_ROLE_PERIPHERAL)
+    if (connsm->conn_role == BLE_LL_CONN_ROLE_PERIPHERAL) {
+        return BLE_LL_CTRL_UNKNOWN_RSP;
+    }
+#endif
+
+    if ((ble_ll_read_supp_features() & BLE_LL_FEAT_CONN_SUBRATING_HOST) == 0) {
+        ble_ll_ctrl_rej_ext_ind_make(BLE_LL_CTRL_SUBRATE_REQ,
+                                     BLE_ERR_UNSUPP_REM_FEATURE, rsp);
+        return BLE_LL_CTRL_REJECT_IND_EXT;
+    }
+
+    srp->subrate_min = get_le16(req + 0);
+    srp->subrate_max = get_le16(req + 2);
+    srp->max_latency = get_le16(req + 4);
+    srp->cont_num = get_le16(req + 6);
+    srp->supervision_tmo = get_le16(req + 8);
+
+    rc = ble_ll_conn_subrate_req_llcp(connsm, srp);
+    if (rc < 0) {
+        if (rc == -EINVAL) {
+            err = BLE_ERR_INV_LMP_LL_PARM;
+        } else if (rc == -ENOTSUP) {
+            err = BLE_ERR_UNSUPP_REM_FEATURE;
+        } else if (rc == -EBUSY) {
+            err = BLE_ERR_DIFF_TRANS_COLL;
+        } else {
+            err = BLE_ERR_UNSPECIFIED;
+        }
+
+        ble_ll_ctrl_rej_ext_ind_make(BLE_LL_CTRL_SUBRATE_REQ, err, rsp);
+
+        return BLE_LL_CTRL_REJECT_IND_EXT;
+    }
+
+    return BLE_ERR_MAX;
+}
+
+static uint8_t
+ble_ll_ctrl_rx_subrate_ind(struct ble_ll_conn_sm *connsm, uint8_t *req,
+                           uint8_t *rsp)
+{
+    struct ble_ll_conn_subrate_params params;
+    struct ble_ll_conn_subrate_params *sp = &params;
+    uint32_t t1, t2;
+
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    if (connsm->conn_role == BLE_LL_CONN_ROLE_CENTRAL) {
+        return BLE_LL_CTRL_UNKNOWN_RSP;
+    }
+#endif
+
+    sp->subrate_factor = get_le16(req + 0);
+    sp->subrate_base_event = get_le16(req + 2);
+    sp->periph_latency = get_le16(req + 4);
+    sp->cont_num = get_le16(req + 6);
+    sp->supervision_tmo = get_le16(req + 8);
+
+    /* This is probably not really useful since we shall apply new parameters
+     * immediately after receiving LL_SUBRATE_IND and central shall apply those
+     * parameters after receiving ack which it already did, so it's too late
+     * here to do anything useful. Let's just send LL_REJECT_EXT_IND anyway just
+     * for debugging purposes and reset to subrate factor of 1 and no latency,
+     * perhaps we can find some connection event from central and send our PDU.
+     */
+    t1 = connsm->conn_itvl * sp->subrate_factor * (sp->periph_latency + 1) *
+         BLE_LL_CONN_ITVL_USECS;
+    t2 = sp->supervision_tmo * BLE_HCI_CONN_SPVN_TMO_UNITS * 1000 / 2;
+    if ((sp->subrate_factor < 1) || (sp->subrate_factor > 500) ||
+        (sp->cont_num > sp->subrate_factor - 1) ||
+        (sp->subrate_factor * (sp->periph_latency + 1) > 500) || (t1 >= t2)) {
+
+        sp->subrate_factor = 1;
+        sp->subrate_base_event = connsm->event_cntr;
+        sp->periph_latency = 0;
+        sp->cont_num = 0;
+        sp->supervision_tmo = connsm->supervision_tmo;
+
+        return BLE_ERR_MAX;
+    }
+
+    ble_ll_conn_subrate_set(connsm, sp);
+    ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_REQ);
+
+    return BLE_ERR_MAX;
+}
+#endif
+
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ISO)
 static uint8_t
 ble_ll_ctrl_rx_cis_req(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
@@ -1865,6 +1991,13 @@ ble_ll_ctrl_rx_reject_ind(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
         ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SCA_UPDATE);
         break;
 #endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    case BLE_LL_CTRL_PROC_SUBRATE_REQ:
+        /* TODO: send event to host */
+        ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE);
+        break;
+#endif
+
     default:
         break;
     }
@@ -2376,6 +2509,21 @@ ble_ll_ctrl_proc_init(struct ble_ll_conn_sm *connsm, int ctrl_proc, void *data)
             opcode = BLE_LL_CTRL_CIS_REQ;
             ble_ll_ctrl_cis_create(connsm, ctrdata);
             break;
+#endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+#if MYNEWT_VAL(BLE_LL_ROLE_PERIPHERAL)
+        case BLE_LL_CTRL_PROC_SUBRATE_REQ:
+            opcode = BLE_LL_CTRL_SUBRATE_REQ;
+            ble_ll_ctrl_subrate_req_make(connsm, ctrdata, &connsm->subrate_req);
+            break;
+#endif
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+        case BLE_LL_CTRL_PROC_SUBRATE_UPDATE:
+            opcode = BLE_LL_CTRL_SUBRATE_IND;
+            ble_ll_ctrl_subrate_ind_make(connsm, ctrdata,
+                                         &connsm->subrate_trans);
+            break;
+#endif
 #endif
         default:
             BLE_LL_ASSERT(0);
@@ -2840,6 +2988,14 @@ ble_ll_ctrl_rx_pdu(struct ble_ll_conn_sm *connsm, struct os_mbuf *om)
     case BLE_LL_CTRL_PERIODIC_SYNC_IND:
         rsp_opcode = ble_ll_ctrl_rx_periodic_sync_ind(connsm, dptr);
         break;
+#endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    case BLE_LL_CTRL_SUBRATE_REQ:
+        rsp_opcode = ble_ll_ctrl_rx_subrate_req(connsm, dptr, rspdata);
+        break;
+    case BLE_LL_CTRL_SUBRATE_IND:
+        rsp_opcode = ble_ll_ctrl_rx_subrate_ind(connsm, dptr, rspdata);
+        break;
 #endif
     default:
         /* Nothing to do here */
@@ -2924,6 +3080,21 @@ ble_ll_ctrl_reject_ind_send(struct ble_ll_conn_sm *connsm, uint8_t rej_opcode,
     return rc;
 }
 
+int
+ble_ll_ctrl_tx_start(struct ble_ll_conn_sm *connsm, struct os_mbuf *txpdu)
+{
+    uint8_t opcode;
+
+    opcode = txpdu->om_data[0];
+    switch (opcode) {
+    case BLE_LL_CTRL_SUBRATE_IND:
+        connsm->csmflags.cfbit.subrate_trans = 1;
+        break;
+    }
+
+    return 0;
+}
+
 /**
  * Called when a Link Layer Control pdu has been transmitted successfully.
  * This is called when we have a received a PDU during the ISR.
@@ -3017,6 +3188,14 @@ ble_ll_ctrl_tx_done(struct os_mbuf *txpdu, struct ble_ll_conn_sm *connsm)
                     ble_ll_ctrl_phy_tx_transition_get(txpdu->om_data[2]);
         break;
 #endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    case BLE_LL_CTRL_SUBRATE_IND:
+        connsm->csmflags.cfbit.subrate_trans = 0;
+        connsm->csmflags.cfbit.subrate_ind_txd = 1;
+        break;
+#endif /* BLE_LL_CTRL_SUBRATE_IND */
+#endif /* BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE */
     default:
         break;
     }


[mynewt-nimble] 04/04: nimble/ll: Add HCI for connection subrating

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git

commit 44f119a73ad3b85b05229636475aa25fa303b010
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Feb 7 12:47:43 2022 +0100

    nimble/ll: Add HCI for connection subrating
---
 nimble/controller/include/controller/ble_ll_conn.h |  3 +
 nimble/controller/include/controller/ble_ll_ctrl.h |  4 +
 nimble/controller/include/controller/ble_ll_hci.h  |  2 +-
 nimble/controller/src/ble_ll_conn.c                | 83 +++++++++++++++++-
 nimble/controller/src/ble_ll_conn_hci.c            | 97 ++++++++++++++++++++++
 nimble/controller/src/ble_ll_conn_priv.h           |  6 ++
 nimble/controller/src/ble_ll_ctrl.c                |  3 +-
 nimble/controller/src/ble_ll_hci.c                 |  9 ++
 nimble/controller/src/ble_ll_hci_ev.c              | 32 +++++++
 nimble/controller/src/ble_ll_supp_cmd.c            | 16 ++++
 nimble/include/nimble/hci_common.h                 | 30 +++++++
 11 files changed, 282 insertions(+), 3 deletions(-)

diff --git a/nimble/controller/include/controller/ble_ll_conn.h b/nimble/controller/include/controller/ble_ll_conn.h
index a50e6e69..269d881c 100644
--- a/nimble/controller/include/controller/ble_ll_conn.h
+++ b/nimble/controller/include/controller/ble_ll_conn.h
@@ -134,6 +134,7 @@ union ble_ll_conn_sm_flags {
         uint32_t pending_initiate_dle:1;
         uint32_t subrate_trans:1;
         uint32_t subrate_ind_txd:1;
+        uint32_t subrate_host_req:1;
     } cfbit;
     uint32_t conn_flags;
 } __attribute__((packed));
@@ -494,6 +495,8 @@ void ble_ll_conn_created_on_aux(struct os_mbuf *rxpdu,
 #endif
 
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int ble_ll_conn_subrate_req_hci(struct ble_ll_conn_sm *connsm,
+                                struct ble_ll_conn_subrate_req_params *srp);
 int ble_ll_conn_subrate_req_llcp(struct ble_ll_conn_sm *connsm,
                                  struct ble_ll_conn_subrate_req_params *srp);
 void ble_ll_conn_subrate_set(struct ble_ll_conn_sm *connsm,
diff --git a/nimble/controller/include/controller/ble_ll_ctrl.h b/nimble/controller/include/controller/ble_ll_ctrl.h
index 17300dc2..379960ec 100644
--- a/nimble/controller/include/controller/ble_ll_ctrl.h
+++ b/nimble/controller/include/controller/ble_ll_ctrl.h
@@ -346,6 +346,10 @@ void ble_ll_hci_ev_sca_update(struct ble_ll_conn_sm *connsm,
                               uint8_t status, uint8_t peer_sca);
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+void ble_ll_hci_ev_subrate_change(struct ble_ll_conn_sm *connsm, uint8_t status);
+#endif
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/nimble/controller/include/controller/ble_ll_hci.h b/nimble/controller/include/controller/ble_ll_hci.h
index f45aa3ae..37d6d205 100644
--- a/nimble/controller/include/controller/ble_ll_hci.h
+++ b/nimble/controller/include/controller/ble_ll_hci.h
@@ -28,7 +28,7 @@ extern "C" {
 #include "nimble/transport.h"
 
 /* For supported commands */
-#define BLE_LL_SUPP_CMD_LEN (45)
+#define BLE_LL_SUPP_CMD_LEN (47)
 extern const uint8_t g_ble_ll_supp_cmds[BLE_LL_SUPP_CMD_LEN];
 
 /* The largest event the controller will send. */
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index 4c60c461..177b3693 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -957,6 +957,7 @@ ble_ll_conn_chk_csm_flags(struct ble_ll_conn_sm *connsm)
         connsm->subrate_trans.subrate_factor = 0;
         ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE);
         connsm->csmflags.cfbit.subrate_ind_txd = 0;
+        connsm->csmflags.cfbit.subrate_host_req = 0;
     }
 #endif /* BLE_LL_CTRL_SUBRATE_IND */
 #endif /* BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE */
@@ -4040,6 +4041,77 @@ err_periph_start:
 #endif
 
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int
+ble_ll_conn_subrate_req_hci(struct ble_ll_conn_sm *connsm,
+                            struct ble_ll_conn_subrate_req_params *srp)
+{
+    uint32_t t1, t2;
+
+    if ((srp->subrate_min < 0x0001) || (srp->subrate_min > 0x01f4) ||
+        (srp->subrate_max < 0x0001) || (srp->subrate_max > 0x01f4) ||
+        (srp->max_latency > 0x01f3) || (srp->cont_num > 0x01f3) ||
+        (srp->supervision_tmo < 0x000a) || (srp->supervision_tmo > 0x0c80)) {
+        return -EINVAL;
+    }
+
+    if (srp->subrate_max * (srp->max_latency + 1) > 500) {
+        return -EINVAL;
+    }
+
+    t1 = connsm->conn_itvl * srp->subrate_max * (srp->max_latency + 1) *
+         BLE_LL_CONN_ITVL_USECS;
+    t2 = srp->supervision_tmo * BLE_HCI_CONN_SPVN_TMO_UNITS * 1000 / 2;
+    if (t1 > t2) {
+        return -EINVAL;
+    }
+
+    if (srp->subrate_max < srp->subrate_min) {
+        return -EINVAL;
+    }
+
+    if (srp->cont_num >= srp->subrate_max) {
+        return -EINVAL;
+    }
+
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    if ((connsm->conn_role == BLE_LL_CONN_ROLE_CENTRAL) &&
+        !ble_ll_conn_rem_feature_check(connsm,
+                                       BLE_LL_FEAT_CONN_SUBRATING_HOST)) {
+        return -ENOTSUP;
+    }
+#endif
+
+    if (connsm->cur_ctrl_proc == BLE_LL_CTRL_PROC_CONN_PARAM_REQ) {
+        return -EBUSY;
+    }
+
+    switch (connsm->conn_role) {
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    case BLE_LL_CONN_ROLE_CENTRAL:
+        connsm->subrate_trans.subrate_factor = srp->subrate_max;
+        connsm->subrate_trans.subrate_base_event = connsm->event_cntr;
+        connsm->subrate_trans.periph_latency = srp->max_latency;
+        connsm->subrate_trans.cont_num = srp->cont_num;
+        connsm->subrate_trans.supervision_tmo = srp->supervision_tmo;
+        connsm->csmflags.cfbit.subrate_host_req = 1;
+        ble_ll_ctrl_proc_start(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE, NULL);
+        break;
+#endif
+#if MYNEWT_VAL(BLE_LL_ROLE_PERIPHERAL)
+    case BLE_LL_CONN_ROLE_PERIPHERAL:
+        connsm->subrate_req = *srp;
+        connsm->csmflags.cfbit.subrate_host_req = 1;
+        ble_ll_ctrl_proc_start(connsm, BLE_LL_CTRL_PROC_SUBRATE_REQ, NULL);
+        break;
+#endif
+    default:
+        BLE_LL_ASSERT(0);
+    }
+
+
+    return 0;
+}
+
 int
 ble_ll_conn_subrate_req_llcp(struct ble_ll_conn_sm *connsm,
                              struct ble_ll_conn_subrate_req_params *srp)
@@ -4089,9 +4161,16 @@ ble_ll_conn_subrate_set(struct ble_ll_conn_sm *connsm,
 {
     int16_t event_cntr_diff;
     int16_t subrate_events_diff;
+    uint8_t send_ev;
 
     /* Assume parameters were checked by caller */
 
+    send_ev = connsm->csmflags.cfbit.subrate_host_req ||
+              (connsm->subrate_factor != sp->subrate_factor) ||
+              (connsm->periph_latency != sp->periph_latency) ||
+              (connsm->cont_num != sp->cont_num) ||
+              (connsm->supervision_tmo != sp->supervision_tmo);
+
     connsm->subrate_factor = sp->subrate_factor;
     connsm->subrate_base_event = sp->subrate_base_event;
     connsm->periph_latency = sp->periph_latency;
@@ -4103,7 +4182,9 @@ ble_ll_conn_subrate_set(struct ble_ll_conn_sm *connsm,
     subrate_events_diff = event_cntr_diff / connsm->subrate_factor;
     connsm->subrate_base_event += connsm->subrate_factor * subrate_events_diff;
 
-    /* TODO send hci event */
+    if (send_ev) {
+        ble_ll_hci_ev_subrate_change(connsm, 0);
+    }
 }
 #endif
 
diff --git a/nimble/controller/src/ble_ll_conn_hci.c b/nimble/controller/src/ble_ll_conn_hci.c
index 08ee4205..4a07ab76 100644
--- a/nimble/controller/src/ble_ll_conn_hci.c
+++ b/nimble/controller/src/ble_ll_conn_hci.c
@@ -20,6 +20,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 #include "syscfg/syscfg.h"
 #include "os/os.h"
 #include "nimble/ble.h"
@@ -1692,6 +1693,102 @@ ble_ll_conn_req_peer_sca(const uint8_t *cmdbuf, uint8_t len,
 }
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int
+ble_ll_conn_hci_set_default_subrate(const uint8_t *cmdbuf, uint8_t len,
+                                    uint8_t *rspbuf, uint8_t *rsplen)
+{
+    const struct ble_hci_le_set_default_subrate_cp *cp = (const void *)cmdbuf;
+    struct ble_ll_conn_global_params *gcp = &g_ble_ll_conn_params;
+    uint16_t subrate_min;
+    uint16_t subrate_max;
+    uint16_t max_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+
+    subrate_min = le16toh(cp->subrate_min);
+    subrate_max = le16toh(cp->subrate_max);
+    max_latency = le16toh(cp->max_latency);
+    cont_num = le16toh(cp->cont_num);
+    supervision_tmo = le16toh(cp->supervision_tmo);
+
+    if ((subrate_min < 0x0001) || (subrate_min > 0x01f4) ||
+        (subrate_max < 0x0001) || (subrate_max > 0x01f4) ||
+        (max_latency > 0x01f3) || (cont_num > 0x01f3) ||
+        (supervision_tmo < 0x000a) || (supervision_tmo > 0x0c80)) {
+        return BLE_ERR_INV_HCI_CMD_PARMS;
+    }
+
+    if (subrate_max * (max_latency + 1) > 500) {
+        return BLE_ERR_INV_HCI_CMD_PARMS;
+    }
+
+    if (subrate_max < subrate_min) {
+        return BLE_ERR_INV_HCI_CMD_PARMS;
+    }
+
+    if (cont_num >= subrate_max) {
+        return BLE_ERR_INV_HCI_CMD_PARMS;
+    }
+
+    gcp->acc_subrate_min = subrate_min;
+    gcp->acc_subrate_max = subrate_max;
+    gcp->acc_max_latency = max_latency;
+    gcp->acc_cont_num = cont_num;
+    gcp->acc_supervision_tmo = supervision_tmo;
+
+    return 0;
+}
+
+int
+ble_ll_conn_hci_subrate_req(const uint8_t *cmdbuf, uint8_t len,
+                            uint8_t *rspbuf, uint8_t *rsplen)
+{
+    const struct ble_hci_le_subrate_req_cp *cp = (const void *)cmdbuf;
+    struct ble_ll_conn_subrate_req_params srp;
+    struct ble_ll_conn_sm *connsm;
+    uint16_t conn_handle;
+    int rc;
+
+    conn_handle = le16toh(cp->conn_handle);
+    srp.subrate_min = le16toh(cp->subrate_min);
+    srp.subrate_max = le16toh(cp->subrate_max);
+    srp.max_latency = le16toh(cp->max_latency);
+    srp.cont_num = le16toh(cp->cont_num);
+    srp.supervision_tmo = le16toh(cp->supervision_tmo);
+
+    connsm = ble_ll_conn_find_by_handle(conn_handle);
+    if (!connsm) {
+        return BLE_ERR_UNK_CONN_ID;
+    }
+
+    rc = ble_ll_conn_subrate_req_hci(connsm, &srp);
+    if (rc < 0) {
+        if (rc == -EINVAL) {
+            return BLE_ERR_INV_HCI_CMD_PARMS;
+        } else if (rc == -EBUSY) {
+            return BLE_ERR_CTLR_BUSY;
+        } else if (rc == -ENOTSUP) {
+            return BLE_ERR_UNSUPP_REM_FEATURE;
+        } else {
+            return BLE_ERR_UNSPECIFIED;
+        }
+    }
+
+#if MYNEWT_VAL(BLE_LL_ROLE_CENTRAL)
+    if (connsm->conn_role == BLE_LL_CONN_ROLE_CENTRAL) {
+        connsm->acc_subrate_min = srp.subrate_min;
+        connsm->acc_subrate_min = srp.subrate_max;
+        connsm->acc_max_latency = srp.max_latency;
+        connsm->acc_cont_num = srp.cont_num;
+        connsm->acc_supervision_tmo = srp.supervision_tmo;
+    }
+#endif
+
+    return 0;
+}
+#endif
+
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_PING)
 /**
  * Read authenticated payload timeout (OGF=3, OCF==0x007B)
diff --git a/nimble/controller/src/ble_ll_conn_priv.h b/nimble/controller/src/ble_ll_conn_priv.h
index c05e1955..7a1e56d8 100644
--- a/nimble/controller/src/ble_ll_conn_priv.h
+++ b/nimble/controller/src/ble_ll_conn_priv.h
@@ -229,6 +229,12 @@ int ble_ll_conn_hci_rd_auth_pyld_tmo(const uint8_t *cmdbuf, uint8_t len,
 int ble_ll_conn_req_peer_sca(const uint8_t *cmdbuf, uint8_t len,
                              uint8_t *rspbuf, uint8_t *rsplen);
 #endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+int ble_ll_conn_hci_set_default_subrate(const uint8_t *cmdbuf, uint8_t len,
+                                        uint8_t *rspbuf, uint8_t *rsplen);
+int ble_ll_conn_hci_subrate_req(const uint8_t *cmdbuf, uint8_t len,
+                                uint8_t *rspbuf, uint8_t *rsplen);
+#endif
 
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_PING)
 void ble_ll_conn_auth_pyld_timer_start(struct ble_ll_conn_sm *connsm);
diff --git a/nimble/controller/src/ble_ll_ctrl.c b/nimble/controller/src/ble_ll_ctrl.c
index f29d3154..5111d6d3 100644
--- a/nimble/controller/src/ble_ll_ctrl.c
+++ b/nimble/controller/src/ble_ll_ctrl.c
@@ -1265,6 +1265,7 @@ ble_ll_ctrl_rx_subrate_ind(struct ble_ll_conn_sm *connsm, uint8_t *req,
 
     ble_ll_conn_subrate_set(connsm, sp);
     ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_REQ);
+    connsm->csmflags.cfbit.subrate_host_req = 0;
 
     return BLE_ERR_MAX;
 }
@@ -1993,7 +1994,7 @@ ble_ll_ctrl_rx_reject_ind(struct ble_ll_conn_sm *connsm, uint8_t *dptr,
 #endif
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
     case BLE_LL_CTRL_PROC_SUBRATE_REQ:
-        /* TODO: send event to host */
+        ble_ll_hci_ev_subrate_change(connsm, ble_error);
         ble_ll_ctrl_proc_stop(connsm, BLE_LL_CTRL_PROC_SUBRATE_UPDATE);
         break;
 #endif
diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c
index bc9160f1..9666b023 100644
--- a/nimble/controller/src/ble_ll_hci.c
+++ b/nimble/controller/src/ble_ll_hci.c
@@ -655,6 +655,7 @@ ble_ll_hci_le_cmd_send_cmd_status(uint16_t ocf)
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_SCA_UPDATE)
     case BLE_HCI_OCF_LE_REQ_PEER_SCA:
 #endif
+    case BLE_HCI_OCF_LE_SUBRATE_REQ:
         rc = 1;
         break;
     default:
@@ -1293,6 +1294,14 @@ ble_ll_hci_le_cmd_proc(const uint8_t *cmdbuf, uint8_t len, uint16_t ocf,
         rc = ble_ll_conn_req_peer_sca(cmdbuf, len,
                                       rspbuf, rsplen);
         break;
+#endif
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    case BLE_HCI_OCF_LE_SET_DEFAULT_SUBRATE:
+        rc = ble_ll_conn_hci_set_default_subrate(cmdbuf, len, rspbuf, rsplen);
+        break;
+    case BLE_HCI_OCF_LE_SUBRATE_REQ:
+        rc = ble_ll_conn_hci_subrate_req(cmdbuf, len, rspbuf, rsplen);
+        break;
 #endif
     default:
         rc = BLE_ERR_UNKNOWN_HCI_CMD;
diff --git a/nimble/controller/src/ble_ll_hci_ev.c b/nimble/controller/src/ble_ll_hci_ev.c
index 8543d989..6da2545c 100644
--- a/nimble/controller/src/ble_ll_hci_ev.c
+++ b/nimble/controller/src/ble_ll_hci_ev.c
@@ -491,6 +491,38 @@ ble_ll_hci_ev_sca_update(struct ble_ll_conn_sm *connsm, uint8_t status,
 
 #endif
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+void
+ble_ll_hci_ev_subrate_change(struct ble_ll_conn_sm *connsm, uint8_t status)
+{
+    struct ble_hci_ev_le_subev_subrate_change *ev;
+    struct ble_hci_ev *hci_ev;
+
+    if (!ble_ll_hci_is_le_event_enabled(BLE_HCI_LE_SUBEV_SUBRATE_CHANGE)) {
+        return;
+    }
+
+    hci_ev = ble_transport_alloc_evt(0);
+    if (!hci_ev) {
+        return;
+    }
+
+    hci_ev->opcode = BLE_HCI_EVCODE_LE_META;
+    hci_ev->length = sizeof(*ev);
+    ev = (void *)hci_ev->data;
+
+    ev->subev_code = BLE_HCI_LE_SUBEV_SUBRATE_CHANGE;
+    ev->status = status;
+    ev->conn_handle = htole16(connsm->conn_handle);
+    ev->subrate_factor = htole16(connsm->subrate_factor);
+    ev->periph_latency = htole16(connsm->periph_latency);
+    ev->cont_num = htole16(connsm->cont_num);
+    ev->supervision_tmo = htole16(connsm->supervision_tmo);
+
+    ble_ll_hci_event_send(hci_ev);
+}
+#endif
+
 void
 ble_ll_hci_ev_send_vs_assert(const char *file, uint32_t line)
 {
diff --git a/nimble/controller/src/ble_ll_supp_cmd.c b/nimble/controller/src/ble_ll_supp_cmd.c
index 6903c8d2..1c408fbf 100644
--- a/nimble/controller/src/ble_ll_supp_cmd.c
+++ b/nimble/controller/src/ble_ll_supp_cmd.c
@@ -582,6 +582,20 @@
     BLE_SUPP_CMD_LE_SET_HOST_FEATURE  \
 )
 
+/* Octet 46 */
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+#define BLE_SUPP_CMD_LE_SET_DEFAULT_SUBRATE     (1 << 0)
+#define BLE_SUPP_CMD_LE_SUBRATE_REQ             (1 << 1)
+#else
+#define BLE_SUPP_CMD_LE_SET_DEFAULT_SUBRATE     (0 << 0)
+#define BLE_SUPP_CMD_LE_SUBRATE_REQ             (0 << 1)
+#endif
+#define BLE_LL_SUPP_CMD_OCTET_46                        \
+(                                                       \
+    BLE_SUPP_CMD_LE_SET_DEFAULT_SUBRATE               | \
+    BLE_SUPP_CMD_LE_SUBRATE_REQ                         \
+)
+
 /* Defines the array of supported commands */
 const uint8_t g_ble_ll_supp_cmds[BLE_LL_SUPP_CMD_LEN] =
 {
@@ -630,4 +644,6 @@ const uint8_t g_ble_ll_supp_cmds[BLE_LL_SUPP_CMD_LEN] =
     BLE_LL_SUPP_CMD_OCTET_42,
     BLE_LL_SUPP_CMD_OCTET_43,
     BLE_LL_SUPP_CMD_OCTET_44,
+    0,
+    BLE_LL_SUPP_CMD_OCTET_46,
 };
diff --git a/nimble/include/nimble/hci_common.h b/nimble/include/nimble/hci_common.h
index 7d1c3ac9..c52dea20 100644
--- a/nimble/include/nimble/hci_common.h
+++ b/nimble/include/nimble/hci_common.h
@@ -1062,6 +1062,25 @@ struct ble_hci_le_set_host_feat_cp {
     uint8_t val;
 } __attribute__((packed));
 
+#define BLE_HCI_OCF_LE_SET_DEFAULT_SUBRATE               (0x007D)
+struct ble_hci_le_set_default_subrate_cp {
+    uint16_t subrate_min;
+    uint16_t subrate_max;
+    uint16_t max_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+} __attribute__((packed));
+
+#define BLE_HCI_OCF_LE_SUBRATE_REQ                       (0x007E)
+struct ble_hci_le_subrate_req_cp {
+    uint16_t conn_handle;
+    uint16_t subrate_min;
+    uint16_t subrate_max;
+    uint16_t max_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+} __attribute__((packed));
+
 /* --- Vendor specific commands (OGF 0x003F) */
 /* Read Random Static Address */
 #define BLE_HCI_OCF_VS_RD_STATIC_ADDR                   (MYNEWT_VAL(BLE_HCI_VS_OCF_OFFSET) + (0x0001))
@@ -1821,6 +1840,17 @@ struct ble_hci_ev_le_subev_biginfo_adv_report {
     uint8_t encryption;
 } __attribute__((packed));
 
+#define BLE_HCI_LE_SUBEV_SUBRATE_CHANGE             (0x23)
+struct ble_hci_ev_le_subev_subrate_change {
+    uint8_t subev_code;
+    uint8_t status;
+    uint16_t conn_handle;
+    uint16_t subrate_factor;
+    uint16_t periph_latency;
+    uint16_t cont_num;
+    uint16_t supervision_tmo;
+} __attribute__((packed));
+
 /* Data buffer overflow event */
 #define BLE_HCI_EVENT_ACL_BUF_OVERFLOW      (0x01)
 


[mynewt-nimble] 02/04: nimble/ll: Add calculations for subrated conn events

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git

commit d95165ce75d566b0b6f52fe762d0df139fc3ed59
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Tue Feb 8 14:06:12 2022 +0100

    nimble/ll: Add calculations for subrated conn events
    
    This adds basic calculations to support subrated connection events. If
    feature is disabled, we always use subrate_factor=1 and most of the
    additional code is compiled out.
---
 nimble/controller/include/controller/ble_ll_conn.h |  9 +-
 nimble/controller/src/ble_ll_conn.c                | 95 +++++++++++++++++-----
 nimble/controller/syscfg.yml                       |  9 ++
 3 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/nimble/controller/include/controller/ble_ll_conn.h b/nimble/controller/include/controller/ble_ll_conn.h
index aaa72fb1..85f59bcf 100644
--- a/nimble/controller/include/controller/ble_ll_conn.h
+++ b/nimble/controller/include/controller/ble_ll_conn.h
@@ -269,7 +269,6 @@ struct ble_ll_conn_sm
 
     /* Connection timing */
     uint16_t conn_itvl;
-    uint16_t periph_latency;
     uint16_t supervision_tmo;
     uint16_t min_ce_len;
     uint16_t max_ce_len;
@@ -283,6 +282,14 @@ struct ble_ll_conn_sm
     uint32_t periph_cur_window_widening;
     uint32_t last_rxd_pdu_cputime;  /* Used exclusively for supervision timer */
 
+    uint16_t periph_latency;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    uint16_t subrate_base_event;
+    uint16_t subrate_factor;
+    uint16_t cont_num;
+    uint16_t last_pdu_event;
+#endif
+
     /*
      * Used to mark that identity address was used as InitA
      */
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index 6d131095..b328296d 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -1853,6 +1853,13 @@ ble_ll_conn_sm_new(struct ble_ll_conn_sm *connsm)
     connsm->conn_rssi = BLE_LL_CONN_UNKNOWN_RSSI;
     connsm->inita_identity_used = 0;
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    connsm->subrate_base_event = 0;
+    connsm->subrate_factor = 1;
+    connsm->cont_num = 0;
+    connsm->last_pdu_event = 0;
+#endif
+
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PERIODIC_ADV_SYNC_TRANSFER)
     connsm->sync_transfer_sync_timeout = g_ble_ll_conn_sync_transfer_params.sync_timeout_us;
     connsm->sync_transfer_mode = g_ble_ll_conn_sync_transfer_params.mode;
@@ -2191,8 +2198,8 @@ ble_ll_conn_move_anchor(struct ble_ll_conn_sm *connsm, uint16_t offset)
 static int
 ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
 {
-    uint16_t latency;
-    uint32_t itvl;
+    uint32_t conn_itvl_us;
+    uint32_t ce_duration;
 #if MYNEWT_VAL(BLE_LL_ROLE_PERIPHERAL)
     uint32_t cur_ww;
     uint32_t max_ww;
@@ -2200,6 +2207,12 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
     struct ble_ll_conn_upd_req *upd;
     uint8_t skip_anchor_calc = 0;
     uint32_t usecs;
+    uint8_t use_periph_latency;
+    uint16_t base_event_cntr;
+    uint16_t next_event_cntr;
+    uint8_t next_is_subrated;
+    uint16_t subrate_factor;
+    uint16_t event_cntr_diff;
 
     /* XXX: deal with connection request procedure here as well */
     ble_ll_conn_chk_csm_flags(connsm);
@@ -2217,6 +2230,22 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
         connsm->periph_latency = 0;
     }
 
+    next_is_subrated = 1;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    base_event_cntr = connsm->subrate_base_event;
+    subrate_factor = connsm->subrate_factor;
+
+    if ((connsm->cont_num > 0) &&
+        (connsm->event_cntr + 1 == connsm->subrate_base_event +
+                                   connsm->subrate_factor) &&
+        (connsm->event_cntr - connsm->last_pdu_event < connsm->cont_num)) {
+        next_is_subrated = 0;
+    }
+#else
+    base_event_cntr = connsm->event_cntr;
+    subrate_factor = 1;
+#endif
+
     /*
      * XXX: TODO Probably want to add checks to see if we need to start
      * a control procedure here as an instant may have prevented us from
@@ -2230,22 +2259,39 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
      * the instant
      */
     /* Set event counter to the next connection event that we will tx/rx in */
-    itvl = connsm->conn_itvl * BLE_LL_CONN_ITVL_USECS;
-    latency = 1;
-    if (connsm->csmflags.cfbit.allow_periph_latency     &&
-        !connsm->csmflags.cfbit.conn_update_sched       &&
-        !CONN_F_PHY_UPDATE_SCHED(connsm)                &&
-        !connsm->csmflags.cfbit.chanmap_update_scheduled) {
-        if (connsm->csmflags.cfbit.pkt_rxd) {
-            latency += connsm->periph_latency;
-            itvl = itvl * latency;
+
+    use_periph_latency = next_is_subrated &&
+                         connsm->csmflags.cfbit.allow_periph_latency &&
+                         !connsm->csmflags.cfbit.conn_update_sched &&
+                         !connsm->csmflags.cfbit.phy_update_sched &&
+                         !connsm->csmflags.cfbit.chanmap_update_scheduled &&
+                         connsm->csmflags.cfbit.pkt_rxd;
+
+    if (next_is_subrated) {
+        next_event_cntr = base_event_cntr + subrate_factor;
+        if (use_periph_latency) {
+            next_event_cntr += subrate_factor * connsm->periph_latency;
         }
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+        /* Make this our next base event. This is technically incorrect, because
+         * subrate base event is determined by LL_SUBRATE_IND and shall only be
+         * changed if counter wrapped, but that does not really matter as once
+         * set it's only used internally.
+         */
+        connsm->subrate_base_event = next_event_cntr;
+#endif
+    } else {
+        next_event_cntr = connsm->event_cntr + 1;
     }
-    connsm->event_cntr += latency;
+
+    event_cntr_diff = next_event_cntr - connsm->event_cntr;
+    BLE_LL_ASSERT(event_cntr_diff > 0);
+
+    connsm->event_cntr = next_event_cntr;
 
 #if MYNEWT_VAL(BLE_LL_CONN_STRICT_SCHED)
     if (connsm->conn_role == BLE_LL_CONN_ROLE_CENTRAL) {
-        connsm->css_period_idx += latency;
+        connsm->css_period_idx += event_cntr_diff;
 
         /* If this is non-reference connection, we set anchor from reference
          * instead of calculating manually.
@@ -2261,13 +2307,15 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
         /* Calculate next anchor point for connection.
          * We can use pre-calculated values for one interval if latency is 1.
          */
-        if (latency == 1) {
+        if (event_cntr_diff == 1) {
             connsm->anchor_point += connsm->conn_itvl_ticks;
             ble_ll_tmr_add_u(&connsm->anchor_point, &connsm->anchor_point_usecs,
                              connsm->conn_itvl_usecs);
         } else {
+            conn_itvl_us = connsm->conn_itvl * BLE_LL_CONN_ITVL_USECS;
+
             ble_ll_tmr_add(&connsm->anchor_point, &connsm->anchor_point_usecs,
-                           itvl);
+                           conn_itvl_us * event_cntr_diff);
         }
     }
 
@@ -2399,7 +2447,7 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
 #endif
 
     /* Calculate data channel index of next connection event */
-    connsm->data_chan_index = ble_ll_conn_calc_dci(connsm, latency);
+    connsm->data_chan_index = ble_ll_conn_calc_dci(connsm, event_cntr_diff);
 
     /*
      * If we are trying to terminate connection, check if next wake time is
@@ -2416,8 +2464,8 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
      * Calculate ce end time. For a peripgheral, we need to add window widening
      * and the transmit window if we still have one.
      */
-    itvl = ble_ll_tmr_u2t(MYNEWT_VAL(BLE_LL_CONN_INIT_SLOTS) *
-                          BLE_LL_SCHED_USECS_PER_SLOT);
+    ce_duration = ble_ll_tmr_u2t(MYNEWT_VAL(BLE_LL_CONN_INIT_SLOTS) *
+                                 BLE_LL_SCHED_USECS_PER_SLOT);
 
 #if MYNEWT_VAL(BLE_LL_ROLE_PERIPHERAL)
     if (connsm->conn_role == BLE_LL_CONN_ROLE_PERIPHERAL) {
@@ -2431,11 +2479,12 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
         }
         cur_ww += BLE_LL_JITTER_USECS;
         connsm->periph_cur_window_widening = cur_ww;
-        itvl += ble_ll_tmr_u2t(cur_ww + connsm->periph_cur_tx_win_usecs);
+        ce_duration += ble_ll_tmr_u2t(cur_ww +
+                                      connsm->periph_cur_tx_win_usecs);
     }
 #endif
-    itvl -= g_ble_ll_sched_offset_ticks;
-    connsm->ce_end_time = connsm->anchor_point + itvl;
+    ce_duration -= g_ble_ll_sched_offset_ticks;
+    connsm->ce_end_time = connsm->anchor_point + ce_duration;
 
     return 0;
 }
@@ -3162,6 +3211,10 @@ ble_ll_conn_rx_data_pdu(struct os_mbuf *rxpdu, struct ble_mbuf_hdr *hdr)
     acl_len = rxbuf[1];
     llid = hdr_byte & BLE_LL_DATA_HDR_LLID_MASK;
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE)
+    connsm->last_pdu_event = connsm->event_cntr;
+#endif
+
 
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION)
     if (BLE_MBUF_HDR_MIC_FAILURE(hdr)) {
diff --git a/nimble/controller/syscfg.yml b/nimble/controller/syscfg.yml
index ebf7e1dd..1cd43355 100644
--- a/nimble/controller/syscfg.yml
+++ b/nimble/controller/syscfg.yml
@@ -327,6 +327,15 @@ syscfg.defs:
         restrictions:
             - '(BLE_VERSION >= 52) if 1'
 
+    BLE_LL_CFG_FEAT_LL_ENHANCED_CONN_UPDATE:
+        description: >
+            Enables support LE Enhanced Connection Update.
+            This allows to use Conenction Subrate Update and Connection Subrate
+            Request procedures to modify subrate paramters for a connection.
+        value: 0
+        restrictions:
+            - '(BLE_VERSION >= 53) if 1'
+
     BLE_LL_CFG_FEAT_LL_ISO:
         description: >
             This option is used to enable/disable support for LE Isochronous Channels


[mynewt-nimble] 01/04: nimble/ll: Add helpers to check/add peer features

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git

commit d2539b118926806386c725d2484d21ff30505699
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Feb 7 13:11:33 2022 +0100

    nimble/ll: Add helpers to check/add peer features
---
 nimble/controller/include/controller/ble_ll_conn.h | 26 ++++++++++++++++++++++
 nimble/controller/src/ble_ll_adv.c                 |  2 +-
 nimble/controller/src/ble_ll_conn.c                |  6 ++---
 nimble/controller/src/ble_ll_conn_hci.c            |  4 ++--
 nimble/controller/src/ble_ll_ctrl.c                |  2 +-
 nimble/controller/src/ble_ll_sync.c                |  8 ++-----
 6 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/nimble/controller/include/controller/ble_ll_conn.h b/nimble/controller/include/controller/ble_ll_conn.h
index ec13d009..aaa72fb1 100644
--- a/nimble/controller/include/controller/ble_ll_conn.h
+++ b/nimble/controller/include/controller/ble_ll_conn.h
@@ -389,6 +389,32 @@ struct ble_ll_conn_sm
 #define CONN_IS_PERIPHERAL(csm)     (false)
 #endif
 
+static inline int
+ble_ll_conn_rem_feature_check(struct ble_ll_conn_sm *connsm, uint64_t feature)
+{
+    uint8_t byte_idx;
+
+    /* 8 lsb are conn features */
+    feature >>= 8;
+
+    byte_idx = __builtin_ctzll(feature) / 8;
+    return connsm->remote_features[byte_idx] & (feature >> (byte_idx * 8));
+}
+
+
+static inline void
+ble_ll_conn_rem_feature_add(struct ble_ll_conn_sm *connsm, uint64_t feature)
+{
+    uint8_t byte_idx;
+
+    /* 8 lsb are conn features */
+    feature >>= 8;
+
+    byte_idx = __builtin_ctzll(feature) / 8;
+    connsm->remote_features[byte_idx] |= (feature >> (byte_idx * 8));
+}
+
+
 struct ble_ll_conn_sm *ble_ll_conn_find_by_handle(uint16_t handle);
 struct ble_ll_conn_sm *ble_ll_conn_find_by_peer_addr(const uint8_t* addr,
                                                      uint8_t addr_type);
diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c
index b3cf5f98..1aaeeb4d 100644
--- a/nimble/controller/src/ble_ll_adv.c
+++ b/nimble/controller/src/ble_ll_adv.c
@@ -4094,7 +4094,7 @@ ble_ll_adv_periodic_set_info_transfer(const uint8_t *cmdbuf, uint8_t len,
       *
       * Allow initiate LL procedure only if remote supports it.
       */
-     if (!(connsm->remote_features[2] & (BLE_LL_FEAT_SYNC_TRANS_RECV >> (8 * 3)))) {
+     if (!ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_SYNC_TRANS_RECV)) {
          rc = BLE_ERR_UNSUPP_REM_FEATURE;
          goto done;
      }
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index 9bbcfd4f..6d131095 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -1763,7 +1763,7 @@ ble_ll_conn_init_phy(struct ble_ll_conn_sm *connsm, int phy)
         connsm->rem_max_tx_time = BLE_LL_CONN_SUPP_TIME_MIN_CODED;
         connsm->rem_max_rx_time = BLE_LL_CONN_SUPP_TIME_MIN_CODED;
         /* Assume peer does support coded */
-        connsm->remote_features[0] |= (BLE_LL_FEAT_LE_CODED_PHY >> 8);
+        ble_ll_conn_rem_feature_add(connsm, BLE_LL_FEAT_LE_CODED_PHY);
     } else {
         connsm->max_tx_time = conngp->conn_init_max_tx_time_uncoded;
         connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_UNCODED;
@@ -2389,8 +2389,8 @@ ble_ll_conn_next_event(struct ble_ll_conn_sm *connsm)
          */
         if (((connsm->phy_data.cur_tx_phy == BLE_PHY_CODED) ||
              (connsm->phy_data.cur_rx_phy == BLE_PHY_CODED)) &&
-            !(connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8))) {
-            connsm->remote_features[0] |= (BLE_LL_FEAT_LE_CODED_PHY >> 8);
+            !ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_LE_CODED_PHY)) {
+            ble_ll_conn_rem_feature_add(connsm, BLE_LL_FEAT_LE_CODED_PHY);
             connsm->max_rx_time = BLE_LL_CONN_SUPP_TIME_MAX_CODED;
             ble_ll_ctrl_initiate_dle(connsm);
         }
diff --git a/nimble/controller/src/ble_ll_conn_hci.c b/nimble/controller/src/ble_ll_conn_hci.c
index 82abdcb1..08ee4205 100644
--- a/nimble/controller/src/ble_ll_conn_hci.c
+++ b/nimble/controller/src/ble_ll_conn_hci.c
@@ -1476,7 +1476,7 @@ ble_ll_conn_hci_set_data_len(const uint8_t *cmdbuf, uint8_t len,
     connsm->host_req_max_tx_time = txtime;
 
     /* If peer does not support coded, we cannot use value larger than 2120us */
-    if (!(connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8))) {
+    if (!ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_LE_CODED_PHY)) {
         txtime = min(txtime, BLE_LL_CONN_SUPP_TIME_MAX_UNCODED);
     }
 #endif
@@ -1677,7 +1677,7 @@ ble_ll_conn_req_peer_sca(const uint8_t *cmdbuf, uint8_t len,
         return BLE_ERR_UNK_CONN_ID;
     }
 
-    if (!(connsm->remote_features[2] & (BLE_LL_FEAT_SCA_UPDATE >> 24))) {
+    if (!ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_SCA_UPDATE)) {
         return BLE_ERR_UNSUPP_REM_FEATURE;
     }
 
diff --git a/nimble/controller/src/ble_ll_ctrl.c b/nimble/controller/src/ble_ll_ctrl.c
index c30f4e3e..04b93543 100644
--- a/nimble/controller/src/ble_ll_ctrl.c
+++ b/nimble/controller/src/ble_ll_ctrl.c
@@ -1970,7 +1970,7 @@ ble_ll_ctrl_update_features(struct ble_ll_conn_sm *connsm, uint8_t *feat)
          * LE Coded PHY. However, once we know that peer does support it we can
          * update those values to ones applicable for coded PHY.
          */
-        if (connsm->remote_features[0] & (BLE_LL_FEAT_LE_CODED_PHY >> 8)) {
+        if (ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_LE_CODED_PHY)) {
             if (connsm->host_req_max_tx_time) {
                 connsm->max_tx_time = max(connsm->max_tx_time,
                                           connsm->host_req_max_tx_time);
diff --git a/nimble/controller/src/ble_ll_sync.c b/nimble/controller/src/ble_ll_sync.c
index 0adc67b7..72da2d97 100644
--- a/nimble/controller/src/ble_ll_sync.c
+++ b/nimble/controller/src/ble_ll_sync.c
@@ -2183,12 +2183,8 @@ ble_ll_sync_transfer(const uint8_t *cmdbuf, uint8_t len,
         goto done;
     }
 
-    /* TODO should not need to shift
-     * byte 3 (0 byte is conn_feature) , bit 1
-     *
-     * Allow initiate LL procedure only if remote supports it.
-     */
-    if (!(connsm->remote_features[2] & (BLE_LL_FEAT_SYNC_TRANS_RECV >> (8 * 3)))) {
+     /* Allow initiate LL procedure only if remote supports it. */
+    if (!ble_ll_conn_rem_feature_check(connsm, BLE_LL_FEAT_SYNC_TRANS_RECV)) {
         rc = BLE_ERR_UNSUPP_REM_FEATURE;
         goto done;
     }