[GitHub] [mynewt-core] mlaz commented on a change in pull request #1783: Modified the nRF5x timer and system init code to use the nRF HFXO manager

[GitBox <gi...@apache.org>]

mlaz commented on a change in pull request #1783: Modified the nRF5x timer and system init code to use the nRF HFXO manager
URL: https://github.com/apache/mynewt-core/pull/1783#discussion_r308009947
 
 

 ##########
 File path: hw/mcu/nordic/nrf51xxx/src/nrf51_clock.c
 ##########
 @@ -39,7 +39,11 @@ nrf51_clock_hfxo_request(void)
     __HAL_DISABLE_INTERRUPTS(ctx);
     assert(nrf51_clock_hfxo_refcnt < 0xff);
     if (nrf51_clock_hfxo_refcnt == 0) {
+        /* Make sure that the HFXO has not previously been started manually */
+        assert(NRF_CLOCK->EVENTS_HFCLKSTARTED == 0);
 
 Review comment:
   According to the  [documentation](https://infocenter.nordicsemi.com/index.jsp?topic=%2Fcom.nordic.infocenter.nrf52832.ps.v1.1%2Fperipheral_interface.html&cp=3_1_0_14&anchor=concept_twz_bpz_vr), EVENTS_HFCLKSTARTED is an event register thus it signals a state change and not a state, this means its value won't change back to zero when the clock is stopped. Documentation also states that after an event(in this case HFCLK stating) this register's value won't be changed by the peripheral until the firmware sets it to 0. So, to stay safe I believe we should be testing HFCLKSTAT, which is a state register, similarly to how we do it on the original implementation.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services