Repository navigation
Feature Request: Option to avoid read-modify-write #305
Description
Basically what the title says. I want to avoid a read-modify-write when I'm writing to fields. The basic idea is to keep track of current value of the neighboring fields (a shadow copy) and encode the values accordingly.
The challenge: Hardware writable fields
- Hardware could have written to a field since the last time the shadow copy was updated.
- Currently there is a branch to check if the reg is read-write or write-only. An additional check may be needed to tell if there are hardware-writable fields in the reg
- Technically, current read-modify-write solution also has a race-condition wrt hardware-writable fields
Possible solutions:
- Keep a shadow copy and read-from that when appropriate for read-modify-writes
- Offload this to the callback. Then the callback needs to know that a given read was a part of a read-modify-write sequence. I'm personally not a fan of this. The callback will not have enough information to do the hardware-writable fields check
Workaround:
User keeps two instances of the top-level reg_model. One for writing and the other for reading. Then, in the writing reg_model, all reads happen from a shadow copy
Activity
krcb197 commentedon May 19, 2026
peakrld-python has to operate on the whole register; therefore, it must write all the fields. There are two ways of doing this:
write_fieldsmethod of a register, in which all the field values must be providedIt is possible to use
write_fieldsin combination withread_fieldsto implement the behaviour suggested. But that would need a layer that site above the RAL.The design philosophy of peakrdl-python todate was to make it stateless, it only holds state with the context managers (for example the
single_read_modify_write). There is a suggestion to implement a read cache, where it is read once and then retained, see #9, I think this request is similar.I agree with the concern about a race condition with hardware writable registers, for this reason most register maps I have seen avoid it. What is the use case for a register field that is both hardware and software writable?
parthkalgaonkar commentedon May 20, 2026
Yes this is exactly like #9. The concern about the race condition was just that, a concern and is not a use case for me
eblankenberg-trt commentedon May 20, 2026
My main headache with the forced read step in read-modify-write is that I have a lot of registers that are read sensitive using SWACC (ex. for auto-incrementing view pointers into LUTs) so rather than do 50 consecutive writes for a 50 entry LUT that increments on SWACC or on SWMOD you would have to reset the pointer each time.
I was going to suggest doing a write only context manager but raising an error if any unspecified fields don't have a safe default value that can be inferred along with some intelligence for creating these safe default values given the properties of the target field. As stated above the "safe" default value could be a cached register written value from the last write if there is no hardware write path, but this could also be not writing a triggering value (ex. writing one for a positive singlepulse register).
Regarding RW set for both HW and SW, I use registers that are like this for these LUT views since the current entry needs to be readable by software (thus software readable and hardware writeable) as well as providing a path for a software write to propagate to the LUT (thus software writeable and hardware readable).
krcb197 commentedon May 22, 2026
@eblankenberg-trt
The use case you describe makes sense to me. On reflection, I think this is slightly different from a register cache, where the state is shadowed to optimise slow connections (issue #9).
The
write_fieldsmethod almost does what you need. In the case of a WriteOnly register, it forces every single field value to be specified. However, on a read/write register, it behaves slightly differently, the field arguments on the method are optional, which does not force every value to be defined and does the read-modify-write to fill in the missing fields.I am reluctant to change the
write_fieldsmethod on the read/write register or introduce additional arguments that change the mode of operation. Additional control arguments will then need to avoid name clashes with existing field names, which could cause issues with backward compatibility to existing code.so my proposal is as follows:
single_writecontext manager to theRegWriteOnlyandRegReadWritelibrary classes that will accept an initial state of the register (rather than doing a read). This context manager will not force all fields to be written to.write_all_fields_without_readmethod on a read/write register that does not perform an initial read and takes all field values as non-optional arguments. It will use a seed register state that is a separate property of the class, so it can be overridden (for example, if there are hidden fields or another good reason not to set it to0).write_fieldsmethod ofRegWriteOnlyto use a similar property for the initial state of the register rather than an assumed0as it does today).Does this seem sensible?
eblankenberg-trt commentedon May 22, 2026
I was not aware of
write_fieldsbut for my specific use case the existing behavior would work. I would suggest adding a warning if you are doing an inferred read on a register that is read sensitive (ex. has SWACC). It took me awhile to realize what was happening in my code.I think having a function call as you suggested like
write_all_fields_without_readwhere it is guaranteed that no reads happen (even for readable fields) is important. Understood that you can't add more args to the existing methods to achieve this.I would suggest that the write-without-read methods have the ability to infer a safe default write value where it can be guaranteed rather than requiring that the user supply it explicitly (and throwing a ValueError if there is insufficient write coverage) since this could reduce verbosity in some cases. I agree that caching register values to supply defaults, even for registers that are supposedly only software accessible, is dangerous for the reasons that you have pointed out.
I am not certain that supplying the assumed register state as a number is very safe as doing so correctly essentially requires that one construct a second RAL independently of the current RAL. The alternative would be supplying a mapping of fields to the state, but I think this is equivalent to just writing all of the fields in the write anyways. I understand that it is possible for registers to be omitted from the Python RAL but still require a non-zero write and for this use case you can't create such a mapping. Arguably if I had a binary-blob type field to write in a vendor supplied module I think I would still have a field for it but the name would be something like "RESERVED" so it would still be treated like a field.
eblankenberg-trt commentedon May 22, 2026
If you think that inferring safe write values to reduce verbosity has merit I would be open to working on that, perhaps as an extension to your implementation.
krcb197 commentedon May 23, 2026
@eblankenberg-trt
Thank you for the feedback.
With respect to
I am not normally in favour of warnings unless used in a very limited way otherwsie people suffer "warning fatigue" when they keep seeing the same warning. I will try to improve the documentation and docstrings to make it much clearer.
With respect to the assumed values of all other register bits
The feedback about the practicality of building the initial state of the register is good. I will add a new method to build a register value from field values and show an example using default field values rather than require the user to make a "binary-blob" as this is very much in the spirt of peakrdl-python to avoid the user needing to bitmask/shift/logic.
I had planned to build the assumed register state based on the system rdl
resetvalue of the fields. The system rdl register has no concept of the reset or default state.I will make a version and allow you the opportuntity to make suggestions
Finally if you want to provide a example usecase to ensure the behaviour is as intended, please supply a systemrdl example and consider adding to the unit tests.
krcb197 commentedon May 24, 2026
I have prototyped an implementation, currently not very well tested, but I have pushed the work in progress to the branch so you can have a look.
There is some work to be done:
eblankenberg-trt commentedon Jun 1, 2026
Sounds good, I will take a look.
Generating a register value from field values directly (rather than indirectly storing from the callback) is helpful for other use cases I have as well (ex. creating initialization of a formatted LUT that is otherwise accessible via register writes). In the past I was just storing the writes in a dictionary then retrieving them later which is a bit messy.
krcb197 commentedon Oct 27, 2026
Thanks to everyone for their feedback, this improvement is released in the latest 4.0.0