andrzej-kaczmarek commented on code in PR #3321: URL: https://github.com/apache/mynewt-core/pull/3321#discussion_r1810924624
########## hw/mcu/dialog/da1469x/src/da1469x_lpclk.c: ########## @@ -38,6 +38,7 @@ static void da1469x_lpclk_settle_tmr_cb(void *arg) { da1469x_clock_lp_xtal32k_switch(); + da1469x_clock_lp_xtal32k_calibrate(); Review Comment: Calibration of XTAL32K should be optional and disabled by default - we should assume after settling it's accurate and has its native frequency. ########## hw/mcu/dialog/da1469x/src/da1469x_clock.c: ########## @@ -342,29 +385,36 @@ da1469x_clock_calibrate(uint8_t clock_sel, uint16_t ref_cnt) assert(!(ANAMISC_BIF->CLK_REF_SEL_REG & ANAMISC_BIF_CLK_REF_SEL_REG_REF_CAL_START_Msk)); ANAMISC_BIF->CLK_REF_CNT_REG = ref_cnt; - ANAMISC_BIF->CLK_REF_SEL_REG = (clock_sel << ANAMISC_BIF_CLK_REF_SEL_REG_REF_CLK_SEL_Pos) | - ANAMISC_BIF_CLK_REF_SEL_REG_REF_CAL_START_Msk; + /* Select reference clock & calibrated clock */ + ANAMISC_BIF->CLK_REF_SEL_REG = + (DA1469X_REF_SEL << ANAMISC_BIF_CLK_REF_SEL_REG_CAL_CLK_SEL_Pos) | + (clock_sel << ANAMISC_BIF_CLK_REF_SEL_REG_REF_CLK_SEL_Pos); + + /* Start measurement */ + ANAMISC_BIF->CLK_REF_SEL_REG |= ANAMISC_BIF_CLK_REF_SEL_REG_REF_CAL_START_Msk; + + /* Wait for meaurement to complete */ while (ANAMISC_BIF->CLK_REF_SEL_REG & ANAMISC_BIF_CLK_REF_SEL_REG_REF_CAL_START_Msk); ref_val = ANAMISC_BIF->CLK_REF_VAL_REG; da1469x_pd_release(MCU_PD_DOMAIN_PER); - return 32000000 * ref_cnt / ref_val; + return CAL_REF_FREQ * ref_cnt / ref_val; } void da1469x_clock_sys_rc32m_calibrate(void) { - g_mcu_clock_rc32m_freq = da1469x_clock_calibrate(1, 100); + g_mcu_clock_rc32m_freq = da1469x_clock_calibrate(DA1469X_CALIB_RC32M, RC32M_CAL_REF_CNT); Review Comment: ref cnt should be configurable via syscfg for consistency ########## hw/mcu/dialog/da1469x/src/da1469x_clock.c: ########## @@ -534,3 +534,27 @@ da1469x_clock_lp_rcx_freq_get(void) return g_mcu_clock_rcx_freq; } + +uint32_t +da1469x_clock_lp_freq_get(void) +{ +#if MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, RCX) + return da1469x_clock_lp_rcx_freq_get(); +#elif MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, RC32K) + return da1469x_clock_lp_rc32k_freq_get(); +#elif MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, XTAL32K) + return da1469x_clock_lp_xtal32k_freq_get(); +#endif +} + +void +da1469x_clock_lp_calibrate(void) +{ +#if MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, RCX) + da1469x_clock_lp_rcx_calibrate(); +#elif MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, RC32K) + da1469x_clock_lp_rc32k_calibrate(); +#elif MYNEWT_VAL_CHOICE(MCU_LPCLK_SOURCE, XTAL32K) + da1469x_clock_lp_xtal32k_calibrate(); Review Comment: as in other commit, this should be optional ########## hw/mcu/dialog/da1469x/src/da1469x_clock.c: ########## @@ -305,11 +355,18 @@ da1469x_clock_calibrate(uint8_t clock_sel, uint16_t ref_cnt) } void -da1469x_clock_lp_rc32m_calibrate(void) +da1469x_clock_sys_rc32m_calibrate(void) { g_mcu_clock_rc32m_freq = da1469x_clock_calibrate(1, 100); } +void +da1469x_clock_lp_xtal32k_calibrate(void) +{ + g_mcu_clock_xtal32k_freq = + da1469x_clock_calibrate(2, MYNEWT_VAL(MCU_CLOCK_RCX_CAL_REF_CNT)); Review Comment: this should have separate syscfg ########## hw/hal/include/hal/hal_os_tick.h: ########## Review Comment: This code should be optional as it only applies to RCX. For devices which use XTAL32K and e.g. 128 OS ticks per seconds there's really no need to have extra overhead for ticks calculation. -- 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: notifications-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org