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

riscv-peripheral: add standard peripherals #124

Open
2 of 3 tasks
romancardenas opened this issue Feb 20, 2023 · 12 comments
Open
2 of 3 tasks

riscv-peripheral: add standard peripherals #124

romancardenas opened this issue Feb 20, 2023 · 12 comments

Comments

@romancardenas
Copy link
Contributor

romancardenas commented Feb 20, 2023

  • PLIC
  • (A)CLINT
  • CLIC
@romancardenas romancardenas changed the title Add "standard" peripherals Add standard peripherals Mar 21, 2023
@techmccat
Copy link

techmccat commented Jul 13, 2023

I was planning on writing some initial peripheral code for the CLIC, but since it's not standardized yet i'm not sure how to go about it.
Ideally this generic version should be implemented against the latest draft spec, but there's already hardware out implementing older versions of the spec (my BL602 dev board has a SiFive E24 core) which seem to have incompatible register definitions.

@romancardenas
Copy link
Contributor Author

Good catch, not sure about how to proceed with this. I suggest you implement the peripheral for your board, so you can test it. Then we can use feature flags to select a pre-release CLIC or a standard CLIC once done. In any case, please consider using the same approach as in #135, which is quite different from the one currently used in master for PLIC.

@techmccat
Copy link

Wrote some initial code and pushed it to https://github.com/techmccat/riscv/tree/sifive-clic

Needs some work on CLINT (in)compatibility and separating che common code

@techmccat
Copy link

After getting around to porting bl602-hal to vectored mode I think the volatile-register approach might be better than the one used in #135, mainly because it allows to interact with the interrupt controller via associated functions, like how it's done in the cortex-m crate with the NVIC, without needing to keep a global CLIC struct or instantiating one whenever you need it.

@romancardenas
Copy link
Contributor Author

Hmm I see... the main issue with the volatile-register approach and the Rust ecosystem is that base addresses of peripherals are target-dependent. Therefore we need to deal with const generics and type aliases everywhere. I'm open to having a discussion about this and evaluating pros and cons, let me know your opinions!

@almindor
Copy link
Contributor

Hmm I see... the main issue with the volatile-register approach and the Rust ecosystem is that base addresses of peripherals are target-dependent. Therefore we need to deal with const generics and type aliases everywhere. I'm open to having a discussion about this and evaluating pros and cons, let me know your opinions!

This might be a good discussion for the embedded-group weekly. Could you bring this up on an upcoming meeting?

@romancardenas
Copy link
Contributor Author

I'm on vacation and I'll be traveling by the time of the meeting. I may have an Internet connection and be able to join, but I'm not 100% sure. @almindor are you available? Or should we leave it to the next week?

@almindor
Copy link
Contributor

I'm on vacation and I'll be traveling by the time of the meeting. I may have an Internet connection and be able to join, but I'm not 100% sure. @almindor are you available? Or should we leave it to the next week?

Yeah no rush on this, let's find a date that works and present it then.

@romancardenas
Copy link
Contributor Author

As discussed in the embedded-group weekly meeting, we will use a raw pointer-based approach for RISC-V peripherals, mainly because of the different unsoundnesses of the volatile-register crate. However, I appreciate your comments, @techmccat, and I'm working on a "middle-way" that allows you to use associated functions.

RISC-V peripherals will be implemented in my personal riscv-peripheral repo to speed up the development process. This repo is still "experimental". Thus, we can explore different approaches to evaluating ergonomics for the end-users. Please, take a look and let me know what you think. You will see that now all the functions are associated, and a per-peripheral trait is provided to define the base address and ensure safety. I'll probably end up developing macros to make PACs' life even easier and allowing both associated functions and getter-setters, so everyone is happy.
Once we have a nice interface for the peripherals, I will ask the @rust-embedded/risc-v group to maintain the peripherals repo (or maybe merge it to this repo, we'll see).

Of course, comments, PRs, suggestions, etc. are more than welcome!

@romancardenas
Copy link
Contributor Author

In case you are curious, I pushed a macro for generating CLINT peripherals and easing the access to mtimecmp registers for the different HARTS, you can take a look here (at the end of the file there is a test to give you an idea of the ergonomics).

I'll do something similar for the PLIC peripheral too in the near future.

@romancardenas
Copy link
Contributor Author

romancardenas added a commit that referenced this issue Nov 17, 2023
Check CHANGELOG.md on PRs to master
@romancardenas
Copy link
Contributor Author

Related PR: #164

@romancardenas romancardenas changed the title Add standard peripherals riscv-peripheral: add standard peripherals Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants