Re: [avr-libc-dev] [EEPROM] patch: Fix mis-feature of lacking EEPROM support

[Haase Bjoern (PT-BEU/EMT) *]

Dimitry K wrote:
>On Monday 11 July 2005 06:38, Björn Haase wrote: 
>> Hi, 
>> 
>> find enclosed a re-write of the AVR eeprom routines that 1.) fixes the bug 
>> of lacking support for many targets and 2.) uses a more code-efficient 
>> implementation. 
>... Hi, yours code is very economical in registers usage! 
>
>Notes: 
>
>1. If I have true understand, you use next code to increment address of EEPROM 
>location: 
>Ld __tmp_reg__,X+ /* Increment x register */ 
>This is not the best method. 
>For example, the address in EEPROM can coincide with the address of the 
>register of data UART that will lead to
>loss accepted byte. 
>'ADIW' instraction take the same number of clocks and words, as above. 

Thank you for reviewing the patch. I had made use of the auto-increment 
instruction because ADIW, according to the documentation, is not available on 
all of the devices. I did not consider that a read access to memory might have 
other side-effects. So in fact one would probably need again to distinguish the 
different families. Since in the present implementation one should not assume 
that __zero_reg__ is indeed zero (it's used as 8 bit counter), auto-increment 
without adiw would need one conditional branch :-(.

>2. Perhaps, it is necessary to make the counter of byte in procedure '_EELABEL 
>(eeprom_write_block)' 16-bits? 
>It considerably will reduce the size inline functions 'eeprom_write_block'. 
>Loss of one clock will be absolutely not appreciable on a background of 
>expectation of readiness. 

I had considered using a 16 bit counter transfered in r0:r1. IMO, the solution 
I had suggested was the one I thought to be better:

a) IMO, the number of bytes to be transfered is almost always known at compile 
time and b) it is almost always smaller than 257. This case that I considered 
to be the most common one is handled most efficiently by the code I have 
suggested. Also my code for the block transfer loop could make use of the write 
byte function stub since it could use __tmp_reg__ to transfer data since it 
does not need it for the counter value. This way code size again is decreased.
The overhead we are having in the expanded inline code for byte counts > 256 is 
minimal. I think we would probably still always be better than the present 
standard block transfer functions. In case that only one transfer > 256 bytes 
is present in the program, the inline code variant even is more efficient than 
a 16 bit transfer function since there would be, anyway the need for 
implementing the loop.

What would however be, IMO, worth considering is to offer a dedicated 
write_word function with the value being transfered in 
__tmp_reg__/__zero_reg__. My last suggestion made use of two independent calls 
for each individual byte. This would possibly make the word access slightly 
more efficient on devices that are having the movw instructions and if the 
application makes lots of scattered word accesses.

Concerning the speed of the accesses I agree with you. The implementation I 
have been posting is not optimized for speed but for code size. IMO, those that 
*really* need speedy eeprom access anyway need to study the documentation of 
the eeprom in detail and should be asked to write an own set of functions.

Yours,

Björn