From 5e360b81c2ff50e150e7f2104e963b434f1869da Mon Sep 17 00:00:00 2001 From: Andries Kruithof Date: Thu, 27 Oct 2022 14:17:02 +0200 Subject: [PATCH] Bluetooth: controller: llcp: update rx parameters upon receipt of PDU According to specification, when executing a remote procedure the rx parameters used need to be updated when the REQ from the peer is received. Currently this is done only when an ACK is received on the RSP PDU send out. This commit updates the RX parameters upon receipt of the REQ PDU Signed-off-by: Andries Kruithof --- subsys/bluetooth/controller/ll_sw/ull_conn.c | 65 ++++++++++++++----- .../controller/ll_sw/ull_conn_internal.h | 2 + .../controller/ll_sw/ull_llcp_common.c | 6 +- .../controller/ll_sw/ull_llcp_internal.h | 6 ++ 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index 234a0e4445a..0f6f5c18d75 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -8141,40 +8141,75 @@ void ull_dle_max_time_get(struct ll_conn *conn, uint16_t *max_rx_time, return dle_max_time_get(conn, max_rx_time, max_tx_time); } +/* + * TODO: this probably can be optimised for ex. by creating a macro for the + * ull_dle_update_eff function + */ uint8_t ull_dle_update_eff(struct ll_conn *conn) { uint8_t dle_changed = 0U; - const uint16_t eff_tx_octets = - MAX(MIN(conn->lll.dle.local.max_tx_octets, conn->lll.dle.remote.max_rx_octets), - PDU_DC_PAYLOAD_SIZE_MIN); + /* Note that we must use bitwise or and not logical or */ + dle_changed = ull_dle_update_eff_rx(conn); + dle_changed |= ull_dle_update_eff_tx(conn); + + return dle_changed; +} + +uint8_t ull_dle_update_eff_rx(struct ll_conn *conn) +{ + uint8_t dle_changed = 0U; + const uint16_t eff_rx_octets = MAX(MIN(conn->lll.dle.local.max_rx_octets, conn->lll.dle.remote.max_tx_octets), PDU_DC_PAYLOAD_SIZE_MIN); #if defined(CONFIG_BT_CTLR_PHY) - unsigned int min_eff_tx_time = (conn->lll.phy_tx == PHY_CODED) ? - PDU_DC_PAYLOAD_TIME_MIN_CODED : PDU_DC_PAYLOAD_TIME_MIN; unsigned int min_eff_rx_time = (conn->lll.phy_rx == PHY_CODED) ? PDU_DC_PAYLOAD_TIME_MIN_CODED : PDU_DC_PAYLOAD_TIME_MIN; - const uint16_t eff_tx_time = - MAX(MIN(conn->lll.dle.local.max_tx_time, conn->lll.dle.remote.max_rx_time), - min_eff_tx_time); const uint16_t eff_rx_time = MAX(MIN(conn->lll.dle.local.max_rx_time, conn->lll.dle.remote.max_tx_time), min_eff_rx_time); - if (eff_tx_time != conn->lll.dle.eff.max_tx_time) { - conn->lll.dle.eff.max_tx_time = eff_tx_time; - dle_changed = 1; - } if (eff_rx_time != conn->lll.dle.eff.max_rx_time) { conn->lll.dle.eff.max_rx_time = eff_rx_time; dle_changed = 1; } #else conn->lll.dle.eff.max_rx_time = PDU_DC_MAX_US(eff_rx_octets, PHY_1M); +#endif + + if (eff_rx_octets != conn->lll.dle.eff.max_rx_octets) { + conn->lll.dle.eff.max_rx_octets = eff_rx_octets; + dle_changed = 1; + } + + return dle_changed; +} + +uint8_t ull_dle_update_eff_tx(struct ll_conn *conn) + +{ + uint8_t dle_changed = 0U; + + const uint16_t eff_tx_octets = + MAX(MIN(conn->lll.dle.local.max_tx_octets, conn->lll.dle.remote.max_rx_octets), + PDU_DC_PAYLOAD_SIZE_MIN); + +#if defined(CONFIG_BT_CTLR_PHY) + unsigned int min_eff_tx_time = (conn->lll.phy_tx == PHY_CODED) ? + PDU_DC_PAYLOAD_TIME_MIN_CODED : PDU_DC_PAYLOAD_TIME_MIN; + + const uint16_t eff_tx_time = + MAX(MIN(conn->lll.dle.local.max_tx_time, conn->lll.dle.remote.max_rx_time), + min_eff_tx_time); + + if (eff_tx_time != conn->lll.dle.eff.max_tx_time) { + conn->lll.dle.eff.max_tx_time = eff_tx_time; + dle_changed = 1; + } +#else conn->lll.dle.eff.max_tx_time = PDU_DC_MAX_US(eff_tx_octets, PHY_1M); #endif @@ -8182,10 +8217,6 @@ uint8_t ull_dle_update_eff(struct ll_conn *conn) conn->lll.dle.eff.max_tx_octets = eff_tx_octets; dle_changed = 1; } - if (eff_rx_octets != conn->lll.dle.eff.max_rx_octets) { - conn->lll.dle.eff.max_rx_octets = eff_rx_octets; - dle_changed = 1; - } return dle_changed; } @@ -8258,7 +8289,7 @@ void ull_dle_init(struct ll_conn *conn, uint8_t phy) * Part B, section 4.5.10 we can call ull_dle_update_eff * for initialisation */ - ull_dle_update_eff(conn); + (void)ull_dle_update_eff(conn); /* Check whether the controller should perform a data length update after * connection is established diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn_internal.h b/subsys/bluetooth/controller/ll_sw/ull_conn_internal.h index 887ec6b129d..1a4cd92ca43 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_conn_internal.h @@ -97,6 +97,8 @@ void ull_dle_max_time_get(struct ll_conn *conn, uint16_t *max_rx_time, uint16_t *max_tx_time); uint8_t ull_dle_update_eff(struct ll_conn *conn); +uint8_t ull_dle_update_eff_tx(struct ll_conn *conn); +uint8_t ull_dle_update_eff_rx(struct ll_conn *conn); void ull_dle_local_tx_update(struct ll_conn *conn, uint16_t tx_octets, uint16_t tx_time); diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index ba399d74273..fd05c214038 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -942,11 +942,12 @@ static void rp_comm_rx_decode(struct ll_conn *conn, struct proc_ctx *ctx, struct case PDU_DATA_LLCTRL_TYPE_LENGTH_REQ: llcp_pdu_decode_length_req(conn, pdu); /* On reception of REQ mark RSP open for local piggy-back - * Pause data tx, to ensure we can later (on RSP tx ack) update DLE without + * Pause data tx, to ensure we can later (on RSP tx ack) update TX DLE without * conflicting with out-going LL Data PDUs * See BT Core 5.2 Vol6: B-4.5.10 & B-5.1.9 */ llcp_tx_pause_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH); + ctx->data.dle.ntf_dle = ull_dle_update_eff_rx(conn); break; #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) @@ -1276,8 +1277,9 @@ static void rp_comm_st_wait_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, u #if defined(CONFIG_BT_CTLR_DATA_LENGTH) case PROC_DATA_LENGTH_UPDATE: { /* Apply changes in data lengths/times */ - uint8_t dle_changed = ull_dle_update_eff(conn); + uint8_t dle_changed = ull_dle_update_eff_tx(conn); + dle_changed |= ctx->data.dle.ntf_dle; llcp_tx_resume_data(conn, LLCP_TX_QUEUE_PAUSE_DATA_DATA_LENGTH); if (dle_changed && !llcp_ntf_alloc_is_available()) { diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h index 707d7b7f0ac..1ec7c70432e 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h @@ -197,6 +197,12 @@ struct proc_ctx { } pu; #endif /* CONFIG_BT_CTLR_PHY */ +#if defined(CONFIG_BT_CTLR_DATA_LENGTH) + struct { + uint8_t ntf_dle; + } dle; +#endif + /* Connection Update & Connection Parameter Request */ struct { uint8_t error;