NRF_51_DK - adapt lp/us ticker drivers to the new standards

[mprse]

Description

1. us ticker driver for NRF_51_DK board:
   According to new ticker standards the following requirements for us ticker are not met on NRF_5xxx boards:

• has a frequency between 250KHz and 8MHz (currently is driven by 32kHz clock)
• ticker increments by 1 each tick (currently is scaled to 1 MHz by incrementing counter by ~31)

Since BLE softdevice uses TIMER0 the proposition is to use high speed TIMER1 for us ticker configured as follows:

• TIMER counter width: 16 bits (max)
• TIMER frequency: 1MHz
  This solution also uses Timer's capture/compare register 0 to specify interrupt time and Timer's capture/compare register 1 to read current timer value.

2. sleep/deep-sleep driver for NRF_5xxx boards: According to new sleep mode standards the following requirement is not met on NRF_5xxx boards: - High-speed clocks are turned off in deep-sleep mode. Additionally when ticker resolution is 31 us, then it is hard to verify that wake-up time does not exceeds 10 us (fix for us ticker solve this problem).

The proposition is to stop timer used by us ticker before going to deep-sleep and enable it after wake-up. This operation should result in disabling Timer clock sources in deep-sleep mode.

According to documentation:
If the system does not require any of the clocks provided by the HFCLK clock controller, the HFCLK
controller may enter a power saving mode automatically and switch off the selected clock source. This
occurs if all peripherals that require either PCLK1M, PCLK16M are appropriately stopped or disabled, and
the CPU is sleeping and thereby no longer requesting HCLK.

3. lp ticker driver for NRF51_DK board:

• According to NRF51_DK reference manual rtc interrupt cannot be controlled by rtc event. In the previous implementation interrupts were enabled permanently and specific interrupt was enabled/disabled by enabling/disabling the specific event. If event is enabled, then event signal is provided to Programmable Peripheral Interconnect (PPI). If interrupt is enabled, then interrupt signal is provided to Nested Vector Interrupt Controller (NVIC). Disable all events permanently. Enable lp ticker overflow interrupt permanently(needed for RTC), disable lp ticker capture/compare interrupt on init (lp_ticker_init) , enable lp ticker interrupt when lp ticker interrupt is set (lp_ticker_set_interrupt), disable lp ticker interrupt on disable request(lp_ticker_disable_interrupt).
• Provide lp ticker data for higher level (freq: 32kHz / len: 24 bits),
• Add the following features to init routine: disable lp ticker interrupt.
• Make ticker driver to operate on ticks instead of us.
• Simplify lp ticker read and set interrupt routines (upper layers handles conversion to us and interrupt scheduling).

Status

IN DEVELOPMENT

Migrations

YES

API is not changed, but the behavior of us ticker and deep-sleep mode changes as follows:

• us ticker is driven by Timer 1 incremented by 1 with frequency 1MHz/16 bits,
• Timer's Capture/Compare register 0 is used to set interrupt time,
• Timer's Capture/Compare register 1 is used to get current timer count
• us ticker init routine disables us ticker interrupt,
• lp ticker driver operates on ticks,
• lp ticker init routine disables lp ticker interrupt
• lp ticker is not scaled to 1MHz

[pan-]

The change looks good however it will have a massive impact on power consumption: The timer will stay on during sleep for BLE applications because they generally use the event queue which internally use a Timer object.

On NRF52, the Internal HFCLK consume 60uA while the external one consume 250uA; on top of that power consumption of the timer peripheral must be added: 5uA @ 1MHz.

On NRF52 power consumption during sleep is less than 6uA; if this patch is applied the result would be a power consumption an order of magnitude higher during sleep. That is not acceptable for battery powered devices.

Could you investigate the impact on power consumption of this patch ?

[bulislaw]

CC: @c1728p9 @0xc0170

[bulislaw]

@mprse that should probably go to the branch with HAL changes
@pan- any hints how to conform this board to our new HAL APIs without hurting power?

[pan-]

@bulislaw I think the situation could be better if the event queue used a Timer object with an lp_ticker rather than the a us_ticker (note that this trap is not documented in the header, the default constructor documentation is empty).

[bulislaw]

Is the accuracy of LP good enough?

[pan-]

The event queue works at the ms scale (1kHz); not the us scale. According to the specification lp tickers should have a reported frequency between 8KHz and 64KHz (see here).

So yes, the lp ticker should be enough for the event queue.
@c1728p9 @geky Could you confirm ?

[geky]

👍 Yep, 1ms resolution is all that's needed, we just haven't defaulted to LowPowerTickers since it's not clearly defined what resolution garuntees there are.

[mprse]

Power consumption tests were performed on NRF51_DK board (battery powered). Board has been modified according to the NRF51_DK user guide (chapter 6.7.1 - link) as follows:

1. Cut the PCB track shorting solder bridge SB9 to put P22 in series with the load.
2. Short solder bridge SB11 (if using coin cell battery) or SB12 (if using external power supply) to
   bypass the protection diode which would otherwise give a voltage drop.

Test scenario:

- init lp, us tickers
- wait - 10 sec
- sleep - 10 sec
- deep-sleep - 10 sec

Test results:

	branch: master	branch: nrf5x_use_high_speed_timer_for_us_timer
Normal operation	4 [uA]	558 [uA]
Sleep mode	4 [uA]	558 [uA]
Deep-sleep mode	4 [uA]	4 [uA]

I have additionally checked power consumption before us ticker init routine is called and it was equal to 558 [uA]. I'm a little confused, how this is possible (us ticker should be disabled before init)?

[pan-]

@mprse Thank you for the analysis, that is really helpful. Could you do the power analysis with a BLE application like the BLE beacon example ? To avoid initialization of the UART peripheral, please remove line 84.

[mprse]

@mprse Thank you for the analysis, that is really helpful. Could you do the power analysis with a BLE application like the BLE beacon example ? To avoid initialization of the UART peripheral, please remove line 84.

	branch: master	branch: nrf5x_use_high_speed_timer_for_us_timer
BLE beacon example (without line 84)	3.5 [mA]	4.4 [mA]

[mprse]

I found that typical capacity of CR2032 battery is 240 mAh, so BLE beacon example can be run on battery power:

• 64,5 h when us ticker uses low power timer (master),
• 53,5 h when us ticker uses high speed timer (nrf5x_use_high_speed_timer_for_us_timer).

So the difference is 11 h (performance decreased by 17%).

[pan-]

@mprse I'm a bit surprised by the current results; did you compile with the release profile or develop profile ?

Note that MBED_TICKLESS is enabled on the NRF51 and one of the first thing done by the RtosTimer is to enable the us_ticker.

[mprse]

@mprse I'm a bit surprised by the current results; did you compile with the release profile or develop profile ?

With the default profile:
mbed test -t GCC_ARM -m NRF51_DK -n tests-mbed_hal-ble --compile -v -c

[bulislaw]

@pan- what do you think?

[pan-]

@bulislaw @mprse I'm really surprised by the numbers. On The NRF52; if I disable the trace that print the MAC address and compile with the release profile then the power consumption of the Beacon example is very low with master: ~20uA.

The Beacon example doesn't work, on NRF52, with that PR however.

[mprse]

@pan- I noticed that the Beacon example doesn't work on NRF51 on this branch neither, so the provided power consumption tests are invalid.

The Beacon example generates exception during BLE initialisation in the following line. I observed that sd_softdevice_enable system call returns 4097 error code which does not tell me much. It looks like these changes have influence on BLE drivers. Maybe someone who is familiar with BLE driver can provide some hint?

[pan-]

@mprse The error return is NRF_ERROR_SDM_INCORRECT_INTERRUPT_CONFIGURATION; it means that the softsevice interrupt is already enabled, or an enabled interrupt has an illegal priority level.

In our case it is an illegal priority level of the us_ticker IRQ. Please use the lowest priority for the us ticker interrupt (see lp_ticker).

[mprse]

@pan- Thanks for the provided explanation of the problem. I have provided a fix to us_ticker_init routine in order to enable TIMER0 irq with the lowest priority.
Unfortunately I still get the same error in the Beacon example program.

[mprse]

It looks like enabling TIMER0 irq (nrf_drv_common_irq_enable(TIMER0_IRQn, <priority>)) regardless of irq priority which is requested causes that BLE initialisation (sd_softdevice_enable()) fails with the NRF_ERROR_SDM_INCORRECT_INTERRUPT_CONFIGURATION error code. If TIMER0 interrupt is disabled, then BLE initialisation is successful.

[pan-]

@mprse Looking at the product specification of the softdevice, TIMER0 is used by the softdevice and shouldn't be used by application code while the softdevice is active. TIMER1 and TIMER2 are free however. My comment on priority stands.

[mprse]

@pan- Thanks. This was the problem. I used TIMER1 instead of TIMER0 and Beacon example runs now on NRF51_DK board. Unfortunately TIMER1 and TIMER2 are 8 or 16 bits wide, so I had to slightly modify us ticker driver. In current version us ticker has 16 bits and its frequency is 250 KHz.

Checked that HAL tests for for us/lp ticker and sleep passes on this branch.
Updated PR description.

[mprse]

Current measurement results on NRF51_DK:

	wait()	hal_sleep()	hal_deepsleep()	BLE beacon example (without line 84)
master	~4.3 [mA]	~4 [uA]	~3 [uA]	~4.5 [mA]
this branch	~4.5 [mA]	~555 [uA]	~4 [uA]	~4.5 [mA]

It looks like the results are very close. Main difference is between sleep mode on both branches. This is because on master sleep and deep-sleep modes are the same and high frequency timer is not used for us ticker. On this branch in sleep mode high frequency timer - TIMER1 counts, but in deep-sleep mode TIMER1 is stopped.

BLE beacon example has been compiled using the following command:
mbed test -t GCC_ARM -m NRF51_DK -n tests-mbed_hal-ble --compile -v -c --profile release.

[bulislaw]

I think that's fair enough, @pan- what do you think?

[bulislaw]

That should be on the new ticker hal api branch I think.

[mprse]

Added fix for lp ticker driver (setting interrupt - min value written to cc register must be now + 2 ticks) and re-enabled sleep support for NRF51_DK.

[mprse]

@0xc0170 Can we run morph after last updates?

[0xc0170]

@0xc0170 Can we run morph after last updates?

Will label this as needs: CI (still howeve waiting for all request changes to approved!). At the moment , release candidate is under test (+ one fix that needs to get in there).

[cmonr]

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 1826
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5629/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

[mbed-ci]

Test : SUCCESS

Build number : 1632
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5629/1632

[mbed-ci]

Exporter Build : FAILURE

Build number : 1470
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5629/

[cmonr]

Going on a hunch that a Jenkins machine left too soon.

Will leave for @0xc0170 to confirm and restart, since the build artifacts contain more than just the usual PASS directory.

[0xc0170]

/morph export-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 1478
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5629/

[cmonr]

@pan- @donatieng Are y'all happy with this PR?

[donatieng]

Everything good from my point of view. @c1728p9 can you confirm you're happy with the new tolerance definitions?

[c1728p9]

The tolerance definitions look good to me.

The PR was updated

[donatieng]

Great stuff, approved!