Skip to content

drivers: timer: Use global sys_clock_hw_cycles_per_tick in nrf_rtc_tmer. #8249

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.

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

Merged
merged 1 commit into from
Jun 11, 2018

Conversation

pizi-nordic
Copy link
Collaborator

@pizi-nordic pizi-nordic commented Jun 7, 2018

The nrf_rtc_timer used own method to calculate number of timer cycles
per tick. As the result value was different than the one used by
the kernel, the reported tick time was incorrect.

This commit fixes the problem described above by using sys_clock_hw_cycles_per_tick
instead of custom RTC_TICKS_PER_SYS_TICK define.

Signed-off-by: Piotr Zięcik [email protected]

Note: Together with #8259 this commit solves timer issues on Nordic SoCs in non-tickless mode. Patches for tickless configuration will follow.

@codecov-io
Copy link

codecov-io commented Jun 7, 2018

Codecov Report

Merging #8249 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8249   +/-   ##
=======================================
  Coverage   64.53%   64.53%           
=======================================
  Files         420      420           
  Lines       40122    40122           
  Branches     6763     6763           
=======================================
  Hits        25892    25892           
  Misses      11110    11110           
  Partials     3120     3120

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e89206...56c2c82. Read the comment docs.

@pizi-nordic pizi-nordic requested review from nvlsianpu, cvinayak, carlescufi and anangl and removed request for nvlsianpu June 7, 2018 12:49
/* Maximum difference for RTC counter values used. Half the maximum value is
* selected to be able to detect overflow (a negative value has the same
* representation as a large positive value).
*/
#define RTC_MASK 0x00FFFFFF
Copy link
Member

Choose a reason for hiding this comment

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

This should not be moved here, since the comment above is for RTC_HALF, not for this one.
Perhaps it would be better to move it to the line after #define RTC_CC_EVENT.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@anangl: Fixed. Thank you for spotting that. I misunderstood comment and thought that it should be moved.

@carlescufi
Copy link
Member

Delegating this one to @cvinayak and @ioannisg

@@ -55,6 +49,8 @@ static u32_t expected_sys_ticks;
s32_t _get_max_clock_time(void);
#endif /* CONFIG_TICKLESS_KERNEL */

extern s32_t _sys_idle_elapsed_ticks;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it redundant? It is declared in system_timer.h which is included above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

The nrf_rtc_timer used own method to calculate number of timer cycles
per tick. As the result value was different than the one used by
the kernel, the reported time was incorrect.

This commit fixes the problem described above by using global
sys_clock_hw_cycles_per_tick instead of custom RTC_TICKS_PER_SYS_TICK.

Signed-off-by: Piotr Zięcik <[email protected]>
@nashif nashif merged commit 3f3a907 into zephyrproject-rtos:master Jun 11, 2018
@pizi-nordic pizi-nordic deleted the nrf-rtc-timer-fix branch June 15, 2018 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants