Wait API updated to remove deepsleep lock #8189

deepikabhavnani · 2018-09-19T19:57:44Z

Description

FOR MINOR RELEASE

ThisThread::sleep_for() allows thread to enter deep sleep mode, and hence saves power. But allowed only tick resolution for wait.
wait API's earlier applied deepsleep lock to avoid entering deep sleep mode and use busy wait loop for usec precision.

Aim here is to get precision as per the API name, and enable devices to enter sleep mode.

Updates:
wait and wait_us - Give usec precision till 10msec and perform busy wait loop, no power saving here. But if value passed to these API's is more than 10msec, then behavior will be same as wait_ms with sub-millisec value rounded.

wait_ms - Will have msec precision and saves power. However it calls ThisThread::sleep_for() -> osDelay() API's which are tick based, i.e actual time delay may be up to one timer tick less. ~~In order to avoid zero delay for wait_ms we do busy wait for 1ms.~~

Pull request type

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

Related : #6969 #7154

geky

Looks good to me 👍

geky · 2018-09-19T20:40:19Z

platform/mbed_wait_api.h

- *    You can avoid it by using Thread::wait().
+ *    If the RTOS is present, this function spins to get the exact number of microseconds,
+ *    (for 1/100th of sec) which potentially affects power (such as preventing deep sleep) and 
+ *    multithread performance. You can avoid it by using ThisThread::sleep_for() / wait_ms()


minor nit: This slash made me think math was involved. Maybe say "ThisThread::sleep_for() or wait_ms()" ?

geky · 2018-09-19T20:41:10Z

platform/mbed_wait_api.h

- *    which potentially affects power (such as preventing deep sleep) and multithread performance.
+ *    If the RTOS is present, this function spins to get the exact number of microseconds,
+ *    (for 1/100th of sec) which potentially affects power (such as preventing deep sleep) and 
+ *    multithread performance. You can avoid it by using ThisThread::sleep_for() / wait_ms()


Isn't this true even if the RTOS is not present?

If the RTOS is present, this function spins to get the exact number of microseconds - This is true if RTOS not present

c1728p9 · 2018-09-19T22:01:52Z

platform/mbed_wait_api_rtos.cpp

+        ThisThread::sleep_for(ms);
+    }
+
+    if (sub_ms > 0) {


Could this be the else case of the above code? That way delays above the threshold never busy wait.

They still don;t in current logic since we set sub_ms = 0;
But i see your reduced instruction code with above comment ms = ((us + 500) / 1000); will update as per that

c1728p9 · 2018-09-19T22:02:41Z

platform/mbed_wait_api_rtos.cpp

-        sleep_manager_lock_deep_sleep();
-        Thread::wait((uint32_t)us / 1000);
-        sleep_manager_unlock_deep_sleep();
+        ms = (us / 1000);


you could do the rounding as part of this statement:

ms = ((us + 500) / 1000);

I'm not really seeing the motivation for changing wait_us. If someone is calling it specifically, they want the precision, or else they'd be calling wait_ms, right? The obvious exception is current wait(float), but that is a long way from the normal user.

I already have wait_ms(20) to get 19-20ms. I don't need wait_us(20000) to give me 19-20ms as well.

Personally, I'm really not a fan of size-of-argument-dependent behaviour here. Ideally something should be a sleeping thread-only function or a non-sleeping any-context function.

I would rather wait_us actually lost the sleep functionality altogether and became a non-sleeping function, and we maybe audited people using it with excessive values (warning/error on debug builds). I don't actually see the utility of wait_us(20000) with microsecond precision.

I think we may be forced to have value-dependent behaviour on wait(float), but I can live with that as long as there are alternative wait_ms and wait_us calls with solid behaviour.

c1728p9 · 2018-09-19T22:05:03Z

platform/mbed_wait_api_rtos.cpp

-    wait_us(ms * 1000);
+    // Wait equivalent to 1 ms near the tick might result into zero wait.
+    // Hence ms precision can be used for > 1 ms wait only.
+    if ((ms >= 1) && core_util_are_interrupts_enabled()) {


wait_us already chooses between thread wait and busy wait. Why do you need the checking here?

ThisThread::sleep_for(ms); in the if condition will assert if inside the interrupt.

ms >= 1? If it's not, it's negative or zero.

And I would say "wait_ms()" if interrupts are disabled is breaking the system. Interrupts should not be disabled for as long as a millisecond, so I don't like coding to permit this. Although I guess the existing code does do it, so maybe backwards compatibility. But if changing anyway...

People willing to do a big wait in interrupt could do it as wait_us(5000) where the big number kind of exposes the enormity of how bad they're being.

c1728p9 · 2018-09-19T22:08:58Z

platform/mbed_wait_api.h

- *    If the RTOS is present, this function always spins to get the exact number of microseconds,
- *    which potentially affects power (such as preventing deep sleep) and multithread performance.
- *    You can avoid it by using Thread::wait().
+ *    If the RTOS is present, this function spins to get the exact number of microseconds,


Microsecond precision is no longer maintained when a delay of greater than 10ms is used. In this case the tolerance would be wait_time - 500us to wait_time +500us

kjbracey · 2018-09-20T07:18:56Z

platform/mbed_wait_api_rtos.cpp

-    wait_us(ms * 1000);
+    // Wait equivalent to 1 ms near the tick might result into zero wait.
+    // Hence ms precision can be used for > 1 ms wait only.
+    if ((ms >= 1) && core_util_are_interrupts_enabled()) {


ms >= 1? If it's not, it's negative or zero.

And I would say "wait_ms()" if interrupts are disabled is breaking the system. Interrupts should not be disabled for as long as a millisecond, so I don't like coding to permit this. Although I guess the existing code does do it, so maybe backwards compatibility. But if changing anyway...

People willing to do a big wait in interrupt could do it as wait_us(5000) where the big number kind of exposes the enormity of how bad they're being.

kjbracey · 2018-09-20T07:31:21Z

platform/mbed_wait_api.h

+ *     The millisec value specifies the number of timer ticks and is therefore an upper bound. 
+ *     The exact time delay depends on the actual time elapsed since the last timer tick. 
+ *     The actual time delay may be up to one timer tick less.
+ *     For a value of 1, the function spins to get the exact 1 millisecond. 


Oh, right. That's not what the code says - you've miscoded it.

This special case would be horrible, and a bizarre discontinuity. Why should wait_ms(1) not wait 1 millisecond less than wait_ms(2)?

And what happens if you call wait_ms(2) 50 times (with the RTOS)? You get a 100ms delay. Without this special case, wait_ms(1) 100 times would give you a 100ms delay. But with the special case added, you'll get a drift effect, due to the failure to lock onto the ticks.

kjbracey · 2018-09-20T07:39:25Z

platform/mbed_wait_api_rtos.cpp

-        sleep_manager_lock_deep_sleep();
-        Thread::wait((uint32_t)us / 1000);
-        sleep_manager_unlock_deep_sleep();
+        ms = (us / 1000);


I'm not really seeing the motivation for changing wait_us. If someone is calling it specifically, they want the precision, or else they'd be calling wait_ms, right? The obvious exception is current wait(float), but that is a long way from the normal user.

I already have wait_ms(20) to get 19-20ms. I don't need wait_us(20000) to give me 19-20ms as well.

Personally, I'm really not a fan of size-of-argument-dependent behaviour here. Ideally something should be a sleeping thread-only function or a non-sleeping any-context function.

I would rather wait_us actually lost the sleep functionality altogether and became a non-sleeping function, and we maybe audited people using it with excessive values (warning/error on debug builds). I don't actually see the utility of wait_us(20000) with microsecond precision.

I think we may be forced to have value-dependent behaviour on wait(float), but I can live with that as long as there are alternative wait_ms and wait_us calls with solid behaviour.

kjbracey · 2018-09-20T09:14:13Z

I think if we're changing it, we should be changing it to something more solid. Specific counter-proposal for the RTOS version:

wait(float) calls wait_ms for >=0.1s, else wait_us. Deprecate it.
wait_ms() is just the thread sleep (so rounding down). Not callable from interrupts, doesn't lock hardware sleep. (For non-RTOS it should use low-power ticker and attempt hardware sleep)
wait_us() is a non-thread-sleeping ticker-based wait. Callable from interrupts, stays in current context, idles the CPU, locks hardware sleep.
In future: wait_ns() is a software-timed CPU-based wait. Callable from interrupts, inherently doesn't sleep.

Whether a call is potentially-thread-sleeping or not is pretty fundamental, and should not depend on values. If it does, someone naively changing what looks like a tuning number past some threshold could radically change the software's behaviour. And at the minute we have no definitely non-thread-sleeping wait.

Possible variants to that:

For backwards compatibility wait_ms() catches calls from interrupts, and if they happen defers to wait_us. This is trapped as an error in debug builds, as this is unacceptable interrupt latency, and misuse of the API. Deprecated behaviour.
JSON-disableable error trap for big wait_us values as these should generally be avoided, and we should be persuading people to use the sleeping wait_ms. Allow it to be disabled as in special cases applications may feel they need to do it and are willing to take consequences.

deepikabhavnani · 2018-09-20T20:50:39Z

@kjbracey-arm - I have update the code as per your suggestion, but with few changes

wait - Condition updated to 0.01s - Allowing busy wait for 100msec seemed incorrect to me.
Query - Do we need to deprecate after all these changes?
wait_ms - For backwards compatibility, even error will be breaking change, hence added busy wait us with warning.
wait_us - Instead json file option added warning (which is for debug only anyways)

deepikabhavnani · 2018-09-20T20:52:24Z

platform/mbed_wait_api_rtos.cpp

 }

 void wait_us(int us)
 {
+    if (us > 10000) {


@kjbracey-arm - 0.1s / 0.01s? I would like to keep it same for https://github.com/ARMmbed/mbed-os/pull/8189/files#diff-5a106edf0944ae7ba2d1bf95153fb0a0R30

Yes, 0.01s makes more sense. I think I probably meant to type that.

0.01s seems fine

kjbracey · 2018-09-21T07:37:22Z

platform/mbed_wait_api_rtos.cpp

 }

+/*  The millisec value specifies the number of timer ticks and is therefore an upper bound.


This statement is also true for the microsecond wait, just with the finer granularity. In both cases, you'd need to add 1 to make it a lower bound. (Assuming hardware resolution exactly equals the unit, which we assert for the RTOS, and is probably true for the microseconds. If it doesn't match, my head starts to hurt).

So probably shouldn't over-emphasise it on this one - restate it on both calls. This kinds of reads as wait_ms working differently from wait_us in this regard, which it doesn't.

Yes, hence I added just as comment and not as part of doxygen. Can remove it as well

kjbracey · 2018-09-21T08:23:54Z

platform/mbed_wait_api_rtos.cpp

+    if (core_util_are_interrupts_enabled()) {
+        ThisThread::sleep_for(ms);
+    } else {
+        MBED_WARNING(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_INVALID_OPERATION),


Does MBED_WARNING actually output a visible string at the moment? I see MBED_WARNING calling mbed_warning with a string, which it then doesn't use?

My preference for this is to be as harsh as an error, but only on debug builds. I think that's consistent with other nasties we check like RTX errors.

Doing a warning here would likely result in a flood of output or buffer filling, if it wasn't a "once-only" - it could cause extra subtle problems on a normal build, when we're trying to provide a mechanism to catch them.

If you don;t have strong opinion on error, I can fix mbed_warning to add debug print and make this warning once-only.

Adding error will result into mbed_die and next instruction will not be executed anyways. So I don;t see a point of calling wait_us after error unless we add some json config to disable the error.

I was suggesting flagging the error for the interrupt case to be something like

if (in interrupt) { #ifdef MBED_DEBUG // or MBED_TRAP_ERRORS_ENABLED ? error(blah) #else wait_us(ms * 1000); #endif }

At some point in the future, once we think that debug error might have persuaded enough people, we get rid of the interrupt check, just call ThisThread::sleep_for, which would itself go pop-bang if called from interrupt.

If it is a warning, once-only is kind of good when the condition is something like this, which is a design error, likely to be repeated throughout every run, rather than an environmental event. But flagging it does mean static data to track the once-only (or rate limiting).

Fair enough!! Can use MBED_TRAP_ERRORS_ENABLED since it is set for debug profile and mostly used for similar purpose in RTOS.

kjbracey · 2018-09-21T08:25:29Z

platform/mbed_wait_api_rtos.cpp

 }

 void wait_us(int us)
 {
+    if (us > 10000) {


0.01s seems fine

kjbracey · 2018-09-21T08:26:47Z

platform/mbed_wait_api_rtos.cpp

+    if (us > 10000) {
+        MBED_WARNING(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_UNKNOWN),
+                     "wait_us blocks deep sleep, wait_ms recommended for long delays\n");
+    }


I was suggesting dropping the sleep functionality from this entirely - restoring it to be the same as the non-RTOS version, hence the message above. What's your thought on that?

~~Yes can do that~~. Thinking of this again, do you mean remove Thread::wait and just spin?
Instead of polling continuously like non-RTOS version https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_wait_api_no_rtos.c#L36. I would prefer yielding to other threads for RTOS

Well, that's kind of my design point. I also prefer yielding to other threads, but I want to define whether a call yields or not - and we would already have the yielding call, which is wait_ms.

This is a call which already spins for up to a millisecond to do its business, so is already inherently thread-unfriendly - I want people to be using wait_ms to avoid any spin.

Uses of this should be rare. (Can we do a quick audit - how many actual direct wait_us users are there in tree? Most examples I can think of are GPIO "reset" pulses on init.)

And there are some occasions where it is important not to yield, so I'd like to have an API explicitly defined as that, not dependent on parameter.

Lets get more opinions over here
@TacoGrandeTX @c1728p9 @geky @bulislaw @stevew817 @NeilMacMullen - Please share your thoughts for wait_us

Oh, and this behaviour was my original reason to have my value warning above - we're saying "you do realise you're spinning for 10ms, right?". Not just the deep sleep issue.

I also think you've got a better chance of getting the time you were hoping each time with the spin than if you yield, where you might suddenly be hit with a full 10ms timeslice from another running thread.

Basically anyone wanting sub-millisecond resolution is having special requirements that can't really be catered for if you start dragging the RTOS into it. My feeling is that the current wait_us behaviour is purely to handle the wait_ms and wait built on top of it, rather than something I'd want an explicit wait_us to do.

I understand the intent here to have wait_us as spinning only, but to be true I am not sure of all use cases it is used for currently, hence will like to know from others as well.

Also agree that current wait_us behavior is to handle wait built on top of it.

FWIW, just searched through the in-tree source, and I can see around a 100 calls to wait_us, but I can't immediately identify any calls that might ask for >=1000us, except for wait_ms and wait themselves.

Actually, no, I see a couple in one of the "onboard_modem_api.c" files, in "wiggle the reset line" code. They're a full second, so would hit the error/warning.

Its wait_ms - Should not have issue, unless called from interrupt
https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NXP/TARGET_LPC176X/TARGET_UBLOX_C027/onboard_modem_api.c#L32

One more thought, we can change wait_us to be always spinning, and update wait to current behavior of wait_us

deepikabhavnani · 2018-09-21T15:59:06Z

CC @SenRamakri

c1728p9 · 2018-09-21T16:41:14Z

platform/mbed_wait_api_rtos.cpp


 void wait(float s)
 {
-    wait_us(s * 1000000.0f);
+    if (s >= 0.01f) {
+        wait_ms(s * 1000.0f);


If wait is used from an interrupt handler it will only trap if the delay is >= 10ms. Is this acceptable?

Fixed.. wait will not trap

deepikabhavnani · 2018-09-28T18:55:52Z

Hi All,

I have updated the code as :

wait(float) calls wait_ms for >=0.01s and not in interrupt, else spin in wait with usec precision and yield for msec wait.
wait_ms() is just the thread sleep and doesn't lock hardware sleep. In order to have backward
compatibility, if used in ISR wait_us is called if MBED_TRAP_ERRORS_ENABLED is false
wait_us() is a ticker-based wait, always spinning.

Please review

pan-

Thanks @deepikabhavnani that's very useful to cleanup that area of our code. I do have few observations:

The documentation of these functions should be precise regards to what happen in interrupt/non interrupt mode, time guarantee and power consumption.
We can improve consistency: wait works in interrupt but wait_ms doesn't if MBED_TRAP_ERRORS_ENABLED is enabled. That is confusing from a user perspective.
us precision can only be achieved from inside a critical section as a context switch or an interrupt of higher priority will disrupt time measured. If a critical section is not used then using a non low power timer makes little sense as precision can be random due to interrupt or context switches ...
Wouldn't it be possible to have a best-effort event driven wait_us implementation:
- Save current thread priority
- Set current thread priority to real time.
- Register an interrupt handler in the us_ticker that sends a signal to the current thread (or release a semaphore).
- Wait for the signal (or acquire the semaphore).
- Restore thread priority
Wouldn't it be useful to have more categories of wait function with different and clear guarantees ?

pan- · 2018-10-02T09:02:17Z

platform/mbed_wait_api.h

- *    You can avoid it by using Thread::wait().
+ *    If the RTOS is present, this function spins to get the exact number of microseconds for 
+ *    usec precision upto 10msec. If delay is larger then 10msec and not in ISR, it is same as 
+ *    `wait_ms`. `wait_us` and `wait_ms` are recommended over `wait`


According to Kevin's comment that function should be deprecated.
Question: should we have a wait_s function that accepts an integral type ?

@kjbracey-arm - Still work for wait will be needed in future and I would like to see sleep_for and sleep_until replacing wait. Shall we keep deprecation for next level of improvement on wait?

pan- · 2018-10-02T09:09:15Z

platform/mbed_wait_api_rtos.cpp

+        MBED_ERROR(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_INVALID_OPERATION),
+                    "Deprecated behavior: milli-sec delay should not be used in interrupt.\n");
+#else
+        wait_us(ms * 1000);


As stated by Kevin, we could use the low power ticker in that case.
It would be useful to at least print a warning here as behaviour with and without MBED_TRAP_ERRORS_ENABLED sets are too very different (trap or execute the intended action).
Note that this is a breaking change and it should be documented as one.

I really don't want to use additional ticker for backwards compatibility, we should be discouraging users from using wait_ms in interrupt mode.

Note that this is a breaking change and it should be documented as one.

I am not sure how we do this, I have updated the API description and will make sure code does not get in patch release. Do let me know how we can communicate this in better way?

Ok we either trap or not, having it error out in one mode, but happily work without even a warning, in other doesn't make much sense to me.

I don't get this part and I really hope every use of MBED_ERROR doesn't forces users to add this if:

#if defined(MBED_TRAP_ERRORS_ENABLED) && MBED_TRAP_ERRORS_ENABLED MBED_ERROR(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_INVALID_OPERATION), "Deprecated behavior: milli-sec delay should not be used in interrupt.\n"); #else

I'm happy to show a warning and change the behaviour for next major.

deepikabhavnani · 2018-10-03T18:10:10Z

We can improve consistency: wait works in interrupt but wait_ms doesn't if MBED_TRAP_ERRORS_ENABLED is enabled. That is confusing from a user perspective.

@pan- that was the initial design, but wait is combination of wait_ms and wait_us, so if we trap after argument > 10ms, it will be confusing as well. Please see comment from @c1728p9 "If wait is used from an interrupt handler it will only trap if the delay is >= 10ms. Is this acceptable?"

Do you have any other suggestion for wait behavior?

kjbracey · 2018-10-04T07:33:50Z

I agree wait(float) is confusing, but not sure we can do anything about it, hence my deprecation suggestion.

us precision can only be achieved from inside a critical section as a context switch or an interrupt of higher priority will disrupt time measured.

Even then, probably can't achieve close to microsecond precision using timers - see #7397 . We would likely need a CPU software loop implementation to achieve that.

Wouldn't it be possible to have a best-effort event driven wait_us implementation

Yes, but I don't think raising the priority inside the call would achieve anything - if it mattered, the moment you lowered priority on return, you'd get pre-empted and delayed anyway, right? But you could have a "sleep thread with high-res ticker interrupt wake", leaving it up to the user to make sure it was used from a high-priority thread.

That would probably get you typical 100us precision, working on my general assumption of interrupt latency. Potentially useful - better than the regular 1ms tick, without the "spin" horror.

The documentation of these functions should be precise regards to what happen in interrupt/non interrupt mode, time guarantee and power consumption.... Wouldn't it be useful to have more categories of wait function with different and clear guarantees ?

Fully agree, and that's what I was reaching for - have each "wait_xxx" be a clear class.

Having looked at C++11 <chrono> for a bit, I've reconsidered and think that inferring behaviour from units is possibly a dead end. It should be more explicit in the function name - continue down the path started by ThisThread::sleep_for. The problem we're having here is really just down to how vague the word wait inherently is.

Also, many people needing "precision" from a "wait for" call would be much better served by a "wait until", which is missing here. Without that they're subject to significant drift.

bulislaw · 2018-11-01T13:33:42Z

I think that's still better that have a function that works or doesn't work depending on the debug level, that doesn't make any sense. It also brakes compatibility (sometimes).

kjbracey · 2018-11-01T14:56:30Z

We've got no existing precedent for "run-time console warning for deprecated API use detected at run-time".

Having the debug build trap out for this doesn't seem that bad - it's what we did for, say, mutexes. Somewhere around 5.7 debug builds started trapping mutex lock/unlock errors (via EvrRtxMutexError) , and in 5.10 develop builds now do the same (MBED_ASSERT in Mutex::lock()).

Adding extra warning output in develop builds could itself be a compatibility problem anyway.

Personally, I'd be fine with just letting it always error now, rather than trying to drop hints about a later clampdown. Make it

/* No-one should be waiting for milliseconds in non-thread context. */
/* Use `wait_us` if needing to wait in interrupts. */
/* Blocking interrupts for >100us is strongly discouraged and may affect system performance */
MBED_ASSERT(!core_util_is_isr_active() && core_util_are_interrupts_enabled());

Assert isn't even really necessary as you'd hit an assert inside ThisThread::sleep_for() anyway, but this assert at least would lead offending code to a lecturing comment.

deepikabhavnani · 2018-11-01T15:28:04Z

@c1728p9 @bulislaw @pan- Thoughts on new proposal to always error if wait_ms() is used in ISR #8189 (comment)

bulislaw · 2018-11-02T16:14:52Z

It makes more sense. We should avoid this little behaviour changes, but I agree it's a corner case. In any case if someone is using ms wait in ISR somethings not right.

deepikabhavnani · 2018-11-02T17:08:48Z

It makes more sense. We should avoid this little behaviour changes, but I agree it's a corner case. In any case if someone is using ms wait in ISR somethings not right.

@bulislaw And do you have similar concerns with wait API behavior as well, as user can call wait for msec / usec wait times and it can be from ISR as well. In current implementation wait call for milli-sec time will be successful from ISR as well, it will be busy wait

bulislaw · 2018-11-05T15:03:56Z

Yeah our wait story is messy and requires cleanup involving breaking changes, but that should wait till next major version.

deepikabhavnani · 2018-11-08T17:05:28Z

wait_ms implementation with low power ticker was suggested by @bulislaw instead of calling ThisThread::sleepfor(). @c1728p9 @kjbracey-arm - Do you see any concerns with that approach?

c1728p9 · 2018-11-08T18:15:39Z

You'll need to go through the OS to sleep even with the low power ticker. If you call sleep directly then other ready to run threads won't get a chance and low priority threads may never get a chance to run. Because of this I don't think there is much of an advantage to using the low power ticker.

kjbracey · 2018-11-09T08:44:45Z

Not quite following the suggestion - in normal thread use, wait_ms absolutely has to be an OS "sleep this thread", leaving other threads running. (And that comes out as something low power if all threads are idle and NO_SYSTICK).

If called to wait milliseconds from ISR/critical section, and you let them do it, you're spinning the CPU anyway, so lptimer versus ustimer doesn't matter. You're not doing either type of sleep.

cmonr · 2018-11-15T17:50:28Z

@c1728p9 @kjbracey-arm Is conversation done? Sounds like there's still some misunderstandings to sort through.

deepikabhavnani · 2018-11-15T18:40:49Z

@cmonr - Suggestions by @bulislaw are not feasible, need to have conversation.

0xc0170 · 2018-11-20T09:11:04Z

@deepikabhavnani Please keep us updated. We might want to remove 5.11 if this is not ready soon

deepikabhavnani · 2018-11-21T05:36:28Z

@0xc0170 - PR is ready and approved by @bulislaw as well, with some more changes in future.
But considering current CI failures, will like to keep this change for 5.12 and merge it to master in early phase of 5.12.

bulislaw · 2018-11-21T09:56:26Z

As discussed off-line, it may be lesser evil. We'll think about redesigning our wait(delay) APIs.

cmonr · 2018-11-22T02:45:23Z

CI job started.

mbed-ci · 2018-11-22T04:25:34Z

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 15
Build artifacts
Build logs

deepikabhavnani requested review from c1728p9, kjbracey, geky and stevew817 September 19, 2018 19:58

deepikabhavnani force-pushed the wait_updated branch from bce7da1 to 9b8d038 Compare September 19, 2018 20:08

geky approved these changes Sep 19, 2018

View reviewed changes

deepikabhavnani force-pushed the wait_updated branch from 9b8d038 to aa906ce Compare September 19, 2018 21:51

c1728p9 reviewed Sep 19, 2018

View reviewed changes

0xc0170 added the needs: review label Sep 20, 2018

kjbracey reviewed Sep 20, 2018

View reviewed changes

deepikabhavnani mentioned this pull request Sep 20, 2018

NRF52832 not going back to sleep completely after a wait_ms() call #7847

Closed

TacoGrandeTX mentioned this pull request Sep 20, 2018

nrf52 power regression using wait_ms #7763

Closed

deepikabhavnani force-pushed the wait_updated branch 2 times, most recently from 21c35eb to 25dafb7 Compare September 20, 2018 20:46

deepikabhavnani commented Sep 20, 2018

View reviewed changes

kjbracey reviewed Sep 21, 2018

View reviewed changes

0xc0170 requested review from bulislaw and donatieng September 21, 2018 12:14

deepikabhavnani force-pushed the wait_updated branch from 25dafb7 to 22114e7 Compare September 21, 2018 14:53

c1728p9 reviewed Sep 21, 2018

View reviewed changes

deepikabhavnani force-pushed the wait_updated branch from 22114e7 to 3c8b8c6 Compare September 28, 2018 18:50

pan- reviewed Oct 2, 2018

View reviewed changes

c1728p9 mentioned this pull request Oct 4, 2018

Error path tightening: use MBED_NORETURN; add+use core_util_atomic_flag #8328

Merged

adbridge added the release-version: 5.11.0-rc1 label Oct 12, 2018

kjbracey changed the title ~~Wait API updated to remove deepsleep lock~~ pWait API updated to remove deepsleep lock Nov 2, 2018

kjbracey changed the title ~~pWait API updated to remove deepsleep lock~~ Wait API updated to remove deepsleep lock Nov 2, 2018

jeromecoutant mentioned this pull request Nov 8, 2018

Improve RTOS behavior with deep sleep #8223

Merged

deepikabhavnani added needs: work and removed needs: review labels Nov 8, 2018

NirSonnenschein added the risk: A label Nov 18, 2018

bulislaw approved these changes Nov 21, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Nov 21, 2018

0xc0170 added ready for merge and removed needs: CI labels Nov 22, 2018

0xc0170 merged commit 70dfbbf into ARMmbed:master Nov 22, 2018

0xc0170 removed the ready for merge label Nov 22, 2018

deepikabhavnani mentioned this pull request Mar 6, 2019

Use LP tickers for waiting in no RTOS builds when available #9960

Merged

		}

		/* The millisec value specifies the number of timer ticks and is therefore an upper bound.

Wait API updated to remove deepsleep lock #8189

Wait API updated to remove deepsleep lock #8189

Conversation

deepikabhavnani commented Sep 19, 2018 • edited Loading

Description

Pull request type

geky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kjbracey commented Sep 20, 2018 • edited Loading

deepikabhavnani commented Sep 20, 2018

deepikabhavnani Sep 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepikabhavnani Sep 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepikabhavnani commented Sep 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepikabhavnani commented Sep 28, 2018

pan- left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepikabhavnani commented Oct 3, 2018

kjbracey commented Oct 4, 2018

bulislaw commented Nov 1, 2018

kjbracey commented Nov 1, 2018 • edited Loading

deepikabhavnani commented Nov 1, 2018

bulislaw commented Nov 2, 2018

deepikabhavnani commented Nov 2, 2018

bulislaw commented Nov 5, 2018

deepikabhavnani commented Nov 8, 2018

c1728p9 commented Nov 8, 2018

kjbracey commented Nov 9, 2018

cmonr commented Nov 15, 2018

deepikabhavnani commented Nov 15, 2018

0xc0170 commented Nov 20, 2018

deepikabhavnani commented Nov 21, 2018

bulislaw commented Nov 21, 2018

cmonr commented Nov 22, 2018

mbed-ci commented Nov 22, 2018

deepikabhavnani commented Sep 19, 2018 •

edited

Loading

kjbracey commented Sep 20, 2018 •

edited

Loading

deepikabhavnani Sep 20, 2018 •

edited

Loading

deepikabhavnani Sep 21, 2018 •

edited

Loading

pan- left a comment •

edited

Loading

kjbracey commented Nov 1, 2018 •

edited

Loading