[Top][All Lists]

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

Re: [avr-libc-dev] New eeprom library. Comments please

From: Joerg Wunsch
Subject: Re: [avr-libc-dev] New eeprom library. Comments please
Date: Fri, 22 Jul 2005 23:50:33 +0200
User-agent: Mutt/

As Nigel Winterbottom wrote:

> The Attachment was missing (me being a buffoon). Now Added.

OK, I had a look at it.  At first, please let me thank you for your
effort and time spent into it.  I'm definately interested.

Anyway, as already mentioned, I'd much prefer a diff -u output (like
"cvs diff -u"), instead of a hacked-up version.  I.e., instead of
supplying a new file eeprom1.S, and changing all #include statements,
just supply a new eeprom.S file.  Please make sure to not include
arbitrary white-space changes, as they make tracking the actual code
changes much harder (and I would have to un-edit them before
committing to CVS to not poison the CVS repository with both code and
no-code changes in a single commit).

Some further remarks:

;      #ifndef EEARH
;      #define EEARH (EEARL+1)
;      #endif

Don't comment out code that's no longer used.  Just drop it.  CVS will
keep track of the old version.

; Internal routine to read one byte from EEPROM. Address in rP1:rP0 or Z 
depending on entry point

       .global Rd
1:     sbic    _SFR_IO_ADDR(EECR), EEWE

Don't use internal names like that.  First, pick a descriptive name in
any case, IMHO at least the "EE" should be retained even for an
internal symbol.

If it's truely internal, just drop the .global, and it should be OK.

If it needs to be global (what happens if multiple EEPROM functions
are included into the same final ELF file, don't the global functions
clash then?), pick a name starting with two underscores, as these
names are reserved for ``the implementation'' (i.e. the library or the
compiler itself).  That way, they are guaranteed to never clash with
an application-defined name.

I'd appreciate if Marek could have a look at the internal logic as
well, as he wrote the previous implementation.

Finally, please submit it as a patch tracker so it won't get lost.
It's too easy to forget about a certain article sent to a

cheers, J"org               .-.-.   --... ...--   -.. .  DL8DTL

http://www.sax.de/~joerg/                        NIC: JW11-RIPE
Never trust an operating system you don't have sources for. ;-)

reply via email to

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