us/lp ticker - port K64F platform to new HAL api #6052

mprse · 2018-02-09T11:24:58Z

Description

This PR ports us/lp ticker K64F drivers to the new HAL API, enables support for ticker on K64F and also provides little modifications to the HAL ticker tests (resolve issues found while porting ticker driver).

Adapt K64F us ticker driver to the new standards.

Here only cosmetic changes were required since US ticker is already driven by 1MHz clock, so us <-> ticks conversion is not required.
Init routine needed to be modified since according to the requirements init should disable us ticker interrupt.

Adapt K64F lp ticker driver to the new standards.

Low power ticker time counter is created based on RTC which is driven by 32KHz clock. Additional low power timer is used to generate interrupts (in case when sleep time does not exceed 2 sec otherwise interrupt is scheduled in two steps - first full seconds are scheduled using RTC Alarm interrupt and then remaining time using LPTMR).

In previous version RTC time was converted to us before was passed to the upper layer and vice versa. In the new version upper layer is informed that lp ticker is driven by 32KHz clock, ticker driver operates on ticks and ticks <-> us conversion is handled by the upper ticker layer.
Init routine needed to be modified since according to the requirements init should disable lp ticker interrupt.

lp_us_tickers test - provide counter overflow protection.

Since counter overflow is handled by the upper layer new drivers does not check if counter has overflown. Added protection against counter overflow during test case execution.

lp_us_tickers test - add tolerance to interrupt time.

On some platforms (e.g. K64F) different counters are used for time measurement and interrupt generation.
Because of that we should relax "interrupt test case" and give additional time before checking if interrupt handler has been executed.

Status

HOLD

Migrations

YES

Low power ticker K64F drivers operates now on ticks and ticks <--> us conversion is done in the upper layer.

mprse · 2018-02-20T13:57:01Z

Waiting for the following patches:

mprse · 2018-02-20T19:57:33Z

@cmonr All reqired patches are now on master. Could you rebase destination branch on master?

cmonr · 2018-02-21T02:55:19Z

@mprse I've rebased the target branch. You should be good to go once you rebase this PR!

- provide ticker configuration: 1MHz/32 bit counter. - us_ticker_init() routine disables interrupts. - adapt comments.

0xc0170 · 2018-02-21T09:14:18Z

/morph build

mbed-ci · 2018-02-21T09:53:37Z

Build : FAILURE

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

mprse · 2018-02-21T11:00:18Z

Build : FAILURE

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

It looks like the conflicts for the following commit have been incorrectly resolved while rebasing on master:
7f88de9 (Disable boards which does not fulfill new ticker standards.)

Support for lp/us ticker should be disabled for all CI boards by above commit:
ARCH_PRO - disabled
K22F - disabled
K64F - disabled
NCS36510 - "LOWPOWERTIMER" should be removed:

mbed-os/targets/targets.json

Line 3667 in 7f88de9

    
           "device_has": ["ANALOGIN", "SERIAL", "I2C", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SERIAL_FC", "SLEEP", "SPI", "LOWPOWERTIMER", "TRNG", "SPISLAVE"],

NRF51_DK - disabled
NRF52_DK - "LOWPOWERTIMER" should be removed:

mbed-os/targets/targets.json

Line 3482 in 7f88de9

    
           "device_has_add": ["ANALOGIN", "I2C", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_ASYNCH", "SERIAL_FC", "SLEEP", "SPI", "SPI_ASYNCH", "SPISLAVE", "FLASH"],

NUCLEO_F401RE - disabled
NUCLEO_F429ZI - "LOWPOWERTIMER" should be removed:

mbed-os/targets/targets.json

Line 1198 in 7f88de9

    
           "device_has_add": ["ANALOGOUT", "CAN", "LOWPOWERTIMER", "SERIAL_ASYNCH", "SERIAL_FC", "TRNG", "FLASH"],

NUCLEO_F746ZG - disabled
UBLOX_EVK_ODIN_W2 - disabled

@cmonr @0xc0170 What can we do with this issue?
Can you fix this commit as described or maybe I can add another commit with fix?

0xc0170 · 2018-02-21T11:08:29Z

Can you fix this commit as described or maybe I can add another commit with fix?

both should be fine. I expect that feature branch will be rebased later for integration so this will be cleaned up.

mprse · 2018-02-21T11:16:29Z

Can you fix this commit as described or maybe I can add another commit with fix?

both should be fine. I expect that feature branch will be rebased later for integration so this will be cleaned up.

So I will add additional commit with the fix and explanation in this PR.

0xc0170 · 2018-02-21T14:12:56Z

/morph build

mbed-ci · 2018-02-21T14:42:25Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-02-21T16:31:25Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-02-21T19:37:14Z

Test : FAILURE

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

mprse · 2018-02-22T07:24:36Z

Test : FAILURE

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

All ok for K64F! tests-mbed_drivers-rtc has failed for all compilers on NRF51_DK. This test is new and uses us ticker to perform the delay so it will be disabled until USTICKER is enabled for NRF51_DK.

0xc0170 · 2018-02-22T09:09:35Z

/morph build

mbed-ci · 2018-02-22T09:47:19Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-02-22T12:10:24Z

Exporter Build : SUCCESS

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

0xc0170 · 2018-02-23T08:09:10Z

/morph test

mbed-ci · 2018-02-23T11:02:23Z

Test : SUCCESS

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

mprse · 2018-02-23T11:05:11Z

CI passed - please review @bulislaw @c1728p9.

c1728p9 · 2018-02-23T21:03:33Z

targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/api/lp_ticker.c

+     * count[0 - 14] - ticks (RTC->TPR)
+     * count[15 - 31] - seconds (RTC->TSR)
+     */
+    const uint32_t count = ((RTC->TSR << SEC_SHIFT) | (RTC->TPR & TICKS_MASK));


The Kinetis RTC is based on a ripple counter so if it is read at the right time it can glich. To guarantee a glitch free value, the same count needs to be read twice in a row. A psuedo code example of this is here:

mbed-os/hal/lp_ticker_api.h

Lines 143 to 149 in 2f14057

* // Loop until the same tick is read twice since this

* // is ripple counter on a different clock domain.

* count = LPTMR_COUNT;

* do {

* last_count = count;

* count = LPTMR_COUNT;

* } while (last_count != count);

This would be a good case to add a test for if we don't have one already. Repeatedly call lp_ticker_read in a tight loop and if the value every glitches into the past then fail the test.

I have fixed lp_ticker_read as suggested and added glitch test cases for lp ticker.

c1728p9 · 2018-02-23T21:16:11Z

targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/api/lp_ticker.c

        delta_ticks = 1;
    }

    if (delta_ticks > MAX_LPTMR_SLEEP) {
-        /* Using RTC if wait time is over 16b (2s @32kHz) */
-        uint32_t delta_sec;
+        /* Using RTC if wait time is over 16b (2s @32kHz). */


Does the RTC alarm interrupt need to be used here? It seems like this code could be made simpler and thus more robust by only using the LPTMR. You would just need to adjust the supported number of bits to 15 so delta_ticks will never be greater than MAX_LPTMR_SLEEP.

I agree that this can be implemented using only RTC->TPR as 15 bit time counter and only LPTMR to generate interrupts, but on the other hand combining 32 bit time counter from RTC->TSR and RTC->TPR is also ok and we got wider counter range.
I suggest to leave 32-bit counter in this PR and consider change to 15-bit counter in another PR (I will create it) since we need to have some example implementations before meting with silicon partners and it looks like this version is stable.

Low power ticker time counter is created based on RTC which is driven by 32KHz clock. Additional low power timer is used to generate interrupts. We need to adapt driver to operate on ticks instead of us. Perform the following updates: - provide lp ticker configuration: 32KHz/32 bit counter. - lp_ticker_init() routine disables lp ticker interrupts . - adapt the driver functions to operate on ticks instead us. - adapt comments. - add us_ticker_free() routine.

Since according to the ticker requirements min acceptable counter size is 12 bits (low power timer) for which max count is 4095, then all test cases must be executed in this time window. HAL ticker layer handles overflow and it is not handled in the target ticker drivers.

On some platforms (e.g. K64F) different counters are used for time measurement and interrupt generation. Because of that we should relax interrupt test case and give additional time before checking if interrupt handler has been executed.

…lts on all compilers count_ticks() function counts ticker ticks elapsed during execution of N cycles of empty while loop. In current version N value (cycles) is given as volatile paramater in order to disable possible compiler optimalisation. There was a problem with measured time on different compilers and additionally results on ARM compiler were unexpected (the difference beetween measured elapsed ticks for the same number of cycles was to large). This might be caused by the memory access in order to store updated variable in memory. To fix this issue given numer of cycles has been stored into register and register is decremented (no memory access). With this fix count_ticks(NUM_OF_CYCLES, 1) call returns 2500 +/-1 for us ticker ticks using each compiler (K64F).

This has been done already here: ARMmbed@7f88de9 Unfortunately targets.json file is changed quite often and it is hard to maintain conflicts while rebasing feature branch on master. This will have to be cleaned up later for integration with master. Note: lp ticker is required to have flash support on NRF52_DK. Disable flash support for this board.

…pecific drivers. Since target specific ticker drivers are not ready also features which uses ticker in upper layers may not work correctly and tests for these features. We need to disable also failing higher level ticker related tests (by adding check if DEVICE_USTICKER symbol is available and raise #error [NOT_SUPPORTED] if not). Note: tests-mbed_drivers-rtc is new and uses us ticker to perform a delay.

Test that lp ticker does not glitch backwards due to an incorrectly implemented ripple counter driver.

0xc0170 · 2018-02-26T10:34:22Z

/morph build

mbed-ci · 2018-02-26T11:09:35Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-02-26T14:28:00Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-02-26T14:44:27Z

Test : SUCCESS

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

jeromecoutant · 2018-04-09T12:40:48Z

TESTS/mbed_hal/lp_ticker/main.cpp

+    uint32_t last = lp_ticker_read();
+    const uint32_t start = last;
+
+    /* Set test time to 2 sec. */


Hi
This test is not protected for overflow.
Regards,

I'll try to provide fix tomorrow.

Fix can be found here: PR #6584.

0xc0170 added the needs: review label Feb 9, 2018

0xc0170 requested review from c1728p9 and bulislaw February 9, 2018 11:26

This was referenced Feb 9, 2018

Ticker interrupt not scheduled when interrupt has been set in the past #6054

Closed

K64F lp ticker driver - calculation bug fix. #6055

Merged

cmonr force-pushed the feature-hal-spec-ticker branch from b57f60e to f2e7043 Compare February 13, 2018 18:15

mprse force-pushed the feature-hal-spec-ticker_k64f_drivers branch from 6c6605f to ea62ab7 Compare February 16, 2018 09:04

mprse mentioned this pull request Feb 20, 2018

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

Merged

cmonr force-pushed the feature-hal-spec-ticker branch from f2e7043 to 2f14057 Compare February 21, 2018 02:54

mprse force-pushed the feature-hal-spec-ticker_k64f_drivers branch from ea62ab7 to 98d54f7 Compare February 21, 2018 08:47

mprse added 2 commits February 21, 2018 10:09

Enable ticker support for K64F board.

f35f932

Adapt K64F us ticker driver to the new standards.

3a48afa

- provide ticker configuration: 1MHz/32 bit counter. - us_ticker_init() routine disables interrupts. - adapt comments.

mprse force-pushed the feature-hal-spec-ticker_k64f_drivers branch from 98d54f7 to fbdfa51 Compare February 21, 2018 09:11

mprse force-pushed the feature-hal-spec-ticker_k64f_drivers branch 2 times, most recently from 57423d9 to 9ba82bb Compare February 21, 2018 14:04

cmonr added needs: CI and removed needs: review labels Feb 23, 2018

cmonr added ready for merge needs: review and removed needs: CI ready for merge labels Feb 23, 2018

c1728p9 reviewed Feb 23, 2018

View reviewed changes

mprse added 7 commits February 26, 2018 08:35

lp_ticker test - provide lp ticker glitch test case

c74c6c7

Test that lp ticker does not glitch backwards due to an incorrectly implemented ripple counter driver.

mprse force-pushed the feature-hal-spec-ticker_k64f_drivers branch from cdfb9ec to c74c6c7 Compare February 26, 2018 08:45

cmonr added release-version: 5.7.7 and removed release-version: 5.7.7 labels Feb 27, 2018

cmonr merged commit 5440cc4 into ARMmbed:feature-hal-spec-ticker Feb 27, 2018

cmonr removed the needs: review label Feb 27, 2018

jeromecoutant reviewed Apr 9, 2018

View reviewed changes

jeromecoutant mentioned this pull request Apr 9, 2018

STM32 LPTICKER update for targets supporting LPTIMER #6474

Merged

	* // Loop until the same tick is read twice since this
	* // is ripple counter on a different clock domain.
	* count = LPTMR_COUNT;
	* do {
	* last_count = count;
	* count = LPTMR_COUNT;
	* } while (last_count != count);

us/lp ticker - port K64F platform to new HAL api #6052

us/lp ticker - port K64F platform to new HAL api #6052

Uh oh!

Conversation

mprse commented Feb 9, 2018

Description

Status

Migrations

Uh oh!

mprse commented Feb 20, 2018

Uh oh!

mprse commented Feb 20, 2018

Uh oh!

cmonr commented Feb 21, 2018

Uh oh!

0xc0170 commented Feb 21, 2018

Uh oh!

mbed-ci commented Feb 21, 2018

Uh oh!

mprse commented Feb 21, 2018

Uh oh!

0xc0170 commented Feb 21, 2018

Uh oh!

mprse commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Feb 21, 2018

Uh oh!

mbed-ci commented Feb 21, 2018

Triggering tests

Uh oh!

mbed-ci commented Feb 21, 2018

Uh oh!

mbed-ci commented Feb 21, 2018

Uh oh!

mprse commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Feb 22, 2018

Uh oh!

mbed-ci commented Feb 22, 2018

Triggering tests

Uh oh!

mbed-ci commented Feb 22, 2018

Uh oh!

0xc0170 commented Feb 23, 2018

Uh oh!

mbed-ci commented Feb 23, 2018

Uh oh!

mprse commented Feb 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Feb 26, 2018

Uh oh!

mbed-ci commented Feb 26, 2018

Triggering tests

Uh oh!

mbed-ci commented Feb 26, 2018

Uh oh!

mbed-ci commented Feb 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mprse Apr 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mprse commented Feb 21, 2018 •

edited

Loading

mprse commented Feb 22, 2018 •

edited

Loading

mprse Apr 9, 2018 •

edited

Loading