Re: [avr-gcc-list] port access with avr-gdb

[David Brown]

----- Original Message -----
From: "E. Weddington" <address@hidden>


> [Apologies if this doesn't make it to avr-gcc-list, I've had some recent
> problems and I don't know if they're fixed. That's why I've CC'd
interested
> parties.]
>

I don't think it made it to the list, so I'm keeping your post below intact
for others to see rather than snipping bits that I haven't commented on.


>
> On 17 Jun 2004 at 10:34, David Brown wrote:
>
> > #define's are not archaic, but having your computer shout at you in all
caps
> > is archaic - keyboards have had small letters for some years now.  So
when
> > breaking with tradition and using bitfield structs, why not go the whole
hog and
> > use small letters, thus avoiding the conflicts?
> >
> > For some uses, I like bit-field structs best, while for others,
traditional
> > #define'ed registers are better.  There is no reason why they can't both
be used
> > - keep the standard #define's as they are, and add some structs as well
(I
> > haven't updated my msp430 gcc for a while, but I believe that's how they
do it
> > now).  If seperate structs are defined for much-used registers rather
than
> > attempting to use a huge struct for the whole device, then you avoid
many of the
> > problems described below while getting a number of advantages. In
particular, it
> > means you can write things like
> >
> >     if (pinc.pin4) portb.pin1 = 1;
> >
> > rather than the uglier
> >
> >     if (PINC & _BV(PINC4)) PORTB |= _BV(PB1);
> >
> > Far better, however, is that it means you can write a header file used
by
> > all modules in your project with something like:
> >
> >     #define lightSwitch pinc.pin4
> >     #define lightOn portb.pin1
> >
> > and then in the main code:
> >
> >     if (lightSwitch) lightOn = 1;
>
>
> This is also a recurring conversation on the AVR Freaks website.
>
> 1. Using #defines for registers is not archaic as it has been used this
way on
> many cross-compilers, including commercial ones.
>

"It's used on commercial compilers" is not a strong arguement per se :-)
Common practice is useful enough, but it should not stop us improving
things.  (As I and others have said, however, no one wants to remove the
#defines - we are considering stucts *in addition* .)


> 2. Portability: There could be conflicts about the ordering of bit-fields
> within a struct, or the underlying size of the bitfields, and just
specifying
> the wrong compiler switch could screw this up. Using bit shifts and bit
> operators is more portable because it is very well defined what they are
> supposed to do and compiler switches don't cause misbehaviour.
>

What sort of portability are you looking for?  Being able to take a program
for an AVR and run it unaltered on an MSP430?  Being able to take a program
for gcc-avr and being able to compile it unaltered on iar-avr?  The
portability of C is greatly exagerated - the gcc compiler is portable to
just about every computer under the sun, but ./configure takes half as long
as the build just to rearrange the C code to match the platform.  If I want
to move code between targets or between compilers, I expect to spend some
time checking the details.  I also expect to know the ordering of bit fields
on the particular compiler and target combination.  It is a pain that it is
not defined in the C standards, but at least any given combination will
normally remain static (with the notable exception of MS's C compiler, which
changed the ordering between two different versions).

I'm not keen on the idea of limiting things to the lowest common
denominator.  I don't use a gcc-specific feature just because it is there,
but I see no reason not to use it just because other compilers don't have
it.  I've used other compilers that don't have as strong bit field support
(for example, they consider bits as small int's, and thus a bit field
structure must be at least 16 bits wide), but that won't stop me using them
on compilers that have good support.

Of course, there are occasions when portability is a prime concern, and you
need to take that into account.  The #defines have to stay in the header
files, giving the user the choice.

> 3. Flexibility: Using defines and bit operators a user can operate on
*multiple
> non-continguous bits simultaneously*. You *cannot* do this with using
structs.
> Example:
>
> // This simultaneously sets bits 0, 2, 4, and 6.
> PORTB |= (_BV(0) | _BV(2) | _BV(4) | _BV(6));
>
> You cannot do the above with a struct of bitfields. And the resulting
assembly
> of above will be more compact then doing a sequence of bitfield
assignments.
>

Quite true (I think I mentioned this breifly, but it got snipped), and
that's part of why structs must be an addition to defines, not a
replacement.

> 4. Macro wrappers: If you hate the way that the above looks you can always
wrap
> things up in macros. I do. And I've put up an RFC for avr-libc to provide
some
> standard wrappers for those users (usually newbies) who don't grok bit
ops.

As it is a RFC, then I can comment on it (my critisisms are supposed to be
constructive)...

Multi-line macros using "do {...} while (0)" are definitely archaic.  They
are ugly to write, inconvenient to maintain (you need a \ at the end of the
lines, making them different from every other line), unclear to read
(especially for a newbie) - what exactly is the point of a "while (0)" loop?
I know it is there so that someone can safely write "if (starting)
motor_init();" but it is not exactly a clear, understandable construct.  Why
not write them as static inline functions:

static inline void led_init(void) {
    led_power_off();
    led_status_off();
    bit_set(led_port_dir, (led_status | led_power));
};

The macros you've used below for bit manipulation are ok, but not ideal - it
means defining several macros for each output bit (leading to mistakes when
things are changed).  I prefer to tie the register address and the bit
number tightly together.  (Although I am arguing some of the benifits of
structs in this post, I use #define'd registers on many targets, while using
bitfield structs mostly for flag registers and similar data.  And I use my
bit macros on about 8 compilers and 6 assemblers - the interface is the
same, although the implementation is different.  So feel free to call me a
hypocrite :-)

My bit macros (in "bit.h" or similar):

#define pinOffset 0   // Offset from port address for pin inputs
#define dirOffset 1   // Offset from port address for pin directions
#define portOffset 2  // Offset from port address for port outputs

#define vbyte(add) (*(volatile byte *) (add))
#define uvbyte(add) ((word) (add))

#define _BitPort(add, bitM) add
#define BitPort(bitAdd) _BitPort(bitAdd)
#define _BitAdd(bitAdd) (word) &(bitAdd)
#define BitAdd(bitAdd) (word) &BitPort(bitAdd)
#define _BitMask(add, bitM) bitM
#define BitMask(bitAdd) _BitMask(bitAdd)

#define Bit(n) (1 << (n))

#define HiByte(n) ((n) >> 8)
#define LoByte(n) ((byte) (n))

#define Byte(v) (*(byte *) (&(v)))
#define Word(v) (*(word *) (&(v)))

#define BitBase(bitAdd) (3*(_BitAdd(bitAdd) / 3))
#define _BitPortIn(add, bitM) vbyte(BitBase(add) + pinOffset)
#define BitPortIn(bitAdd) vbyte(BitBase(bitAdd) + pinOffset)
#define _BitPortDir(add, bitM) vbyte(BitBase(add) + dirOffset)
#define BitPortDir(bitAdd) vbyte(BitBase(bitAdd) + dirOffset)
#define _BitPortOut(add, bitM) vbyte(BitBase(add) + portOffset)
#define BitPortOut(bitAdd) vbyte(BitBase(bitAdd) + portOffset)

#define PullHigh(bitAdd) _BitPortOut(bitAdd) |= _BitMask(bitAdd)
#define OutputEnable(bitAdd) _BitPortDir(bitAdd) |= _BitMask(bitAdd)
#define OutputDisable(bitAdd) _BitPortDir(bitAdd) &= ~_BitMask(bitAdd)

#define MakeBit(port, bitNo) port, (1 << bitNo)

#define SetBit(bitAdd) _BitPort(bitAdd) |= _BitMask(bitAdd)
#define ResetBit(bitAdd) _BitPort(bitAdd) &= ~_BitMask(bitAdd)
#define ToggleBit(bitAdd) _BitPort(bitAdd) ^= _BitMask(bitAdd)
#define GetBit(bitAdd) (_BitPort(bitAdd) & _BitMask(bitAdd))


These might look a bit horrible, but they can be used as:

#define ledStatus MakeBit(PORTD, 0)
#define ledPower MakeBit(PORTD, 3)
#define motorStatus MakeBit(PINE, 7)

static inline void initPorts(void) {
    SetBit(ledStatus);
    OutputEnable(ledStatus);
    SetBit(ledPower);
    OutputEnable(ledPower);
}

int main(void) {
    initPorts();

    while (1) {
        if (GetBit(motorStatus)) {
            ResetBit(ledStatus);
        } else {
            SetBit(ledStatus);
        };
    };
}


Mostly it's a matter of style.  Because my macros tie the port address and
the bit number together in one line, however, there are far fewer lines in
the project-specific port definition file (roughly a quarter as many), and
the same style can be used for inputs, outputs and data flags.  There are
also several more macros defined and ready for use as necessary without
adding more pin-specific macros.  Of course, there is nothing to stop you
combining the methods:

    #define led_power_on() ResetBit(ledPower)
    #define led_power_off() SetBit(ledPower)

I do that with signals whose polarity might change between card revisions.

mvh.,

David





> Examples:
>
> // Save in bit.h
> #define bit_set(v, m) (v) |= (m)
> #define bit_clear(v, m) (v) &= ~(m)
> #define bit_toggle(v, m) (v) ^= (m)
> #define bit_read(v, m) (v) & (m)
>
> (One can also define separate macros to work with strictly 8-bit types to
avoid
> integer promotion.)
>
> Then you can define macros based on application specific functionality:
>
> -------------------------------------------------------------
> // I've taken the liberty to put certain things in lowercase for those who
feel
> // like they're being shouted at.
> #include <avr/io.h>
> #include "bit.h"
>
> #define led_port PORTD
> #define led_port_dir DDRD
> #define led_status _BV(0)
> #define led_power _BV(3)
>
> #define led_power_on() bit_clear(led_port, led_power)
> #define led_power_off() bit_set(led_port, led_power)
> #define led_status_on() bit_clear(led_port, led_status)
> #define led_status_off() bit_set(led_port, led_status)
>
> // Set the output states before setting the port direction.
> // Note: in seting the port direction,
> //      you can't do this on one line with a bitfield struct.
> #define led_init() \
> do{ \
> led_power_off(); \
> led_status_off(); \
> bit_set(led_port_dir, (led_status | led_power)); \
> }while(0)
>
> #define motor_port PORTE
> #define motor_port_dir DDRE
> #define motor_status _BV(7)
> #define is_motor_on() bit_read(motor_port, motor_status)
>
> // Setup port as input.
> #define motor_init() \
> do{ \
> bit_clear(motor_port_dir, motor_status); \
> }while(0)
>
> int main(void)
> {
> // Initialize ports.
> led_init();
> motor_init();
>
> // Show the world we're alive.
> led_power_on();
>
> for(;;)
> {
> // If this isn't self-documenting, then what is?
> if (is_motor_on())
> led_status_on();
> else
> led_status_off();
> }
>
> return;
> }
> -------------------------------------------------------------
>
> Eric Weddington
>
>