-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
tests: kernel: timer: Fix failing tests for custom k_busy_wait() #73068
base: main
Are you sure you want to change the base?
tests: kernel: timer: Fix failing tests for custom k_busy_wait() #73068
Conversation
0a2f17e
to
80f1268
Compare
/* On other platforms assume the clocks are perfectly aligned. */ | ||
#define BUSY_TICK_SLEW_PPM 0U | ||
/* On other platforms the maximum skew allowed is +/- 2%. */ | ||
#define BUSY_TICK_SLEW_PPM 20000U |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer to make excuses based on specific platform clock behavior. A busy_wait being "custom" doesn't necessarily mean it's inaccurate. See the comment above about nRF, where the skew is an understood thing that can't be reasonably worked around (well, without changing the CPU clock to be a PLL based off the same crystal the timer driver uses vs. the internal oscillator, which AIUI has power impact that Nordic doesn't want to accept on the default config).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer to make excuses based on specific platform clock behavior. A busy_wait being "custom" doesn't necessarily mean it's inaccurate. See the comment above about nRF, where the skew is an understood thing that can't be reasonably worked around (well, without changing the CPU clock to be a PLL based off the same crystal the timer driver uses vs. the internal oscillator, which AIUI has power impact that Nordic doesn't want to accept on the default config).
Okay, how about that?
09c1caa
to
8955679
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
8955679
to
d6ae2a4
Compare
@@ -4,6 +4,8 @@ tests: | |||
- kernel | |||
- timer | |||
- userspace | |||
platform_exclude: | |||
- nrf54h20dk/nrf54h20/cpuppr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's frustrating. Can you file a bug on this, we should probably look at splitting this into two tests then. timer_api is one of the core/most-important/best-validation-smokes tests and we really don't want to be disabling it for technicalities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, and I swear I'm not pointing fingers here, failures in timer_api are often very subtle things that look like "the test is wrong" or "it's just random/unfixable". It's happened again and again that tiny edge case failures on obscure platforms or configurations turn out to be evidence of bugs in the driver or timer subsystem. We really want this test to be on for everyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discovered that it is enough to reduce assertions level like here, however for the timer_behavior
test it won't work. Is it acceptable to skip it for this target?
d6ae2a4
to
532dac2
Compare
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
The nRF54H20 PPR target has not enough memory to run some of kernel/timer tests. Upstream PR: zephyrproject-rtos/zephyr#73068 Signed-off-by: Adam Kondraciuk <[email protected]>
This commit provides an additional threshold value for comparison when a custom k_busy_wait() implementation is chosen.