-1

I'm trying to combine two typedef unions of a GPIO port of an ARM SoC into one, and address pointers into one. Currently, I have something which looks like this:

.h file:

//GPIO00 port
typedef union {
  struct {
    uint32_t GPIO000:1;
    uint32_t GPIO001:1;
    ...
    uint32_t GPIO0017:1;
  };
  struct {
    uint32_t w:18;
  };
} __GPIO00portbits_t;

volatile __GPIO00portbits_t * PTR_GPIO00portbits;
#define GPIO00portbits (*PTR_GPIO00portbits)


//GPIO01 port
typedef union {
  struct {
    uint32_t GPIO010:1;
    uint32_t GPIO011:1;
    ...
    uint32_t GPIO0117:1;
  };
  struct {
    uint32_t w:18;
  };
} __GPIO01portbits_t;

volatile __GPIO01portbits_t * PTR_GPIO01portbits;
#define GPIO01portbits (*PTR_GPIO01portbits)

.c file:

//GPIO 00 port
volatile __GPIO00portbits_t * PTR_GPIO00portbits = (__GPIO00portbits_t *) (AXIBRIDGE_BASE_ADDR + GPIO_00_BASE);


//GPIO 01 port
volatile __GPIO01portbits_t * PTR_GPIO01portbits = (__GPIO01portbits_t *) (AXIBRIDGE_BASE_ADDR + GPIO_01_BASE);
}

I can use this to control GPIO ports of the ARM SoC. I.e. I can control a single pin of GPIO00 by changing GPIO00portbits.GPIO00x. It works the same for GPIO01.

In reality, GPIO00 and GPIO01 are actually one port called GPIO0, where GPIO00 is pin 0-17 and GPIO01 is pin 18-35, so I would also like to combine GPIO00 and GPIO01 into one stuct which can be controlled by changing GPIO0portbits.GPIO0x.

So I would like to have something like this:

typedef union {
  struct {
    uint64_t GPIO00:1 = GPIO00portbits.GPIO000;
    uint64_t GPIO01:1 = GPIO00portbits.GPIO001;
    ...
    uint64_t GPIO035:1 = GPIO01portbits.GPIO0117;
  };
  struct {
    uint32_t w:36;
  };
} __GPIO0portbits_t;

How can I do this?

Thank you in advance.

artless noise
  • 21,212
  • 6
  • 68
  • 105
Roy Meijer
  • 13
  • 3
  • Are you asking how to make a union of unions? Or a struct of unions? – Owl Jan 30 '21 at 14:28
  • I'm asking how to make new union of structs called GPIO0portbits_t, which consists of elements that point to elements of other two unions of struct which are descibed in my question. Something like this would be perfect: typedef union { struct { uint64_t GPIO00:1 = GPIO00portbits.GPIO000; uint64_t GPIO01:1 = GPIO00portbits.GPIO001; ... uint64_t GPIO035:1 = GPIO01portbits.GPIO0117; }; struct { uint32_t w:36; }; } __GPIO0portbits_t; – Roy Meijer Jan 30 '21 at 14:41
  • `typedef` and `union` are separate things. You can certainly `typedef` a `union`, but it's important to understand which properties pertain to unions and which to typedefs. – John Bollinger Jan 30 '21 at 14:44
  • You say "members that point to", but it looks like you are not, in fact, talking about using pointers. – John Bollinger Jan 30 '21 at 14:46
  • I think it is a X-Y not existing problem. Why do you need it? – 0___________ Jan 30 '21 at 17:05
  • Allowing a compiler to generate code to access registers is often wrong. The registers are not normal memory. When someone tries to port this to a new SOC variant, what ever assumptions the compiler might meet now, are unlikely to fit the new SOC. It is better to **NOT** have code compile so the porter can look at it. So the only reason is readability. There are other ways to make this readable; a macro or inline assembler functions in 'C' and be as efficient (if not more). The best 'C' map is the bit operators, but often barrier instructions are needed. – artless noise Jan 30 '21 at 21:17

2 Answers2

0

Data types generally

You have defined two distinct types, __GPIO00portbits_t and __GPIO01portbits_t, with identical structure and closely related use. This is pointless, and it may even get in your way. I would probably do this, instead:

typedef union {
  struct {
    uint32_t GPIO0:1;
    uint32_t GPIO1:1;
    ...
    uint32_t GPIO17:1;
  };
  uint32_t w:18;
} __GPIOhalfportbits_t;

extern volatile __GPIOhalfportbits_t *PTR_GPIO00portbits;
#define GPIO00portbits (*PTR_GPIO00portbits)

extern volatile __GPIOhalfportbits_t * PTR_GPIO01portbits;
#define GPIO01portbits (*PTR_GPIO01portbits)

Note, by the way, that you need the externs if the header is going to be used in more than one .c file, and that in that case exactly one of those .c files should contain definitions you show.


Your specific request

I would also like to combine GPIO00 and GPIO01 into one stuct which can be controlled by changing GPIO0portbits.GPIO0x

It seems like you may not be maintaining the appropriate mental distinction between objects and their data types. That would explain your odd duplication of data types, and also the way you describe what you're looking for. If you want to be able to have the option to treat the data as either a full 36 bits or two 18-bit halves, then you could imagine continuing the above with something like this:

// XXX: see below
typedef union {
  struct {
      __GPIOhalfportbits_t group0;
      __GPIOhalfportbits_t group1;
  };
  struct {
    uint32_t GPIO0:1;
    uint32_t GPIO1:1;
    ...
    uint32_t GPIO35:1;
  };
  uint64_t w:36;  // uint32_t isn't wide enough
} __GPIOportbits_t;

In principle, then, you might access an object of that type either by directly accessing the bits ...

__GPIOportbits_t portbits;

// ...

if (portbits.GPIO23) {
    // ...
}

... or via the half-port pieces ...

if (portbits.group1.GPIO5) {
    // ...
}

Something like that might work under different circumstances, but in your case, this will not work. The problem is that the number of bits in your half-port pieces is not a multiple of the number of bits in a char (8 on your hardware). The size of char is the unit in which object sizes are measured, and, accordingly, the finest possible granularity for addresses.

That means that the size of my __GPIOhalfportbits_t and your __GPIO00portbits_t and __GPIO01portbits_t is at least 24 bits, not 18 bits. Therefore, if you lay two of them out one after the other then the bitfields cannot be laid out as a contiguous 36-bit range starting at the beginning of the object. There are at least 6 (padding) bits of the first object that need to go somewhere before the bits of the second half-port object.

For substantially the same reason, there are no pointer tricks that can accomplish what you're after, either. If you have a region of 36 contiguous bits then the second half does not start on an addressible boundary, so you cannot form a pointer to it.

On the other hand, if the two halves are not contiguous in the first place, then you might be able to go with something like this:

typedef struct {
    __GPIOhalfportbits_t group0;
    __GPIOhalfportbits_t group1;
} __GPIOportbits_t;

You would have to pay attention to alignment of the two half-port pieces, but there is probably an implementation-specific way to get that right. Given that the underlying data (we have now assumed) is not presented as a contiguous span of 36 bits in the first place, forming a union with a 36-bit bitfield does not make sense. It might nevertheless be possible to use a union to map individual single-bit bitfields on top of that pair of structures by inserting explicit padding of the appropriate size, but you need to consider whether any of this is actually worth doing. In particular, see below.


Important other considerations

Bitfields are a tricky business in general, and C makes very few guarantees about their behavior -- many fewer than a lot of people suppose or expect. It is a particularly poor idea to use bitfields to write to hardware ports, because you cannot write fewer than CHAR_BIT bits at once, and if you're writing via a bitfield whose size is not a power-of-two multiple of CHAR_BIT then you will be writing additional bits as well, whose values are unspecified.

I generally recommend avoiding bitfields altogether, except possibly for usage of bitfields in C-language programming interfaces provided by the relevant hardware manufacturer, in a manner consistent with those interfaces' documentation.


Alternatives

You could conceivably come up with some wrapper macros for accessing the GPIO port in terms of two half ports, and even in terms of individual bits within those ports. But this answer is already long, and such a macro-centric approach would be a whole other story.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
0
  1. You can't do that as they live under different addresses in memory.
  2. Using objects to access hardware registers is very inefficient. On this level of programming, you need to optimize code as much as possible.

https://godbolt.org/z/ncbr8o

YOu can only "combine" them by having additional object where you will read the data from that actual registers, and after changes save it to registers.

#include <stdint.h>

#define AXIBRIDGE_BASE_ADDR 0x12340000
#define GPIO_00_BASE  0x400
#define GPIO_01_BASE  0x800

//GPIO00 port
typedef union {
  struct {
    uint32_t GPIO000:1;
    uint32_t GPIO001:1;
    uint32_t GPIO002:1;
    uint32_t GPIO003:1;
    uint32_t GPIO004:1;
    uint32_t GPIO005:1;
    uint32_t GPIO006:1;
    uint32_t GPIO007:1;
    uint32_t GPIO008:1;
    uint32_t GPIO009:1;
    uint32_t GPIO010:1;
    uint32_t GPIO011:1;
    uint32_t GPIO012:1;
    uint32_t GPIO013:1;
    uint32_t GPIO014:1;
    uint32_t GPIO015:1;
    uint32_t GPIO016:1;
    uint32_t GPIO017:1;
  };
  struct {
    uint32_t w:18;
  };
} __GPIO00portbits_t;



typedef union {
  struct {
    uint32_t GPIO000:1;
    uint32_t GPIO001:1;
    uint32_t GPIO002:1;
    uint32_t GPIO003:1;
    uint32_t GPIO004:1;
    uint32_t GPIO005:1;
    uint32_t GPIO006:1;
    uint32_t GPIO007:1;
    uint32_t GPIO008:1;
    uint32_t GPIO009:1;
    uint32_t GPIO010:1;
    uint32_t GPIO011:1;
    uint32_t GPIO012:1;
    uint32_t GPIO013:1;
    uint32_t GPIO014:1;
    uint32_t GPIO015:1;
    uint32_t GPIO016:1;
    uint32_t GPIO017:1;
    uint32_t GPIO100:1;
    uint32_t GPIO101:1;
    uint32_t GPIO102:1;
    uint32_t GPIO103:1;
    uint32_t GPIO104:1;
    uint32_t GPIO105:1;
    uint32_t GPIO106:1;
    uint32_t GPIO107:1;
    uint32_t GPIO108:1;
    uint32_t GPIO109:1;
    uint32_t GPIO110:1;
    uint32_t GPIO111:1;
    uint32_t GPIO112:1;
    uint32_t GPIO113:1;
    uint32_t GPIO114:1;
    uint32_t GPIO115:1;
    uint32_t GPIO116:1;
    uint32_t GPIO117:1;
  };
  struct {
    uint64_t GPIO1w:18;
    uint64_t GPIO2w:18;
  };
} __GPIO12portbits_t;


#define GPIO1       ((volatile __GPIO00portbits_t *)(AXIBRIDGE_BASE_ADDR + GPIO_00_BASE))
#define GPIO2       ((volatile __GPIO00portbits_t *)(AXIBRIDGE_BASE_ADDR + GPIO_01_BASE))

#define COMBINE()   (&(__GPIO12portbits_t){.GPIO1w = GPIO1 -> w, .GPIO2w = GPIO2 -> w})
#define UPDATEGPIO(ptr)  do{GPIO1 -> w = ptr -> GPIO1w; GPIO2 -> w = ptr -> GPIO2w;}while(0)

void foo()
{
    __GPIO12portbits_t *ptr = COMBINE();

    ptr -> GPIO014 = 1;
    ptr -> GPIO110 = 1;

    UPDATEGPIO(ptr);
}

void bar()
{
    GPIO1 -> GPIO014 = 1;
    GPIO2 -> GPIO010 = 1;
}

But it is very inefficient https://godbolt.org/z/jMsc7j

0___________
  • 60,014
  • 4
  • 34
  • 74