Just wondering what the best practice regarding I²C register maps in C or rather what other people use often/prefer.
Up to this point, I have usually done lots of defines, one for every register and one for all the bits, masks, shifts etc. However, lately I've seen some drivers use (possibly packed) structs instead of defined. I think these were Linux kernel modules.
Anyway, they would
struct i2c_sensor_fuu_registers {
uint8_t id;
uint16_t big_register;
uint8_t another_register;
...
} __attribute__((packed));
Then they'd use offsetof (or a macro) to get the i2c register and use sizeof for the number of bytes to read.
I find that both approaches have their merit:
struct approach:
define approach:
Basically, I'm looking for a smarter way to handle these cases. I often find myself typing lots and lots of agonizingly long symbol names for each and every register and each bit and possibly masks and shifts (latter two depending on data type) as well, just to end up using just a few of them (but hating to redefine missing symbols later on, which is why I type all in one session). Still, I notice that sizes of bytes to read/write are mostly magic numbers and usually reading the datasheet and source code side-by-side is required to understand even the most basic interaction.
I wonder how other people handle these kinds of situations? I found some examples online where people also arduously typed every single register, bit etc. in a big header, but nothing quite definitive... However, neither of the two options above seems too smart at this point :(
WARNING: The method described here uses bitfields, whose arrangement in memory is implementation specific. If you do this, make sure you know how your compiler works in this regard.
As you point out, there are advantages and disadvantages to each method. I like a hybrid approach. You can define register offsets, but then use a struct for the contents and a union to specify the bits or the entire register. Inside the union, use the correct size variable for the size of the register (as you mentioned sometimes they're not byte addressable). You don't need quite as many defines, and you're less likely to mess up bit shifts and don't need masks. For example:
#define unsigned char u8;
#define unsigned short u16;
#define CTL_REG_ADDR 0x1234
typedef union {
struct {
u16 not_used:10; //top 10 bits ununsed
u16 foo_bits:3; //a multibit register
u16 bar_bit:1; //just one bit
u16 baz_bits:2; //2 more bits
} fields;
u16 raw;
} CTL_REG_DATA;
#define STATUS_REG_ADDR 0x58
typedef union {
struct {
u8 bar_bits:4; //upper nibble
u8 baz_bits:4; //lower nibble
} fields;
u8 raw;
} STATUS_REG_DATA;
//use them like the following
u16 readregister(u16);
void writeregister(u16,u16);
CTL_REG_DATA reg;
STATUS_REG_DATA rd;
rd = readregister(STATUS_REG_ADDR);
if (rd.fields.bar_bit) {
reg.raw = 0xffff; //set every bit
reg.fields.bar_bit = 0; //but clear this one bit
writeregister(CTL_REG_ADDR, reg);
}
In my ideal world, the hardware designer would supply a header file compatible with C++, C, and ASM. One that was auto-generated based on the actual hardware registers. One that defined every register and bit/field via both #defines (for ASM) and typedef'd structures (for C and C++). One that indicated the access properties of every bit and field (read-only, write-only, write-clear, etc.). One that included comments defining the use and purpose of each register and its bits/fields. It would also need to account for target endianness and compiler, to make sure any registers and bitfields were ordered correctly.
I got as close to this ideal as I could at a previous job. I wrote a script that would parse a register description file (of a format I defined) and auto-generate a full header (structures and #defines) as well as a function to dump all the readable registers for debugging purposes. I've seen similar approaches at other companies, but none that took it to that extent.
I'll point out that if you use a typedef struct to define your register layout then you can easily account for large register gaps in the definition. e.g. Just add a "reserved[80]" or "unused[94]" or "unimplemented[2044]" or "gap[42]" array element to define the gap. You'll always use the struct definition as a pointer to the hardware base address anyway, so it won't take up the actual size of the struct anywhere in memory.
Hope that helps.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With