avr-libc-dev
[Top][All Lists]
Advanced

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

RE: [avr-libc-dev] [RFC] New eeprom.h


From: Weddington, Eric
Subject: RE: [avr-libc-dev] [RFC] New eeprom.h
Date: Tue, 29 Jan 2008 13:40:51 -0700

 

> -----Original Message-----
> From: Wouter van Gulik [mailto:address@hidden 
> Sent: Tuesday, January 29, 2008 12:23 PM
> To: Weddington, Eric
> Cc: address@hidden
> Subject: Re: [avr-libc-dev] [RFC] New eeprom.h
> 
> So I got the header and my first consideration in is the copyright... 
> All previous contributers are now gone? I hope this is WorkInProgress.

Mainly because this is a total rewrite of the implementation. The API
(interface) stays the same. I'm certainly fine with putting the old
copyrights back on.


> Second why all these hideous define with there nasty \ ?

Because these are multi-line macros. The whole point of the
reimplementation of the EEPROM API is to get rid of the compiled code
(per architecture) and implement it in a way that is only in the header
file which can be targeted per device. Macros are just one way to do it.
Someone suggested implementing it as inlined C functions in the header
file. I chose the macros method of implementing as we have done that in
other headers.

 
> For eeprom read an inline function I came up with this (for 
> E2ND> 0xFF)
> 
> //static inline uint8_t __eeprom_read_byte_address_word(uint8_t*) 
> __attribute__((always_inline));
> static inline uint8_t __eeprom_read_byte_address_word(uint8_t* eeptr)
> {
>      uint16_t __address_word = (uint16_t)(eeptr);
>      uint8_t __result;
>      do{}while(!eeprom_is_ready());
>      EEARL = __address_word & 0xFF;
>      EEARH = __address_word >> 8;
>      EECR |= 1<<EERE;
>      __result = EEDR;
>      return __result;
> }
> 
> This is plain C, but it's forced to be in this specific 
> order, since all 
> (!) access is to volatile memory. There is no timing constraint on 
> instruction sequence so there is no problem.
>
> For -O0 it generates large but valid code.
> For -O[123] it gives all the same small code
> For -Os it might give a function call, so I added the always_inline 
> constraint. Then it gives the same results, however it's 
> arguable to leave
> the inlining to gcc, since it might be smaller (in case of 
> several calls) 
> 
> My C function gives much, much smaller code. GCC gets its 
> hands tide up
> because of the assembler, it forgets it can recycle registers etc...
> 
> So I would recommend this C function approach for all read routines.
> Of course read word could be made more sophisticated using 
> gcc ability to
> check on constants (__builtin_constant_p), so you could then 
> check for page
> crossing and write the high address only once.

This sounds promising. Do you care to modify the header?

 
> The write sequence is best to be in asm since it needs a 
> special timing and
> interrupt clearing.
>  
> One last thing, why is there a loop before every function? In 
> the old code
> there is no loop (although the documentation state there is!?!).

There is a loop, or should be. The loop is checking the EEWE flag (or
equivalent) to make sure that a read or write is possible.

> 
> And the old code is not disabling interrupts during eeprom 
> write... which is
> no good! Maybe this was the problem of the report for the tiny13?

No, the old code is disabling the interrupts for the write routines. You
have to look in the libc/misc/eeprom.S file in the avr-libc source.

Eric Weddington




reply via email to

[Prev in Thread] Current Thread [Next in Thread]