0

I am working on a Motorola HCS08 µCU in CodeWarrior V10.6, I am trying to create an extern bitfield which has bits from existing registers. The way the bitfields are created in the µCU header is like

typedef unsigned char byte;
typedef union {
  byte Byte;
  struct {
    byte PTAD0       :1;
    byte PTAD1       :1;                                     
    byte PTAD2       :1;                                     
    byte PTAD3       :1;                                     
    byte PTAD4       :1;                                     
    byte PTAD5       :1;                                     
    byte PTAD6       :1;                                     
    byte PTAD7       :1;                                     
  } Bits;
} PTADSTR;
extern volatile PTADSTR _PTAD @0x00000000;
#define PTAD                            _PTAD.Byte
#define PTAD_PTAD0                      _PTAD.Bits.PTAD0
#define PTAD_PTAD1                      _PTAD.Bits.PTAD1
#define PTAD_PTAD2                      _PTAD.Bits.PTAD2
#define PTAD_PTAD3                      _PTAD.Bits.PTAD3
#define PTAD_PTAD4                      _PTAD.Bits.PTAD4
#define PTAD_PTAD5                      _PTAD.Bits.PTAD5
#define PTAD_PTAD6                      _PTAD.Bits.PTAD6
#define PTAD_PTAD7                      _PTAD.Bits.PTAD7

Which will let the register value be changed either by PTAD = 0x01, or PTAD_PTAD0 = 1, for example. This definition is basically the same for PTAD, PTBD, PTCD, ... PTGD, the only thing changing is the address.

My attemp to create a custom bitfield out of the previous existing variables is

typedef union {
  byte Byte;
  struct {
    byte *DB0;
    byte *DB1;
    byte *DB2;
    byte *DB3;
    byte *DB4;
    byte *DB5;
    byte *DB6;
    byte *DB7;
  } Bits;
} LCDDSTR;

I would create and initialize the bitfield as LCDDSTR lcd = {{&PTGD_PTGD6, &PTBD_PTBD5, ...}}, because by some reason, the initialization like LCDSTR lcd = {*.Bits.DB0 = &PTGD_PTGD6, *.Bits.DB1 = &PTBD_PTBD5, ...} (treating it as a struct, please correct me again) advice in How to initialize a struct in accordance with C programming language standards does not work with this compiler (it does work on an online compiler).

However, as you may see I am sort of grouping the bits, and (if it would work) I would be able to change the values of the actual register by doing *lcd.Bits.DB0 = 1, or something like that, but if I do lcd.Byte = 0x00, I would be changing the last (I think) byte of the memory address contained in lcd.Bits.DB0, you know, because the struct doesn't actually contains the data, but the pointers instead.

How would I go on achieving a struct that is able to contain and modify bits from several registers? (I guess the problem here is that in memory the bits are not one next to the other, which I guess would make it easier). Is it even possible? I hope it is.

Community
  • 1
  • 1
Hans
  • 224
  • 2
  • 13
  • Are there memory addresses that are mapped to particular registers? For instance memory address 0x00000000 is mapped to Register 0 and memory address 0x00000001 is mapped to Register 1 (assuming 1 byte or 8 bit registers)? I would expect to use the same `PTADSTR` struct for the definition and then create create a pointer to the beginning of the memory area where the registers are located and reference them as an array of the `PTADSTR` struct. – Richard Chambers Jan 13 '17 at 05:23
  • You cannot have a pointer to a bitfield. Look at http://stackoverflow.com/questions/13547352/c-cannot-take-address-of-bit-field. – Rishikesh Raje Jan 13 '17 at 05:37
  • @RichardChambers I can say that further in the definition a PTADDSTR struct is defined to 0x00000001, and then PTBDSTR to 0x00000002, then PTBDDSTR to 0x00000003 and so on, and yes 8 bit registers, I guess that answers your first questions, but I don't get your idea, create an array with pointers to the bits of each struct, you say? – Hans Jan 13 '17 at 05:45
  • Pointers not to each bit of the struct but rather a pointer to the beginning of the memory area mapped to the 8 bit registers which would be an array of the PTADSTR struct. Then use `_PTAD[0].Byte = 4;` or `_PTAD[0].Bits.PTAD1 = 1;` with `volatile PTADSTR (* const _PTAD) = 0x00000000;` which I understand is the similar to the meaning of `extern volatile PTADSTR _PTAD @0x00000000;`. You would want to use some name other than `_PTAD` though since the system headers already use that. – Richard Chambers Jan 13 '17 at 05:51
  • However I may be misunderstanding your question and what you want to accomplish as it is not totally clear to me what it is you are wanting to do. – Richard Chambers Jan 13 '17 at 05:56
  • This is the textbook definition of the worst thing you can do. bitfields are your first problem, using a union in this way is incorrect, and using structure across compile domains and three strikes you are out. Might work initially but becomes a maintenance problem down the road. – old_timer Jan 13 '17 at 14:49

4 Answers4

1

How would I go on achieving a struct that is able to contain and modify bits from several registers? (I guess the problem here is that in memory the bits are not one next to the other..

I don't think you can do it with a struct. That is because bitfields by definition have to occupy the same or contiguous addresses.

However macros may be useful here

#define DB0  PTGD_PTGD6
#define DB1  PTBD_PTBD5
....

And to clear the bits to all 0's or set to all 1's you can use a multiline macro

#define SET_DB(x) do { \
    PTGD_PTGD6 = x;    \
    PTBD_PTBD5 = x;    \
    ......             \
} while(0) 
Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31
  • Thank you so much for your answer. I like this approach, I was expecting something sort of `DB = 0xAB` as stated on my question, but I guess I could write a macro `SET_DB(x)` such that I can access each bit in `x` and assign it to each BD bit, for example, let me try it out to see if it works as expected. I also have to see if that is enough (I have in mind that for some reason I need the bits contained in a variable). – Hans Jan 13 '17 at 06:25
  • Made plenty of tests and worked like a charm, thank you so much. – Hans Jan 19 '17 at 23:53
1

How would I go on achieving a struct that is able to contain and modify bits from several registers?

You can't.

A structure must represent a single, continuous block of memory -- otherwise, operations like taking the sizeof the structure, or performing operations on a pointer to one would make no sense.

If you want to permute the bits of a value, you will need to find some way of doing so explicitly. If the order of your bits is relatively simple, this may be possible with a few bitwise operations; if it's weirder, you may need to use a lookup table.

Beyond that: bitfields in C are pretty limited. The language does not make a lot of guarantees about how a structure containing bitfields will end up laid out in memory; they are generally best avoided for portable code. (Which doesn't apply here, as you're writing code for a specific compiler/microcontroller combination, but it's worth keeping in mind in general.)

1

Your union does unfortunately not make any sense, because it forms a union of one byte and 8 byte*. Since a pointer is 16 bit on HCS08, this ends up as 8*2 = 16 bytes of data, which can't be used in any meaningful way.

  • Please note that the C structure called bit-fields is very poorly specified by the standard and therefore should be avoided in any program. See this.
  • Please note that the Codewarrior register maps aren't remotely close to following the C standard (nor MISRA-C).
  • Please note that structs in general are problematic for hardware register mapping, since structs can contain padding. You don't have that problem on HCS08 specifically, since it doesn't require alignment of data. But most MCUs do require that.

It is therefore better to roll out your own register map in standard C if you have that option. The port A data register could simply be defined like this:

#define PTAD    (*(volatile uint8_t*)0x0000U)
#define PTAD7   (1U << 7)
#define PTAD6   (1U << 6)
#define PTAD5   (1U << 5)
#define PTAD4   (1U << 4)
#define PTAD3   (1U << 3)
#define PTAD2   (1U << 2)
#define PTAD1   (1U << 1)
#define PTAD0   (1U << 0)

As we can tell, defining the bit masks is mostly superfluous anyway, as PTAD |= 1 << 7; is equally readable to PTAD |= PTAD7;. This is because this was a pure I/O port. Defining textual bit masks for status and control registers on the other hand, increases the readability of the code significantly.


If you want to modify bits from several registers, you'd do something like the following:

Assume we have a RGB (red-green-blue) LED, common cathode, with 3 colors connected to 3 different pins on 3 different ports. Instead of beating up the PCB designer, you could do this:

#define RGB_RED_PTD     PTAD
#define RGB_RED_PTDD    PTADD
...
#define RGB_BLUE_PTD    PTBD
#define RGB_BLUE_PTDD   PTBDD
...
#define RGB_GREEN_PTD   PTDD
#define RGB_GREEN PTDD  PTDDD

#define RGB_RED_PIN    1
#define RGB_BLUE_PIN   5
#define RGB_GREEN_PIN  3

You can now set these independently of where they happen to be located on the hardware:

void rgb_init (void)
{
  RGB_RED_PTDD   |= (1 << RGB_RED_PIN);
  RGB_BLUE_PTDD  |= (1 << RGB_BLUE_PIN);
  RGB_GREEN_PTDD |= (1 << RGB_GREEN_PIN);
}

void rgb_yellow (void)
{
  RGB_RED_PTD    |=  (1 << RGB_RED_PIN);
  RGB_BLUE_PTD   &= ~(1 << RGB_BLUE_PIN);
  RGB_GREEN_PTD  |=  (1 << RGB_GREEN_PIN);
}

And so on. Examples were for HCS08 but the same can of course be used universally on any MCU with direct port I/O.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396
0

It sounds like an approach such as the following is along the lines of where you would like to go with a solution.

I have not tested this as I do not have the hardware however this should provide an alternative to look at.

This assumes that you want to turn on particular pins or turn off particular pins but there will not be a case where you will want to turn on some pins and turn off other pins for a particular device in a single operation. If that should be the case I would consider making the type of RegPinNo be an unsigned short to include an op code for each register/pin number combination.

This also assumes that timing of operations is not a critical constraint and that the hardware has sufficient horsepower such that small loops are not much of a burden on throughput and hogging CPU time needed for other things. So this code may need changes to improve optimization if that is a consideration.

I assume that you want some kind of a easily readable way of expressing a command that will turn on and off a series of bits scattered across several areas of memory.

The first thing is to come up with a representation of what such a command would look like and it seems to me that borrowing from a char array to represent a string would suffice.

typedef byte RegPinNo;    // upper nibble indicates register number 0 - 7, lower nibble indicates pin number 0 - 7

const byte REGPINNOEOS = 0xff;   // the end of string for a RegPinNo array.

And these would be used to define an array of register/pin numbers as in the following.

RegPinNo myLed[] = { 0x01, 0x12, REGPINNOEOS };  // LED is addressed through Register 0, Pin 0 and Register 1, Pin 1 (zero based)

So at this point we have a way to describe that a particular device, an LED in this case, is addressed through a series of register/pin number items.

Next lets create a small library of functions that will use this representation to actually modify the specific pins in specific registers by traversing this array of register/pin numbers and performing an operation on it such as setting the bit in the register or clearing the bit in the register.

typedef unsigned char byte;
typedef union {
    byte Byte;
    struct {
        byte PTAD0 : 1;
        byte PTAD1 : 1;
        byte PTAD2 : 1;
        byte PTAD3 : 1;
        byte PTAD4 : 1;
        byte PTAD5 : 1;
        byte PTAD6 : 1;
        byte PTAD7 : 1;
    } Bits;
} PTADSTR;

// Define a pointer to the beginning of the register area. This area is composed of
// 8 different registers each of which is one byte in size.
// We will address these registers as Register 0, Register 1, ... Register 7 which just happens
// to be how C does its zero based indexing.
// The bits representing pins on the PCB we will address as Pin 0, Pin 1, ... Pin 7.
extern volatile PTADSTR (* const _PTAD)  = 0x00000000;

void SetRegPins(RegPinNo *x)
{
    byte pins[] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80 };
    int i;
    for (i = 0; x[i] != REGPINNOEOS; i++) {
        byte bRegNo = (x[i] >> 4) & 0x07;  // get the register number, 0 - 7
        byte bPinNo = x[i] & 0x07;         // get the pin number, 0 - 7
        _PTAD[bRegNo].Byte |= pins[bPinNo];
    }
}

void ClearRegPins(RegPinNo *x)
{
    byte pins[] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80 };
    int i;
    for (i = 0; x[i] != REGPINNOEOS; i++) {
        byte bRegNo = (x[i] >> 4) & 0x07;  // get the register number, 0 - 7
        byte bPinNo = x[i] & 0x07;         // get the pin number, 0 - 7
        _PTAD[bRegNo].Byte &= ~pins[bPinNo];
    }
}

void ToggleRegPins(RegPinNo *x)
{
    byte pins[] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80 };
    int i;
    for (i = 0; x[i] != REGPINNOEOS; i++) {
        byte bRegNo = (x[i] >> 4) & 0x07;  // get the register number, 0 - 7
        byte bPinNo = x[i] & 0x07;         // get the pin number, 0 - 7
        _PTAD[bRegNo].Byte ^= pins[bPinNo];
    }
}

You would use the above something like the following. Not sure what a time delay function would look like in your environment so I am using a function Sleep() which takes an argument as to the number of milliseconds to delay or sleep.

void LightLed (int nMilliSeconds)
{
    RegPinNo myLed[] = { 0x01, 0x12, REGPINNOEOS };  // LED is addressed through Register 0, Pin 0 and Register 1, Pin 1 (zero based)

    SetRegPins(myLed);    // turn on the LED
    Sleep(nMilliSeconds); // delay for a time with the LED lit
    ClearRegPins(myLed);  // turn the LED back off
}

Edit - A Refinement

A more efficient implementation that would allow multiple pins to be set in a particular register at the same time would be to define the use of RegPinNo as being an unsigned short` with the upper byte being the register number and the lower byte being the pins to manipulate as a bit mask for the byte.

With this approach you would have a SetRegPins() function that would look like the following. A similar change would be needed for the other functions.

void SetRegPins(RegPinNo *x)
{
    int i;
    for (i = 0; x[i] != REGPINNOEOS; i++) {
        byte bRegNo = (x[i] >> 8) & 0x07;  // get the register number, 0 - 7
        byte bPinNo = x[i] & 0xFF;         // get the pin mask
        _PTAD[bRegNo].Byte |= bPinNo;
    }
}

And the typedefs would look like:

typedef unsigned short RegPinNo;    // upper byte indicates register number 0 - 7, lower byte provides pin mask

const byte REGPINNOEOS = 0xffff;   // the end of string for a RegPinNo array.

And these elements would be used like:

void LightLed (int nMilliSeconds)
{
    RegPinNo myLed[] = { 0x0002, 0x0103, REGPINNOEOS };  // LED is addressed through Register 0, Pin 1 and Register 1, Pin 0 and Pin 1 (zero based)

    SetRegPins(myLed);    // turn on the LED
    Sleep(nMilliSeconds); // delay for a time with the LED lit
    ClearRegPins(myLed);  // turn the LED back off
}
Richard Chambers
  • 16,643
  • 4
  • 81
  • 106