-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Revert "drivers/timer/hpet: Extend qemu workaround" #32790
Conversation
This reverts commit 4ae44dd. See: zephyrproject-rtos#32724 Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Not opposed to merging this, but are we absolutely sure it's actually happening in master and not just on a PR branch, and that this is the sole source? I'm concerned about voodoo here based on one or two results, basically. Again, on my build machine this platform is absolutely rock solid; literally no visible failures at all over hundreds of trials. This particular fix wasn't a common failure mode, but it was the source of actual observed rollover conditions in the z_clock_set_timeout() path. I'm not opposed to reverting it, really, I just worry we're looking at the wrong thing. |
I share your concerns, but https://buildkite.com/zephyr/zephyr/builds/22543#185a9bbb-68d0-4c47-8b58-73e4a4d39059 shows failures on these configurations:
many of which should not have been affected by the k_work changes. They go away with this patch, and were never observed until the reverted commit was added to master. I'd be more comfortable if we could find the similar failures on other PRs, but since they're intermittent I think people just forced a re-run and they disappeared. |
FWIW: I just tried the reversion on my rig: 22 runs with one failure (seemingly timing-related thing), which seems acceptable. I also tried a variant with the entire workaround removed (including the pre-existing one) and got no failures in 8 more runs. So there's really no reason I can see not to merge this this given that it's just a workaround for testing configurations anyway. |
This reverts commit 4ae44dd.
Fixes #32724. See also: #29618 (comment)