Minor bug with fields' swwel property

[tolaszi]

Minor bug with fields' swwel property

[amykyta3]

Thanks for reporting!
It looks like I completely misinterpreted the spec's description of the swwe and swwel properties, thinking that they override the sw property at RDL compile-time. Instead, setting either to True results in a hardware input signal that changes the field's behavior.
This means that in fact, the entire block of code you referenced should simply be removed from the elaborate stage.
Should be an easy fix, but I just want to make sure I don't miss any other assumptions.

Thanks again for reporting! It helps to have multiple people interpreting the spec :-).

[amykyta3]

Revised interpretation results in a slightly different behavior of the FieldNode.is_sw_writable and FieldNode.implements_storage utility properties.

If swwe or swwel are set to true or a reference, then the field is capable of being sw writable (perhaps not all the time). Also implies that the field has a storage element.

[tolaszi]

I think there are two things being intertwined here. The purpose of the swwe and swwel properties are to be able to explicitly define a write enable signal in the HW description of the field. However, your initial assumption of this property affecting the sw property wasn't wrong either and it came from 7.6.1.-d of the spec.
My personal opinion here is that the spec is trying to be too smart. In my mind setting sw=r and swwe=true are conflicting settings and should be reported as such. The spec decided resolve these conflicts and as it happens swwe and swwel takes precedence due to the compile order so what you have in the code is not wrong and is faithful to the spec.
There is one problem though. A read-write field, by definition, has a write enable signal and normally this is high active by default. So the assumptions described in 7.6.1-d should go both ways and if sw=rw this implicitly could result in swwe=true. At the moment the compiler returns swwe as false unless explicitly defined. I guess it is up you to decide whether swwe should return true or false when not explicitly defined because the spec just opened up the issue but left some areas for interpretation.

[amykyta3]

I mostly agree with your interpretation. I agree that the combination of swwe=true and sw=r makes no sense, but I disagree that the assumptions described in 7.6.1-d should go both ways. Reading through the spec a bit more, I noticed this in the intro paragraph to section 9.6 (or section 7.6 in the RDL 1.0 spec):

Some of these properties perform operations that directly effect the value of a field (rclr, woset, and woclr), others allow the surrounding logic to effect software operations (swwe and swwel), and still others allow software operations effecting the surrounding logic (swmod and swacc)

I believe the critical text that is missing is clarifying that the swwe and swwel properties expose a signal where the user has the ability to augment the sw property at runtime. If a boolean true is assigned, then it implies the hardware provides an input to the register file that can be driven by external logic. Alternatively if a signal or field reference is assigned, then the state of the corresponding signal or field determines software's ability to write to the field. The polarity of this user-accessible signal is determined whether swwe or swwel is used.

Based on that, I feel that the compiler should do the following:

• Keep existing code that overrides the sw property based on swwe or swwel (with the fix you originally suggested):
  • If the designer set the swwe or swwel properties to anything other than false, then the sw property should be upgraded to writable.
  • Reason to keep this is to prevent downstream tools from having to deal with odd conflicting scenarios like the one you mentioned: swwe=true and sw=r
  • Add: Emit a warning to the user. This kind of design input should be discouraged since it doesn't make sense.
• Do not override the value of swwe or swwel
  • These properties seem to be exclusively for adding hardware that augments the field's "writability" at runtime. You're correct that internally, a field will have its own implicit "write-enable" signal if sw=rw but I don't think that's what these properties are intended to denote. These properties imply an entirely unique "write-enable from software is allowed" signal that masks the internal "write-enable" signal.
  • Overriding swwe or swwel would be confusing to downstream tools since it would destroy the ability to distinguish whether the designer wanted an explicit input signal to their block or not.
• No change to the FieldNode.is_sw_writable and FieldNode.implements_storage utility properties.
  • Since the compiler will automatically warn and patch the value of the sw property, these will continue to function as expected.

Does the above sound reasonable? If so, I may add some of this to the spec errata page I have been maintaining.

[tolaszi]

I think we are mostly in agreement here and I think your suggestion sounds reasonable, I just add a few notes to clarify what I meant.

For a normal read-write and write field there is a write enable signal which is basically active when the bus has a valid transaction, it is a write transaction and the address bus value matches the register offset. I agree that the purpose of the swwe and swwel properties would be to augment this by AND'ing this write enable with either a field value (SW controlled lock mechanism) or a signal value (HW controlled lock mechanism). Having the option to define these additional write enables as high or low active makes a lot of sense.

Now using these purely as boolean makes much less sense to me. I don't see a use case for selectively setting normal write enables to low active. Also, the presence or absence of (high active) normal write enable should be determined by the value of the sw property as I wrote earlier. I think the only reason for having the boolean as an option in the spec is to allow the dynamic reference assignment later.

So I think the downstream tools would really only need to care whether swwe or swwel has a reference (although technically they could cover the case when the normal write enable is set to low active but like I said, that seems odd to me). That's why I argued that setting sw based on the true/false status swwe value seems unnecessary but if it is done it may as well go both ways.
So all in all, I agree with the plan that you described. This will be faithful to the spec and will work well for any design descriptions that actually makes sense in HW.

[amykyta3]

Re-opening based on some discussions I had with @taichi-ishitani.

From section 9.6.1-d:

swwe and swwel have precedence over the software access property in determining its current
access state, e.g., if a field is declared as sw=rw, has a swwe property, and the value is currently
false, the effective software access property is sw=r.

I think we were correct to understand that the swwe/swwel infers logic to provide a mechanism for a field to be writable by software or not. Upon re-reading this thread, and conversations with Taichi, I don't think it is appropriate for the swwe/swwel properties to override the sw property at compile-time at all. This is purely a runtime effect.

Based on that, I am revising the implementation as follows:

• Never override the value of the sw property, regardless of what swwe/swwel is set to.
• If swwe or swwel is assigned a signal or field reference, or a boolean of true, then the field must also be writable. If it is not, validation should emit an error.

Also, I think its worth starting a documentation page that provides more detailed interpretations of what logic certain properties should infer. The SystemRDL spec could have been a little more clear on how swwe/swwel is supposed to be interpreted.