Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

best common practice I2C register map

Tags:

c

i2c

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:

  • (+) Register offsets are all logically contained inside a struct instead of having to spell each register out in a define.
  • (+) Entry sizes are explicitly stated using a data type of appropriate size.
  • (-) This doesn't account for bit fields which are widely used
  • (-) This doesn't account for register maps that aren't byte mapped (e.g. LM75), where one reads 2 bytes from offset n+0x00, yet n+0x01 is another register, not the high/low byte of register n+0x00
  • (-) This doesn't account for large gaps in address space (e.g. registers at 0x00, 0x01, 0x80, 0xAA, no in-betweens...) and (I think?) relies on compiler optimization to get rid of the struct.

define approach:

  • (+) Each of the registers along with its bits is usually defined in a block, making finding the right symbol easy and relying on a naming convention.
  • (+) Transparent/unaware of address space gaps.
  • (-) Each of the registers have to be defined individually, even when there are no gaps
  • (-) Because defines tend to be global, the names are usually very long, somewhat littering the source code with dozens of long symbol names.
  • (-) Sizes of data to read are usually either hard-coded magic numbers or (end - start + 1) style computations with possibly long symbol names.
  • (o) Transparent/unaware of data size vs. address in map.

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 :(

like image 526
FRob Avatar asked Oct 05 '22 22:10

FRob


2 Answers

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);
}
like image 120
engineerC Avatar answered Oct 08 '22 10:10

engineerC


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.

like image 43
Andrew Cottrell Avatar answered Oct 08 '22 11:10

Andrew Cottrell