Taking advantage of cotiguous bytes in struct = portable?

K

Kobu

In embedded systems (programmed in C), often times structure
declarations are used to group together several status/control/data
registers of external hardware (or even internal registers). The
example below (from GBD's THE C BOOK) uses this. Is this a portable
method? [Note, there is no issue with structure "padding" because no
structure object is defined, only a declaration is used to access
existing hardware].




/*
* Declare the device registers
* Whether to use int or short
* is implementation dependent
*/

struct devregs{
unsigned short volatile csr;
unsigned short const volatile data;
};


/* bit patterns in the csr */
#define ERROR 0x1
#define READY 0x2
#define RESET 0x4

/* absolute address of the device */
#define DEVADDR ((struct devregs *)0xffff0004)

/* number of such devices in system */
#define NDEVS 4

/*
* Busy-wait function to read a byte from device n.
* check range of device number.
* Wait until READY or ERROR
* if no error, read byte, return it
* otherwise reset error, return 0xffff
*/
unsigned int read_dev(unsigned devno){

struct devregs * const dvp = DEVADDR + devno;

if(devno >= NDEVS)
return(0xffff);

while((dvp->csr & (READY | ERROR)) == 0)
; /* NULL - wait till done */

if(dvp->csr & ERROR){
dvp->csr = RESET;
return(0xffff);
}

return((dvp->data) & 0xff);
}



Any portability issues? At first sight this looks like an awesome
technique!
 
R

REH

Kobu said:
[Note, there is no issue with structure "padding" because no
structure object is defined, only a declaration is used to access
existing hardware].

How does mapping the structure directly over that hardware registers'
address space, and not creating an object of the type, alleviate there being
any issues with the layout of the structure?
 
E

Eric Sosman

Kobu said:
In embedded systems (programmed in C), often times structure
declarations are used to group together several status/control/data
registers of external hardware (or even internal registers). The
example below (from GBD's THE C BOOK) uses this. Is this a portable
method? [Note, there is no issue with structure "padding" because no
structure object is defined, only a declaration is used to access
existing hardware].

That's a non sequitur. The struct declaration defines
a memory layout; that memory layout may contain padding; if
you use the padded layout with un-padded hardware things
may not line up correctly. For a particular compiler on a
particular piece of hardware things may work just fine --
but portability problems loom, and you'll need to re-verify
every time you change compiler or machine.

A few other points:
/* absolute address of the device */
#define DEVADDR ((struct devregs *)0xffff0004)

Conversion between integers and pointers is allowed
(in both directions), but the results are implementation-
defined and thus not portable. A construct like the one
you've written will probably work just fine on most "flat
address" machines, but embedded systems are perhaps more
likely than most to have "crinkly" address spaces. This
is another non-portability you'll need to re-verify each
time out.
struct devregs * const dvp = DEVADDR + devno;

Here's where you impose the possibly-padded struct's
memory layout on whatever actually lives at the desired
location. Note that if the struct is padded (and hence
larger than you want), the location will be mis-calculated
if `devno' is non-zero.
if(devno >= NDEVS)
return(0xffff);

It would be better to make this test before trying
to initialize `dvp'. On many systems it's harmless to
generate a bogus pointer value (provided you never use
it), but in principle the mere act of "thinking about"
an illegal address can be a mortal sin. Besides (sigh),
it's "more efficient" to avoid the erroneous calculation
whose result you'll never use anyhow.
Any portability issues?

As noted. However, it might not be worth while to
worry about them overmuch; what you're trying to do is
inherently non-portable, so you're forced to walk on the
wild side. Try to find ways to get the compiler to verify
your non-portable assumptions -- for example, here's an
ugly hack that will generate a compile error if `struct
devregs' contains any padding:

/* Message To Those Who Come After Me: I don't
* really care about this array type, and I'll
* never actually use it for anything; it's just
* here to make the compiler whine if there's any
* padding in a `struct devregs'. If you get a
* complaint about an invalid array size or some
* such thing, it's not really about this array
* at all: it just means that `struct devregs'
* isn't laid out the way I want, and that code
* using `struct devregs' won't work with this
* compiler. Too bad -- but at least you found
* out at compile time, instead of by trying to
* debug some mysterious misbehavior.
*/
typedef char bogus_array_type[
sizeof(struct devregs) == 2 * sizeof(short) ];

(When using such a hack, the long-winded comment is an
essential component; don't omit it.)
 
C

Christian Bau

"Kobu said:
In embedded systems (programmed in C), often times structure
declarations are used to group together several status/control/data
registers of external hardware (or even internal registers). The
example below (from GBD's THE C BOOK) uses this. Is this a portable
method? [Note, there is no issue with structure "padding" because no
structure object is defined, only a declaration is used to access
existing hardware].




/*
* Declare the device registers
* Whether to use int or short
* is implementation dependent
*/

struct devregs{
unsigned short volatile csr;
unsigned short const volatile data;
};


/* bit patterns in the csr */
#define ERROR 0x1
#define READY 0x2
#define RESET 0x4

/* absolute address of the device */
#define DEVADDR ((struct devregs *)0xffff0004)

/* number of such devices in system */
#define NDEVS 4

/*
* Busy-wait function to read a byte from device n.
* check range of device number.
* Wait until READY or ERROR
* if no error, read byte, return it
* otherwise reset error, return 0xffff
*/
unsigned int read_dev(unsigned devno){

struct devregs * const dvp = DEVADDR + devno;

if(devno >= NDEVS)
return(0xffff);

while((dvp->csr & (READY | ERROR)) == 0)
; /* NULL - wait till done */

if(dvp->csr & ERROR){
dvp->csr = RESET;
return(0xffff);
}

return((dvp->data) & 0xff);
}



Any portability issues? At first sight this looks like an awesome
technique!

Best to throw that book away before it does any more damage.

The code will crash with any modern operating system that will
absolutely refuse to let you access hardware directly. And rightfully
so. Device drivers access hardware, maybe code running on some
embededded systm, but nothing else does.
 
A

Alan Balmer

Best to throw that book away before it does any more damage.

The code will crash with any modern operating system that will
absolutely refuse to let you access hardware directly. And rightfully
so. Device drivers access hardware, maybe code running on some
embededded systm, but nothing else does.

See the OP's first line.
 
K

kingzog

Kobu said:
In embedded systems (programmed in C), often times structure
declarations are used to group together several status/control/data
registers of external hardware (or even internal registers). The
example below (from GBD's THE C BOOK) uses this. Is this a portable
method? [Note, there is no issue with structure "padding" because no
structure object is defined, only a declaration is used to access
existing hardware].


/*
* Declare the device registers
* Whether to use int or short
* is implementation dependent
*/

struct devregs{
unsigned short volatile csr;
unsigned short const volatile data;
};


As others have pointed out, structure padding can still bite you,
because it could cause devregs.data to refer to the wrong memory
location in the first structure if there are padding bytes between csr
and data. Also later structures (DEVADDR + n) might not line up with
the hardware if the first has some tail packing bytes.

Also, I don't like using structures like this for i/o, because some
platforms may need special instructions to do the access - e.g. x86
with IN and OUT, or Risc chips which need memory barrier instructions.
Having said that, it's a popular technique if you develop for something
where neither of these apply.

I usually use something like this

/* Structure packing for ARMCC */
#ifdef __arm
#define PACKED_STRUCT __packed
#else
#define PACKED_STRUCT
/* check the structure size for compilers other than ARMCC!
build with DEBUG defined, check the printfs. Also check Ux types! */
#endif

typedef unsigned char U8;
typedef unsigned short U16;
typedef unsigned int U32;

typedef PACKED_STRUCT struct {
U16 abc;
U16 def;
} HW_STRUCT;

I'd at least do a debug check that the sizeof the structure is what you
expect - e.g

#ifdef DEBUG
if (sizeof(U16) != 2 )
printf ("Fix U16 typedef!\n" );
if (sizeof(HW_STRUCT) !=4))
printf ("Fix packing\n" );
#endif

E.g. for GCC on Arm the the types would be the same, but structure
packing is done with __attribute__ ((packed)) on the elements. IIRC you
can use this in a macro too - e.g.

#ifdef __GNUC__
#define PACKED_ELEM __attribute__ ((packed))
#endif

typedef struct
{
U8 xyz;
U32 abc PACKED_ELEM;
U32 def PACKED_ELEM;
} struct;

You're better off asking this sort of question in comp.arch.embedded
BTW - I've set the follow ups.

Zog.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,578
Members
45,052
Latest member
LucyCarper

Latest Threads

Top