Question

I am trying to perform a less-than-32bit read over the PCI bus to a VME-bridge chip (Tundra Universe II), which will then go onto the VME bus and picked up by the target.

The target VME application only accepts D32 (a data width read of 32bits) and will ignore anything else.

If I use bit field structure mapped over a VME window (nmap'd into main memory) I CAN read bit fields >24 bits, but anything less fails. ie :-

struct works {
    unsigned int a:24;
};

struct fails {
    unsigned int a:1;
    unsigned int b:1;
    unsigned int c:1;
};

struct main {
    works work;
    fails fail;
}
volatile *reg = function_that_creates_and_maps_the_vme_windows_returns_address()

This shows that the struct works is read as a 32bit, but a read via fails struct of a for eg reg->fail.a is getting factored down to a X bit read. (where X might be 16 or 8?)

So the questions are :
a) Where is this scaled down? Compiler? OS? or the Tundra chip?
b) What is the actual size of the read operation performed?

I basiclly want to rule out everything but the chip. Documentation on that is on the web, but if it can be proved that the data width requested over the PCI bus is 32bits then the problem can be blamed on the Tundra chip!

edit:-
Concrete example, code was:-

struct SVersion
{
    unsigned title         : 8;
    unsigned pecversion    : 8;
    unsigned majorversion  : 8;
    unsigned minorversion  : 8;
} Version;

So now I have changed it to this :-

union UPECVersion
{
    struct SVersion
    {
        unsigned title         : 8;
        unsigned pecversion    : 8;
        unsigned majorversion  : 8;
        unsigned minorversion  : 8;
    } Version;
    unsigned int dummy;
};

And the base main struct :-

typedef struct SEPUMap
{
    ...
    ...
    UPECVersion PECVersion;

};

So I still have to change all my baseline code

// perform dummy 32bit read
pEpuMap->PECVersion.dummy;

// get the bits out
x = pEpuMap->PECVersion.Version.minorversion;

And how do I know if the second read wont actually do a real read again, as my original code did? (Instead of using the already read bits via the union!)

Was it helpful?

Solution

As an example, the Linux kernel has inline functions that explicitly handle memory-mapped IO reads and writes. In newer kernels it's a big macro wrapper that boils down to an inline assembly movl instruction, but it older kernels it was defined like this:

#define readl(addr) (*(volatile unsigned int *) (addr))
#define writel(b,addr) ((*(volatile unsigned int *) (addr)) = (b))

OTHER TIPS

Your compiler is adjusting the size of your struct to a multiple of its memory alignment setting. Almost all modern compilers do this. On some processors, variables and instructions have to begin on memory addresses that are multiples of some memory alignment value (often 32-bits or 64-bits, but the alignment depends on the processor architecture). Most modern processors don't require memory alignment anymore - but almost all of them see substantial performance benefit from it. So the compilers align your data for you for the performance boost.

However, in many cases (such as yours) this isn't the behavior you want. The size of your structure, for various reasons, can turn out to be extremely important. In those cases, there are various ways around the problem.

One option is to force the compiler to use different alignment settings. The options for doing this vary from compiler to compiler, so you'll have to check your documentation. It's usually a #pragma of some sort. On some compilers (the Microsoft compilers, for instance) it's possible to change the memory alignment for only a very small section of code. For example (in VC++):

#pragma pack(push)      // save the current alignment
#pragma pack(1)         // set the alignment to one byte
// Define variables that are alignment sensitive
#pragma pack(pop)       // restore the alignment

Another option is to define your variables in other ways. Intrinsic types are not resized based on alignment, so instead of your 24-bit bitfield, another approach is to define your variable as an array of bytes.

Finally, you can just let the compilers make the structs whatever size they want and manually record the size that you need to read/write. As long as you're not concatenating structures together, this should work fine. Remember, however, that the compiler is giving you padded structs under the hood, so if you make a larger struct that includes, say, a works and a fails struct, there will be padded bits in between them that could cause you problems.

On most compilers, it's going to be darn near impossible to create a data type smaller than 8 bits. Most architectures just don't think that way. This shouldn't be a huge problem because most hardware devices that use datatypes of smaller than 8-bits end up arranging their packets in such a way that they still come in 8-bit multiples, so you can do the bit manipulations to extract or encode the values on the data stream as it leaves or comes in.

For all of the reasons listed above, a lot of code that works with hardware devices like this work with raw byte arrays and just encode the data within the arrays. Despite losing a lot of the conveniences of modern language constructs, it ends up just being easier.

I am wondering about the value of sizeof(struct fails). Is it 1? In this case, if you perform the read by dereferencing a pointer to a struct fails, it looks correct to issue a D8 read on the VME bus.

You can try to add a field unsigned int unused:29; to your struct fails.

The size of a struct is not equal to the sum of the size of its fields, including bit fields. Compilers are allowed, by the C and C++ language specifications, to insert padding between fields in a struct. Padding is often inserted for alignment purposes.

The common method in embedded systems programming is to read the data as an unsigned integer then use bit masking to retrieve the interesting bits. This is due to the above rule that I stated and the fact that there is no standard compiler parameter for "packing" fields in a structure.

I suggest creating an object ( class or struct) for interfacing with the hardware. Let the object read the data, then extract the bits as bool members. This puts the implementation as close to the hardware. The remaining software should not care how the bits are implemented.

When defining bit field positions / named constants, I suggest this format:

#define VALUE (1 << BIT POSITION)
// OR
const unsigned int VALUE = 1 << BIT POSITION;

This format is more readable and has the compiler perform the arithmetic. The calculation takes place during compilation and has no impact during run-time.

Ian - if you want to be sure as to the size of things you're reading/writing I'd suggest not using structs like this to do it - it's possible the sizeof of the fails struct is just 1 byte - the compiler is free to decide what it should be based on optimizations etc- I'd suggest reading/writing explicitly using int's or generally the things you need to assure the sizes of and then doing something else like converting to a union/struct where you don't have those limitations.

It is the compiler that decides what size read to issue. To force a 32 bit read, you could use a union:

union dev_word {
    struct dev_reg {
        unsigned int a:1;
        unsigned int b:1;
        unsigned int c:1;
    } fail;
    uint32_t dummy;
};

volatile union dev_word *vme_map_window();

If reading the union through a volatile-qualified pointer isn't enough to force a read of the whole union (I would think it would be - but that could be compiler-dependent), then you could use a function to provide the required indirection:

volatile union dev_word *real_reg; /* Initialised with vme_map_window() */

union dev_word * const *reg_func(void)
{
    static union dev_word local_copy;
    static union dev_word * const static_ptr = &local_copy;

    local_copy = *real_reg;
    return &static_ptr;
}

#define reg (*reg_func())

...then (for compatibility with the existing code) your accesses are done as:

reg->fail.a

The method described earlier of using the gcc flag -fstrict-volatile-bitfields and defining bitfield variables as volatile u32 works, but the total number of bits defined must be greater than 16.

For example:

typedef     union{
    vu32    Word;
    struct{
        vu32    LATENCY     :3;
        vu32    HLFCYA      :1;
        vu32    PRFTBE      :1;
        vu32    PRFTBS      :1;  
    };
}tFlashACR;
.
tFLASH* const pFLASH    =   (tFLASH*)FLASH_BASE;
#define FLASH_LATENCY       pFLASH->ACR.LATENCY
.
FLASH_LATENCY = Latency;

causes gcc to generate code

.
ldrb r1, [r3, #0]
.

which is a byte read. However, changing the typedef to

typedef     union{
    vu32    Word;
    struct{
        vu32    LATENCY     :3;
        vu32    HLFCYA      :1;
        vu32    PRFTBE      :1;
        vu32    PRFTBS      :1;
        vu32                :2;

        vu32    DUMMY1      :8;

        vu32    DUMMY2      :8;
    };
}tFlashACR;

changes the resultant code to

.
ldr r3, [r2, #0]
.

I believe the only solution is to
1) edit/create my main struct as all 32bit ints (unsigned longs)
2) keep my original bit-field structs
3) each access I require,
3.1) I have to read the struct member as a 32bit word, and cast it into the bit-field struct,
3.2) read the bit-field element I require. (and for writes, set this bit-field, and write the word back!)

(1) Which is a same, because then I lose the intrinsic types that each member of the "main/SEPUMap" struct are.

End solution :-
Instead of :-

printf("FirmwareVersionMinor: 0x%x\n", pEpuMap->PECVersion);

This :-

SPECVersion ver = *(SPECVersion*)&pEpuMap->PECVersion;

printf("FirmwareVersionMinor: 0x%x\n", ver.minorversion);

Only problem I have is writting! (Writes are now Read/Modify/Writes!)

// Read - Get current
_HVPSUControl temp = *(_HVPSUControl*)&pEpuMap->HVPSUControl;

// Modify - set to new value
temp.OperationalRequestPort = true;

// Write
volatile unsigned int *addr = reinterpret_cast<volatile unsigned int*>(&pEpuMap->HVPSUControl);

*addr = *reinterpret_cast<volatile unsigned int*>(&temp);

Just have to tidy that code up into a method!

#define writel(addr, data) ( *(volatile unsigned long*)(&addr) = (*(volatile unsigned long*)(&data)) )

I had same problem on ARM using GCC compiler, where write into memory is only through bytes rather than 32bit word.

The solution is to define bit-fields using volatile uint32_t (or required size to write):

union {
    volatile uint32_t XY;
    struct {
        volatile uint32_t XY_A : 4;
        volatile uint32_t XY_B : 12;
    };
};

but while compiling you need add to gcc or g++ this parameter:

-fstrict-volatile-bitfields

more in gcc documentation.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top