From c905dcfcca72402d502f8e92a573f0010afb8e06 Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Wed, 5 Feb 2020 18:29:21 +0100 Subject: [PATCH] nimble/ll: Fix PHY change on slave request This fixes PHY switch in case slave tries to change from asymmetric to symmetric PHY and the target PHY is one of currently used PHYs. The issue is because we first calculate PHY change in both directions and only then check if slave requested symmetric PHY. However, since either m_to_s or s_to_m are already set to 0 (due to no change required), the subseqent checks assume we should not change PHY in either direction. The current behavior is technically correct, since master is always allowed to leave PHY in both directions unchanged, but it's just dumb since we should make a change assuming our PHY preference allows it. To fix this we basically skip PHY calculation if we determined that slave requested symmetric PHY and we can apply it in both directions. If requested symmetric PHY cannot be applied in at least one direction, we leave both directions unchanged. If there was no request for symmetric PHY from slave, we calculated PHYs as usual. --- nimble/controller/src/ble_ll_ctrl.c | 39 ++++++++++++++++++----------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/nimble/controller/src/ble_ll_ctrl.c b/nimble/controller/src/ble_ll_ctrl.c index d8e8a5843..ea2ba8349 100644 --- a/nimble/controller/src/ble_ll_ctrl.c +++ b/nimble/controller/src/ble_ll_ctrl.c @@ -738,29 +738,38 @@ ble_ll_ctrl_phy_update_ind_make(struct ble_ll_conn_sm *connsm, uint8_t *dptr, s_to_m = connsm->phy_data.req_pref_rx_phys_mask & tx_phys; } - /* Find new phys. If not different than current, set to 0 */ + if (is_slave_sym) { + /* + * If either s_to_m or m_to_s is 0, it means for at least one direction + * requested PHY is not our preferred one so make sure we keep current + * PHY in both directions + * + * Core 5.2, Vol 6, PartB, 5.1.10 + * If the slave specified a single PHY in both the TX_PHYS and + * RX_PHYS fields and both fields are the same, the master shall + * either select the PHY specified by the slave for both directions + * or shall leave both directions unchanged. + */ + if ((s_to_m == 0) || (m_to_s == 0)) { + s_to_m = 0; + m_to_s = 0; + } else { + BLE_LL_ASSERT(s_to_m == m_to_s); + } + } + + /* Calculate new PHYs to use */ m_to_s = ble_ll_ctrl_find_new_phy(m_to_s); + s_to_m = ble_ll_ctrl_find_new_phy(s_to_m); + + /* Make sure we do not indicate PHY change if the same as current one */ if (m_to_s == connsm->phy_data.cur_tx_phy) { m_to_s = 0; } - - s_to_m = ble_ll_ctrl_find_new_phy(s_to_m); if (s_to_m == connsm->phy_data.cur_rx_phy) { s_to_m = 0; } - /* - * Core 5.0, Vol 6, PartB, 5.1.10 - * If the slave specified a single PHY in both the TX_PHYS and RX_PHYS - * fields and both fields are the same, the master shall either select - * the PHY specified by the slave for both directions or shall leave - * both directions unchanged. - */ - if (is_slave_sym && (s_to_m != m_to_s)) { - s_to_m = 0; - m_to_s = 0; - } - /* At this point, m_to_s and s_to_m are not masks; they are numeric */ /*