-
Notifications
You must be signed in to change notification settings - Fork 3
Added control for which access type to use #3
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds support for controlling which access type properties (software vs hardware) the Rust code generator uses when determining register and field access permissions. This is particularly useful when the Rust code runs on the hardware side of an I2C register map where the software interface is external to the Rust environment.
Key Changes:
- Added
access_modeparameter to the exporter with three options: "software" (default), "hardware", and "read_only" - Modified access determination logic to respect the selected access mode
- Added validation to ensure only valid access modes are accepted
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_access_mode.py | New test file verifying all three access modes work correctly and invalid modes are rejected |
| tests/rdl_src/access_modes.rdl | Test RDL file with various software/hardware access combinations to validate the feature |
| src/peakrdl_rust/utils.py | Updated reg_access() and field_access() functions to accept and use access_mode parameter |
| src/peakrdl_rust/exporter.py | Added documentation for the new access_mode parameter |
| src/peakrdl_rust/design_state.py | Added validation logic for access_mode and passes it to the ContextScanner |
| src/peakrdl_rust/component_context.py | Updated ContextScanner to accept and propagate access_mode to all access determination calls |
| docs/configuring.rst | Added comprehensive documentation for the new access_mode configuration option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @joecrop , thanks for the PR. This is an interesting use case that I haven't encountered before. Can you elaborate a bit on when you'd want to use the hardware access modes for the software generated by this exporter? Similarly for the read-only registers. What's the use case for the feature? |
|
We have a chip that has an I2C target interface that it exposes to an external host. The I2C registers are written to by "hardware" and read from by "software". In our case, the "software" side is an I2C host microcontroller (not necessarily written in Rust). The "hardware" side is our Rust code running on the I2C target microcontroller. So, we want the the Rust code on our microcontroller to have write access, but the external microcontroller should have read access. Does that make sense? We also have a use case where we create a special I2C (or other bus interface) that is truly read-only. For example, we have a 'helper' microcontroller in a safety application that wants to monitor the status of the main microcontroller, but this monitoring should be read-only, regardless of if the registers are allowed to be written of not. So, I added a read-only access mode for convince so that we don't have to have a separate RDL file for this mode... |
|
Ok, I can see the use case. But would this setup require an alternate implementation of the read/wire memory accesses? Right now all register reads/writes are non volatile pointer accesses. Would your use case require an interface to translate reads/writes to i2c commands? As for the read-only registers, I'm ok adding a flag for this. But I think it should just remove write access, not make everything readable. I believe your current implementation changes write-only fields to read-only, which won't work. |
In some cases, the access types used by the generator should be hardware instead of software. For example, when the Rust code is used on the hardware side of an I2C register map whose software interface is external to the Rust environment. In this case, we should use the hardware access types. I added a parameter to the generator to select which access types to choose from, and also added a read-only type that sets all registers to read only for the purpose of an observation-only interface.