Re: [PATCH 02/10] MCDE: Add configuration registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 12 November 2010, Russell King - ARM Linux wrote:
> On Fri, Nov 12, 2010 at 04:14:51PM +0100, Arnd Bergmann wrote:
> > Some people prefer to express all this in C instead of macros:
> > 
> > struct mcde_registers {
> > 	enum {
> > 		mcde_cr_dsicmd2_en = 0x00000001,
> > 		mcde_cr_dsicmd1_en = 0x00000002,
> > 		...
> > 	} cr;
> > 	enum {
> > 		mcde_conf0_syncmux0 = 0x00000001,
> > 		...
> > 	} conf0;
> > 	...
> > };
> > 
> > This gives you better type safety, but which one you choose is your decision.
> 
> It is a bad idea to describe device registers using C structures, and
> especially enums.
> 
> The only thing C guarantees about structure layout is that the elements
> are arranged in the same order which you specify them in your definition.
> It doesn't make any guarantees about placement of those elements within
> the structure.

Right, I got carried away when seeing the macro overload. My example
would work on a given architecture since the ABI is not changing, but
we should of course not advocate nonportable code.

Normally what I do is to describe the data structure in C and define the
values in a separate enum. The main advantage of using the struct instead
of offset defines is that you have a bit more type safety, i.e. you cannot
accidentally do readw() on a __be32 member.

Using #define for the actual values makes it possible to interleave the
values with the structure definition like 

struct mcde_registers {
 	__le32 cr;
#define MCDE_CR_DSICMD2_EN 0x00000001
#define MCDE_CR_DSICMD1_EN 0x00000002
	__le32 conf0;
 	...
};

whereas the enum has the small advantage of putting the identifiers
into the C language namespace rather than the preprocessor macro
namespace.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux