-
Notifications
You must be signed in to change notification settings - Fork 188
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
Port to Zephyr v3.5.0 + Fix System Timer #111
Conversation
Fantastic! This has been on my wish list for a long time. I haven't had time to look at your work yet, but regarding the msleep issue, there's one thing I would check first. At least in Zephyr 2.4.0, there's a bug that causes Hope to take a closer look at your work in the coming days |
5463e63
to
ff5b25f
Compare
This looks good to me. I did however come across a bug when running the dining philosophers demo.
I don't know if this is a new problem or if the checks in Zephyr have just become stricter, but I want to hold off merging this for a bit to investigate. |
Can confirm that there is a problem here. With the corresponding check removed, the demo runs, but somehow only two of the six philosophers seem to eat:
Like you said, hold off merging this until we fix this and the timer overflow issue. |
Interesting. On a tickful kernel, they never pick up or drop any forks at all. It just locks up after the initial prints. Something is very wrong here 🙈 |
I think we have several problems. I just realigned serv_timer to the upstream riscv_machine_timer from zephyr 3.5.0 and there were quite a few changes. But I also believe there are other problems in the RTL and/or the board support code. One thing I have knowingly cheated with in the RTL is to not make the MPIE bit in the MSTATUS register accessible from software. Perhaps this worked before, but not any more. I also remember that a lot of the files in the SERV-specific zephyr/soc directory had to be copied there because of some limitation in Zephyr when using an external soc dir. Maybe these files need to be refreshed (or best of all removed!) now. |
Oof, you're probably right. I just noticed that not even printf is working properly. It prints something like "0g0" when I try to use %d or %x. I feel like most of our issues at this point are not interrupt related even, but I have no idea how to get started troubleshooting this right now. |
Okay that problems seems to be because I still have MUL instructions in picolibc code. As far as I know, that library is precompiled and ships with their toolchain (I'm on v0.16.4). |
I see. Perhaps it's easier in that case to just build Servant with the M extension enabled. First add the mdu library with |
Yup, that fixes this issue. So that's most likely an upstream bug. |
Ok, that's a relief at least |
Oh, neat, that also improves the philosophers situation. It's still unhappy about the assertion, but once it is removed (in |
Yup, this seems to also fix the 10-minute lockup (has been running for half an hour now with multiple overflows). So I'll have to apologize to @alinja, PR #109 seems to do what it promises, the overflow works. (Again, I didn't check that there aren't any weird timing artifacts, but the basic functionality looks good.) Log, 1 s sleep: hello_world_with_timer.log |
Ok, so to sum it up, things seems to work just fine now with Zephyr 3.5.0, except for the assert crash? If so, I think we can merge it. That assert thing is likely another problem and should be analyzed separately. |
I agree, this should be good to merge. I think it's a good idea to open an Issue regarding the assertion just to keep track of it, and the (possibly upstream) compiler issue I'll try to chase down later in the week. |
I filed #113 now to keep track of that issue. Thanks for working on this. Picked and pushed! |
I did a quick and dirty port to the latest Zephyr OS v3.5.0.
Here's the
hello_world
sample running on servant in verilator:To make this cleaner, one should add the registers for the UART (and timer) to the device tree, and read them from there, but I see this as a separate clean-up commit rather than part of this port. If I have the time, I will implement that and open a separate PR for that once this one is merged. Also, we should probably be able to re-use/include some generic code from the RISC-V upstream repo, but again, I think this is a separate PR. Let me know what you think.
EDIT: Also fixes #112 by calling the timer init function using the
SYS_INIT
macro.