5

I'm looking for input on the most elegant interface to put around a memory-mapped register interface where the target object is split in the register:

union __attribute__ ((__packed__)) epsr_t {
    uint32_t storage;
    struct {
        unsigned reserved0    : 10;
        unsigned ICI_IT_2to7  :  6; // TOP HALF
        unsigned reserved1    :  8;
        unsigned T            :  1;
        unsigned ICI_IT_0to1  :  2; // BOTTOM HALF
        unsigned reserved2    :  5;
    } bits;
};

In this case, accessing the single bit T or any of the reserved fields work fine, but to read or write the ICI_IT requires code more like:

union epsr_t epsr;
// Reading:
uint8_t ici_it = (epsr.bits.ICI_IT_2to7 << 2) | epsr.bits.ICI_IT_0to1;
// Writing:
epsr.bits.ICI_IT_2to7 = ici_it >> 2;
epsr.bits.ICI_IT_0to1 = ici_it & 0x3;

At this point I've lost a chunk of the simplicity / convenience that the bitfield abstraction is trying to provide. I considered the macro solution:

#define GET_ICI_IT(_e)      ((_e.bits.ICI_IT_2to7 << 2) | _e.bits.ICI_IT_0to1)
#define SET_ICI_IT(_e, _i)  do {\
    _e.bits.ICI_IT_2to7 = _i >> 2;\
    _e.bits.ICI_IT_0to1 = _i & 0x3;\
    while (0);

But I'm not a huge fan of macros like this as a general rule, I hate chasing them down when I'm reading someone else's code, and far be it from me to inflict such misery on others. I was hoping there was a creative trick involving structs / unions / what-have-you to hide the split nature of this object more elegantly (ideally as a simple member of an object).

timrau
  • 22,578
  • 4
  • 51
  • 64
Pat
  • 1,882
  • 2
  • 15
  • 22
  • stick with macros, really. if you want to make your code fancy, assuming you have read_epsr / write_epsr functions, make them accept struct two fields (T and ICC_IT) and convert it from/to espr_t inside funcs. – iced Jan 14 '13 at 03:53
  • 3
    Bitfields are very unreliable, esp used in this manner (to point at something in another compile domain or hardware). The hardware is not dynamic, not going to change (dont need to define once and use many). The bitfield will result in a mask and shift anyway, just code the mask and shift, either directly or in a macro. – old_timer Jan 14 '13 at 04:20
  • Do you have any pointers or references for when they're actually a problem? This is a pretty common idiom in the embedded space -- certainly not invented by me.. – Pat Jan 14 '13 at 04:26
  • Dan Saks has covered this issue in great detail at www.embedded.com over the years. Here is a [link](http://www.embedded.com/design/programming-languages-and-tools/4027659/Alternative-Models-for-Memory-Mapped-Devices) to one of his articles but there are several others that may help you. – kkrambo Jan 14 '13 at 14:25
  • Just a little remark. You don't need to name the unused fields of your structure. Removing the reserved0, reserved1 and reserved2 names works absolutely fine. This has the advantage for initialisers where these fields do not need to be set. – Patrick Schlüter Jan 14 '13 at 14:26

3 Answers3

8

I don't think there's ever a 'nice' way, and actually I wouldn't rely on bitfields... Sometimes it's better to just have a bunch of exhaustive macros to do everything you'd want to do, document them well, and then rely on them having encapsulated your problem...

#define ICI_IT_HI_SHIFT   14
#define ICI_IT_HI_MASK    0xfc
#define ICI_IT_LO_SHIFT   5
#define ICI_IT_LO_MASK    0x02

// Bits containing the ICI_IT value split in the 32-bit EPSR
#define ICI_IT_PACKED_MASK  ((ICI_IT_HI_MASK << ICI_IT_HI_SHIFT) |     \
                             (ICI_IT_LO_MASK << ICI_IT_LO_SHIFT))

// Packs a single 8-bit ICI_IT value x into a 32-bit EPSR e
#define PACK_ICI_IT(e,x)  ((e & ~ICI_IT_PACKED_MASK) |                 \
                           ((x & ICI_IT_HI_MASK) << ICI_IT_HI_SHIFT) | \
                           ((x & ICI_IT_LO_MASK) << ICI_IT_LO_SHIFT)))

// Unpacks a split 8-bit ICI_IT value from a 32-bit EPSR e
#define UNPACK_ICI_IT(e)  (((e >> ICI_IT_HI_SHIFT) & ICI_IT_HI_MASK) | \
                           ((e >> ICI_IT_LO_SHIFT) & ICI_IT_LO_MASK)))

Note that I haven't put type casting and normal macro stuff in, for the sake of readability. Yes, I get the irony in mentioning readability...

paddy
  • 60,864
  • 6
  • 61
  • 103
  • Curious why you avoid bitfields? Things are pretty well defined for a single-word sized structure on a 32-bit architecture (the present context) – Pat Jan 14 '13 at 04:18
  • 1
    As far as I understand, bitfields are not pinned down by the C standard. The compiler can more or less implement them in any way it chooses. If you are using a single compiler for a single platform, maybe it's fine. But I prefer not to rely on them. – paddy Jan 14 '13 at 04:22
  • By my understanding, it is at least obligated to keep the fields in order. At that point, an early assert(sizeof(union epsr_t) == 4) is sufficient for the paranoid to validate that things are as expected. At that point I find the readability massively improved. Plus debuggers and other tools gain nice features when macros are avoided. – Pat Jan 14 '13 at 04:28
  • That's fine. I'm no expert, and maybe I'm also a little set in my ways. But when I'm laying out chunks of bits for a very well-defined protocol, I just don't trust bitfields as much as I trust cold hard bit operations. Doing it all with macros can make code harder to maintain, but once I develop a set of macros for some purpose, I would test them very carefully and then never touch them again. – paddy Jan 14 '13 at 04:39
  • That's reasonable, it just doesn't scale. Start something with order 100 registers, most of which have 5~20 components and you really start to appreciate the scalability and readability of the bitfield approach. Regardless, I thought I was reaching with this question, just thought I'd throw it out there for the world. – Pat Jan 14 '13 at 04:45
  • Hmm, didn't realise this was intended to be scaled. That would be a very nasty looking header whether you used bitfields or not. I dunno what I'd do in that case. Might end up making a very simple definitions file and using it to generate a header (stubbornly using my own bit operations, of course!) – paddy Jan 14 '13 at 04:50
  • The non-contiguous issue is pretty unique, but otherwise yes, things need to scale. At that point any raw data source you'd use for such a generator starts to look a lot like a bitfield already... :) – Pat Jan 14 '13 at 04:52
  • @Pat Bit fields are indeed very poorly defined by the standard. There are so many issues with them that one could page after page on why they are so bad an unreliable. [More info](http://stackoverflow.com/questions/6043483/why-bit-endianness-is-an-issue-in-bitfields/6044223#6044223) – Lundin Jan 14 '13 at 07:50
  • @Pat: the problem with bitfields is that different builds can get you different layouts. A real-world example I ran into is that a bitfield intended to model an MPEG2 header would get laid out differently by GCC targeting x86 and GCC targeting ARM Cortex-A8. – Michael Burr Jan 15 '13 at 09:44
0

If you dislike macros that much just use an inline function, but the macro solution you have is fine.

James
  • 9,064
  • 3
  • 31
  • 49
0

Does your compiler support anonymous unions?

I find it an elegant solution which gets rid of your .bits part. It is not C99 compliant, but most compilers do support it. And it became a standard in C11.

See also this question: Anonymous union within struct not in c99?.

Community
  • 1
  • 1
lorcap
  • 173
  • 2
  • 8
  • What would it be like you solution that gets rid of the .bits? – EnzoR Apr 23 '21 at 13:46
  • Looking back at my answer (after so many years), it doesn't make much sense: it doesn't provide a solution. Bit juggling is still required. – lorcap Apr 25 '21 at 08:42