[avrdude-dev] Re: avrdude 4.2.0 Release

[Theodore A. Roth]

On Thu, 4 Sep 2003, Brian Dean wrote:

:)On Thu, Sep 04, 2003 at 07:33:26PM -0700, Theodore A. Roth wrote:
:)
:)> I think this patch breaks the encapsulation of the avr910 module. It
:)> makes calls to avr_write_byte_default() and
:)> avr_read_byte_default(). I'm not sure if that is the best route to
:)> take since if may come back to bite us in the future if there are
:)> changes made to avr.c.
:)> 
:)> Brian, what's the policy here, if any? 
:)
:)Let me see if I understand this - the avr910 currently doesn't support
:)writing fuse bits, nor did it used to support a "universal command"
:)command?  And now, the new AVR910 firmware supports a "universal
:)command" which can be used to access fuse bits?  So the idea is that
:)the prefered byte-writing function for flash and eeprom is
:)avr910_write_byte(), but if we happen to be writing something other
:)than flash or eeprom (presumable fuse bits), then we instead call the
:)"default" byte writing routine which happens to call the "universal
:)command" for the programmer if it has one?  Is that close?
:)
:)I tend to agree that several assumptions are being made about what
:)avr_write_byte_default() does and how it works.  Can you think of a
:)cleaner way to do the equivalent without too much code duplication and
:)without too many changes just before the release?
:)
:)I'm OK with whatever you decide.  It's also fine with me if you decide
:)to let this go in as is and then maybe revisit the issue after the
:)release to make it work cleaner, in all your free time, of course :-)
:)
:)Maybe something as simple as renaming 'avr_write_byte_default()' to be
:)something like 'avr_write_byte_universal()', and create a new
:)'avr_write_byte_default()' to just call that.  Then, in
:)avr910_write_byte() make it call the "universal" command directly.
:)That way we are still free to change the implementation of
:)avr_write_byte_default() without affecting anything else.  Just a
:)thought.
:)
:)In direct answer to your question, there is no strict policy, but my
:)hope is that the code stays (becomes) clean(er) and doesn't devolve
:)into a maintenance nightmare where little changes here have unexpected
:)results over there.  I know, not a very useful answer :-)

I've tested the attached patch as best I can since my 8515's don't support
SPI access to fuses and my only mega163 is temporarily hosed with the SPIEN
fuse disabled (need HiV to fix it). :-\ Seems to me it issues the universal 
commands correctly.

It's a little bit of a tweak to Jan's patch. His patch generated warnings
due to avr_{read,write}_byte_default() not having protos in avr.h in
addition to the broken encapsulation issue.

Essentially the same code gets executed in both patches, but I think mine is
a bit more clear as to what is getting called when looking at avr.c (of
course, that's open to debate ;-).

If there's no objections to my hack, I'll commit it tomorrow.

Ted Roth

avr910-fuse-troth.diff
Description: Text document