Re: [avr-libc-dev] [RFC] sleep api

[Joerg Wunsch]

As Theodore A. Roth wrote:

> Attached is new header implementing an api for the SLEEP insn.
> 
> Comments?

Looks fine.

I notice there's no #ifndef __ASSEMBLER__ around it, so it can be used
with both, the assembler and the C environment.  Makes sense, but then
it should expand into valid assembler code for the __ASSEMBLER__ case.

>     \note FIXME: TRoth/2002-11-01: These modes were taken from the mega128
>     datasheet and might not be applicable or correct for all devices. */

I think we could fix that, and provide the macros in a
device-dependent fashion.  That way, someone who tries to use
SLEEP_MODE_ADC with an older device will get a compile-time error.

> #define sleep_mode() {                         \
>     MCUCR |= _BV (SE);                         \
>     __asm__ __volatile__ ("sleep" "\n\t" :: ); \
>     MCUCR &= ~_BV (SE);                        \
> }

Why not write it completely in assembler code?

> As far as I know, the only place in a #define where space is an issue is
> immediately after the name of the macro.
> 
> This is wrong:
> 
>   #define bad (arg) (_BV(arg))
> 
> as is this:
> 
>   #define bad( arg ) (_BV(arg))

Nope, that's already OK (the preprocessor knows that bad will be a
macro with an argument, based on the first paren seen).  But i agree
that we should adhere to a consistent style.

> I made some dox improvements since posting this, so the new file is
> attached. Will probably commit it tomorrow if there's no noise.

Please commit it despite my nitpicking above.  We can easily fix this
afterwards.

-- 
J"org Wunsch                                           Unix support engineer
address@hidden        http://www.interface-systems.de/~j/