Re: [avrdude-dev] Butterfly (was: Before a release...)

[Michael Mayer]

On Thu, Nov 20, 2003 at 19:37:08 +0100, Jan-Hinnerk Reichert wrote:
[reporting the butterfly patch works for his AVR910]
> However, I still think the patch could backfire. AFAIK, there are many 
> patched versions of the AVR910 out there. If any of them uses the 'b' 
> command for some own improvements, we could loose syncronisation on 
> these. And we don't want to lose users with broken programmers, do 
> we?
> 
> Do avoid this. we could add a subtype-parameter to the prommer 
> definition.
> 
> programmer
>   id    = "avr910";
>   desc  = "Atmel Low Cost Serial Programmer";
>   type  = avr910;
>   subtype = avr910;
> ;
> 
> programmer
>   id    = "butterfly";
>   desc  = "Atmel Butterfly Development Board";
>   type  = avr910;
>   subtype = butterfly;
> ;

I agree with you, even home-patched AVR910 versions have to work with the
patched driver. But I wonder what is the best way to do this.

You suggest the definition of a subtype field, Eric prefers to define a new
type. Today I checked out the impact of both alternatives to the overall
structure of the sources. The subtype-way needs only very small
modifications in pgm.h, config_gram.y and avr910.c leaving all the structure
intact, with sounds good to me.

Introducing a new type is more difficult, because it would need something
like butterfly.c which duplicates most of the functionality of avr910.c
making it difficult to keep the common parts in sync. One possible solution
may be to split avr910.c in two parts, one specific part and a part, which
is shareable with the new, small butterfly.c. But this will break the
current modularisation scheme, some of the static functions in avr910 will
have to be public. In detail all these functions should be shared between
avr910.c and butterfly.c and should be moved to a new file like avr_common.c
oder serial_common.c:

static int avr910_send
static int avr910_recv
static int avr910_drain
static void avr910_vfy_cmd_sent
static int avr910_rdy_led
static int avr910_err_led
static int avr910_pgm_led
static int avr910_vfy_led
static int avr910_chip_erase
static void avr910_enter_prog_mode
static void avr910_leave_prog_mode
static int avr910_program_enable
static void avr910_powerup
static void avr910_powerdown
static int avr910_save
static void avr910_restore
static void avr910_disable
static void avr910_enable
static int avr910_cmd
static void avr910_open
static void avr910_close
static void avr910_display
static void avr910_set_addr
static int avr910_read_sig_bytes
static void avr910_write_setup


Only these few functions are specific for the two programmers and will be
called butterfly_xxx respectivly:

static int avr910_initialize
static int avr910_paged_write
void avr910_initpgm
static int avr910_read_byte
static int avr910_paged_write_eeprom
static int avr910_write_byte
static int avr910_read_byte_flash
static int avr910_read_byte_eeprom
static int avr910_paged_write_flash
static int avr910_paged_load

In order to avoid the split, one can simply add the butterfly specific parts
to avr910, so one file defines two different programmer types. But again,
the clean structure gets lost.

I would like to avoid messing up the structure and propose to do a
autodetection to distinguish between a real avr910 and a butterfly.
Currently, I use the response to the new 'b'-command. Not the best choice,
as Jan mentioned.

The id string returned from the 'S'-command can be used as well. The
S-command is well defined and a valid answer is required anyway. The
butterfly returns "AVRBOOT", is this string unique or is there a avr910
which does return this, too?

Or the 'v'-command: A real avr910 is expected to return a two byte string
showing the hardware version. The butterfly doesn't implement this command
and just returns a single '?'.

What do you think about this proposal?

  Michael