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.

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

Some drivers return invalid z_timer_cycle_get_32() value #31510

Closed
sslupsky opened this issue Jan 22, 2021 · 1 comment
Closed

Some drivers return invalid z_timer_cycle_get_32() value #31510

sslupsky opened this issue Jan 22, 2021 · 1 comment

Comments

@sslupsky
Copy link
Contributor

sslupsky commented Jan 22, 2021

I noticed what I believe is an issue with the z_timer_cycle_get() function and the corresponding drivers that implement this function.

From the Zephyr documentation, the kernel presents a “cycle” count via the k_cycle_get_32() API. The frequency of this counter is required to be steady over time, and is available from sys_clock_hw_cycles_per_sec() (which on almost all platforms is a runtime constant that evaluates to CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC).

Moreover, for asynchronous timekeeping, the kernel defines a “ticks” concept. A “tick” is the internal count in which the kernel does all its internal uptime and timeout bookeeping. Interrupts are expected to be delivered on tick boundaries to the extent practical, and no fractional ticks are tracked. The choice of tick rate is configurable via CONFIG_SYS_CLOCK_TICKS_PER_SEC.

It appears from inspection of some drivers they mistakenly return clock ticks as hardware cycles. For instance, the nrf_rtc_timer driver returns clock ticks from z_timer_cycle_get_32() determined as follows:

	uint32_t ret = counter_sub(counter(), last_count) + last_count;

The sam0_rtc_timer driver similarly returns clock ticks from z_timer_cycle_get_32() determined as follows:

	ret = rtc_count();

I suspect other drivers are affected by this issue as well. I think people have worked around this issue in the past by setting CONFIG_SYS_HW_CYCLES_PER_SEC to the cycle rate of the rtc timer. That does not appear to be a correct interpretation of how the kernel intends to use the hw cycle timer.

Thus there is a problem here that needs to be resolved in one of two ways:

  1. Fix the drivers so that they correctly determine the hw cycles. This will require the driver to control two different counters. For instance the rtc counter and the SysTick counter.
  2. Remove the hw cycles function from the offending drivers and create a new class of driver specifically for tracking hw cycles. That is, separate the clock timer driver API from the hw cycle timer API. Some work would have to be done here to work out how to exclude overlapping functions of the two drivers. That is, SysTick may be used for both hw cycle counting and clock cycle counting. So, if using the SysTick for hw cycle counting and the rtc for clock cycle counting, there needs to be a way to turn off the clock cycle counting of the SysTick driver.
@sslupsky
Copy link
Contributor Author

I think there is a third way to fix this. Convert return value of z_timer_cycle_get_32() to cycles using k_ticks_to_cyc. Will close for now.

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

No branches or pull requests

1 participant