Skip to content

Big timer API refactoring #10160

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

Closed
wants to merge 13 commits into from
Closed

Big timer API refactoring #10160

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
65a0211
sys_clock.h: Remove asm guards
Sep 19, 2018
3f9d566
kernel.h: Header hygine, move clock/timer handling
Sep 19, 2018
ce4a658
sys_clock.h: Make "global variable" APIs into proper functions
Sep 19, 2018
edd31be
clock: Remove CONFIG_TICKLESS_KERNEL_TIME_UNIT_IN_MICRO_SECS
Sep 19, 2018
7c8af14
sys_clock.h: Remove sys_clock_ticks_per_sec()
Sep 19, 2018
8fb84e0
sys_clock: Make sys_clock_hw_cycles_per_tick() a proper API
Sep 19, 2018
8c1fcf1
sys_clock: Make clock_always_on true by default
Sep 19, 2018
8d8b365
sys_clock: Fix unsafe tick count usage
Sep 19, 2018
5cc3607
system_timer.h: Remove ASMLANGUAGE guard
Sep 19, 2018
c2f9c62
sys_clock: Fix up tick announce API
Sep 19, 2018
93d2440
drivers/timer: Unify timeout setting APIs
Sep 20, 2018
840922a
include/system_timer.h: Timer API cleanup
Sep 21, 2018
84ff95f
timer: Clean up hairy tickless APIs
Sep 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions include/kernel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1554,26 +1554,16 @@ __syscall s64_t k_uptime_get(void);
/**
* @brief Enable clock always on in tickless kernel
*
* This routine enables keeping the clock running when
* there are no timer events programmed in tickless kernel
* scheduling. This is necessary if the clock is used to track
* passage of time.
* This routine enables keeping the clock running (that is, it always
* keeps an active timer interrupt scheduled) when there are no timer
* events programmed in tickless kernel scheduling. This is necessary
* if the clock is used to track passage of time (e.g. via
* k_uptime_get_32()), otherwise the internal hardware counter may
* roll over between interrupts.
*
* @retval prev_status Previous status of always on flag
*/
static inline int k_enable_sys_clock_always_on(void)
{
#ifdef CONFIG_TICKLESS_KERNEL
int prev_status = _sys_clock_always_on;

_sys_clock_always_on = 1;
_enable_sys_clock();

return prev_status;
#else
return -ENOTSUP;
#endif
}
int k_enable_sys_clock_always_on(void);

/**
* @brief Disable clock always on in tickless kernel
Expand All @@ -1583,12 +1573,7 @@ static inline int k_enable_sys_clock_always_on(void)
* scheduling. To save power, this routine should be called
* immediately when clock is not used to track time.
*/
static inline void k_disable_sys_clock_always_on(void)
{
#ifdef CONFIG_TICKLESS_KERNEL
_sys_clock_always_on = 0;
#endif
}
void k_disable_sys_clock_always_on(void);

/**
* @brief Get system uptime (32-bit version).
Expand Down
23 changes: 22 additions & 1 deletion kernel/sys_clock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ volatile u64_t _sys_clock_tick_count;
* system clock to track passage of time without interruption.
* To save power, this should be turned on only when required.
*/
int _sys_clock_always_on;
int _sys_clock_always_on = 1;

static u32_t next_ts;
#endif
Expand Down Expand Up @@ -341,3 +341,24 @@ void _nano_sys_clock_tick_announce(s32_t ticks)
}
#endif
}

int k_enable_sys_clock_always_on(void)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO if we are not in tickless mode we we should return:

  • 1 from k_enable_sys_clock_always_on()
  • -ENOTSUP from k_disable_sys_clock_always_on() (requires API change).

Now the API tells that you can always disable this feature, but re-enabling might be not supported.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, though a little out of scope for this particular pull request. As mentioned in the commit note, this API is sort of broken by design. Personally I'd argue we want to remove it in general and just replace it with a kconfig where the app can tell the kernel whether it wants to allow the clock to lie after system idle or not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for Kconfig option.

{
#ifdef CONFIG_TICKLESS_KERNEL
int prev_status = _sys_clock_always_on;

_sys_clock_always_on = 1;
_enable_sys_clock();

return prev_status;
#else
return -ENOTSUP;
#endif
}

void k_disable_sys_clock_always_on(void)
{
#ifdef CONFIG_TICKLESS_KERNEL
_sys_clock_always_on = 0;
#endif
}
2 changes: 0 additions & 2 deletions samples/philosophers/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ static s32_t get_random_delay(int id, int period_in_ms)
* and the current uptime to create some pseudo-randomness. It produces
* a value between 0 and 31.
*/
k_enable_sys_clock_always_on();
s32_t delay = (k_uptime_get_32()/100 * (id + 1)) & 0x1f;
k_disable_sys_clock_always_on();

/* add 1 to not generate a delay of 0 */
s32_t ms = (delay + 1) * period_in_ms;
Expand Down
2 changes: 0 additions & 2 deletions subsys/net/lib/sntp/sntp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ static u32_t get_uptime_in_sec(void)
{
u64_t time;

k_enable_sys_clock_always_on();
time = k_uptime_get_32();
k_disable_sys_clock_always_on();

return time / MSEC_PER_SEC;
}
Expand Down