klemen_dovrtel wrote: > But i still don't know if this register
definition are correct.
> Shouldn't be there another REG_8 _pad0[3]; after the REG_8 feed;?
> Isn't the WDTV pointing at wrong adress (0x0xE000 0009)?
>
> // lpcWD.h
> // Watchdog Registers
> typedef struct
> {
> REG_8 mod; // Watchdog Mode Register
> REG_8 _pad0[3];
> REG32 tc; // Watchdog Time Constant Register
> REG_8 feed; // Watchdog Feed Register
> REG32 tv; // Watchdog Time Value Register
> } wdRegs_t;
>
> //lpc21xx.h
> #define WD ((wdRegs_t *)0xE0000000)
> // Watchdog Registers
> #define WDMOD WD->mod /* Watchdog Mode Register */
> #define WDTC WD->tc /* Watchdog Time Constant
> Register */
> #define WDFEED WD->feed /* Watchdog Feed Register */
> #define WDTV WD->tv /* Watchdog Time Value Register */
Yes there should be, but should probably be called _pad1[3]. However as
pointed out in other replies, the code will probably work as is unless
the struct is marked as 'packed'.
Reply by Brendan Murphy●November 7, 20062006-11-07
--- In l..., "klemen_dovrtel"
wrote: >
> But i still don't know if this register definition are correct.
> Shouldn't be there another REG_8 _pad0[3]; after the REG_8 feed;?
> Isn't the WDTV pointing at wrong adress (0x0xE000 0009)?
>
> // lpcWD.h
> // Watchdog Registers
> typedef struct
> {
> REG_8 mod; // Watchdog Mode Register
> REG_8 _pad0[3];
> REG32 tc; // Watchdog Time Constant Register
> REG_8 feed; // Watchdog Feed Register
> REG32 tv; // Watchdog Time Value Register
> } wdRegs_t;
>
> //lpc21xx.h
> #define WD ((wdRegs_t *)0xE0000000)
> // Watchdog Registers
> #define WDMOD WD->mod /* Watchdog Mode Register */
> #define WDTC WD->tc /* Watchdog Time Constant
> Register */
> #define WDFEED WD->feed /* Watchdog Feed Register */
> #define WDTV WD->tv /* Watchdog Time Value Register */
>
The short answer is that nobody can answer your question ("is the
definition correct?"), as it's entirely dependent on what compiler
you're using.
That's why I recommended not using structs to map onto hardware.
If you insist on doing this, you'll have to either read the
compiler's documentation and/or experiment to see what it's doing
and then play around with packing to get it to work as expected.
Brendan.
Reply by Karl Olsen●November 7, 20062006-11-07
--- In l..., "klemen_dovrtel"
wrote: >
> But i still don't know if this register definition are correct.
> Shouldn't be there another REG_8 _pad0[3]; after the REG_8 feed;?
> Isn't the WDTV pointing at wrong adress (0x0xE000 0009)?
>
> // lpcWD.h
> // Watchdog Registers
> typedef struct
> {
> REG_8 mod; // Watchdog Mode Register
> REG_8 _pad0[3];
> REG32 tc; // Watchdog Time Constant Register
> REG_8 feed; // Watchdog Feed Register
> REG32 tv; // Watchdog Time Value Register
> } wdRegs_t;
>
> //lpc21xx.h
> #define WD ((wdRegs_t *)0xE0000000)
> // Watchdog Registers
> #define WDMOD WD->mod /* Watchdog Mode Register */
> #define WDTC WD->tc /* Watchdog Time Constant
> Register */
> #define WDFEED WD->feed /* Watchdog Feed Register */
> #define WDTV WD->tv /* Watchdog Time Value Register */
Unless you tell the compiler to "pack" the struct fields, it most
likely inserts padding between feed and tv so that the 32-bit tv
value ends up on an aligned address, and it then happens to work.
Best to check for this and other potential problems by checking if
the final size of the structure is as expected.
In my own struct definitions I just declare all registers (except the
explicitly halfword- and byte-addressable FIO registers) as 32-bit.
Karl Olsen
Reply by klemen_dovrtel●November 7, 20062006-11-07
But i still don't know if this register definition are correct.
Shouldn't be there another REG_8 _pad0[3]; after the REG_8 feed;?
Isn't the WDTV pointing at wrong adress (0x0xE000 0009)?
Reply by Brendan Murphy●November 7, 20062006-11-07
--- In l..., Tom Walsh wrote: > When using a typedef, you are maintaining the
abstraction. Keeping the > IDEA of the operation in the abstract on the coding
level leaves the > IMPLEMENTATION up to the compiler. Since the
compiler is a 'automatic' > process, the compiler (et.al) does the grunt work of
implementing your idea. >
Agreed 100%. Software works on abstract objects, so best to keep
them like that.
> With a #define you can build complex structures but
with a lot of
> tedious detail. With a typedef, you keep it in the abstract. For
> example, this is the Transmit Status Vector information returned by the > ENC28J60 ethernet chip:
> typedef union {
> BYTE v[7];
> struct {
> WORD ByteCount;
> unsigned CollisionCount:4;
> unsigned CRCError:1;
> unsigned LengthCheckError:1;
> unsigned LengthOutOfRange:1;
> unsigned Done:1;
> unsigned Multicast:1;
> unsigned Broadcast:1;
> unsigned PacketDefer:1;
> unsigned ExcessiveDefer:1;
> unsigned MaximumCollisions:1;
> unsigned LateCollision:1;
> unsigned Giant:1;
> unsigned Underrun:1;
> WORD BytesTransmittedOnWire;
> unsigned ControlFrame:1;
> unsigned PAUSEControlFrame:1;
> unsigned BackpressureApplied:1;
> unsigned VLANTaggedFrame:1;
> unsigned Zeros:4;
> } bits;
> } TXSTATUS;
>
The problem with this is that it relies on information on how the
compiler stores structs, unions, words (little or big-end?), none of
which are defined by the language. You have to:
(a) carefully lay out the structure, using either published info or
(worse) experimentation to ensure you got it right
(b) do this every time you move to a new compiler
(c) do thie every time you upgrade your compiler
That to me is a lot of (error-prone) work.
> If you wanted to get the number of
BytesTransmittedOnWire, how would you > do that using a #define? Something like this?
>
> #define BytesTransmitted 1
> number = (WORD) *TxStatusVector [BytesTransmitted];
>
Absolutely not! This is non-portable as well.
I'd do something like:
/* pointer to block of memory with TX status vector: */
unsigned char *tx_status_vector;
/* the "abstract" info we want (could be part of struct typedef) */
> The problem with a define is that you MUST remember
to cast the pointer > value into a WORD, if you forget to do that then you
will get an 8bit > value instead. With a typedef, you merely access the
desired member of > the structure and 'fahget about it'. ;-)
Here it is, still in the > abstraction:
>
> number = *TxStatusVector.BytesTransmitted;
>
This is great, but it is absolutely not portable, which is why I
wouldn't use it in this context (i.e. mapping directly to memory).
> It is really difficult to create a problem for
yourself with a
> typedef! If, for example, "number" is an "unsigned char" then the
> compiler will dump errors or warning back to you. If, when using a > #define, you forget to cast the result to a WORD, the
compiler may
> silently do it.
>
Again: agreed 100%.
> OH, keep in mind too that when I determined that "BytesTransmitted" had > an index value of "1", I had to count the bytes to
make sure I was
> referencing the correct one, with a typedef, you don't need to count > anything (beyond when you initially define the
structure).
>
Either way, you only have to define the layout once. With using
#define to define the offsets you just have to match what's in the
data sheet or protocol spec. With defining structs to map onto
memory you have (as well as this) to have an intimate knowledge of
how the compiler stores information, and are left wiuth something
that is likely to cause problems down the line. I know which I'd
prefer.
Brendan
Reply by Tom Walsh●November 6, 20062006-11-06
Brendan Murphy wrote: >
> --- In lpc2000@yahoogroups .com ,
> Tom Walsh wrote:
> >
> > Brendan Murphy wrote:
> > >
> > > --- In lpc2000@yahoogroups .com
> 40yahoogroups. com>,
> > > "Karl Olsen" wrote:
> > > > If you use GCC, structs have the advantage that they allow
> better
> > > > code generation. When you access multiple registers within one
> > > > peripheral, such as WDMOD and WDFEED, GCC can load one base
> > > address
> > > > in a register, and then use different offsets from that base
> > > > address. It cannot do that with #defines that just define the
> > > full
> > > > register addresses.
> > > >
> > >
> > > Karl,
> > >
> > > Mapping structure definitions to hardware is I think just too
> > > problematic to encourage.
> > >
> > > You're correct about gcc. I'd make the following points:
> > >
> > > - in most cases the efficiency of access makes no difference
> > > whatsoever to system throughput
> > >
> > > - if it really does make a difference, there are other ways to
> > > achieve the same efficiency of access that don't rely in
> knowledge
> > > of how the compiler stores structs to work (use a #define for the
> > > base address and #defines for all the offsets)
> > >
> > I disagree, use the typedef approach. Using #defines can get
> really
> > messy. With a typedef you encapsulate the names of registers
> inside of
> > a struct, this enforces scope upon the names. With a #define, you
> don't
> > take advantage of the compiler and C language to avoid coding
> errors
> > (like using a name in the wrong context).
> > Tom,
>
> In general, I'd strongly agree with you, for the reasons you give:
> the pre-processor is in general something to be avoided.
>
> I guess it's a matter of personal opinion which you consider worse:
> using the pre-processor or something inherently non-portable like
> structs to access hardware registers. BTW when I say "non-portable"
> I mean non-poratble between different compilers or even versions of
> the same compiler, rather than portable to a different platform.
> Yes, it is a matter of personal choice as to how you "get the job
done". Having said that, a #define approach takes the abstract idea of
setting / clearing a bit out of context and exacts a specific set of
steps to affect the operation.
When using a typedef, you are maintaining the abstraction. Keeping the
IDEA of the operation in the abstract on the coding level leaves the
IMPLEMENTATION up to the compiler. Since the compiler is a
'automatic'
process, the compiler (et.al) does the grunt work of implementing your idea.
With a #define you can build complex structures but with a lot of
tedious detail. With a typedef, you keep it in the abstract. For
example, this is the Transmit Status Vector information returned by the
ENC28J60 ethernet chip:
typedef union {
BYTE v[7];
struct {
WORD ByteCount;
unsigned CollisionCount:4;
unsigned CRCError:1;
unsigned LengthCheckError:1;
unsigned LengthOutOfRange:1;
unsigned Done:1;
unsigned Multicast:1;
unsigned Broadcast:1;
unsigned PacketDefer:1;
unsigned ExcessiveDefer:1;
unsigned MaximumCollisions:1;
unsigned LateCollision:1;
unsigned Giant:1;
unsigned Underrun:1;
WORD BytesTransmittedOnWire;
unsigned ControlFrame:1;
unsigned PAUSEControlFrame:1;
unsigned BackpressureApplied:1;
unsigned VLANTaggedFrame:1;
unsigned Zeros:4;
} bits;
} TXSTATUS;
If you wanted to get the number of BytesTransmittedOnWire, how would you
do that using a #define? Something like this?
#define BytesTransmitted 1
number = (WORD) *TxStatusVector [BytesTransmitted];
The problem with a define is that you MUST remember to cast the pointer
value into a WORD, if you forget to do that then you will get an 8bit
value instead. With a typedef, you merely access the desired member of
the structure and 'fahget about it'. ;-) Here it is, still in the
abstraction:
number = *TxStatusVector.BytesTransmitted;
It is really difficult to create a problem for yourself with a
typedef! If, for example, "number" is an "unsigned char" then the
compiler will dump errors or warning back to you. If, when using a
#define, you forget to cast the result to a WORD, the compiler may
silently do it.
OH, keep in mind too that when I determined that "BytesTransmitted" had
an index value of "1", I had to count the bytes to make sure I was
referencing the correct one, with a typedef, you don't need to count
anything (beyond when you initially define the structure).
Regards,
Reply by Brendan Murphy●November 6, 20062006-11-06
--- In l..., Tom Walsh wrote: >
> Brendan Murphy wrote:
> >
> > --- In lpc2000@yahoogroups .com
40yahoogroups.com>, > > "Karl Olsen" wrote:
> > > If you use GCC, structs have the advantage that they allow better > > > code generation. When you access multiple
registers within one
> > > peripheral, such as WDMOD and WDFEED, GCC can load one base
> > address
> > > in a register, and then use different offsets from that base
> > > address. It cannot do that with #defines that just define the
> > full
> > > register addresses.
> > >
> >
> > Karl,
> >
> > Mapping structure definitions to hardware is I think just too
> > problematic to encourage.
> >
> > You're correct about gcc. I'd make the following points:
> >
> > - in most cases the efficiency of access makes no difference
> > whatsoever to system throughput
> >
> > - if it really does make a difference, there are other ways to
> > achieve the same efficiency of access that don't rely in knowledge > > of how the compiler stores structs to work (use a
#define for the
> > base address and #defines for all the offsets)
> >
> I disagree, use the typedef approach. Using #defines can get really > messy. With a typedef you encapsulate the names of
registers inside of > a struct, this enforces scope upon the names. With a
#define, you don't > take advantage of the compiler and C language to
avoid coding errors > (like using a name in the wrong context).
>
Tom,
In general, I'd strongly agree with you, for the reasons you give:
the pre-processor is in general something to be avoided.
I guess it's a matter of personal opinion which you consider worse:
using the pre-processor or something inherently non-portable like
structs to access hardware registers. BTW when I say "non-portable"
I mean non-poratble between different compilers or even versions of
the same compiler, rather than portable to a different platform.
Brendan.
Reply by Tom Walsh●November 6, 20062006-11-06
Brendan Murphy wrote: >
> --- In lpc2000@yahoogroups .com ,
> "Karl Olsen" wrote:
> > If you use GCC, structs have the advantage that they allow better
> > code generation. When you access multiple registers within one
> > peripheral, such as WDMOD and WDFEED, GCC can load one base
> address
> > in a register, and then use different offsets from that base
> > address. It cannot do that with #defines that just define the
> full
> > register addresses.
> > Karl,
>
> Mapping structure definitions to hardware is I think just too
> problematic to encourage.
>
> You're correct about gcc. I'd make the following points:
>
> - in most cases the efficiency of access makes no difference
> whatsoever to system throughput
>
> - if it really does make a difference, there are other ways to
> achieve the same efficiency of access that don't rely in knowledge
> of how the compiler stores structs to work (use a #define for the
> base address and #defines for all the offsets)
> I disagree, use the typedef approach. Using #defines can get really
messy. With a typedef you encapsulate the names of registers inside of
a struct, this enforces scope upon the names. With a #define, you don't
take advantage of the compiler and C language to avoid coding errors
(like using a name in the wrong context).
Reply by Brendan Murphy●November 6, 20062006-11-06
--- In l..., "Karl Olsen" wrote: > If you use GCC, structs have the advantage that they
allow better
> code generation. When you access multiple registers within one
> peripheral, such as WDMOD and WDFEED, GCC can load one base address > in a register, and then use different offsets from
that base
> address. It cannot do that with #defines that just define the full > register addresses.
>
Karl,
Mapping structure definitions to hardware is I think just too
problematic to encourage.
You're correct about gcc. I'd make the following points:
- in most cases the efficiency of access makes no difference
whatsoever to system throughput
- if it really does make a difference, there are other ways to
achieve the same efficiency of access that don't rely in knowledge
of how the compiler stores structs to work (use a #define for the
base address and #defines for all the offsets)
Brendan
Reply by Karl Olsen●November 6, 20062006-11-06
--- In l..., "rtstofer" wrote: >
> --- In l..., "klemen_dovrtel"
> wrote:
>>
>> I have a question about register definitions (in WinARM). I read in
>> user manual that WD for instance has four registers:
>> 0xE000 0000
>> 0xE000 0004
>> 0xE000 0008
>> 0xE000 000C
>> Shouldn't be there another REG_8 _pad0[3]; after the REG_8 feed;,
>> because now WDTV is pointing at wrong adress (it is pointing at
>> 0x0xE000 0009 right?)?
>>
>> // lpcWD.h
>> // Watchdog Registers
>> typedef struct
>> {
>> REG_8 mod; // Watchdog Mode Register
>> REG_8 _pad0[3];
>> REG32 tc; // Watchdog Time Constant Register
>> REG_8 feed; // Watchdog Feed Register
>> REG32 tv; // Watchdog Time Value Register
>> } wdRegs_t;
>>
>> //lpc21xx.h
>> #define WD ((wdRegs_t *)0xE0000000)
>> // Watchdog Registers
>> #define WDMOD WD->mod /* Watchdog Mode Register */
>> #define WDTC WD->tc /* Watchdog Time Constant Register */
>> #define WDFEED WD->feed /* Watchdog Feed Register */
>> #define WDTV WD->tv /* Watchdog Time Value Register */
>> I don't think I would do it that way. Among other things, you
don't
> have the registers defined as 'volatile'.
>
> The LPC214x.h file from Philips does it this way:
>
> /* Watchdog */
> #define WDG_BASE_ADDR 0xE0000000
> #define WDMOD (*(volatile unsigned long *)(WDG_BASE_ADDR + 0x00))
> #define WDTC (*(volatile unsigned long *)(WDG_BASE_ADDR + 0x04))
> #define WDFEED (*(volatile unsigned long *)(WDG_BASE_ADDR + 0x08))
> #define WDTV (*(volatile unsigned long *)(WDG_BASE_ADDR + 0x0C))
>
> Typedefs and structs are nice but I don't think this is the best
> place to use them. To have the struct version work correctly, you should make the
wdRegs_t volatile:
#define WD ((volatile wdRegs_t *)0xE0000000)
If you use GCC, structs have the advantage that they allow better
code generation. When you access multiple registers within one
peripheral, such as WDMOD and WDFEED, GCC can load one base address
in a register, and then use different offsets from that base
address. It cannot do that with #defines that just define the full
register addresses.
With the struct definitions, GCC 4.0.1 turns this program:
