<tpw_rules>
looks like the RFC has been updated too?
<jfng[m]>
yes, in addition to the aforementioned changes, some classes that where present in the implementation but unspecified in the RFC have been added
<jfng[m]>
namely csr.RegisterMap and csr.Bridge
<tpw_rules>
okay. i think the rfc could use some more separation between fields and actions
<jfng[m]>
the goal of RegisterMap and Bridge is to allow the definition of a group ("cluster") of registers to be decoupled from the notion of an address, which depend on external factors such as bus address and data widths
<jfng[m]>
tpw_rules: the RFC mostly uses the term "field instance" to cover actions, but also FieldMap/FieldArray objects
<jfng[m]>
as the latter are basically instances of a group of fields
<jfng[m]>
when writing it (and also documenting the implementation), i have been bothered by the lack of a better definition than "a Component whose signature has a FieldPort.Signature member named 'port', with an In direction"
<tpw_rules>
(man "field" is really bad for semantic satiation)
<tpw_rules>
i think that should be a tautology if you will. define somewhere an "action" as that
<tpw_rules>
FieldMap/FieldArray are not field instances though?
<jfng[m]>
they are collections of field instances
<tpw_rules>
then what is Field?
<jfng[m]>
a field definition
<jfng[m]>
or type, if you will
<tpw_rules>
why are FieldMap and FieldArray not also that?
<tpw_rules>
does FieldMap's constructor call its passed Field.create() functions?
<jfng[m]>
because they work with field instances; they are created from a dict of Field objects, but they only store their instances (what is returned by Field.create()`)
<tpw_rules>
i talk about the naming because imo it's confusing and hindering my understanding of the RFC and its intent
<tpw_rules>
i don't have any further naming complaints per se however
<cr1901>
I think I'm fine with the RFC as-is, and would like to hear more about tpw_rules' naming concerns if it's not holding up anything else
<tpw_rules>
yes, overall it's good. like i said earlier, "i think the rfc could use some more separation between fields and actions", in order to clarify how the pieces fit together
<tpw_rules>
in a few words, i think logically the actions should be completely separate from the rest.
<jfng[m]>
would there be downsides to making a FieldAction base class, that would inherit from Component and check that the signature is valid (e.g. has a `"port": In(FieldPort.Signature(...))` member) ?
<tpw_rules>
i don't think that's a bad idea, but i don't think it's really my main concern. does the Field constructor (or Field.create() method) already do such a check?
<jfng[m]>
yes, currently
<tpw_rules>
okay. then i don't see any urgent need to make such a class
<jfng[m]>
functionally speaking no, but it would allow us to refer to a field action as an instance of `FieldAction`, and will simplify the mental model users have to make about this API
Bluefoxicy has joined #amaranth-lang
<tpw_rules>
to explain more thoroughly, i see that there are (in sort of hierarchical order) Bridge, Multiplexer, RegisterMap, Element, Register. these all deal with an address space and what happens when you access a particular address. each address has whatever metadata, plus a read/write/readwrite csr.Element.Access correct?
<tpw_rules>
then there's a (hypothetical) FieldAction that actually does something when read/written and is what the user connects their logic too
<tpw_rules>
s/too/to/
<tpw_rules>
and Field connects that to a Register
<jfng[m]>
it depends on the point of view
<jfng[m]>
from an implementation pov, the hierarchy is Bridge -> Multiplexer -> (multiple) Register -> (multiple) field instances (to be named FieldAction?)
<jfng[m]>
from a "description" pov, the hierarchy is MemoryMap -> RegisterMap -> (multiple) FieldMap/FieldArray -> (multiple) Field
<jfng[m]>
the interface between a Multiplexer and a Register is a csr.Element
<jfng[m]>
the interface between a Register and a FieldAction is a FieldPort
<tpw_rules>
okay, i think that's a good manifestation of my troubles. in the RFC the points of view are mixed
<jfng[m]>
there are thus two different access mode types: Element.Access and FieldPort.Access
<tpw_rules>
(and in the older RFCs too)
<tpw_rules>
is FieldPort.Access a thing that would be cared about during description?
<jfng[m]>
no
<jfng[m]>
it is another public-yet-invisible thing
<jfng[m]>
unless users want to define their own field actions, that is
<jfng[m]>
which is very much an intended use-case for the API
<tpw_rules>
does it provide internal value? the "no connection" case seems unnecessary
<jfng[m]>
FieldPort.Access.NC is necessary for reserved fields
<tpw_rules>
i don't understand why
<tpw_rules>
wouldn't you want to annotate the access of their nominal capabilities? all are really Element.RW in some sense i think
<tpw_rules>
Element.Access.RW*
<tpw_rules>
i guess it could be helpful in making sure the action doesn't secretly do something
<jfng[m]>
users can reserve a range of bits for future use, using reserved fields (ResRAW0 and friends)
<tpw_rules>
ye
<tpw_rules>
s
<jfng[m]>
in such a case, you don't want them to actually be instantiated
<jfng[m]>
so their field actions have the "not connected" access mode
<tpw_rules>
they are already though, correct?
<jfng[m]>
and elaborate to an empty module
<jfng[m]>
no; a field action is connected to a register element read path if its port is readable, and write path if writable
<tpw_rules>
that's connected, not instantiated
<jfng[m]>
our built-in reserved fields are neither
<whitequark[cis]>
tpw_rules: that makes the access rules into a proper lattice, and removes the weird case where there is an RW field you're not supposed to read from or write to; it declares intent
<jfng[m]>
tpw_rules: an empty `Module()` is instantiated and left unconnected
<whitequark[cis]>
(NC is the bottom, RW is the top, R and W are between them)
<tpw_rules>
whitequark[cis]: then Element should gain an NC too?
<jfng[m]>
no
<jfng[m]>
or rather, why ?
<tpw_rules>
for propagation of intent
<jfng[m]>
what would be the purpose of a register that is neither readable nor writable ?
<tpw_rules>
afaict the only thing FieldPort.Access.NC does for you is that if you connect to my_reg.f.my_field.port.*, your logic would silently do nothing
<tpw_rules>
as opposed to being read or written
<tpw_rules>
but i would think .port is not to be touched by user code anyway, that's for the Register machinery, correct?
<jfng[m]>
but in the case of a placeholder for a reserved field, that is what you want no ? i also considered just having access=None for such fields, but i think NC is more elegant for the reason Catherine mentioned
<jfng[m]>
tpw_rules: unless users want to define their own custom Register-like class
<tpw_rules>
well it only matters if you are already breaking the rules. as for "an RW field you're not supposed to read from or write to", the distinguishing attribute between all the Res* is how you are supposed to read from or write to them
<tpw_rules>
that would imo fall under "Register machinery". i guess by user code i should have said "action user"
<tpw_rules>
i agree in general with the justification of "it declares intent" too, i just am trying to work out to whom it declares that intent
<jfng[m]>
tpw_rules: policies such as "read-any/write-last" etc, are meant for software drivers, in order to maintain forward or backward compatibility with silicon
<tpw_rules>
would a doc generator label a field with an ResRAWL action as "don't access"?
<jfng[m]>
it should recognize the "ResRAWL" action and document it as such
<jfng[m]>
or rather, the doc generator will not use this at all
<galibert[m]>
It would label it as "reverse-engineering starts here"
<jfng[m]>
it would the kind of metadata that is the topic of RFC 30
<jfng[m]>
use*
<tpw_rules>
yes
zyp[m] has joined #amaranth-lang
<zyp[m]>
you can't not access a single field while accessing the other fields of the same register, what you're not allowed to do is change the value
<zyp[m]>
hence WL = Write Last
<tpw_rules>
well you are for some of them
<tpw_rules>
(namely ResRAW0)
<zyp[m]>
yeah, WI = Writes Ignored can be written with whatever
<zyp[m]>
for W0, you're not allowed to write a nonzero value
<whitequark[cis]>
you still access the field though (since every field is accessed when you read or write the register)
<tpw_rules>
anyway, to conclude my point (sorry): 1. there are a lot of things in the RFC and the "public-yet-invisible thing" are not clear IMO. 2. FieldPort.Access.NC seems to only be used by the internal connection machinery to signify a connection which can be safely made but wouldn't do anything anyway if it were 3. so FieldPort.Access should be removed and replaced with Element.Access
<jfng[m]>
@libera_tpw_rules:catircservices.org a reserved field (e.g. ResRAW0) may be replaced by an actual field in a later silicon revision, while allowing existing software to remain compatible
<whitequark[cis]>
I disagree with 2 and 3
<zyp[m]>
the point of having different policies is handling forward compatibility, telling the user how to access the field in a forward compatible manner
<jfng[m]>
the "public-yet-invisible" parts are what allows user to customize part of the API without having to reimplement it entirely
<tpw_rules>
i'm confused about the disagreement with 2. that's a fact that i've ascertained. do you see future uses not currently present in the RFC, or a use that I missed?
<tpw_rules>
jfng[m]: to be clear i think they should be there, just delineated.
<whitequark[cis]>
FieldPort isn't entirely internal; FieldPort.Signature for example is a public entity that you can introspect
<jfng[m]>
2. a FieldPort.Access.NC connection cannot be made safely; consider a read-only register (Element.Access.R) that has a ResRAW0 field; if the field port was RW, then it would be a hard error
<jfng[m]>
FieldPort.Access.R cannot be a good default value either, as a csr.Element can be write-only
<tpw_rules>
is that a suitable use for a ResRAW0 field type? would such a register become Element.Access.RW in a future version once that field was implemented?
<whitequark[cis]>
(and now JF is explaining my point about it forming a lattice between RW, R, W, NC)
<jfng[m]>
tpw_rules: yes, but changing the element access mode to RW will break the forward compatibility assumption
<jfng[m]>
software drivers will need to be rebuilt, etc
<tpw_rules>
i see the point about it conflicting with a read only register. but based on my current understanding that's the wrong thing to do anyway
<tpw_rules>
but they would have to be once the field is implemented
<whitequark[cis]>
all of the reserved fields specify both read and write behavior while requiring neither connection
<jfng[m]>
tpw_rules: not if the software drivers have defined semantics for the value 0
<whitequark[cis]>
unless there is a mode of access dedicated to such fields, there would have to be some sort of sniffing to go on to make sure both read-only and write-only registers may contain reserved fields at all
<jfng[m]>
jfng[m]: sorry, this is incorrect; no because existing software drivers would ignore that field
<jfng[m]>
(whereas for ResR0W*, they would need to have defined semantics for 0 to remain forward compatible)
<tpw_rules>
i think the solution i'm coming to is that the reserved actions would be split up into the separate read and write semantics, but that is a lot of cases
<tpw_rules>
and perhaps unnecessarily limiting
<whitequark[cis]>
that's not really feasible due to the amount of cases, nor does it fit into the existing system of fields
<whitequark[cis]>
it would make the CSR system worse
<jfng[m]>
jfng[m]: sorry, i'm starting to be tired; the field that replaces a ResR0W* will need to define an appropriate semantic value for 0
<jfng[m]>
the whole idea of compatibility between silicon and software has a section in the guide-level part of the RFC
<whitequark[cis]>
a reserved action isn't something that's desirable in itself but rather a stand-in for future expansion; it should be as unobtrusive as we can feasibly make it, rather than bend the rest of the API to its properties
<whitequark[cis]>
* something that's not desirable in
<tpw_rules>
so going back to the earlier example of "consider a read-only register (Element.Access.R) that has a ResRAW0 field". suppose you replace that with a RW1S. would you also have to update the register to RW?
<whitequark[cis]>
yes, you would have to
<jfng[m]>
yes; otherwise it will be hard error
<tpw_rules>
the answer is yes, but is this what is wanted? i would say it should be .RW in the first place
<tpw_rules>
it sounds like that is what is wanted based on the unobtrusive comment a moment ago
<jfng[m]>
yes, it is what we want
<whitequark[cis]>
huh? you would need a time machine to make the register .RW retroactively
<jfng[m]>
such a change needs to be visible in documentation, tooling, software, etc
<tpw_rules>
it's not really retroactive, if you put a ResRAW0 then i think that means at some future point in time you would want it to be writable
<whitequark[cis]>
no matter what kind of reserved field you put there, they all have a write specification
<tpw_rules>
and splitting them up is indeed ugly and limiting
<whitequark[cis]>
this happens because otherwise we would have 12 different reserved fields and an annoying system that mandates that you use the right kind in the register with the right access
<whitequark[cis]>
(maybe less than 12, but still too many. 4 is already a pretty high number)
<zyp[m]>
one problem with saying «this reserved field is RW because we might make it writable in the future» is that it doesn't say anything about what you're allowed/supposed to write to it
<zyp[m]>
registers don't have the same issue, you can always add more access than a register had before
<zyp[m]>
upgrading a readonly register to RW doesn't break existing software, because it's not illegal to not write a RW register
<jfng[m]>
yes, unless existing software writes to a read-only register
<jfng[m]>
which wouldn't have side-effects previously, but may when it becomes RW
<zyp[m]>
no amount of policies will fix software that doesn't follow policies
<jfng[m]>
in such a case, this is actually fine if the software followed the reserved field policy (e.g. always wrote 0 if ResRAW0)
<whitequark[cis]>
going for a "swiss cheese model" makes sense to me; i.e. mark the register as read-only but also "if you absolutely have to write to it, at least write 0"
<whitequark[cis]>
also, I absolutely expect there to be the case where an internally RW register is downgraded to RO because e.g. it contains chicken bits set by the bootloader
<tpw_rules>
okay, i think the FieldPort makes sense to me now. i have some other comments but i recognize i've monopolized this meeting pretty heavily. i'll bring them up over the next week. i see the checkboxes on the PR is done which is great but i think the RFC needs a little revision and clarity before i would want to see it accepted
<whitequark[cis]>
RAWL would hint away at that, but also prevent someone from subtly breaking their SoC
<zyp[m]>
does it really make sense for registers to have an access parameter?
<whitequark[cis]>
it's there in SVD, so we should probably support it
<whitequark[cis]>
aside from the declaration of intent in the source code
<jfng[m]>
whitequark[cis]: yes, though in such a case, this would only be about metadata; the actual field wouldn't be ResRAWL
<tpw_rules>
ok, sounds like the meeting is officially over
<whitequark[cis]>
jfng: didn't we decide against adding `FieldAction` class? I might be missing something
<tpw_rules>
we decided against changing FieldPort.Access for sure
<jfng[m]>
we haven't, and i asked if someone identified any downsides of doing so
<jfng[m]>
in the absence of an answer, i seldom went with the change
<whitequark[cis]>
I also (like tpw_rules) felt that it wouldn't necessarily solve anything)
<whitequark[cis]>
* I also (like tpw_rules) felt that it wouldn't necessarily solve anything
<tpw_rules>
i'd like to see it but i'm not sure if it could justify itself. the only thing i see is documentation/RFC clarity, not any asset to the implementaiton
<whitequark[cis]>
I think I'd want to look at how the implementation looks like before committing one way or another
<whitequark[cis]>
that's something we can do next week
<jfng[m]>
it would allow me to say that something returns a FieldAction in documentation, for example, rather than a Component and having to explain what a field action is every time
<jfng[m]>
also, when documenting the exceptions that are raised by a method, i'll be able to just say "if x is not a FieldAction"
<whitequark[cis]>
that's a good argument for it
<whitequark[cis]>
would it just have a port?
<tpw_rules>
is there a taxonomy for terms in the docs? for sure it would be easier to have it be a language type though
<jfng[m]>
my usual cue is that if something is awkward to document, it is awkward in practice
<jfng[m]>
tpw_rules: well, there are no guide-level docs so far; we only have some reference-level docs, as docstrings
<jfng[m]>
whitequark[cis]: yes
<jfng[m]>
but its subclasses may have more members
<whitequark[cis]>
makes sense
<whitequark[cis]>
I guess it would require a dict of members in the constructor
<jfng[m]>
yeah
<jfng[m]>
and a FieldPort.Access value
<jfng[m]>
... and a shape
<tpw_rules>
there is the guide level explanation in the RFC
<tpw_rules>
isn't the access value part of the port?
<tpw_rules>
and the shape?
<tpw_rules>
i guess you would want to get help setting up the port
<jfng[m]>
the FieldAction constructor will pass these two parameters to FieldPort.Signature, yes
<tpw_rules>
and set up the port itself? okay
<tpw_rules>
that wouldn't mix with attributes then would it
<jfng[m]>
not more so than when a component calls super().__init__(...) to create its signature
<whitequark[cis]>
fields would usually be variable width anyway
<whitequark[cis]>
except for flags
<tpw_rules>
ah, they pretty much already are anyway
<jfng[m]>
also flags, as they can be grouped as a multibit field