`-c stk500` only works with devices that's running stk500v2 firmware

MCUdude commented 1 year ago

Even though -c stk500 is supposed to work with both the stk500v1 and stk500v2 protocol, it only works with hardware running stk500v2 firmware.

I agree that -c stk500 should try the v2 protocol, but shouldn't it switch to v1 automatically if it can't connect, ideally after only one failed attempt?

https://github.com/avrdudes/avrdude/blob/5e715e2851eeea5593d323bd670da6ee1ea8afb4/src/avrdude.conf.in#L1350-L1366

$ ./avrdude -cstk500 -P /dev/cu.usbserial -patmega8515
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout communicating with programmer
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout communicating with programmer
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout
avrdude error: timeout communicating with programmer
^C

$ ./avrdude -cstk500v1 -P /dev/cu.usbserial -patmega8515 -v

avrdude: Version 7.1-20230108
         Copyright the AVRDUDE authors;
         see https://github.com/avrdudes/avrdude/blob/main/AUTHORS

         System wide configuration file is /Users/hans/Downloads/avrdude-terminal/src/avrdude.conf
         User configuration file is /Users/hans/.avrduderc

         Using Port                    : /dev/cu.usbserial
         Using Programmer              : stk500v1
         AVR Part                      : ATmega8515
         Chip Erase delay              : 9000 us
         PAGEL                         : PD7
         BS2                           : PA0
         RESET disposition             : dedicated
         RETRY pulse                   : SCK
         Serial program mode           : yes
         Parallel program mode         : yes
         Timeout                       : 200
         StabDelay                     : 100
         CmdexeDelay                   : 25
         SyncLoops                     : 32
         PollIndex                     : 3
         PollValue                     : 0x53
         Memory Detail                 :

                                           Block Poll               Page                       Polled
           Memory Type Alias    Mode Delay Size  Indx Paged  Size   Size #Pages MinW  MaxW   ReadBack
           ----------- -------- ---- ----- ----- ---- ------ ------ ---- ------ ----- ----- ---------
           eeprom                  4    20   128    0 no        512    1      0  9000  9000 0xff 0xff
           flash                  33    10    64    0 yes      8192   64    128  4500  4500 0xff 0x00
           lfuse                   0     0     0    0 no          1    1      0  4500  4500 0x00 0x00
           hfuse                   0     0     0    0 no          1    1      0  4500  4500 0x00 0x00
           lock                    0     0     0    0 no          1    1      0  4500  4500 0x00 0x00
           signature               0     0     0    0 no          3    1      0     0     0 0x00 0x00
           calibration             0     0     0    0 no          4    1      0     0     0 0x00 0x00

         Programmer Type : STK500
         Description     : Atmel STK500 version 1.x firmware
         Hardware Version: 2
         Firmware Version: 1.14
         Vtarget         : 5.1 V
         Varef           : 3.1 V
         Oscillator      : 1.229 MHz
         SCK period      : 1.1 us
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9306 (probably m8515)

avrdude done.  Thank you.

mcuee commented 1 year ago

This has been there for a while. So the question is whether we want to change or not.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c stk500v1 -P COM48 -p m16a
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9403 (probably m16a)

avrdude done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c stk500 -P COM48 -p m16a -v

avrdude: Version 7.1-20230715 (5e715e28)
         Copyright the AVRDUDE authors;
         see https://github.com/avrdudes/avrdude/blob/main/AUTHORS

         System wide configuration file is C:\work\avr\avrdude_test\avrdude_bin\avrdude.conf
         User configuration file is C:\work\avr\avrdude_test\avrdude_bin\avrdude.rc

         Using Port                    : COM48
         Using Programmer              : stk500
avrdude stk500v2_recv() error: timeout
avrdude stk500v2_recv() error: timeout
avrdude stk500v2_recv() error: timeout

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude71 -C .\avrdude71.conf -c stk500 -P COM48 -p m16a
avrdude71 error: timeout

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude7 -C .\avrdude7.conf -c stk500 -P COM48 -p m16a
avrdude7.exe: stk500v2_ReceiveMessage(): timeout

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude63 -C .\avrdude63.conf -c stk500 -P COM48 -p m16
avrdude63.exe: stk500v2_ReceiveMessage(): timeout

mcuee commented 1 year ago

Even the first avrdude release with Windows binary (5.11 patch 7610 version) has this issue. http://download.savannah.gnu.org/releases/avrdude/

PS C:\work\avr\avrdude_test\avrdude_binary_archive\avrdude-5.11-Patch7610-win32> .\avrdude -c stk500 -P COM48 -p m16
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_getsync(): timeout communicating with programmer

avrdude.exe: stk500v2_ReceiveMessage(): timeout
avrdude.exe: stk500v2_ReceiveMessage(): timeout

stefanrueger commented 1 year ago

-c stk500 runs the stump src/stk500generic.c programmer. That code hasn't changed in donkey's years, and I am pretty sure there was no change after 7.0. The code looks like it's meant to probe stk500v1, then stk500v2, and either way recommends the user either go with -c stk500v1 or -c stk500v2.

However, what really happens is that the setup() call initialises the programmer as an stk500v2 thereby overwriting the stk500generic_open() that is meant to do the probing and therefore carries on with the full-monty stk500v2 code. That is, it behaves as you describe @MCUdude; it simply switches to be a stk500v2 programmer.

I guess, the intended code would happen if you were to insert the line pgm->open = stk500generic_open; as last line in stk500generic_setup(), but there are other problems with that approach.

We could fiddle in other ways with the intended code.

Try removing the stk500generic_setup() and stk500generic_teardown() functions and not initialise these pointers; instead execute pgm->setup(pgm); after stk500v2_initpgm() in stk500generic_open().
Alternatively, the stk500generic_initpgm() could simply print an error message asking the user to figure out which firmware their programmers have and if unknown try one then the other.

mcuee commented 1 year ago

@stefanrueger

Yet another easy fix is to say - stk500 is now same as -c stk500v2.

stefanrueger commented 1 year ago

@mcuee Well, -c stk500 was really meant to do some sort of intelligent switching (probe stk500v1 first, and if that works go with it but recommend using -c stk500v1; if stk500v1 doesn't try stk500v2)

mcuee commented 1 year ago

@stefanrueger

I understand. However, this behavior has been there since 2011 (or maybe earlier), that is where my proposal comes from.

And actually last year we were thinking to deprecate the real STK500 with v1 FW because of the EEPROM issues. It is just later you fixed it.

But yes it is better if there is a easy fix without regressions.

avrdudes / avrdude

`-c stk500` only works with devices that's running stk500v2 firmware #1467