-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implement RFC 16 (CSR register API) #40
Conversation
Rebased on the |
Rebased back to |
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 really excited to see this. It will remove a lot of fiddly and annoying code work, and there are a lot of interesting things that can be done by combining with the component metadata work, among other things.
But it's still rather hard to get into. It felt like an uphill battle trying to inventory all the pieces (Register
s, Field
s, Element
s, Signature
s, Interface
s, csr.Interface
s, Multiplexer
s, Bridge
s) and figure out who is supposed to connect to what and how. I figured it out after a while by diving through the source along with some brute forcing but that's not a pleasant experience.
While all the bits do seem to work logically and properly (once I could see the logic), I would have a hard time personally recommending a merge without at least an illustrated example design to show how the pieces are supposed to fit together, as now there are even more pieces and even more ways to get things wrong compared to just managing Element
s. I'm happy to share my design and help guide such an example.
The RFC text could use an update too, to correct for the changes introduced by the incorporation of lib.wiring
.
I've put a few comments on simple changes that would make things more convenient to use too.
f5bfa22
to
be1f028
Compare
Updated to implement the final version of amaranth-lang/rfcs#16. |
Before this commit, CSR elements were added with `Multiplexer.add()`. Now, a memory map of elements is populated beforehand, then given as argument to `Multiplexer.__init__()`. This decouples the definition of a group of neighboring registers from their implementation by a CSR Multiplexer.
Also, fix FieldPort signature direction.
Before this commit, a Register whose fields were variable annotations couldn't be instantiated more than once, as the latter belonged to the global scope. The Field class is now a factory for user-defined field components, instead of a base class. Every FieldMap or FieldArray sharing a Field will use a different instance of its component, obtained from Field.create(). Also: * update to follow RFCs 37 and 38 * docstring and diagnostics improvements.
This allows csr.Register subclasses that use variable annotations to define their access mode without overriding __init__(). In addition, the default value of access= (which was "rw") is removed. The access mode of a register has an influence on the rest of the SoC and should therefore be explicitly set. As a consequence of access= being a required __init_subclass__() kwarg, the csr.Register class can no longer be directly instantiated and must be subclassed first.
The access= parameter can now be set in either __init_subclass__() or __init__(). It has a default value of None, and __init__() will error out if set in both sites or none of them.
Also, csr.Multiplexer now requires the signature of its memory map resources to contain a csr.Element.Signature member, named "element" and oriented as output.
This PR implements the CSR register RFC.
csr.Element
away fromRecord
Field
csr.action.R
,csr.action.W
,csr.action.RW
,csr.action.RW1C
,csr.action.RW1S
)FieldActionMap
,FieldActionArray
)Register
Builder
Bridge