A new and more modern way to access and modify registers

S

shawn

Well-known member
I've been experimenting with using modern C++ to create a better way to access and modify registers.

Goals:
1. No #defines or macros
2. No special functions
3. Compiles down to code as efficient as writing it the "old" way
4. Reduce cut & paste errors
5. Single definitions and use for a given register, not the quadruplicate "mask, shift, apply" stuff
6. Access or modification is done with single or almost-single statements
7. Usable with bit-twiddling operations
8. Easy to use and read
9. Feels like its use is "out of the way" and without clutter

The repo (see src/main.cpp for some example use cases):

This is a work in progress and should be considered experimental. I'd love some feedback on whether I've truly achieved my goals. Please remember this is only a first go-around. I'm sure there's lots of room for improvement. Note that I've machine-checked (using Codex) the accuracy of all the values by comparing with NXP's SDK, but still, be cautious.

(@jmarsh, @luni: I'd love your input too.)

Other notes:
1. The Readme is not even close to being complete
 
How do you set more than one register field with a single access? I'm thinking of a case where you can only update part of a register if a separate bit is also set at the same time.
 
jmarsh said:
How do you set more than one register field with a single access? I'm thinking of a case where you can only update part of a register if a separate bit is also set at the same time.
Click to expand...
When just setting bits, see line 88 of main.cpp. When needing to clear some bits and set others, just do the same as you normally would:
reg = (reg & ~mask) | bits

Are you referring to point 6? If so, note that it says "single statement" and not single access. (Assuming this is what you're referring to.)

Note also that this API provides both whole and partial register access.

"mask" and "bits" are constructed similar to line 88. Show me a specific example, and I'll show how to do it with this.
 
Last edited:
The SCB AIRCR register for example; if the top 16 bits aren't written as 0x05FA the register write is ignored. So attempting to set SCB_AIRCR_SYSRESETREQ to 1 won't work, you have to set the whole register which makes most of the template stuff redundant.
 
C++: 
SCB->AIRCR = 0x05FA0000 | SCB_AIRCR_SYSRESETREQ(1);
Or:
C++: 
SCB->AIRCR = 0x05FA0004;
Or:
C++: 
SCB->AIRCR = SCB_AIRCR_VECTKEY(0x05fa) | SCB_AIRCR_SYSRESETREQ(1);

There's no "business logic" in anything right now, but that might be a good layer on top. There's still the presumption that you know how to use the registers.
 
Last edited:
shawn said:
C++: 
SCB->AIRCR = 0x05FA0000 | SCB_AIRCR_SYSRESETREQ(1);
Or:
C++: 
SCB->AIRCR = 0x05FA0004;
Or:
C++: 
SCB->AIRCR = SCB_AIRCR_VECTKEY(0x05fa) | SCB_AIRCR_SYSRESETREQ(1);

There's no "business logic" in anything right now, but that might be a good layer on top. There's still the presumption that you know how to use the registers.
Click to expand...
I'll add to this: the `Reg` class, along with its baby sibling, `RegValue`, also act as value types so they can be used in expressions.
 
Last edited:
I'm a bit concerned about this...

Code: 
// Undefine anything defined by Teensyduino's imxrt.h

My guess is these .h header files are meant to be included only by the .cpp source files of libraries, right? But consider what happens if a library author includes it from their library's main .h header? Then programs or other libraries which use that library by including its main header file end up with well established "old style" names redefined to something unexpected.

Or maybe I've misunderstood something here?
 
PaulStoffregen said:
I'm a bit concerned about this...

Code: 
// Undefine anything defined by Teensyduino's imxrt.h

My guess is these .h header files are meant to be included only by the .cpp source files of libraries, right? But consider what happens if a library author includes it from their library's main .h header? Then programs or other libraries which use that library by including its main header file end up with well established "old style" names redefined to something unexpected.

Or maybe I've misunderstood something here?
Click to expand...
You're not wrong. The way I've currently chosen to do the naming clashes with the imxrt.h includes, so I have do that before the name gets defined. Consider this all experimental. (Note that the usage syntax is slightly different and the compiler is doing more work.)

I'm trying this approach just to see how it gets compiled down compared to the other approach, and I think, for the most part, it's pretty good.

What inspired this little rabbit hole is Teensyduino's digitalWriteFast() and friends, where they may get compiled down to one instruction. I wanted a clean way to use registers while reducing redundancy, copy & paste errors, and to let the compiler catch more problems. The more the compiler can catch problems, the better the code, and if I can make the compiled code equivalent, then I consider that a win.

And indeed, there's some more type safety that could be added, etc., but for right now, it just uses numbers. I.e. No enums or other types, and no business logic checking some other things.

Sure, for interoperability with imxrt.h, the naming could use some work. Thanks for having a look! :)
 
Last edited:
@PaulStoffregen I just updated the repo to add a USE_OLD_WAY macro to src/main.cpp that can either be set to 0 or 1, for easy swapping. I've attached both firmware listings to this post so you can see what the compiler does; the two are extremely close (accounting for differences in the locations within the files; you may need to do a little searching). This uses the PlatformIO default settings, which I think is TEENSY_OPT_FASTER, and that's equivalent to just -O2.

If you'd like me to create a version that's more compatible with the Arduino IDE, then just say the word and I'll make one.
 

Attachments

  • firmware_new.lst.txt
    668.8 KB · Views: 21
  • firmware_new.sym.txt
    19.4 KB · Views: 25
  • firmware_old.lst.txt
    668.2 KB · Views: 27
  • firmware_old.sym.txt
    19.4 KB · Views: 28
Last edited:
I was thinking about naming some more, and I have a few naming ideas that won't conflict with the imxrt.h header.

For example, namespace separators ("::") instead of underscores.
 
I just added a feature where parts of a register can be read-only and the compiler catches writes to those. I also added a bunch more register groups, with more to come.
 
I find it usually much easier (quicker) to use explicit hex constants so that the code can be compared to the information in the chip datasheet register diagrams directly - naming every little field in every register is not always a good thing (certainly not for conciseness, or even readability)...

Adding a comment/URL pointing to the page in the datasheet can be more illuminating that lots of #defines or template magic, since hardware datasheets always give the layout in visual form, and there are often subtleties to beware of like register fields whose value is one less than the notional value... You need your nose in the datasheet the whole time anyway.
 
That's true. You still need to be looking at the datasheet, but the compiler can catch more errors this way, and there's far less potential for mis-typing constants or cut & paste errors when using a group of similar-looking registers.

...and it compiles down to similar code.
 
Last edited:
MarkT said:
I find it usually much easier (quicker) to use explicit hex constants ...
naming every little field in every register is not always a good thing ... for ... readability
Click to expand...
Maybe easier to write, but a complete pain to read o_O. Compare
I2S0_TCR4 = I2S_TCR4_FRSZ(3) | I2S_TCR4_SYWD(0) | I2S_TCR4_MF | I2S_TCR4_FSP | I2S_TCR4_FSD;
with
I2S0_TCR4 = 0x00030013;

I know which one I'd rather be confronted with. I've not looked at how that would pan out using @shawn's new scheme - presumably at least as good as the first option.
MarkT said:
You need your nose in the datasheet the whole time anyway
Click to expand...
Absolutely - but I2S_TCR4_FRSZ(3) lets me find the right field and its meaning way faster than the magic hex value ... and that's for a reasonably user-friendly register where most fields are nibble-aligned or only one bit.

Occasionally you find things like I2S_TCR4_FRSZ(4-1), which is a handy hint that the Frame Size is 4 (words), but the field "adds one". Harder for fields which are 2^n, or weird table lookups.
 
I'm using "::" namespace separators now instead of underscores to avoid any potential conflicts with including <imxrt.h>. It was either that or undefine all the symbols before they're declared. see posts 8, 11, and 12.

Note also that there's still probably a lot of room for design work. Right now, I'm grappling with how best to represent substructures.
 
I think I cracked the problem of what to do with array elements and sub-structures and arrays of sub-structures. I also figured out how to automatically detect and set mixed w1c and non-w1c fields properly.

The compiler can now also catch:
1. Writes to read-only fields
2. Reads from write-only fields
3. It can also ensure correct masking automatically
 
h4yn0nnym0u5e said:
Maybe easier to write, but a complete pain to read o_O. Compare
I2S0_TCR4 = I2S_TCR4_FRSZ(3) | I2S_TCR4_SYWD(0) | I2S_TCR4_MF | I2S_TCR4_FSP | I2S_TCR4_FSD;
with
I2S0_TCR4 = 0x00030013;

I know which one I'd rather be confronted with. I've not looked at how that would pan out using @shawn's new scheme - presumably at least as good as the first option.
Click to expand...
Either version requires you to have relevant page of the datasheet to understand - if your field names were readable then it would be better, but probably multi-line! Perhaps best to wrap all the register encoding in functions/macros with well named parameters, rather than a mess of inline bit field manipulations.
 
I’m sorry, but … just no.

If you’re happy doing bit shifts and masks in your head, that’s great. I’m not.

The field names match the datasheet (which I agree you need to have to hand - no question). Using “readable” ones would just add a different layer of complexity, as you’d lose the 1:1 correspondence with the datasheet.

Apart from the readability issue, I’ve encountered quite a few bugs over the years resulting from code writers mis-counting bit shifts or otherwise sprinkling their code with magic numbers, hex or otherwise. It’s poor for reading, it’s poor for maintainability, and it’s poor for code quality.
 
@h4yn0nnym0u5e do you feel like trying to use the library with any projects you have that use direct register access? It would be lovely for others to use it and give feedback.

Also, which registers are still missing that you'd like to see?

Note that the library is currently thin on provided named constants.
 
I’m definitely keeping it in mind as a useful resource.

The thing is that most of my register-bashing use is modifying existing libraries, where adding another dependency is unlikely to be especially helpful. It’s hard enough as it is to get a PR merged…

I think the only peripheral I use (historically, a lot!) that you haven’t implemented is SAI, for audio I/O. One thing I did start playing with is reducing the amount of duplicated code in the Audio library - code for SAI1 and SAI2 is entirely separate, rather than passing a base address (or reference) to common methods. There’s a wrinkle there, too, as SAI1 can make use of 5 data pins, whereas SAI2 only has 2.

Glancing briefly at your implementation of LPSPI, I’m not sure your approach allows that. You seem to have quadruplicated the code needed for a single peripheral…
 
Everything is passed as a template parameter and also inlined, that's why it compiles down to something equivalent to using masks and shifts directly.

I believe my approach does allows different peripheral parts to have different bits. I'll throw in the SAI fields and commit that so you can see.
 
You must log in or register to reply here.
Back
Top