Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure us_ticker is initialized before it is used #5194

Merged
merged 2 commits into from Oct 2, 2017
Merged

Ensure us_ticker is initialized before it is used #5194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8183749
Initialize the HAL ticker on first read
c1728p9 Sep 25, 2017
114d60c
Remove direct use of us ticker from platform
c1728p9 Sep 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions hal/mbed_ticker_api.c
View file
Open in desktop
Expand Up @@ -263,6 +263,7 @@ timestamp_t ticker_read(const ticker_data_t *const ticker)

us_timestamp_t ticker_read_us(const ticker_data_t *const ticker)
{
initialize(ticker);
update_present_time(ticker);
return ticker->queue->present_time;
}
Expand Down
3 changes: 1 addition & 2 deletions platform/mbed_retarget.cpp
View file
Open in desktop
Expand Up @@ -1049,9 +1049,8 @@ void operator delete[](void *ptr)
extern "C" clock_t clock()
{
_mutex->lock();
clock_t t = us_ticker_read();
clock_t t = ticker_read(get_us_ticker_data());
t /= 1000000 / CLOCKS_PER_SEC; // convert to processor time
_mutex->unlock();
return t;
}

5 changes: 3 additions & 2 deletions platform/mbed_wait_api_no_rtos.c
View file
Open in desktop
Expand Up @@ -30,8 +30,9 @@ void wait_ms(int ms) {
}

void wait_us(int us) {
uint32_t start = us_ticker_read();
while ((us_ticker_read() - start) < (uint32_t)us);
const ticker_data_t *const ticker = get_us_ticker_data();
uint32_t start = ticker_read(ticker);
while ((ticker_read(ticker) - start) < (uint32_t)us);
}

#endif // #ifndef MBED_CONF_RTOS_PRESENT
Expand Down
9 changes: 7 additions & 2 deletions platform/mbed_wait_api_rtos.cpp
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@
#include "hal/us_ticker_api.h"
#include "rtos/rtos.h"
#include "platform/mbed_critical.h"
#include "platform/mbed_sleep.h"

void wait(float s) {
wait_us(s * 1000000.0f);
Expand All @@ -32,15 +33,19 @@ void wait_ms(int ms) {
}

void wait_us(int us) {
uint32_t start = us_ticker_read();
const ticker_data_t *const ticker = get_us_ticker_data();

uint32_t start = ticker_read(ticker);
// Use the RTOS to wait for millisecond delays if possible
int ms = us / 1000;
if ((ms > 0) && core_util_are_interrupts_enabled()) {
sleep_manager_lock_deep_sleep();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation of wait doesn't allow deep sleep, since the us ticker must be running. Two possible solutions to this are:

  • Make distinction between busy wait and suspended wait (encourage use of Thread::wait again)
  • Use Thread::wait for ms part and busy wait for remainder (start time would need to be read after Thread::wait)

Anyone have a preference or other ideas on how to handle this?

Copy link
Contributor

@0xc0170 0xc0170 Sep 26, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation of wait doesn't allow deep sleep, since the us ticker must be running.

tickers are always running (only initialize function present), are we getting API to stop/resume them?

Make distinction between busy wait and suspended wait (encourage use of Thread::wait again)

From a user perspective, what API would be preferable ?

If I consider the second one (ms part and busy), what about these:

wait_us() - Busy wait with higher accurancy (microseconds) always blocking, no Thread:wait involved. We are specifically requesting high precision and be blocking. Smaller periods (limits to 32bit microseconds).
wait_ms() + wait() (this would not allow us accuracy?) - lower accuracy (task switch allowed), plus tickless possible. Bigger periods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Largely agree with this proposal - see #5216.

Only caveat is that it is something of an API change - wait_ms will wait up to that number of milliseconds, being potentially low by up to a millisecond, rather than waiting at least that number of milliseconds.

cf confusion when RTX changed a similar function: http://www.keil.com/support/docs/3766.htm and linked threads.

Although in this case the change would be towards conventional/expected behaviour, rather than away from.

Thread::wait((uint32_t)ms);
sleep_manager_unlock_deep_sleep();
}
// Use busy waiting for sub-millisecond delays, or for the whole
// interval if interrupts are not enabled
while ((us_ticker_read() - start) < (uint32_t)us);
while ((ticker_read(ticker) - start) < (uint32_t)us);
}

#endif // #if MBED_CONF_RTOS_PRESENT
Expand Down