Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do STK500/arduino/xbee programmers fail EEPROM r/w for some parts? #967

Closed
stefanrueger opened this issue May 16, 2022 · 82 comments
Closed
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@stefanrueger
Copy link
Collaborator

stefanrueger commented May 16, 2022

Reading stk500.c I noticed that for paged load/writes typically word addresses are passed to the external programmer for both flash and EEPROM but under certain circumstances byte addresses (eg, for EEPROM access of parts without EEPROM loadpage SPI programming). Now, I don't have any STK500 programmer at hand, and more crucially neither their firmware source, but does the STK500 communication protocol really switch between word and byte addresses on such a condition without rhyme or reason?

If not, and it's always word addresses, then avrdude -c stk500 should fail to properly read/write EEPROM of the following devices: ATtiny12 ATtiny15 AT90S1200 AT90S4414 AT90S2313 AT90S2333 AT90S2343 AT90S4433 AT90S4434 AT90S8515 AT90S8535 ATmega103 ATmega64 ATmega64A ATmega128 ATmega128A ATmega163 ATmega161 ATmega8 ATmega8A ATmega8515 ATmega8535 ATtiny26

[edit: Actually, only affects ATmega64 ATmega64A ATmega128 ATmega128A ATmega8 ATmega8A, see Issue #970 and below]

Also affects derived programmers, eg, arduino, xbee. I verified that

avrdude -qq -p atmega8 -F -c arduino -b 115200 -P /dev/ttyUSB0 -U eeprom:r:hi.eep:i

with an EEPROM enabled bootloader reads 512 bytes (as it should) but these are wrongly sourced from every other 4-byte page of the 1024 byte EEPROM on the connected 328p (obvs you'd have to store some suitable pattern in the EEPROM to notice).

If stk500.c is right (and my suspicion wrong) then optiboot would need do something about EEPROM addresses of the parts mentioned...

@stefanrueger
Copy link
Collaborator Author

I wonder how an attached STK500 (v1) programmer would suss out whether to expect from AVRDUDE a byte address (eg, ATmega128) or a word address (eg, ATmega328) for the (STK500-protocol) paged EEPROM access (which I guess will be emulated with EEPROM byte read/writes in the physical programmer). It's possible (in theory the firmware is told a device code), but not really plausible.

I have managed to find a couple of sources for the AVRISP firmware: both clearly always expect word addresses (as does optiboot). So looks like the avrisp programmer is affected, too.

Currently, the tally programmers vs stk500.c stands at 2:0

Anyone with a real STK500v1 programmer and some of the above chips with more intel?

@dl8dtl
Copy link
Contributor

dl8dtl commented May 17, 2022

If you're interested, I could get you a genuine STK500v1 firmware image for testing. ;-)

@dl8dtl
Copy link
Contributor

dl8dtl commented May 17, 2022

Maybe I even have one STK that's still at v1, I'm not sure. I used to keep one as a reference …

@stefanrueger
Copy link
Collaborator Author

If you're interested, I could get you a genuine STK500v1 firmware image for testing. ;-)

Thanks, but would need chips/boards of some of the 23 affected chips, too... So no, and best to leave that with the hardware experts :-). Actually, only the firmware source is needed (but I guess Atmel's will be closed?)

But for me this is one more reason to write a dedicated SPM programmer for AVRDUDE that does not rely on the current stk500 implementation. In fact, I noticed this issue when looking through stk500.c to implement "backward compatibility" in that SPM programmer.

@dl8dtl
Copy link
Contributor

dl8dtl commented May 17, 2022

Actually, only the firmware source is needed (but I guess Atmel's will be closed?)

So it is. All we've got is a binary blurb, to fit into the AT90S8535 on the STK.

Please provide a few commands you'd like me to test, and I'll see whether I can run some tests. At least, ATmega8 and ATmega128, I do have some.

@dl8dtl
Copy link
Contributor

dl8dtl commented May 17, 2022

Btw., I'd normally consider the STK500v1 protocol quite obsolete anyway. Atmel did that many years ago. ;-) Let's spend not too much time into such a dead horse.

@stefanrueger
Copy link
Collaborator Author

The stk500.c protocol implementation is used for some programmers and optiboot (with millions and millions of uploads), so we might as well get this right. It's probably more that EEPROM upload/download with the affected parts do not happen that often.

Please provide a few commands

I'd generate a random hex file with as many bytes as the EEPROM holds (512 for the ATmega8, I think)

$ head -512c /dev/urandom | srec_cat - -binary -o - -intel > /tmp/rnd.eep

Upload that to the ATmega8's EEPROM; verification should fail. (Note if you do the same with a file that is half the EEPROM size then verification should pass, but the data would be at the wrong location.)

Or, could prepare two different .hex files:

$ yes The quick brown fox jumps over the lazy dog             | head -512c | srec_cat - -binary -o - -intel >/tmp/en.eep
$ yes Z jagt im komplett verwahrlosten Taxi quer durch Bayern | head -512c | srec_cat - -binary -o - -intel >/tmp/de.eep

Upload /tmp/en.eep to the EEPROM of the ATmega8 (with a reliable programmer)
Upload /tmp/de.eep to the EEPROM of the ATmega8 with the STK500v1 programmer

Look at the eeprom with the reliable programmer:

avrdude -qq -c ... -U eeprom:r:-:r

Should be something like (but not literally so)

Z jaquicgt iown m kojumpmpleer ttt vazy erwaThe hrlok brstenfox  Taxs pvi quhe ler ddog
urchquic Bayown ern
jumpZ jaer tgt iazy m koThe mplek brtt vfox erwas pvhrlohe lstendog
 Taxquici quown er djumpurcher t Bayazy ern
The Z jak brgt ifox m kos pvmplehe ltt vdog
erwaquichrloown stenjump Taxer ti quazy er dThe urchk br Bayfox ern
s pvZ jahe lgt idog
m koquicmpleown tt vjumperwaer thrloazy stenThe  Taxk bri qufox er ds pvurchhe l Baydog
ern
quicZ jaown gt ijumpm koer tmpleazy tt vThe erwak brhrlofox stens pv

If the real STK500v1 programmer passes, I predict that AVRISP programmers fails the test. Optiboot does above when AVRDUDE pretends the 328p is an 8 using -F.

@dl8dtl
Copy link
Contributor

dl8dtl commented May 18, 2022

The "srec_cat" is not needed: AVRDUDE could always write raw binary files (file type "r").
Reprogramming the STK was easy enough to give it a try.
Here's the readout of the "quick brown fox" file with an AVRISPmkII:

% ./build_freebsd/src/avrdude -c avrisp2 -p m8 -t

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9307 (probably m8)
avrdude> d ee
>>> d ee 

Reading | ################################################## | 100% 0.15s

0000  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
0010  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
0020  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
0030  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
0040  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|
0050  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
0060  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
0070  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
0080  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
0090  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|
00a0  65 72 20 74 68 65 20 6c  61 7a 79 20 64 6f 67 0a  |er the lazy dog |
00b0  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
00c0  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
00d0  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
00e0  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
00f0  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|

avrdude> d ee
>>> d ee 

Reading | ################################################## | 100% 0.15s

0100  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
0110  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
0120  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
0130  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
0140  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|
0150  65 72 20 74 68 65 20 6c  61 7a 79 20 64 6f 67 0a  |er the lazy dog |
0160  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
0170  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
0180  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
0190  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
01a0  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|
01b0  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
01c0  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
01d0  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
01e0  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
01f0  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|

avrdude>

I guess that is proof enough that the AVRDUDE stk500v1 protocol implementation matches an STK500v1 firmware …

Device: ATmega8A (after all, they're still in production), STK500 firmware version 1.14 (decimal, as usually in AVRDUDE). I believe that to be the latest V1 firmware. I just keep the firmware in the STK until tonight, in case you'd like me to perform more tests. Afterwards, will reflash it to V2.

@mcuee
Copy link
Collaborator

mcuee commented May 18, 2022

If you're interested, I could get you a genuine STK500v1 firmware image for testing. ;-)

Just wondering how I can check if an STK500 is of V1 or V2 firmware. I have found an unused genuine Atmel STK500 from 2006 (or earlier?) and it does not seem to work with either avrdude (error message: avrdude stk500_recv() programmer is not responding ) or Atmel Studio 4.19 or Atmel Studio 7.0 (firmware upgrade failed). Maybe it is broken. I will try again.

@dl8dtl
Copy link
Contributor

dl8dtl commented May 18, 2022

If you call AVRDUDE with -c stk500, it attempts some auto-probing. It's not that easy, and quite a bit of guesswork.
Drop me a mail if you'd like to get a firmware file to upgrade (or downgrade) yourself; the bootloader (actually a separate AT90S2313 on the STK500) talks avr910 protocol.

@dl8dtl
Copy link
Contributor

dl8dtl commented May 18, 2022

Btw., IMHO AVRDUDE's STK500v1 protocol implementation mostly wraps around the SPI_MULTI command, issuing its own idea of ISP commands. Back two decades ago, when someone contributed us the STK500 implementation, that seemed to be the fastest way to handle STK500s at all: the ISP command stuff was already there (AVRDUDE started out with the "classic" parallel-port bit-bangers of those days), so handling the STK500 protocol took only a bit of wrap-up to initialize the programmer and such.
We've been doing that even in STK500v2 for many years, and only switched to real protocol commands a few years ago.

@stefanrueger
Copy link
Collaborator Author

I guess that is proof enough that the AVRDUDE stk500v1 protocol implementation matches an STK500v1 firmware …

Cool! And thanks for going to the trouble of verifying that.

Which makes dealing with the derived AVRDUDE programmers somewhat harder, because the attached programmers (AVRISP, ATmega8 boards with optiboot's vector bootloader, ...) don't for a moment think that any STK500v1 address could be a byte address: -c avrisp should expose the problem mentioned, as does -c arduino which I see after uploading something with

avrdude -qq -p atmega8 -F -c arduino -b 115200 -P /dev/ttyUSB0 -U eeprom:w:fox.eep:i

I know what modification in arduino.c would solve this issue but not so sure about the other derived programmers.

@stefanrueger
Copy link
Collaborator Author

stefanrueger commented May 18, 2022

The "srec_cat" is not needed: AVRDUDE could always write raw binary files (file type "r")

... and read from pipeline. Cool. I initially tried constructing a pattern and pipe with the file type "d", which does not allow reads:

$ echo {0..255} {255..0} | tr \  , | avrdude -qq -p m328p -c urclock -P /dev/ttyUSB0 -U eeprom:w:-:d
avrdude: fileio: invalid operation=0
avrdude: read from file '-' failed

@dl8dtl
Copy link
Contributor

dl8dtl commented May 18, 2022

It would be interesting to dig out an old version of AVR Studio (4.something), and see what it does when being faced with such a bootloader.

@mcuee
Copy link
Collaborator

mcuee commented May 19, 2022

Does the STK500 v1 communication protocol can explain the question? Or we really have to test on a real STK500 with V1 FW?
http://ww1.microchip.com/downloads/en/AppNotes/doc2525.pdf

STK500 v2 is also documented.
https://ww1.microchip.com/downloads/en/Appnotes/doc2591.pdf

@dl8dtl
Copy link
Contributor

dl8dtl commented May 19, 2022

Or we really have to test on a real STK500 with V1 FW?

That's what I did.

@mcuee
Copy link
Collaborator

mcuee commented May 19, 2022

Which makes dealing with the derived AVRDUDE programmers somewhat harder, because the attached programmers (AVRISP, ATmega8 boards with optiboot's vector bootloader, ...) don't for a moment think that any STK500v1 address could be a byte address.

Now the thing is that probably there is no real need to support real STK500 with V1 protocol any more since it has be obsolete for many years (only V2 support is needed, V2 FW was distributed with AVR Studio 4.11 build 401 or later), that probably means in 2005.

The things to support is the derived programmers which may not exactly follow the STK500 v1 protocol to the letter. Does this make the solution a bit easier?

@mcuee
Copy link
Collaborator

mcuee commented May 19, 2022

It would be interesting to dig out an old version of AVR Studio (4.something), and see what it does when being faced with such a bootloader.

Then you need really old version of Atmel Studio (prior to 4.11 build 401) which is no longer available from Atmel/Microchip (earliest version is 4.13).
https://www.microchip.com/en-us/tools-resources/archives/avr-sam-mcus

Google does find a link. Not so sure how good the Russian link mentioned in the forum post is good or not.
https://www.avrfreaks.net/forum/downloading-old-version-avr-studio-410

@stefanrueger
Copy link
Collaborator Author

@mcuee Atmel's STK500 v1 docu is non-committal about whether the addresses are byte or word addresses and certainly does not say anything about which memories of which parts will be treated as what. Whoever wrote the lines

avrdude/src/stk500.c

Lines 819 to 822 in 4601bee

if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
a_div = 2;
else
a_div = 1;

clearly knew something that cannot be augured from Atmel's 31 page v1 docu. In contrast to this, the STK500 v2 docu on this point is lucid, sane and simple: flash addresses to be sent as word addresses, all other memory addresses as byte addresses.

Given the closed-source nature of Atmel's implementation of its protocol it is quite understandable that others have interpreted the protocol to mean both flash and EEPROM access is always done with word addresses.

AVRDUE only invokes the programmer's *_paged_load() and *_paged_write() when the memory page size exceeds 1. It so turns out that only EEPROM r/w for ATmega64 ATmega64A ATmega128 ATmega128A ATmega8 ATmega8A are affected by this issue. But see also Issue #970 that affects bootloaders.

Jörg has proven AVRDUDE's stk500 implementation correct in these cases. Well, the ATmega8A, but I take the extrapolation to the others as given.

I see some possible solutions:

  1. Deprecate the "pure" STK500 v1 support and change above code to simply a_div = 2 (accept that the STK500 kit with FW version 1.xx is a dead horse)
  2. Create a new AVRDUDE programmer stk500v1w that imports the stk500 (v1) programmer and change paged access to be always through word addresses. Modify .conf for these programmers to be based on stk200v1w
  3. Change all STK500 derived programmers to swap out their paged memory access functions

Thoughts?

@mcuee
Copy link
Collaborator

mcuee commented May 20, 2022

@stefanrueger
IMHO Option 1 should be the preferred way to go. But @dl8dtl will have to agree.

Option 2 is of course okay as well but it is a bit wasted (kind of redundant codes).

@mcuee
Copy link
Collaborator

mcuee commented May 20, 2022

It would be interesting to dig out an old version of AVR Studio (4.something), and see what it does when being faced with such a bootloader.

I have got avrdude 3.56 and 4.10 installed under Windows 11. Just wondering how to test this case.

I have the official Arduino Leonardo board and clone Arduino Uno with CH340.

  1. how to trigger the Arduino board in bootloader mode?
  2. I see one problem that usually the COM port of my Arduino boards can not be reassigned to COM1/2/3/4.

@stefanrueger
Copy link
Collaborator Author

@mcuee

Doesn't have to be an arduino board to use -c arduino. These days (I guess) the AVRDUDE arduino programmer is used to upload to bootloaders that follow a skelton STK500 v1 protocol. Optiboot is probably the most popular of these. Can be compiled for a lot of parts, even those without bootloader support (switch on virtual bootloader or something).

Triggering the bootloader is its own artform. Most simply need to be reset shortly before talking to them (the main difference between -c arduino and -c stk500v1) is that arduino twitches the DTR/RTS line that some boards couple to the mcu's reset via a cap and pullup to Vcc to send a reset pulse. You can do that by hand, but then need to start arduino shortly after, so the bootloader doesn't time out.

Afraid I cannot help with windows driver etc to connect COMx to the various FTDI/CH340/CH330N/... chips.

@stefanrueger
Copy link
Collaborator Author

I'd be happy to do a PR for 1 or 2 if/when the current PRs are processed one way or another.

The second option isn't too much code duplication, as the code base is vvv modular and requires essentially two functions to be overloaded (paged access). And it's neat because it is backwards compatible to very early and old programmer boards/FW.

@mcuee
Copy link
Collaborator

mcuee commented May 21, 2022

Afraid I cannot help with windows driver etc to connect COMx to the various FTDI/CH340/CH330N/... chips.

No problems. I am kind of cross-platform guy (at least Linux, macOS and Windows, sometimes also BSDs) since I carry out testing for cross-platform projects (eg: libusb, hidapi, libftdi, pyusb, libusbdotnet and OpenOCD).

Interestingly I do not have problems to set the FTDI/CH340/CP2102/etc based USB to Serial Converter to use COM1/2/3/4 under Windows but not the two Arduino boards. Very strange.

This seems to be an interesting question from @dl8dtl.

It would be interesting to dig out an old version of AVR Studio (4.something), and see what it does when being faced with such a bootloader.

@mcuee
Copy link
Collaborator

mcuee commented May 22, 2022

Doesn't have to be an arduino board to use -c arduino. These days (I guess) the AVRDUDE arduino programmer is used to upload to bootloaders that follow a skelton STK500 v1 protocol. Optiboot is probably the most popular of these. Can be compiled for a lot of parts, even those without bootloader support (switch on virtual bootloader or something).

I'd be happy to do a PR for 1 or 2 if/when the current PRs are processed one way or another.

The second option isn't too much code duplication, as the code base is vvv modular and requires essentially two functions to be overloaded (paged access). And it's neat because it is backwards compatible to very early and old programmer boards/FW.

Good to know that. In that case, maybe Option 2 is better as we know that this types of bootloaders are not really the same as stk500v1, but rather a subset only.

Ref: https://github.com/Optiboot/optiboot/wiki/HowOptibootWorks
Optiboot implements a small subset of the "STK500 communications protocol" (version 1) defined by Atmel in their Application Note AVR061 In order to conserve code space, many of the protocol commands are ignored and receive "lies" rather than correct response codes, leading to the possibility that the bootloader may not operate correctly with all host-side software. It is known to work with versions of "avrdude" supporting the "-c arduino" programmer type.

@stefanrueger
Copy link
Collaborator Author

OK, seeing that this is the known Issue #421 we might consider doing nothing here.

@dl8dtl has proven the stk500.c implementation correct for the STK500 kit with v1 FW. Folks who have recognised the problem in the current issue a long time ago for their own programmer firmware/bootloader may/will have solved that issue on their side, so any change now will create backward compatibility issues with them.

I still think it is a good idea to provide an alternative programmer to arduino with its own protocol that does not have this, ahem, unusual byte/word protocol interpretation quirk and that solves the shortcomings of AVRDUDE's arduino programmer for compact bootloaders as mentioned in Discussion #940 (eg, that these use atomic page erase and write rather than chip erase or page erase on its own, that these only implement paged access for EEPROM/flash not byte access etc).

@mcuee
Copy link
Collaborator

mcuee commented May 24, 2022

I still think it is a good idea to provide an alternative programmer to arduino with its own protocol that does not have this, ahem, unusual byte/word protocol interpretation quirk and that solves the shortcomings of AVRDUDE's arduino programmer for compact bootloaders as mentioned in Discussion #940.

Yes I agree. I beliver what I mentioned below in #970 is in line with what you are saying. Correct me if I am wrong. Thanks.

What I am trying to say is that they probably should not use -c arduino as -c arduino was probably designed for those official and compatible Arduino clones and works fine for that purpose. Maybe a -c optiboot or things like that makes sense. This is different from your proposed solution.

@stefanrueger
Copy link
Collaborator Author

So, what's outstanding is an actual EEPROM check for "the defining" STK500 kit v1 on ATmega328P et al. That's all.

Hmm, it actually failed.

@mcuee So, you put Atmel's original FW version 1.x on your STK 500 kit, prepared an ATmega328P eeprom with "The big brown fox..." using another known-to-be-working kit, and experienced the following with the STK 500 FW v 1.x kit and the current mainline avdude 7.0?

$ avrdude -qqp m328p -c stk500v1 -P COM4 -t
avrdude> read eeprom 0 32
>>> read eeprom 0 0x200
0000  54 68 71 75 6b 20 6f 77  66 6f 6a 75 73 20 65 72  |Thquk owfojus er|
0010  68 65 61 7a 64 6f 54 68  71 75 6b 20 6f 77 66 6f  |heazdoThquk owfo|

What is returned by

$ avrdude -qqp m328p -c stk500v1 -P COM4 -U eeprom:r:-:r | head

@mcuee
Copy link
Collaborator

mcuee commented Jul 18, 2022

So, what's outstanding is an actual EEPROM check for "the defining" STK500 kit v1 on ATmega328P et al. That's all.

Hmm, it actually failed.

@mcuee So, you put Atmel's original FW version 1.x on your STK 500 kit, prepared an ATmega328P eeprom with "The big brown fox..." using another known-to-be-working kit, and experienced the following with the STK 500 FW v 1.x kit and the current mainline avdude 7.0?

$ avrdude -qqp m328p -c stk500v1 -P COM4 -t
avrdude> read eeprom 0 32
>>> read eeprom 0 0x200
0000  54 68 71 75 6b 20 6f 77  66 6f 6a 75 73 20 65 72  |Thquk owfojus er|
0010  68 65 61 7a 64 6f 54 68  71 75 6b 20 6f 77 66 6f  |heazdoThquk owfo|

Yes that is correct.

What is returned by

$ avrdude -qqp m328p -c stk500v1 -P COM4 -U eeprom:r:-:r | head
PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_main_git.exe -qqp m328p -c stk500v1 -P COM4 -e && echo OK
OK
PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_main_git.exe -qqp m328p -c stk500v1 -P COM4 -U eep
rom:w:.\hex\en.eep && echo OK
avrdude_main_git.exe: verification error, first mismatch at byte 0x0002
                      0x71 != 0x65
avrdude_main_git.exe: verification error; content mismatch

/c/work/avr/avrdude_test/avrdude-7.0_bin64
$ ./avrdude_main_git.exe -qqp m328p -c stk500v1 -P COM4  -U eeprom:r:-:r | head
Thququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ovov□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□

$ ./avrdude_main_git.exe -qqp m328p -c usbasp  -U eeprom:r:-:r | head
Thquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus ov□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□

Note: is actually 0xff.

@mcuee
Copy link
Collaborator

mcuee commented Jul 18, 2022

Another test, using usbasp to write the eeprom correctly and then use STK500V1 to read back, the results are wrong.

$ ./avrdude_main_git.exe -qqp m328p -c usbasp -e

$ ./avrdude_main_git.exe -qqp m328p -c usbasp -U eeprom:w:./hex/en.eep:i

$ ./avrdude_main_git.exe -qqp m328p -c usbasp -U eeprom:r:-:r | head
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog

$ ./avrdude_main_git.exe -qqp m328p -c stk500v1 -P COM4 -U eeprom:r:-:r | head
The e ququicick k brbrowown n fofox x jujumpmps s ovoverer t thehe l lazazy y dodog
g
ThThe e ququicick k brbrowown n fofox x jujumpmps s ovoverer t thehe l lazazy y dodog
g
ThThe e ququicick k brbrowown n fofox x jujumpmps s ovoverer t thehe l lazazy y dodog
g
ThThe e ququicick k brbrowown n fofox x jujumpmps s ovoverer t thehe l lazazy y dodog
g
ThThe e ququicick k brbrowown n fofox x jujumpmps s ovoverer t thehe l lazazy y dodog
g

@mcuee
Copy link
Collaborator

mcuee commented Jul 18, 2022

Same problem for ATmega2560. This time the test is under Linux.

mcuee@UbuntuSwift3:~/build/avr/avrdude_bin$ ./avrdude_main -c stk500v1 -P /dev/ttyUSB0 -qqp m328p 
-U flash:w:../hex/Blink.ino.with_bootloader.standard.hex && echo OK
OK
mcuee@UbuntuSwift3:~/build/avr/avrdude_bin$ ./avrdude_main -c stk500v1 -P /dev/ttyUSB0 -qqp m2560
 -U eeprom:w:../hex/en.eep:i && echo OK 
avrdude_main: verification error, first mismatch at byte 0x0004
              0x6b != 0x71
avrdude_main: verification error; content mismatch
mcuee@UbuntuSwift3:~/build/avr/avrdude_bin$ ./avrdude_main -c stk500v1 -P /dev/ttyUSB0 -qqp m2560 
-U eeprom:r:-:r | head
The k brk brfox fox s ovs ovhe lhe ldog
dog
quicquicown own jumpjumper ter tazy azy The The k brk brfox fox s ovs ovhe lhe ldog
dog
quicquicown own jumpjumper ter tazy azy The The k brk brfox fox s ovs ovhe lhe ldog
dog
quicquicown own jumpjumper ter tazy azy The The k brk brfox fox s ovs ovhe lhe ldog
dog
quicquicown own jumpjumper ter tazy azy The The k brk brfox fox s ovs ovhe lhe ldog
dog
mcuee@UbuntuSwift3:~/build/avr/avrdude_bin$ ./avrdude_main -c usbasp  -qqp m2560 -U eeprom:r:-:r | head
The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumps ov��(remaining skipped)

mcuee@UbuntuSwift3:~/build/avr/avrdude_bin$ ./avrdude_main -c usbasp  -qqp m2560 -t
avrdude> read eeprom 0 0x200
>>> read eeprom 0 0x200 
0000  54 68 65 20 6b 20 62 72  66 6f 78 20 73 20 6f 76  |The k brfox s ov|
0010  68 65 20 6c 64 6f 67 0a  71 75 69 63 6f 77 6e 20  |he ldog quicown |
0020  6a 75 6d 70 65 72 20 74  61 7a 79 20 54 68 65 20  |jumper tazy The |
0030  6b 20 62 72 66 6f 78 20  73 20 6f 76 68 65 20 6c  |k brfox s ovhe l|
0040  64 6f 67 0a 71 75 69 63  6f 77 6e 20 6a 75 6d 70  |dog quicown jump|
0050  65 72 20 74 61 7a 79 20  54 68 65 20 6b 20 62 72  |er tazy The k br|
0060  66 6f 78 20 73 20 6f 76  68 65 20 6c 64 6f 67 0a  |fox s ovhe ldog |
0070  71 75 69 63 6f 77 6e 20  6a 75 6d 70 65 72 20 74  |quicown jumper t|
0080  61 7a 79 20 54 68 65 20  6b 20 62 72 66 6f 78 20  |azy The k brfox |
0090  73 20 6f 76 68 65 20 6c  64 6f 67 0a 71 75 69 63  |s ovhe ldog quic|
00a0  6f 77 6e 20 6a 75 6d 70  65 72 20 74 61 7a 79 20  |own jumper tazy |
00b0  54 68 65 20 6b 20 62 72  66 6f 78 20 73 20 6f 76  |The k brfox s ov|
00c0  68 65 20 6c 64 6f 67 0a  71 75 69 63 6f 77 6e 20  |he ldog quicown |
00d0  6a 75 6d 70 65 72 20 74  61 7a 79 20 54 68 65 20  |jumper tazy The |
00e0  6b 20 62 72 66 6f 78 20  73 20 6f 76 68 65 20 6c  |k brfox s ovhe l|
00f0  64 6f 67 0a 71 75 69 63  6f 77 6e 20 6a 75 6d 70  |dog quicown jump|
0100  73 20 6f 76 ff ff ff ff  ff ff ff ff ff ff ff ff  |s ov............|
0110  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0120  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0130  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0140  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0150  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0160  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0170  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0180  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
0190  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01a0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01b0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01c0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01d0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01e0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
01f0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|

@mcuee
Copy link
Collaborator

mcuee commented Jul 18, 2022

Looks like the issue has always been there. I went back to release 5.6 which is the first release to support ATmega328p and the issue is already there.

I also tested 6.3, 6.01, 5.10 and checked that 5.4/5/5 do not support ATmega328p. Good thing is that all the old version can be built under Ubuntu 20.04, include version 4.4 and 5.0.
Ref: http://download.savannah.gnu.org/releases/avrdude/

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.6$ ./avrdude -C ./avrdude.conf -c stk500v1 
-P /dev/ttyUSB0 -qqp m328p -U eeprom:w:../../hex/en.eep:i && echo OK
avrdude: verification error, first mismatch at byte 0x0002
         0x65 != 0x71
avrdude: verification error; content mismatch

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.6$ ./avrdude -C ./avrdude.conf -c stk500v1 -P /dev/ttyUSB0 -qqp m328p -U eeprom:r:-:r | head
Thququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ererheheazazdodoThThququk k owowfofojujus s ovov�� (skipped the test)

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.6$ ./avrdude -C ./avrdude.conf -c usbasp -qqp m328p -U eeprom:r:-:r | head
Thquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus erheazdoThquk owfojus ov��(skipped the rest)

@mcuee
Copy link
Collaborator

mcuee commented Jul 18, 2022

Using ATtiny13A and I can see that version 5.2 is broken and version 5.1 is fine. 5.2 is the first release version with STK500v1/v2 support. Both were released in 2006. I guess most of the people would upgarde the STK500 to V2 FW later.

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.1$ ./avrdude -C ./avrdude.conf -c stk500 
-P /dev/ttyUSB0 -qqp t13 -U eeprom:w:../../hex/ent13.eep:i && echo OK
avrdude: please define PAGEL and BS2 signals in the configuration file for part ATtiny13
OK

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.1$ ./avrdude -C ./avrdude.conf -c stk500 
-P /dev/ttyUSB0 -qqp t13 -U eeprom:r:-:r | head
avrdude: please define PAGEL 

and BS2 signals in the configuration file for part ATtiny13
The quick brown fox jumps over the lazy dog
The quick brown fox 

mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.2$ ./avrdude -C ./avrdude.conf -c stk500 
-P /dev/ttyUSB0 -qqp t13 -U eeprom:w:../../hex/ent13.eep:i && echo OK
avrdude: successfully opened stk500v1 device -- please use -c stk500v1
avrdude: please define PAGEL and BS2 signals in the configuration file for part ATtiny13
avrdude: verification error, first mismatch at byte 0x0002
         0x65 != 0x71
avrdude: verification error; content mismatch
mcuee@UbuntuSwift3:~/build/avr/avrdude_release/avrdude-5.2$ ./avrdude -C ./avrdude.conf -c stk500 
-P /dev/ttyUSB0 -qqp t13 -U eeprom:r:-:r | head
avrdude: successfully opened stk500v1 device -- please use -c stk500v1
avrdude: please define PAGEL and BS2 signals in the configuration file for part ATtiny13
Thquk owfojus erheazdoThquk owfox  lazy dog

@stefanrueger
Copy link
Collaborator Author

@mcuee Thanks. This has now peeled off all layers in the software archeology going back to 2006, and would indicate that

avrdude/src/stk500.c

Lines 809 to 822 in 4601bee

if (strcmp(m->desc, "flash") == 0) {
memtype = 'F';
}
else if (strcmp(m->desc, "eeprom") == 0) {
memtype = 'E';
}
else {
return -2;
}
if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
a_div = 2;
else
a_div = 1;

is a bug in that the a_div initialisation should only apply to flash, where for EEPROM it should simply be a_div = 1 throughout.

It seems that the world (optiboot, arduino as as ISP, ...) has compensated for this bug by assuming AVRDUDE sends all EEPROM addresses as word address (a_div = 2). Actually these programmers overcompensate for the bug because for six out of the 146 or so of the known SPI programmable parts with EEPROM and page_size > 1, AVRDUDE would still send the EEPROM addresses as byte addresses (a_div = 1): ATmega64 ATmega64A ATmega128 ATmega128A ATmega8 ATmega8A

Given that piece of new info from @mcuee, I now also think Option 1 of #967 (comment) would best deal with the situation. This is how I would write it up in the code (twice, once for paged_load() and once for paged_write()):

  if (strcmp(m->desc, "flash") == 0) {
    memtype = 'F';
    /* this routine is only called for parts with page_size > 1: next line means a_div = 2; */
    a_div = m->op[AVR_OP_LOADPAGE_LO] || m->op[AVR_OP_READ_LO]? 2: 1
  } else if (strcmp(m->desc, "eeprom") == 0) {
    memtype = 'E';
    /*
     * The STK 500 v1 protocol actually expects a_div=1, but since optiboot,
     * arduino as ISP and others now assume a_div = 2, better leave it thus.
     * See https://github.com/avrdudes/avrdude/issues/967
     */
    a_div = 2;        
  } else
    return -2;

Note that, somewhat unusually, this willfully makes the existing bug worse in the sense that the current implementation "only" fails paged EEPROM r/w for all but six SPI programmable parts with EEPROM page_size > 1. With the change it is all those parts.

If the bug was corrected now to reflect the gold reference STK500 FW v1.x, this would upset quite a few programmers that have adopted AVRDUDE's apparent implementation. Above automagically helps those (physical) programmer firmware implementations and optiboot that simply expect a_div = 2 for EEPROM. The suggested change hurts those projects that have fully understood the ramifications of how exactly the 16+ year old bug occurs, and created their own bootloaders/firmware to 100% match the bug. I predict the latter group is in the minority and, more importantly, has the means to follow suit.

@stefanrueger stefanrueger added the bug Something isn't working label Jul 18, 2022
@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

@WestfW's comments here is interesting.
Optiboot/optiboot#235 (comment)

See the conclusion at avrdudes/avrdude#967 (comment) and related discussion. TLDR: a bug was introduced in AVRDUDE back in 2006, and since then other code (like optiboot and Arduino as ISP) has compensated, assuming that the buggy code was intentional.)

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

@stefanrueger
Thanks for the clear summary and conclusion. Now the consensus seems to be with Option 1.

Deprecate the "pure" STK500 v1 support and change above code to simply a_div = 2 (accept that the STK500 kit with FW version 1.xx is a dead horse)

@dl8dtl Any comments here? Thanks.

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

@stefanrueger

Indeed the proposed patch (with a_div =1 for EEPROM) fixed the issue with STK500 (original V1 FW) EEPROM issue.

PS C:\work\avr\avrdude_test\avrdude_issue967> git diff
diff --git a/src/stk500.c b/src/stk500.c
index cb27003..31c7af0 100644
--- a/src/stk500.c
+++ b/src/stk500.c
@@ -808,18 +808,22 @@ static int stk500_paged_write(PROGRAMMER * pgm, AVRPART * p, AVRMEM * m,

   if (strcmp(m->desc, "flash") == 0) {
     memtype = 'F';
-  }
+    if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
+       a_div = 2;
+    else
+        a_div = 1;
+  }
   else if (strcmp(m->desc, "eeprom") == 0) {
     memtype = 'E';
-  }
-  else {
-    return -2;
-  }
-
-  if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
-    a_div = 2;
+    /*
+     * The STK 500 v1 protocol actually expects a_div=1, but since optiboot,
+     * arduino as ISP and others now assume a_div = 2, better leave it thus.
+     * See https://github.com/avrdudes/avrdude/issues/967
+     */
+    a_div = 1;
+  }
   else
-    a_div = 1;
+    return -2;

   n = addr + n_bytes;
 #if 0
@@ -902,18 +906,22 @@ static int stk500_paged_load(PROGRAMMER * pgm, AVRPART * p, AVRMEM * m,

   if (strcmp(m->desc, "flash") == 0) {
     memtype = 'F';
-  }
+    if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
+       a_div = 2;
+    else
+        a_div = 1;
+  }
   else if (strcmp(m->desc, "eeprom") == 0) {
     memtype = 'E';
-  }
-  else {
-    return -2;
-  }
-
-  if ((m->op[AVR_OP_LOADPAGE_LO]) || (m->op[AVR_OP_READ_LO]))
-    a_div = 2;
+    /*
+     * The STK 500 v1 protocol actually expects a_div=1, but since optiboot,
+     * arduino as ISP and others now assume a_div = 2, better leave it thus.
+     * See https://github.com/avrdudes/avrdude/issues/967
+     */
+    a_div = 1;
+  }
   else
-    a_div = 1;
+    return -2;

   n = addr + n_bytes;
   for (; addr < n; addr += block_size) {

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_stk500v1_issue967.exe -qqp m328p 
-c stk500v1 -P COM4 -U eeprom:w:.\hex\en.eep:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_stk500v1_issue967.exe -qqp m328p -c stk500v1
 -P COM4 -U eeprom:r:-:r 
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps ov        

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

Given that piece of new info from @mcuee, I now also think Option 1 of #967 (comment) would best deal with the situation. This is how I would write it up in the code (twice, once for paged_load() and once for paged_write()):

  if (strcmp(m->desc, "flash") == 0) {
    memtype = 'F';
    /* this routine is only called for parts with page_size > 1: next line means a_div = 2; */
    a_div = m->op[AVR_OP_LOADPAGE_LO] || m->op[AVR_OP_READ_LO]? 2: 1
  } else if (strcmp(m->desc, "eeprom") == 0) {
    memtype = 'E';
    /*
     * The STK 500 v1 protocol actually expects a_div=1, but since optiboot,
     * arduino as ISP and others now assume a_div = 2, better leave it thus.
     * See https://github.com/avrdudes/avrdude/issues/967
     */
    a_div = 2;        
  } else
    return -2;

Note that, somewhat unusually, this willfully makes the existing bug worse in the sense that the current implementation "only" fails paged EEPROM r/w for all but six SPI programmable parts with EEPROM page_size > 1. With the change it is all those parts.

@stefanrueger
Just wondering what you mean by the code will make things worse. Do you mean it makes the original STK500 with V1 FW worse? It should help Arduino compatible bootloader or "Arduino as ISP", right?

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

I think the above does help the Arduino compatible bootloader and things like Arduino as ISP.

Reading test. git main failed. The patched version (with with a_div =1 for EEPROM) works with ATmega8 (simulated). Itested wigth both bootloader and Arduino as ISP.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_arduino_issue967.exe -qqp atmega8 -F 
-c usbasp -U eeprom:r:-:r
avrdude_arduino_issue967.exe: Expected signature for ATmega8 is 1E 93 07
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps ov

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git_main.exe -qqp atmega8 -F 
-c arduino -P COM5 -U eeprom:r:-:r
avrdude_git_main.exe: Expected signature for ATmega8 is 1E 93 07
The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jumper tazy The k brfox s ovhe ldog
quicown jump
                                             
PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_arduino_issue967.exe -qqp atmega8 -F
 -c arduino -P COM5 -U eeprom:r:-:r
avrdude_arduino_issue967.exe: Expected signature for ATmega8 is 1E 93 07
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps ov

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

Write test using Arduino as ISP, again git main will fail and the patched version is okay.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_arduino_issue967.exe -qqp atmega328p 
-c stk500v1 -P COM5 -b 19200 -e

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_arduino_issue967.exe -qqp atmega8 -F 
-c stk500v1 -P COM5 -b 19200 -U eeprom:w:.\hex\en.eep:i && echo OK
avrdude_arduino_issue967.exe: Expected signature for ATmega8 is 1E 93 07
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_arduino_issue967.exe -qqp atmega328p 
-c usbasp -U eeprom:r:-:r
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps over the lazy dog
The quick brown fox jumps ov

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git_main.exe -qqp atmega8 -F -c stk500v1 
-P COM5 -b 19200 -U eeprom:w:.\hex\en.eep:i && echo OK
avrdude_git_main.exe: Yikes!  Invalid device signature.
avrdude_git_main.exe: Expected signature for ATmega8 is 1E 93 07
avrdude_git_main.exe: verification error, first mismatch at byte 0x0000
                      0xff != 0x54
avrdude_git_main.exe: verification error; content mismatch

@dl8dtl
Copy link
Contributor

dl8dtl commented Jul 19, 2022

@stefanrueger Thanks for the clear summary and conclusion. Now the consensus seems to be with Option 1.

Deprecate the "pure" STK500 v1 support and change above code to simply a_div = 2 (accept that the STK500 kit with FW version 1.xx is a dead horse)

@dl8dtl Any comments here? Thanks.

I guess pure STK500v1 support is something that could be deprecated, indeed. I can't imagine a good reason for why not upgrade any (real) STK500 to v2.

@WestfW
Copy link

WestfW commented Jul 19, 2022

Deprecate the "pure" STK500 v1 support

What does that mean WRT the STK500v1-subset used by bootloaders like Optiboot?
https://github.com/Optiboot/optiboot/wiki/HowOptibootWorks#implemented-commands

@mcuee
Copy link
Collaborator

mcuee commented Jul 19, 2022

Deprecate the "pure" STK500 v1 support

What does that mean WRT the STK500v1-subset used by bootloaders like Optiboot? https://github.com/Optiboot/optiboot/wiki/HowOptibootWorks#implemented-commands

@WestfW

@stefanrueger's options are as following.

  1. Deprecate the "pure" STK500 v1 support and change above code to simply a_div = 2 (accept that the STK500 kit with FW version 1.xx is a dead horse)
  2. Create a new AVRDUDE programmer stk500v1w that imports the stk500 (v1) programmer and change paged access to be always through word addresses. Modify .conf for these programmers to be based on stk200v1w
  3. Change all STK500 derived programmers to swap out their paged memory access functions
  4. Do nothing

So option 1 is to deprecate the "pure" STK500 v1" (usually genunine STK500 with V1 FW, or rare clones which support V1 FW), and then apply @stefanrueger's proposed fix (a_div = 2 for EEPROM) in favor of the popular STK500v1-subset derivatives like optiboot bootloader and others (bootloader or programmers like Arduino as ISP.

Impact analysis by @stefanrueger for Option 1.

Impacts users with the STK500 kit with vvv old FW, but also bootloader writers using -c arduino who realised what the current implementation does and adapted (the latter group has the skills to adapt again and won't need our help). Automagically helps optiboot users (who have compiled for ATmega8A et al with EEPROM support). Automagically helps Arduino for ISP physical programmers.

@stefanrueger
Copy link
Collaborator Author

What does that mean WRT the STK500v1-subset used by bootloaders like Optiboot?

The proposed change is good for Optiboot. With that STK_LOAD_ADDRESS will actually always get a word address for flash and EEPROM for all devices. Currently EEPROM addresses for 6 parts (ATmega8 etc) are still sent as byte addresses.

Those bootloader writers who understood better what AVRDUDE does and implemented EEPROM byte addresses for these six devices will see a "regression" problem. I think that's the minority (given the popularity of optiboot).

@stefanrueger
Copy link
Collaborator Author

@mcuee

Just wondering what you mean by the code will make things worse. Do you mean it makes the original STK500 with V1 FW worse? It should help Arduino compatible bootloader or "Arduino as ISP", right?

Absolutely correct. On both accounts. Thanks for all the testing!

@mcuee
Copy link
Collaborator

mcuee commented Jul 20, 2022

@stefanrueger
I think you can now create a PR for you patch to go with Option 1 since @dl8dtl agrees with the option.

From @dl8dtl

I guess pure STK500v1 support is something that could be deprecated, indeed. I can't imagine a good reason for why not upgrade any (real) STK500 to v2.

@mcuee
Copy link
Collaborator

mcuee commented Jul 31, 2022

#1046 has been tested by @MCUdude to work well with ATmega8 running optiboot.

@stefanrueger
Copy link
Collaborator Author

Closed by PR #1046

@mcuee
Copy link
Collaborator

mcuee commented Aug 12, 2022

Just an update, I got the simple ATmega8A core board and an Arduino AVR ISP Shield, and the test shows "Arduino as ISP" works fine now (failed if using the 7.0 release version). Verified using usbasp.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c avrisp -P COM8 -b 19200 -p m8a
 -U eeprom:w:.\hex\entest.hex:i

avrdude.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.03s

avrdude.exe: Device signature = 0x1e9307 (probably m8a)
avrdude.exe: reading input file .\hex\entest.hex for eeprom
avrdude.exe: writing 512 bytes eeprom ...

Writing | ################################################## | 100% 25.78s

avrdude.exe: 512 bytes of eeprom written
avrdude.exe: verifying eeprom memory against .\hex\entest.hex

Reading | ################################################## | 100% 2.21s

avrdude.exe: 512 bytes of eeprom verified

avrdude.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m8a -t

avrdude.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.01s

avrdude.exe: Device signature = 0x1e9307 (probably m8a)
avrdude> d ee 0 0x400
>>> d ee 0 0x400

Reading | ################################################## | 100% 0.52s

0000  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
0010  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
0020  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
0030  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
0040  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|
0050  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
0060  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
0070  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
0080  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
0090  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|
00a0  65 72 20 74 68 65 20 6c  61 7a 79 20 64 6f 67 0a  |er the lazy dog |
00b0  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
00c0  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
00d0  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
00e0  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
00f0  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|
0100  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
0110  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
0120  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
0130  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
0140  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|
0150  65 72 20 74 68 65 20 6c  61 7a 79 20 64 6f 67 0a  |er the lazy dog |
0160  54 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |The quick brown |
0170  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|
0180  68 65 20 6c 61 7a 79 20  64 6f 67 0a 54 68 65 20  |he lazy dog The |
0190  71 75 69 63 6b 20 62 72  6f 77 6e 20 66 6f 78 20  |quick brown fox |
01a0  6a 75 6d 70 73 20 6f 76  65 72 20 74 68 65 20 6c  |jumps over the l|
01b0  61 7a 79 20 64 6f 67 0a  54 68 65 20 71 75 69 63  |azy dog The quic|
01c0  6b 20 62 72 6f 77 6e 20  66 6f 78 20 6a 75 6d 70  |k brown fox jump|
01d0  73 20 6f 76 65 72 20 74  68 65 20 6c 61 7a 79 20  |s over the lazy |
01e0  64 6f 67 0a 54 68 65 20  71 75 69 63 6b 20 62 72  |dog The quick br|
01f0  6f 77 6e 20 66 6f 78 20  6a 75 6d 70 73 20 6f 76  |own fox jumps ov|

avrdude> quit
>>> quit

The release 7.0 version will fail.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m8a -e

avrdude.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.01s

avrdude.exe: Device signature = 0x1e9307 (probably m8a)
avrdude.exe: erasing chip

avrdude.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude7 -C .\avrdude7.conf -c stk500v1 -P COM8 -b 19200
 -p m8a -U eeprom:w:.\hex\entest.hex:i

avrdude7.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.03s

avrdude7.exe: Device signature = 0x1e9307 (probably m8a)
avrdude7.exe: reading input file ".\hex\entest.hex"
avrdude7.exe: writing eeprom (512 bytes):

Writing | ################################################## | 100% 25.77s

avrdude7.exe: 512 bytes of eeprom written
avrdude7.exe: verifying eeprom memory against .\hex\entest.hex:

Reading | ################################################## | 100% 2.24s

avrdude7.exe: verification error, first mismatch at byte 0x0000
              0x61 != 0x54
avrdude7.exe: verification error; content mismatch

avrdude7.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m8a -t

avrdude.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.01s

avrdude.exe: Device signature = 0x1e9307 (probably m8a)
avrdude> d ee 0 0x400
>>> d ee 0 0x400

Reading | ################################################## | 100% 0.53s

0000  61 7a 79 20 71 75 69 63  64 6f 67 0a 6f 77 6e 20  |azy quicdog own |
0010  54 68 65 20 6a 75 6d 70  71 75 69 63 65 72 20 74  |The jumpquicer t|
0020  6b 20 62 72 61 7a 79 20  6f 77 6e 20 54 68 65 20  |k brazy own The |
0030  66 6f 78 20 6b 20 62 72  6a 75 6d 70 66 6f 78 20  |fox k brjumpfox |
0040  73 20 6f 76 73 20 6f 76  65 72 20 74 68 65 20 6c  |s ovs over the l|
0050  68 65 20 6c 64 6f 67 0a  61 7a 79 20 71 75 69 63  |he ldog azy quic|
0060  64 6f 67 0a 6f 77 6e 20  54 68 65 20 6a 75 6d 70  |dog own The jump|
0070  71 75 69 63 65 72 20 74  6b 20 62 72 61 7a 79 20  |quicer tk brazy |
0080  6f 77 6e 20 54 68 65 20  66 6f 78 20 6b 20 62 72  |own The fox k br|
0090  6a 75 6d 70 66 6f 78 20  73 20 6f 76 73 20 6f 76  |jumpfox s ovs ov|
00a0  65 72 20 74 68 65 20 6c  68 65 20 6c 64 6f 67 0a  |er the lhe ldog |
00b0  61 7a 79 20 71 75 69 63  64 6f 67 0a 6f 77 6e 20  |azy quicdog own |
00c0  54 68 65 20 6a 75 6d 70  71 75 69 63 65 72 20 74  |The jumpquicer t|
00d0  6b 20 62 72 61 7a 79 20  6f 77 6e 20 54 68 65 20  |k brazy own The |
00e0  66 6f 78 20 6b 20 62 72  6a 75 6d 70 66 6f 78 20  |fox k brjumpfox |
00f0  73 20 6f 76 73 20 6f 76  65 72 20 74 68 65 20 6c  |s ovs over the l|
0100  68 65 20 6c 64 6f 67 0a  61 7a 79 20 71 75 69 63  |he ldog azy quic|
0110  64 6f 67 0a 6f 77 6e 20  54 68 65 20 6a 75 6d 70  |dog own The jump|
0120  71 75 69 63 65 72 20 74  6b 20 62 72 61 7a 79 20  |quicer tk brazy |
0130  6f 77 6e 20 54 68 65 20  66 6f 78 20 6b 20 62 72  |own The fox k br|
0140  6a 75 6d 70 66 6f 78 20  73 20 6f 76 73 20 6f 76  |jumpfox s ovs ov|
0150  65 72 20 74 68 65 20 6c  68 65 20 6c 64 6f 67 0a  |er the lhe ldog |
0160  61 7a 79 20 71 75 69 63  64 6f 67 0a 6f 77 6e 20  |azy quicdog own |
0170  54 68 65 20 6a 75 6d 70  71 75 69 63 65 72 20 74  |The jumpquicer t|
0180  6b 20 62 72 61 7a 79 20  6f 77 6e 20 54 68 65 20  |k brazy own The |
0190  66 6f 78 20 6b 20 62 72  6a 75 6d 70 66 6f 78 20  |fox k brjumpfox |
01a0  73 20 6f 76 73 20 6f 76  65 72 20 74 68 65 20 6c  |s ovs over the l|
01b0  68 65 20 6c 64 6f 67 0a  61 7a 79 20 71 75 69 63  |he ldog azy quic|
01c0  64 6f 67 0a 6f 77 6e 20  54 68 65 20 6a 75 6d 70  |dog own The jump|
01d0  71 75 69 63 65 72 20 74  6b 20 62 72 61 7a 79 20  |quicer tk brazy |
01e0  6f 77 6e 20 54 68 65 20  66 6f 78 20 6b 20 62 72  |own The fox k br|
01f0  6a 75 6d 70 66 6f 78 20  73 20 6f 76 73 20 6f 76  |jumpfox s ovs ov|

avrdude>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants