You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2022/09/09 02:07:37 UTC

[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #1354: nimble/phy: Introduce common phy for nRF52 and nRF53

apache-mynewt-bot commented on PR #1354:
URL: https://github.com/apache/mynewt-nimble/pull/1354#issuecomment-1241416793

   
   <!-- style-bot -->
   
   ## Style check summary
   
   ### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
   #### nimble/drivers/nrf5x/src/ble_hw.c
   <details>
   
   ```diff
   @@ -77,15 +77,15 @@
    
    #if MYNEWT_VAL(BLE_PHY_UBLOX_BMD345_PUBLIC_ADDR)
        /*
   -    * The BMD-345 modules are preprogrammed from the factory with a unique public
   -    * The  Bluetooth device address stored in the CUSTOMER[0] and CUSTOMER[1]
   -    * registers of the User Information Configuration Registers (UICR).
   -    * The Bluetooth device address consists of the IEEE Organizationally Unique
   -    * Identifier (OUI) combined with the hexadecimal digits that are printed on
   -    * a 2D barcode and in human-readable text on the module label.The Bluetooth
   -    * device address is stored in little endian format. The most significant
   -    * bytes of the CUSTOMER[1] register are 0xFF to complete the 32-bit register.
   -    */
   +     * The BMD-345 modules are preprogrammed from the factory with a unique public
   +     * The  Bluetooth device address stored in the CUSTOMER[0] and CUSTOMER[1]
   +     * registers of the User Information Configuration Registers (UICR).
   +     * The Bluetooth device address consists of the IEEE Organizationally Unique
   +     * Identifier (OUI) combined with the hexadecimal digits that are printed on
   +     * a 2D barcode and in human-readable text on the module label.The Bluetooth
   +     * device address is stored in little endian format. The most significant
   +     * bytes of the CUSTOMER[1] register are 0xFF to complete the 32-bit register.
   +     */
    
        /* Copy into device address. We can do this because we know platform */
        addr_low = NRF_UICR->CUSTOMER[0];
   ```
   
   </details>
   
   #### nimble/drivers/nrf5x/src/ble_phy.c
   <details>
   
   ```diff
   @@ -110,23 +110,22 @@
    
    /* NRF_RADIO->PCNF0 configuration values */
    #define NRF_PCNF0               (NRF_LFLEN_BITS << RADIO_PCNF0_LFLEN_Pos) | \
   -                                (RADIO_PCNF0_S1INCL_Msk) | \
   -                                (NRF_S0LEN << RADIO_PCNF0_S0LEN_Pos) | \
   -                                (NRF_S1LEN_BITS << RADIO_PCNF0_S1LEN_Pos)
   +    (RADIO_PCNF0_S1INCL_Msk) | \
   +    (NRF_S0LEN << RADIO_PCNF0_S0LEN_Pos) | \
   +    (NRF_S1LEN_BITS << RADIO_PCNF0_S1LEN_Pos)
    #define NRF_PCNF0_1M            (NRF_PCNF0) | \
   -                                (RADIO_PCNF0_PLEN_8bit << RADIO_PCNF0_PLEN_Pos)
   +    (RADIO_PCNF0_PLEN_8bit << RADIO_PCNF0_PLEN_Pos)
    #define NRF_PCNF0_2M            (NRF_PCNF0) | \
   -                                (RADIO_PCNF0_PLEN_16bit << RADIO_PCNF0_PLEN_Pos)
   +    (RADIO_PCNF0_PLEN_16bit << RADIO_PCNF0_PLEN_Pos)
    #define NRF_PCNF0_CODED         (NRF_PCNF0) | \
   -                                (RADIO_PCNF0_PLEN_LongRange << RADIO_PCNF0_PLEN_Pos) | \
   -                                (NRF_CILEN_BITS << RADIO_PCNF0_CILEN_Pos) | \
   -                                (NRF_TERMLEN_BITS << RADIO_PCNF0_TERMLEN_Pos)
   +    (RADIO_PCNF0_PLEN_LongRange << RADIO_PCNF0_PLEN_Pos) | \
   +    (NRF_CILEN_BITS << RADIO_PCNF0_CILEN_Pos) | \
   +    (NRF_TERMLEN_BITS << RADIO_PCNF0_TERMLEN_Pos)
    
    /* BLE PHY data structure */
   -struct ble_phy_obj
   -{
   +struct ble_phy_obj {
        uint8_t phy_stats_initialized;
   -    int8_t  phy_txpwr_dbm;
   +    int8_t phy_txpwr_dbm;
        uint8_t phy_chan;
        uint8_t phy_state;
        uint8_t phy_transition;
   @@ -139,7 +138,7 @@
        uint8_t phy_tx_phy_mode;
        uint8_t phy_rx_phy_mode;
        uint8_t phy_bcc_offset;
   -    int8_t  rx_pwr_compensation;
   +    int8_t rx_pwr_compensation;
        uint32_t phy_aar_scratch;
        uint32_t phy_access_address;
        struct ble_mbuf_hdr rxhdr;
   @@ -164,7 +163,7 @@
    
    /* RF center frequency for each channel index (offset from 2400 MHz) */
    static const uint8_t g_ble_phy_chan_freq[BLE_PHY_NUM_CHANS] = {
   -     4,  6,  8, 10, 12, 14, 16, 18, 20, 22, /* 0-9 */
   +    4,  6,  8, 10, 12, 14, 16, 18, 20, 22,  /* 0-9 */
        24, 28, 30, 32, 34, 36, 38, 40, 42, 44, /* 10-19 */
        46, 48, 50, 52, 54, 56, 58, 60, 62, 64, /* 20-29 */
        66, 68, 70, 72, 74, 76, 78,  2, 26, 80, /* 30-39 */
   @@ -239,36 +238,36 @@
    
    /* Statistics */
    STATS_SECT_START(ble_phy_stats)
   -    STATS_SECT_ENTRY(phy_isrs)
   -    STATS_SECT_ENTRY(tx_good)
   -    STATS_SECT_ENTRY(tx_fail)
   -    STATS_SECT_ENTRY(tx_late)
   -    STATS_SECT_ENTRY(tx_bytes)
   -    STATS_SECT_ENTRY(rx_starts)
   -    STATS_SECT_ENTRY(rx_aborts)
   -    STATS_SECT_ENTRY(rx_valid)
   -    STATS_SECT_ENTRY(rx_crc_err)
   -    STATS_SECT_ENTRY(rx_late)
   -    STATS_SECT_ENTRY(radio_state_errs)
   -    STATS_SECT_ENTRY(rx_hw_err)
   -    STATS_SECT_ENTRY(tx_hw_err)
   +STATS_SECT_ENTRY(phy_isrs)
   +STATS_SECT_ENTRY(tx_good)
   +STATS_SECT_ENTRY(tx_fail)
   +STATS_SECT_ENTRY(tx_late)
   +STATS_SECT_ENTRY(tx_bytes)
   +STATS_SECT_ENTRY(rx_starts)
   +STATS_SECT_ENTRY(rx_aborts)
   +STATS_SECT_ENTRY(rx_valid)
   +STATS_SECT_ENTRY(rx_crc_err)
   +STATS_SECT_ENTRY(rx_late)
   +STATS_SECT_ENTRY(radio_state_errs)
   +STATS_SECT_ENTRY(rx_hw_err)
   +STATS_SECT_ENTRY(tx_hw_err)
    STATS_SECT_END
    STATS_SECT_DECL(ble_phy_stats) ble_phy_stats;
    
    STATS_NAME_START(ble_phy_stats)
   -    STATS_NAME(ble_phy_stats, phy_isrs)
   -    STATS_NAME(ble_phy_stats, tx_good)
   -    STATS_NAME(ble_phy_stats, tx_fail)
   -    STATS_NAME(ble_phy_stats, tx_late)
   -    STATS_NAME(ble_phy_stats, tx_bytes)
   -    STATS_NAME(ble_phy_stats, rx_starts)
   -    STATS_NAME(ble_phy_stats, rx_aborts)
   -    STATS_NAME(ble_phy_stats, rx_valid)
   -    STATS_NAME(ble_phy_stats, rx_crc_err)
   -    STATS_NAME(ble_phy_stats, rx_late)
   -    STATS_NAME(ble_phy_stats, radio_state_errs)
   -    STATS_NAME(ble_phy_stats, rx_hw_err)
   -    STATS_NAME(ble_phy_stats, tx_hw_err)
   +STATS_NAME(ble_phy_stats, phy_isrs)
   +STATS_NAME(ble_phy_stats, tx_good)
   +STATS_NAME(ble_phy_stats, tx_fail)
   +STATS_NAME(ble_phy_stats, tx_late)
   +STATS_NAME(ble_phy_stats, tx_bytes)
   +STATS_NAME(ble_phy_stats, rx_starts)
   +STATS_NAME(ble_phy_stats, rx_aborts)
   +STATS_NAME(ble_phy_stats, rx_valid)
   +STATS_NAME(ble_phy_stats, rx_crc_err)
   +STATS_NAME(ble_phy_stats, rx_late)
   +STATS_NAME(ble_phy_stats, radio_state_errs)
   +STATS_NAME(ble_phy_stats, rx_hw_err)
   +STATS_NAME(ble_phy_stats, tx_hw_err)
    STATS_NAME_END(ble_phy_stats)
    
    /*
   @@ -309,13 +308,12 @@
     * space for 267 bytes of scratch. I used 268 bytes since not sure if this
     * needs to be aligned and burning a byte is no big deal.
     */
   -//#define NRF_ENC_SCRATCH_WORDS (((MYNEWT_VAL(BLE_LL_MAX_PKT_SIZE) + 16) + 3) / 4)
   +/*#define NRF_ENC_SCRATCH_WORDS (((MYNEWT_VAL(BLE_LL_MAX_PKT_SIZE) + 16) + 3) / 4) */
    #define NRF_ENC_SCRATCH_WORDS   (67)
    
    uint32_t g_nrf_encrypt_scratchpad[NRF_ENC_SCRATCH_WORDS];
    
   -struct nrf_ccm_data
   -{
   +struct nrf_ccm_data {
        uint8_t key[16];
        uint64_t pkt_counter;
        uint8_t dir_bit;
   @@ -434,16 +432,16 @@
    {
    #if (BLE_LL_BT5_PHY_SUPPORTED == 1)
        switch (g_ble_phy_data.phy_cur_phy_mode) {
   -        case BLE_PHY_MODE_1M:
   -            return BLE_PHY_1M;
   -        case BLE_PHY_MODE_2M:
   -            return BLE_PHY_2M;
   -        case BLE_PHY_MODE_CODED_125KBPS:
   -        case BLE_PHY_MODE_CODED_500KBPS:
   -            return BLE_PHY_CODED;
   -        default:
   -            assert(0);
   -            return -1;
   +    case BLE_PHY_MODE_1M:
   +        return BLE_PHY_1M;
   +    case BLE_PHY_MODE_2M:
   +        return BLE_PHY_2M;
   +    case BLE_PHY_MODE_CODED_125KBPS:
   +    case BLE_PHY_MODE_CODED_500KBPS:
   +        return BLE_PHY_CODED;
   +    default:
   +        assert(0);
   +        return -1;
        }
    #else
        return BLE_PHY_1M;
   @@ -515,10 +513,10 @@
                              "   adds %[src], %[src], r4   \n"
                              "   adds %[dst], %[dst], r4   \n"
                              : [dst] "+r" (dst), [src] "+r" (src),
   -                            [len] "+r" (copy_len)
   +                          [len] "+r" (copy_len)
                              :
                              : "r3", "r4", "memory"
   -                         );
   +                          );
    #endif
    
            if ((rem_len < 4) && (block_rem_len >= rem_len)) {
   @@ -545,7 +543,7 @@
                          : [len] "+r" (rem_len)
                          : [dst] "r" (dst), [src] "r" (src)
                          : "r3", "memory"
   -                     );
   +                      );
    #endif
    
        /* Copy header */
   @@ -856,7 +854,7 @@
            NRF_CCM->OUTPTR = (uint32_t)dptr;
            NRF_CCM->SCRATCHPTR = (uint32_t)&g_nrf_encrypt_scratchpad[0];
            NRF_CCM->MODE = CCM_MODE_LENGTH_Msk | CCM_MODE_MODE_Decryption |
   -                                                    ble_phy_get_ccm_datarate();
   +                        ble_phy_get_ccm_datarate();
            NRF_CCM->CNFPTR = (uint32_t)&g_nrf_ccm_data;
            NRF_CCM->SHORTS = 0;
            NRF_CCM->EVENTS_ERROR = 0;
   @@ -899,7 +897,7 @@
         * to take this into account when setting up BCC.
         */
        if (g_ble_phy_data.phy_cur_phy_mode == BLE_PHY_MODE_CODED_125KBPS ||
   -            g_ble_phy_data.phy_cur_phy_mode == BLE_PHY_MODE_CODED_500KBPS) {
   +        g_ble_phy_data.phy_cur_phy_mode == BLE_PHY_MODE_CODED_500KBPS) {
            g_ble_phy_data.phy_bcc_offset = 5;
        } else {
            g_ble_phy_data.phy_bcc_offset = 0;
   @@ -1041,10 +1039,10 @@
         * using CI field.
         */
        if ((phy == BLE_PHY_MODE_CODED_125KBPS) ||
   -                                    (phy == BLE_PHY_MODE_CODED_500KBPS)) {
   +        (phy == BLE_PHY_MODE_CODED_500KBPS)) {
            phy = NRF_RADIO->PDUSTAT & RADIO_PDUSTAT_CISTAT_Msk ?
   -                                   BLE_PHY_MODE_CODED_500KBPS :
   -                                   BLE_PHY_MODE_CODED_125KBPS;
   +              BLE_PHY_MODE_CODED_500KBPS :
   +              BLE_PHY_MODE_CODED_125KBPS;
        }
    #endif
    
   @@ -1083,7 +1081,8 @@
            if (g_ble_phy_data.phy_encrypted) {
                while (NRF_CCM->EVENTS_ENDCRYPT == 0) {
                    /* Make sure CCM finished */
   -            };
   +            }
   +            ;
    
                /* Only set MIC failure flag if frame is not zero length */
                if ((dptr[1] != 0) && (NRF_CCM->MICSTATUS == 0)) {
   @@ -1275,7 +1274,7 @@
            NRF_RADIO->EVENTS_BCMATCH = 0;
            phy_ppi_radio_bcmatch_to_aar_start_enable();
            nrf_radio_bcc_set(NRF_RADIO, (BLE_LL_PDU_HDR_LEN + adva_offset +
   -            BLE_DEV_ADDR_LEN) * 8 + g_ble_phy_data.phy_bcc_offset);
   +                                      BLE_DEV_ADDR_LEN) * 8 + g_ble_phy_data.phy_bcc_offset);
        }
    #endif
    
   @@ -1411,7 +1410,7 @@
    
    #ifdef NRF53_SERIES
        *(volatile uint32_t *)(NRF_RADIO_NS_BASE + 0x774) =
   -        (*(volatile uint32_t* )(NRF_RADIO_NS_BASE + 0x774) & 0xfffffffe) | 0x01000000;
   +        (*(volatile uint32_t * )(NRF_RADIO_NS_BASE + 0x774) & 0xfffffffe) | 0x01000000;
    #if NRF53_ERRATA_16_ENABLE_WORKAROUND
        if (nrf53_errata_16()) {
            /* [16] RADIO: POWER register is not functional */
   @@ -1437,11 +1436,11 @@
    
        /* Enable radio fast ramp-up */
        NRF_RADIO->MODECNF0 |= (RADIO_MODECNF0_RU_Fast << RADIO_MODECNF0_RU_Pos) &
   -                            RADIO_MODECNF0_RU_Msk;
   +                           RADIO_MODECNF0_RU_Msk;
    
        /* Set logical address 1 for TX and RX */
   -    NRF_RADIO->TXADDRESS  = 0;
   -    NRF_RADIO->RXADDRESSES  = (1 << 0);
   +    NRF_RADIO->TXADDRESS = 0;
   +    NRF_RADIO->RXADDRESSES = (1 << 0);
    
        /* Configure the CRC registers */
        NRF_RADIO->CRCCNF = (RADIO_CRCCNF_SKIPADDR_Skip << RADIO_CRCCNF_SKIPADDR_Pos) | RADIO_CRCCNF_LEN_Three;
   @@ -1505,7 +1504,7 @@
                                    "ble_phy");
            assert(rc == 0);
    
   -        g_ble_phy_data.phy_stats_initialized  = 1;
   +        g_ble_phy_data.phy_stats_initialized = 1;
        }
    
        return 0;
   @@ -1531,7 +1530,7 @@
         */
        nrf_wait_disabled();
        if ((NRF_RADIO->STATE != RADIO_STATE_STATE_Disabled) &&
   -            ((NRF_RADIO->STATE & 0x07) != RADIO_STATE_STATE_RxIdle)) {
   +        ((NRF_RADIO->STATE & 0x07) != RADIO_STATE_STATE_RxIdle)) {
            ble_phy_disable();
            STATS_INC(ble_phy_stats, radio_state_errs);
            return BLE_PHY_ERR_RADIO_STATE;
   @@ -2009,7 +2008,8 @@
    }
    
    /* Gets the current access address */
   -uint32_t ble_phy_access_addr_get(void)
   +uint32_t
   +ble_phy_access_addr_get(void)
    {
        return g_ble_phy_data.phy_access_address;
    }
   @@ -2078,7 +2078,8 @@
    #endif
    
    #if MYNEWT_VAL(BLE_LL_DTM)
   -void ble_phy_enable_dtm(void)
   +void
   +ble_phy_enable_dtm(void)
    {
        /* When DTM is enabled we need to disable whitening as per
         * Bluetooth v5.0 Vol 6. Part F. 4.1.1
   @@ -2086,7 +2087,8 @@
        NRF_RADIO->PCNF1 &= ~RADIO_PCNF1_WHITEEN_Msk;
    }
    
   -void ble_phy_disable_dtm(void)
   +void
   +ble_phy_disable_dtm(void)
    {
        /* Enable whitening */
        NRF_RADIO->PCNF1 |= RADIO_PCNF1_WHITEEN_Msk;
   ```
   
   </details>
   
   #### nimble/drivers/nrf5x/src/ble_phy_trace.c
   <details>
   
   ```diff
   @@ -38,7 +38,7 @@
    ble_phy_trace_init(void)
    {
        ble_phy_trace_off =
   -            os_trace_module_register(&g_ble_phy_trace_mod, "ble_phy", 3,
   +        os_trace_module_register(&g_ble_phy_trace_mod, "ble_phy", 3,
                                         ble_phy_trace_module_send_desc);
    }
    #endif
   ```
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org