-
Notifications
You must be signed in to change notification settings - Fork 190
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
Peripheral codegen for Grapefruit / Cosmo #2010
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.
Left a minor comment about some other access types we might see. We have a lot of other stuff that could be exposed from the RDL side, but probably best to deal with it along-and-along as we start experiencing them.
Excited about making this more 1st class!
name: String, | ||
value: u8, | ||
} | ||
|
||
#[derive(Debug, Deserialize)] | ||
#[serde(rename_all = "lowercase")] | ||
pub enum SwAccess { |
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.
As a note, there are a few more possible types here: https://systemrdl-compiler.readthedocs.io/en/stable/api/types.html#systemrdl.rdltypes.builtin_enums.AccessType
that could show up.
We also don't do much with the OnRead and OnWrite types described below that section but am open to doing so if there's code-gen value.
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.
Interesting, I'm going to leave those as unimplemented so that they'll fail loudly if we ever end up using them!
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.
Minor questions but LGTM
@@ -67,14 +65,16 @@ fn main() -> ! { | |||
// Idol loop after the FPGA has been brought up. | |||
let seq = | |||
drv_spartan7_loader_api::Spartan7Loader::from(LOADER.get_task_id()); | |||
seq.ping(); |
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.
Intentional drop of the ping?
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.
Yes, the call to seq.get_token()
is what ensures that the FPGA is loaded.
_ => { | ||
variant_errors.push(compile_error( | ||
t.span(), | ||
&format!( | ||
"unexpected token {t:?}; expected #[idol(...)]" |
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'm guessing most of this is the fallout from syn
update?
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.
Yeah, they revamped the meta syntax tree in v2
15b8bfc
to
4225bea
Compare
This PR adds a new target to
build-fpga-regmap
, generating Rust code for a memory-mapped peripheral. For Grapefruit and Cosmo, the Spartan-7 FPGA is connected over FMC, so we can treat peripherals implemented in the FPGA as native(ish). This PR uses the new codegen paths forcosmo-hf
andgrapefruit-seq
, replacing raw addresses. The peripheral types take adrv_spartan7_loader_api::Spartan7Token
, which proves that the FPGA is loaded and running.Other changes that came along for the ride:
drv/spartan7-loader/grapefruit/
subfolder. I also updated to a more recent build (latestmain
), and removed the HTML file (since the JSON is smaller + has the same information).syn
is bumped to version 2, which requires a few miscellaneous changes