-
Notifications
You must be signed in to change notification settings - Fork 94
Add Support for Banana Pi BPI-F3 #202
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
Conversation
|
TIMER tests are not passing with these changes and I would like some help with that .. (as its my first time writing timer driver) So, without TIMER when I run sel4test I can see these prints .. But when run TIMER tests, it gets stuck waiting for interrupt I believe ... Some more details: In the DTS I see the timer node as : and accordingly, I've set the addresses over in platsupport/mach/timer.h#L12 Can someone please help me debug this? I can create issue on sel4test for better track if you want. |
|
I can have a look if you can provide a datasheet that describes the timer hardware. It's not an sel4test issue, but an problem with your driver, so it should stay here till proven otherwise. |
Indanz
left a comment
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.
I would add debug printf()s everywhere to check that your code is being executed as expected, both init as well as IRQ handling.
There is not such good details but I'been using these: https://docs.banana-pi.org/en/BPI-F3/SpacemiT_K1_datasheet btw I also reviewed the linux kernel patches and I still see incomplete dts for this hardware (I compared the once in their latest bianbu OS vs what in linux kernel / riscv linux tree has), What I believe is they are still working on finalizing this and that is why we don't see any details so far, specially about peripherals. So, I was thinking can we probably go with seL4/seL4#1535 as it is? as I can confirm all tests passes except timer (I can try skip the tests for this board until timer is fixed?) |
I've kind of localized the issue already, It looks like the addressing I used (as mentioned in the DTS) not correct for TIMER either or the TIMER is not being initialized at all, because every time I read the timer using |
But were your driver functions called at all? Is there a functioning Linux timer driver that works? Is it doing anything you aren't, like enabling or configuring the clock source? Bianbu OS is based on Linux, so the source code should be available. There's a lot of information on those pages, but it's near impossible to find what you're looking for. Not proper datasheets anyway. Where did you got the register definitions from? I would prefer that there is a working timer before merging this. |
not sure but I could not find any for now.
Yes, I believe, btw let me drop the logs from u-boot when I load the sel4 image .. I've added multiple prints to find the issue... View code |
So you don't know the register layout and can't write a timer driver. There is no point in trying to write one if you don't know what registers to read and what they mean!
Well, at least you wired it correctly to the util_libs driver infrastructure, so the problem is not on the seL4 side of things. |
|
The closest thing to a technical reference manual for the Spacemit K1 SoC I can find is here. Unfortunately I can't see a description of the peripheral timers anywhere. It seems the only source of truth would be the Linux driver in their fork. |
No, I meant I didnt find the compatible driver so far and the only K1 SoC details I could find is below as I mentioned earliar ..
same which @Ivan-Velickovic shared. The problem is what I configure (as per their datasheet) it's not working, when read from the timer registers, I get no value. I'll figure this out .. the only reason I posted the problem here thought I would get some help :). |
|
The register layout is described in the user manual, as well as the address map. https://developer.spacemit.com/documentation?token=T7TnwVZz1iPBk1kKwAPc6lyKnNb So the best TRM pdf would be this: https://cloud.spacemit.com/prod-api/release/download/tools?token=K1_User_Manual |
Ah thanks I missed that. |
|
Me too. I found the documents, but not the right timer info. So at first guess, what's seems missing is configuration of the Timer Clock Control Register? |
|
The timer you need to use is probably: and doesn't match the register layout of the timer in this PR. |
|
This is PR is ready to review I believe. Timer tests also pass on sel4test with these changes. |
|
CI was failing, updated as per errors. |
|
Not sure why Style is failing. Can someone please help me with this? Edit: I think because I forced pushed in last commit, it was comparing with branch head and found difference, it should be fixed now. |
Indanz
left a comment
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.
Just some nitpicks, nothing important.
Let me know when you're happy with it. Don't forget to redo a final sel4test run to catch last second mistakes.
Thanks for the review. btw I did not remove the |
|
This PR is ready now. let me know if needed any change from myside, Thanks! |
|
Please squash the history into one commit. Ignore the line too long error for the link to the spec, that's okay, but fix the other errors. |
updated. |
|
You may have tabs instead of spaces and that's what the style checker chokes on. |
I think style fails because of force push as also mentioned in https://github.com/seL4/ci-actions/tree/master/style
|
I don't think we use the |
|
Yes, the style error is genuine, please replace the tabs with spaces. In the For the commit message it is Ok to ignore CI on this one. I'm not a fan of links in commit messages (extremely unlikely that this link is still going to work in 10 years, and no way to update it), but there doesn't seem to be a better kind of reference. What could work is putting the link into a comment in the code somewhere at the top. Then it can at least in theory be updated when it breaks. |
The SpacemiT K1 SoC has multiple timers, the 'Timer1' peripheral is used by this driver. Each timer peripheral consists of three individual counters/timers. Below Timer 0 is called TIMER0 and is used as a counter and Timer 1 is called TIMER1 and used for timeouts. Signed-off-by: Akif Ejaz <akifejaz40@gmail.com>
|
I've updated PR as per suggestions. style showed some tabs in |
|
I think everything is ready now. |

related to seL4/seL4#1535
This driver is based on the implementations in PR #167 and PR #190, with necessary updates, such as address and configuration changes applied according to the current DTS for this platform added here https://github.com/seL4/seL4/blob/772179e4e639fec1c48a01c69ea6cea144cab02a/tools/dts/bananapi-f3.dts