Add wait_ns API #9812

kjbracey · 2019-02-22T09:55:47Z

Description

This provides the ability to generate really small delays - it's often
the case that wait_us() takes multiple microseconds to set up, so
having an alternative suitable for <10us delays is useful.

There have been a few local implementations - it makes sense to
centralise them as they need retuning for each new ARM core.

Based on the local implementation inside the Atmel 802.15.4 driver.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@c1728p9, @bulislaw

Release Notes

wait_ns() API added for <10us small software-loop based delays.

ciarmcom · 2019-02-22T10:00:30Z

@kjbracey-arm, thank you for your changes.
@c1728p9 @bulislaw @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

kjbracey · 2019-02-22T13:10:55Z

Added a timing test - this will test both that my tuning number (LOOP_SCALER) is correct for each core type, and also that the targets are setting SystemCoreClock correctly, which we rely on. I don't think we have any existing tests for this.

kjbracey · 2019-02-22T15:15:07Z

Testing shows a bunch of failures: https://mbed-os.mbedcloudtesting.com/job/mbed-os-ci_greentea-test/1244/testReport/

RTL8195 (M3) is running 35% slower than SystemCoreClock indicates according to lp-timer.
Nucleo F7s (M7) are sometimes running twice as fast. (F746ZG-IAR was correct, other 2 F746ZG builds failed and alll F767ZI builds ran double speed). I think this is probably an alignment issue - I suspect the M7 is sometimes dual issuing and managing a 1 cycle loop, depending on alignment.
NRF51_DK (M0) is running 26% slower than SystemCoreClock indicates according to lp-timer. Might just be some interrupt load though? Seems it should be locked at 16MHz.

Looks like I need to do something to make the M7 timing predictable - the nRF51 and RTL8195 seem to have some sort of target-specific problem - either a clock error or unexpectedly high interrupt load.

kjbracey · 2019-02-25T10:43:40Z

Updated with something that I hope stabilises the M7 timing - SUBS NOP NOP BCS seems to reliably take 2 cycles, whereas SUBS BCS seemed to take either 1 or 2.

Still need to figure out RTL8195 and NRF51_DK - maybe some input from relevant teams might be useful.

0xc0170 · 2019-02-25T10:46:11Z

Still need to figure out RTL8195 and NRF51_DK - maybe some input from relevant teams might be useful.

@ARMmbed/team-realtek Can you review time differences for target?

@ARMmbed/mbed-os-pan or @TacoGrandeTX could comment on NRF51_DK ?

kjbracey · 2019-02-25T11:18:19Z

In my latest run RTL8195 and M7 devices passed, nRF51 still fails, and now Nucleo L073 and F072 failed too.

https://mbed-os.mbedcloudtesting.com/job/mbed-os-ci_greentea-test/1257/testReport/

It's possible it's down to alignment again, rather than specific platforms - flash sometimes not able to keep up with the instruction flow? Rerunning that one to see if consistent for one build, and will keep fiddling.

If I can't force alignment (seems to be hard to do), may be able to get it down to a tolerable difference between fast and slow alignment by putting enough NOPs in - smoothing out the flow.

kjbracey · 2019-02-26T15:30:02Z

Some platforms seem to be quite persistently running 30% slower than expected, but not been able to lock down a cause. Falling back to making the upper tolerance bound higher, to try to make sure the test is stable. It would be nice to get it more precise, but for the sort of use case envisaged, taking 40% longer is acceptable.

Trying some repeated test system runs to get confidence.

This provides the ability to generate really small delays - it's often the case that wait_us() takes multiple microseconds to set up, so having an alternative suitable for <10us delays is useful. There have been a few local implementations - it makes sense to centralise them as they need retuning for each new ARM core. Based on the local implementation inside the Atmel 802.15.4 driver.

Nuvoton M2351 was including generic core_armv8mbl.h from CMSIS - we need it to be more specific to identify the specific core for wait_ns. Change to core_cm23.h.

bulislaw · 2019-02-27T17:19:35Z

platform/mbed_wait_api_no_rtos.c

+#elif (__CORTEX_M == 0 && defined __CM0PLUS_REV) || __CORTEX_M == 3 || __CORTEX_M == 4 || \
+      __CORTEX_M == 23 || __CORTEX_M == 33
+// Cortex-M0+, M3, M4, M23 and M33 take 6 cycles per iteration - SUBS = 1, 3xNOP = 2, BCS = 2
+// TODO - check M33


Could we resolve the TODO before committing this?

We don't actually have any M33 targets yet, do we? So don't think I've got anything I can test this on.

My belief is that the M33 is basically an M4 in terms of internal architecture, but no firm evidence. Could leave the __CORTEX_M == 33 case out and deal with it when we have the first actual target.

bulislaw

Looks good, but I don't like the TODO labels, we should either do it now or remove them as it won't happen later.

cmonr · 2019-02-27T19:21:33Z

CI started

cmonr · 2019-02-27T19:25:56Z

@bulislaw A quick search of the Mbed OS repo found that we have 330 files with TODO (command used: rg TODO --count-matches | wc -l).

If that's the only problem with this PR, I wouldn't suggest holding it up.

@kjbracey-arm It would be good to know why this is still here though.

mbed-ci · 2019-02-27T21:18:13Z

Test run: FAILED

Summary: 1 of 8 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

cmonr · 2019-02-27T23:02:43Z

Info: A CI config issue appears to be affecting NUMAKER_PFM_M2351 builds. Please ignore build errors against the target for now.

Other build failures should still be investigated, if any. Will restart CI when appropriate.

bulislaw · 2019-02-28T10:18:23Z

@bulislaw A quick search of the Mbed OS repo found that we have 330 files with TODO (command used: rg TODO --count-matches | wc -l).

If that's the only problem with this PR, I wouldn't suggest holding it up.

I approved the PR, so not holding it back, but I find it pointless to add TODO as it's never going to be resolved as your grep just proved :P

0xc0170 · 2019-02-28T11:31:12Z

CI restarted

kjbracey · 2019-02-28T14:09:43Z

Do I need to rebase this or something?

0xc0170 · 2019-02-28T14:35:58Z

Do I need to rebase this or something?

Not needed, it's in the CI queue (hooks here will be updated once it is executing).

mbed-ci · 2019-02-28T15:32:21Z

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2019-02-28T15:40:35Z

Ci will be restarted (iar8 exporter issue we will resolve separately).

deepikabhavnani

Thanks for adding this @kjbracey-arm

Based on the local implementation inside the Atmel 802.15.4 driver

It will be good to record somewhere that this local implementation should be removed and use the generic one implemented here.

ciarmcom requested review from bulislaw and c1728p9 February 22, 2019 10:00

ciarmcom requested review from a team February 22, 2019 10:00

ciarmcom added the needs: review label Feb 22, 2019

kjbracey force-pushed the wait_ns branch 3 times, most recently from c1372bc to b99806f Compare February 22, 2019 13:04

kjbracey force-pushed the wait_ns branch from b99806f to ddc44f2 Compare February 22, 2019 13:16

kjbracey force-pushed the wait_ns branch 3 times, most recently from f940807 to ae6c8fd Compare February 25, 2019 10:41

c1728p9 approved these changes Feb 25, 2019

View reviewed changes

kjbracey force-pushed the wait_ns branch from 3d88e48 to 51a8c7c Compare February 26, 2019 09:59

0xc0170 approved these changes Feb 26, 2019

View reviewed changes

kjbracey force-pushed the wait_ns branch from 1ed2e2d to 25d6c20 Compare February 26, 2019 13:19

kjbracey added 5 commits February 26, 2019 17:30

Add wait_ns timing test

08a8f16

M2351: include core_cm23.h

66dea08

Nuvoton M2351 was including generic core_armv8mbl.h from CMSIS - we need it to be more specific to identify the specific core for wait_ns. Change to core_cm23.h.

Hand assemble to get alignment

91b22f8

Increase wait_ns test tolerance

d2df5a0

kjbracey force-pushed the wait_ns branch from 25d6c20 to d2df5a0 Compare February 26, 2019 15:31

mikisch81 mentioned this pull request Feb 27, 2019

USTICKER can not be removed from compilation #9853

Closed

bulislaw reviewed Feb 27, 2019

View reviewed changes

bulislaw approved these changes Feb 27, 2019

View reviewed changes

cmonr added needs: CI risk: A release-version: 5.12.0-rc1 and removed needs: review labels Feb 27, 2019

deepikabhavnani approved these changes Feb 28, 2019

View reviewed changes

cmonr added ready for merge and removed needs: CI labels Feb 28, 2019

mikisch81 mentioned this pull request Feb 28, 2019

wait_us without usticker #9896

Merged

cmonr merged commit e6caa12 into ARMmbed:master Mar 1, 2019

cmonr removed the ready for merge label Mar 1, 2019

kjbracey mentioned this pull request Apr 12, 2021

Timer.read_us takes 8.5µs, making wait_us unable to wait for less than that on Nucleo F103RB #7397

Closed

kjbracey deleted the wait_ns branch April 12, 2021 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wait_ns API #9812

Add wait_ns API #9812

kjbracey commented Feb 22, 2019

ciarmcom commented Feb 22, 2019

kjbracey commented Feb 22, 2019

kjbracey commented Feb 22, 2019

kjbracey commented Feb 25, 2019

0xc0170 commented Feb 25, 2019

kjbracey commented Feb 25, 2019 •

edited

Loading

kjbracey commented Feb 26, 2019

bulislaw Feb 27, 2019

kjbracey Feb 28, 2019

bulislaw left a comment

cmonr commented Feb 27, 2019

cmonr commented Feb 27, 2019

mbed-ci commented Feb 27, 2019

cmonr commented Feb 27, 2019

bulislaw commented Feb 28, 2019

0xc0170 commented Feb 28, 2019

kjbracey commented Feb 28, 2019

0xc0170 commented Feb 28, 2019

mbed-ci commented Feb 28, 2019

0xc0170 commented Feb 28, 2019

deepikabhavnani left a comment

Add wait_ns API #9812

Add wait_ns API #9812

Conversation

kjbracey commented Feb 22, 2019

Description

Pull request type

Reviewers

Release Notes

ciarmcom commented Feb 22, 2019

kjbracey commented Feb 22, 2019

kjbracey commented Feb 22, 2019

kjbracey commented Feb 25, 2019

0xc0170 commented Feb 25, 2019

kjbracey commented Feb 25, 2019 • edited Loading

kjbracey commented Feb 26, 2019

bulislaw Feb 27, 2019

Choose a reason for hiding this comment

kjbracey Feb 28, 2019

Choose a reason for hiding this comment

bulislaw left a comment

Choose a reason for hiding this comment

cmonr commented Feb 27, 2019

cmonr commented Feb 27, 2019

mbed-ci commented Feb 27, 2019

Test run: FAILED

cmonr commented Feb 27, 2019

bulislaw commented Feb 28, 2019

0xc0170 commented Feb 28, 2019

kjbracey commented Feb 28, 2019

0xc0170 commented Feb 28, 2019

mbed-ci commented Feb 28, 2019

Test run: FAILED

0xc0170 commented Feb 28, 2019

deepikabhavnani left a comment

Choose a reason for hiding this comment

kjbracey commented Feb 25, 2019 •

edited

Loading