Re: [avrdude-dev] [patch #7720] Bug in EEPROM write

[René Liebscher]

Am 06.02.2012 09:34, schrieb Hannes Weisbach:
> Am 06.02.2012 um 08:36 schrieb Rene Liebscher:
>
>> Follow-up Comment #2, patch #7720 (project avrdude):
>>
>> If the interface of paged_load/write is outdated, so may be better retire 
>> them
>> and put some write/read_page function at their places.
>>
>>
>>  int  (*write_page)    (struct programmer_t * pgm, AVRPART * p, AVRMEM * m, 
>>                          unsigned int page_number, unsigned int n_bytes);
>>  int  (*read_page)     (struct programmer_t * pgm, AVRPART * p, AVRMEM * m,
>>                           unsigned int page_number, unsigned int n_bytes);
>>
>>
>> If you always write/read whole pages the n_bytes parameter can be discarded
>> too.
>>
>> Main reason to change the names of the functions are the patches in the patch
>> tracker and and in other places at the web which would all fail to work if
>> using still the old interface (anybody really cares about the compiler
>> warnings?). The same problem we had already when paged_load/write changed
>> their interfaces. Using new names, people get at least a compiler error and
>> must fix this to be able to use paged functions again.
>>
> Why not implement eeprom_write/read_page() and flash_write/read_page() 
> functions?
> Some programmers already split these functions up and when AVRDUDE calls the 
> appropriate functions directly, a lot of code duplication can be removed.
>
> As for the change: probably a 'grace period' can be used where both 
> interfaces are available.
> This would mean more maintenance overhead, but not all programmers need to be 
> modified at the same time.
> Also, patches could be modified later on. "Obsolete"-warnings could remind 
> people to update their stuff.
And what about other memory regions as application, apptable, boot used
in the xmega family? I think we should like write/read_byte use a single
interface for all kind of memory.

Another solution would be to replace the functions by a pointer to a
table like this.

{
{"flash" , flash_page_write, flash_page_read};
{"eeprom" , eeprom_page_write, eeprom_page_read};
{...};
{NULL,NULL,NULL}; // end marker
}

Then you could provide functions for any kind of memory you want,
without to restrict it to the now used types of memory.

The pointer to the table would be set in the initpgm function of the
programmer.


René