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

[Rick Altherr]

On Feb 29, 2008, at 10:29 AM, Weddington, Eric wrote:

> > -----Original Message-----
> > From:
> > address@hidden
> > [mailto:address@hidden
> > org] On Behalf Of Dmitry K.
> > Sent: Thursday, February 28, 2008 10:23 PM
> > To: address@hidden
> > Subject: Re: [avr-libc-dev] [RFC] New eeprom.h
> >
> > Hi all.
> >
> > I like the rewrite. It looks good. My only concern is not
> > only code size. There is a second question: this functions
> > are not a normal C functions.  That is:
> >
> > . No argument type control at compile time. For expamle,
> > the expressions like
> >    eeprom_write_block ("Apple", s, sizeof("Apple"));
> >    eeprom_write_dword (3.14159, 0);
> > do not type any warnings.
> >
> > . It is impossible to get address of eeprom function
> > to inderect call.
> >
> > . Avr-gcc can not to optimize the EEPROM reading, like it
> > can do this in case of SRAM.
> >
> >
> > Summarizes the Eric's "eeprom.h" and all ideas from above
> > (Wouter, Shaun, Rick) I have prepare two projects.
> >
> > Project 1.
> > It is a very simple: use an inderect calls (not indirect
> > registers). It is eliminates big space expense and notes
> > above: eeprom functions are pure functions. It is needed
> > to add only 6 very short functions into Avr-libc. At this
> > moment this project is ready for test. I can to commite
> > at this week-end.
> >
> > Project 2.
> > Use a set of high quality asm functions. This is not only
> > space effective, also it is fast. Now the set of funtions is
> > ready for test. Some efforts are needed to include into
> > Avr-libc: I am not happy to do this manualy.
> >
> > In attach the "eeprom.h" of project 1.
> >
> > Opinions?
>
>
> - In eeprom.h, I had changed it to "inline static" because I  
> received a
> comment from a tester that 4.1.x gave warnings on "static inline". But
> "inline static" was ok. So you may want to test your version with
> previous versions of GCC (4.1.x) to avoid the same issue.
>
> - We can't have library functions until avr-libc moves to a
> library-per-device design. We have to be able to conditionally compile
> the code to change it for certain devices.

Actually, in this case, you can.  The code in the library is the  
generic code for doing the loops for multibyte operations.  The code  
for a single byte operation is still in the header and will have the  
correct values of EEARL etc substituted at compile time.

> - The ability to taking the addresses of these functions is not that
> critical. How often would this be done in a real AVR application? It's
> not likely.
>
> - Yes there is no type checking, but that requires functions, which
> requires library-per-device design.

Actually, it doesn't.  Dmitry's header shows this.  The exported  
interface is a set of inline functions that will provide for type- 
checking but will ultimately be inlined into the calling function.   
Those simple functions just call the library routines which need an  
extra argument, namely the address of the function for single-byte  
read/write.  That function is also defined in the header.

> At this point, I do NOT care if the generated code is twice the size  
> as
> used to be, as long as the code:
> - Works correctly for *all* AVR devices in the toolchain that has  
> EEPROM
> (see avr-libc bug #21410), with all locations of EEPROM registers
> - Fixes the GCC bug #31644 test case
> - Has a means to allow future handling of the new EEPROM design in the
> XMEGA devices

Dmity's implementation does all of that and keeps the code size down  
since only a single copy of the various routines will exist in the  
code.  The whole trick is that the single-byte operations are inline  
functions.  The actual entry-points are also inline functions, but  
they pass the address of the inline routine to the library routines  
that do the looping for multi-byte operations.  By taking the address  
of an inline function, the compiler is forced to not actually inline  
it, but to generate a separate symbol for it in the binary.  All the  
entry points will be inlined into their callers which will turn into a  
call into the library routine while passing the address of the single- 
byte operation.  The address of the single-byte operation will point  
to the symbol that is the single instance of the single-byte operation.

> It would be nice if we can, in the future, remove all instances of
> __EEPROM_REG_LOCATIONS__, as that was a kludge to begin with. This  
> means
> that we should only depend on typical register definitions in the IO
> header file like EECR, EEDR, EEARL, EEARH, EEAR, etc.
>
> Oh, and can it be done in a week? I need a short-term fix, now. This
> discussion has gone on way too long. If you wish to debate the various
> merits/demerits of the design, then please provide working examples  
> that
> meet the criteria above. I have seen Wouter and Dmitry provide code.
> Shaun, you provided the dword routines (even though they've been
> modified).

Dmitry beat me to implementing the code-size efficient version.  His  
implementation is missing the tiny bit of code that goes in the  
library, but is otherwise complete.  Assuming his testing goes well,  
this should be completely usable now.

> Eric Weddington
>
>
>
> _______________________________________________
> AVR-libc-dev mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

--
Rick Altherr
address@hidden

"He said he hadn't had a byte in three days. I had a short, so I split  
it with him."
 -- Slashdot signature

smime.p7s
Description: S/MIME cryptographic signature