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

Change condition for using paged access in avr_read() and avr_write() #1110

Closed
wants to merge 2 commits into from

Conversation

stefanrueger
Copy link
Collaborator

@stefanrueger stefanrueger commented Oct 5, 2022

Supposed to fix #970.

The fundamental avr_read() and avr_write() functions that carry out -U use if(pgm->paged_... && mem->page_size > 1) to decide whether to use paged access. I believe the condition mem->page_size > 1 serves as a proxy for whether the memory is of flash or eeprom type (application, apptable, ...).

This has disadvantages when it comes to bootloaders and EEPROM for which the newer parts(UPDI / AVR8X) and some older parts (ATmega161 ATmega163 ATmega8515 ATmega8535) declare a page_size of 1. The problem is that bootloaders often only implement paged access for EEPROM and flash, so are bound to fail. See Issue #970

I suggest using a more comprehensive condition of when to use paged access functions:

avrdude/src/avrcache.c

Lines 105 to 117 in 44fe5be

/*
* Paged access?
* - Programmer must have paged routines
* - Memory has positive page size, which is a power of two
* - Memory has positive size, which is a multiple of the page size
* - Memory is flash type or eeprom type
*/
int avr_has_paged_access(const PROGRAMMER *pgm, const AVRMEM *mem) {
return pgm->paged_load && pgm->paged_write &&
mem->page_size > 0 && (mem->page_size & (mem->page_size-1)) == 0 &&
mem->size > 0 && mem->size % mem->page_size == 0 &&
(avr_mem_is_flash_type(mem) || avr_mem_is_eeprom_type(mem));
}

As this is a change for the fundamental strategy how all -U operations are carried out, this requires careful consideration. Comments? Ramifications I overlooked?

@mcuee mcuee added bug Something isn't working enhancement New feature or request labels Oct 6, 2022
@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

@stefanrueger

This seems to be a simple and good fix for #970. But I still do not quite understand the internal working of avrdude very well. Two questions from my side.

  1. It seems to me you explained the change to avr_write_mem() clearly in the above despcription but you did not explain the change to avr_read_mem(). The new line of code does not seem to be equivalent to the two lines of codes it replaced. It could be my misunderstanding though.
@@ -382,8 +382,7 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con
    return avr_mem_hiaddr(mem);
  }

-  if (pgm->paged_load != NULL && mem->page_size > 1 &&
-      mem->size % mem->page_size == 0) {
+  if (avr_has_paged_access(pgm, mem)) {
    /*
     * the programmer supports a paged mode read
     */

  1. Does the change affect memory types other than Flash and EEPROM (fuse, calibration, use signature, etc)? I believe it will not affect those but just want to confirm.

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

@stefanrueger The following is tested with the bootloader in #940 (https://github.com/avrdudes/avrdude/files/8882667/atmega328p.txt)

Previously writing EEPROM will fail if simulating the ATmega161 using ATmega328P.

$ yes hello, world | head -512c | ./avrdude -qq -p m161 -F -c arduino -b 115200 -P COM5 -U eeprom:w:-:r
avrdude.exe: Expected signature for ATmega161 is 1E 94 01
 ***failed;
 ***failed;
 ***failed;
 ***failed;
 ***failed;

Now it seems to be able to write the EEPROM but the results are not correct.

$ yes hello, world | head -512c | ./avrdude -qq -p m161 -F -c arduino -b 115200 -P COM5 -U eeprom:w:-:r
avrdude.exe: Expected signature for ATmega161 is 1E 94 01
avrdude.exe: verification error, first mismatch at byte 0x0000
             0x65 != 0x68
avrdude.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -patmega328p -cavrispmkii -Pusb -t

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

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

avrdude.exe: Device signature = 0x1e950f (probably m328p)
avrdude> dump eeprom
>>> dump eeprom

Reading | ################################################## | 100% 0.34s

0000  65 ff 6c ff 2c ff 77 ff  72 ff 64 ff 68 ff 6c ff  |e.l.,.w.r.d.h.l.|
0010  6f ff 20 ff 6f ff 6c ff  0a ff 65 ff 6c ff 2c ff  |o. .o.l. .e.l.,.|
0020  77 ff 72 ff 64 ff 68 ff  6c ff 6f ff 20 ff 6f ff  |w.r.d.h.l.o. .o.|
0030  6c ff 0a ff 65 ff 6c ff  2c ff 77 ff 72 ff 64 ff  |l. .e.l.,.w.r.d.|
0040  68 ff 6c ff 6f ff 20 ff  6f ff 6c ff 0a ff 65 ff  |h.l.o. .o.l. .e.|
0050  6c ff 2c ff 77 ff 72 ff  64 ff 68 ff 6c ff 6f ff  |l.,.w.r.d.h.l.o.|
0060  20 ff 6f ff 6c ff 0a ff  65 ff 6c ff 2c ff 77 ff  | .o.l. .e.l.,.w.|
0070  72 ff 64 ff 68 ff 6c ff  6f ff 20 ff 6f ff 6c ff  |r.d.h.l.o. .o.l.|
0080  0a ff 65 ff 6c ff 2c ff  77 ff 72 ff 64 ff 68 ff  | .e.l.,.w.r.d.h.|
0090  6c ff 6f ff 20 ff 6f ff  6c ff 0a ff 65 ff 6c ff  |l.o. .o.l. .e.l.|
00a0  2c ff 77 ff 72 ff 64 ff  68 ff 6c ff 6f ff 20 ff  |,.w.r.d.h.l.o. .|
00b0  6f ff 6c ff 0a ff 65 ff  6c ff 2c ff 77 ff 72 ff  |o.l. .e.l.,.w.r.|
00c0  64 ff 68 ff 6c ff 6f ff  20 ff 6f ff 6c ff 0a ff  |d.h.l.o. .o.l. .|
00d0  65 ff 6c ff 2c ff 77 ff  72 ff 64 ff 68 ff 6c ff  |e.l.,.w.r.d.h.l.|
00e0  6f ff 20 ff 6f ff 6c ff  0a ff 65 ff 6c ff 2c ff  |o. .o.l. .e.l.,.|
00f0  77 ff 72 ff 64 ff 68 ff  6c ff 6f ff 20 ff 6f ff  |w.r.d.h.l.o. .o.|

avrdude> quit
>>> quit

@MCUdude
Copy link
Collaborator

MCUdude commented Oct 6, 2022

As this is a change for the fundamental strategy of how all -U operations are carried out, this requires careful consideration. Comments? Ramifications I overlooked?

It's a change that will affect all parts, so we should test it on some actual usual suspect hardware. And perhaps @dl8dtl should have a look as well?

I can test with an ATmega8535 later today, both in ISP and bootloader mode.

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

It will be good to test bootloader for AVR8X parts as well. Unfortunately I have not figured how to get a bootloader working yet with them. I have the official ATmega4809 Arduino Nano Every and two ATmega4808 Nano clones.

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

When I use the wiring bootloader, it seems to work fine, with or without this patch.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -F -qqp m161 -c wiring -P COM5 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
avrdude_pr1110.exe: Expected signature for ATmega161 is 1E 94 01
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -qqp m2560 -c wiring -P COM5 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -F -qqp m161 -c wiring -P COM5 -b 115200 
-U eeprom:w:detest.eep:i && echo OK
avrdude.exe: Expected signature for ATmega161 is 1E 94 01
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -qqp m2560 -c wiring -P COM5 -b 115200 
-U eeprom:v:detest.eep:i && echo OK
OK

@MCUdude
Copy link
Collaborator

MCUdude commented Oct 6, 2022

It will be good to test bootloader for AVR8X parts as well. Unfortunately I have not figured how to get a bootloader working yet with them. I have the official ATmega4809 Arduino Nano Every and two ATmega4808 Nano clones.

I have a few custom-designed boards I can use to test 32 and 48-pin AVR8X chips with, with, and without (Optiboot) bootloader. I'll see if I can test with an AVR128Dx too.

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

@stefanrueger The following is tested with the bootloader in #940 (https://github.com/avrdudes/avrdude/files/8882667/atmega328p.txt)

Previously writing EEPROM will fail if simulating the ATmega161 using ATmega328P.
Now it seems to be able to write the EEPROM but the results are not correct.

So I tested using the optiboot bootloader from @MCUdude's Arduino MegaCore (which supports EEPROM). It has the same results. Both reading and writing are not correct.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -F -qqp m161 -c arduino -P COM5 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
avrdude.exe: Expected signature for ATmega161 is 1E 94 01
 ***failed;
 ***failed;
 ***failed;
 ***failed;

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -F -qqp m161 -c arduino -P COM5 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
avrdude_pr1110.exe: Expected signature for ATmega161 is 1E 94 01
avrdude_pr1110.exe: verification error, first mismatch at byte 0x0000
                    0x68 != 0x54
avrdude_pr1110.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -qqp m2560 -c arduino -P COM5 -b 115200 
-U eeprom:w:entest.eep && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -F -qqp m161 -c arduino -P COM5 -b 115200 
-U eeprom:v:entest.eep:i && echo OK
avrdude_pr1110.exe: Expected signature for ATmega161 is 1E 94 01
avrdude_pr1110.exe: verification error, first mismatch at byte 0x0001
                    0x54 != 0x68
avrdude_pr1110.exe: verification error; content mismatch

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

It will be good to test bootloader for AVR8X parts as well. Unfortunately I have not figured how to get a bootloader working yet with them. I have the official ATmega4809 Arduino Nano Every and two ATmega4808 Nano clones.

Just figured out how to burn the Optiboot-DX bootloader from DXCore for my AVR128DB48 Curiosity Nano board (need to use UART3). Results seem to be the same. It is better than git main but still both writing and reading will fail.

So all in all, it does not work with optiboot as of now.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c arduino -qqp avr128db48 -P COM6 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
 ***failed;
 ***failed;
 ***failed;
 ***failed;

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -c arduino -qqp avr128db48 -P COM6 -b 115200 
-U eeprom:w:entest.eep:i && echo OK
avrdude_pr1110.exe: verification error, first mismatch at byte 0x0000
                    0xff != 0x54
avrdude_pr1110.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -c pkobn_updi -qqp avr128db48 
-U eeprom:w:entest.eep:i && echo OK
                    Vtarget                      : 3.31 V
                    PDI/UPDI clock Xmega/megaAVR : 100 kHz
OK

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110 -c arduino -qqp avr128db48 -P COM6 -b 115200 
-U eeprom:v:entest.eep:i && echo OK
avrdude_pr1110.exe: verification error, first mismatch at byte 0x0001
                    0x54 != 0x68
avrdude_pr1110.exe: verification error; content mismatch

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

@MCUdude

I am wondering if you can help to figure out how to get optiboot working for ATtiny817 Xplained Mini.

I have multiple issues with burning the megaTinyCore optiboot bootloader onto ATtiny817 Xplained Mini.

@MCUdude
Copy link
Collaborator

MCUdude commented Oct 6, 2022

I am wondering if you can help to figure out how to get optiboot working for ATtiny817 Xplained Mini.
I have multiple issues with burning the megaTinyCore optiboot bootloader onto ATtiny817 Xplained Mini.

Can't you just use the upstream version of Avrdude and upload the bootloader file directly?

@mcuee
Copy link
Collaborator

mcuee commented Oct 6, 2022

Can't you just use the upstream version of Avrdude and upload the bootloader file directly?

I tried but it does not work -- I am not so sure how to enter the bootloader. It seems to be tricky. As of now, I fail to understand the following. It seems to me there is no way to get it working out of the box (autoreset is not possible as I need updi, PB4 is not possible). The only way seems to use firmware which seems to be tricky as well.

I will just give up for now.

https://github.com/SpenceKonde/megaTinyCore#bootloader-optiboot-support
https://github.com/SpenceKonde/megaTinyCore/blob/master/megaavr/extras/Ref_Reset.md

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -p attiny817 -c xplainedmini_updi -Pusb -e "-Ufuse0:w:0x00:m" "-Ufuse1:w:0x00:m" "-Ufuse5:w:0b11000101:m" "-Ufuse6:w:0x04:m" "-Ufuse8:w:0x00:m" "-Ufuse7:w:0x00:m"

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

Reading | ################################################## | 100% 0.05s

avrdude.exe: Device signature = 0x1e9320 (probably t817)
avrdude.exe: erasing chip
avrdude.exe: reading input file 0x00 for fuse0/wdtcfg
avrdude.exe: writing 1 byte fuse0/wdtcfg ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse0/wdtcfg written
avrdude.exe: verifying fuse0/wdtcfg memory against 0x00

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse0/wdtcfg verified
avrdude.exe: reading input file 0x00 for fuse1/bodcfg
avrdude.exe: writing 1 byte fuse1/bodcfg ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse1/bodcfg written
avrdude.exe: verifying fuse1/bodcfg memory against 0x00

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse1/bodcfg verified
avrdude.exe: reading input file 0b11000101 for fuse5/syscfg0
avrdude.exe: writing 1 byte fuse5/syscfg0 ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse5/syscfg0 written
avrdude.exe: verifying fuse5/syscfg0 memory against 0b11000101

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse5/syscfg0 verified
avrdude.exe: reading input file 0x04 for fuse6/syscfg1
avrdude.exe: writing 1 byte fuse6/syscfg1 ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse6/syscfg1 written
avrdude.exe: verifying fuse6/syscfg1 memory against 0x04

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse6/syscfg1 verified
avrdude.exe: reading input file 0x00 for fuse8/bootend
avrdude.exe: writing 1 byte fuse8/bootend ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse8/bootend written
avrdude.exe: verifying fuse8/bootend memory against 0x00

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse8/bootend verified
avrdude.exe: reading input file 0x00 for fuse7/append
avrdude.exe: writing 1 byte fuse7/append ...

Writing | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse7/append written
avrdude.exe: verifying fuse7/append memory against 0x00

Reading | ################################################## | 100% 0.02s

avrdude.exe: 1 byte of fuse7/append verified

avrdude.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -p attiny817 -c xplainedmini_updi -P usb -U .\optiboot_xplained817.hex

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

Reading | ################################################## | 100% 0.06s

avrdude.exe: Device signature = 0x1e9320 (probably t817)
avrdude.exe: NOTE: "flash" memory has been specified, an erase cycle will be performed
             To disable this feature, specify the -D option.
avrdude.exe: erasing chip
avrdude.exe: reading input file .\optiboot_xplained817.hex for flash
             with 466 bytes in 2 sections within [0, 0x1ff]
             using 8 pages and 46 pad bytes
avrdude.exe: writing 466 bytes flash ...

Writing | ################################################## | 100% 0.27s

avrdude.exe: 466 bytes of flash written
avrdude.exe: verifying flash memory against .\optiboot_xplained817.hex

Reading | ################################################## | 100% 0.22s

avrdude.exe: 466 bytes of flash verified

avrdude.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -p attiny817 -c arduino -P COM7 -v

avrdude.exe: Version 7.0-20221005 (44fe5be)
             Copyright (c) Brian Dean, http://www.bdmicro.com/
             Copyright (c) Joerg Wunsch

             System wide configuration file is "C:/work/avr/avrdude_test/avrdude_bin/avrdude.conf"

             Using Port                    : COM7
             Using Programmer              : arduino
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 1 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 2 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 3 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 4 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 5 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 6 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 7 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 8 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 9 of 10: not in sync: resp=0x00
avrdude.exe: stk500_recv(): programmer is not responding
avrdude.exe: stk500_getsync() attempt 10 of 10: not in sync: resp=0x00
avrdude.exe: opening programmer "arduino" on port "COM7" failed

avrdude.exe done.  Thank you.

@MCUdude
Copy link
Collaborator

MCUdude commented Oct 6, 2022

I tried but it does not work -- I am not so sure how to enter the bootloader. It seems to be tricky.

The "proper" solution would be to turn the UPDI pin into the reset pin. However, you can't recover the chip unless you have a HV UPDI programmer. I believe megaTinyCore has a bootloader build with an 8-second application start delay. This means the chip would stay in bootloader mode for 8 seconds before timing out and starting the user application.
I don't own any development boards that make tinyAVR-0/1/2 bootloader experimentation convenient.

@stefanrueger
Copy link
Collaborator Author

Basically, git mainline uses mem->page_size > 1 as proxy for paged access, whereas this PR uses the memory name being either eeprom, flash, application, apptable or boot as proxy.

For eeprom the former is probably wrong for all UPDI parts and some classic parts (amongst which ATmega161 ATmega163 ATmega8515 ATmega8535).

I now realise that for flash the PR definition is probably wrong about the following older parts

$ avrdude -p*/At | grep flash.page_size.1$ 
.ptmm	ATtiny11	flash	page_size	1
.ptmm	ATtiny12	flash	page_size	1
.ptmm	ATtiny15	flash	page_size	1
.ptmm	AT90S1200	flash	page_size	1
.ptmm	AT90S4414	flash	page_size	1
.ptmm	AT90S2313	flash	page_size	1
.ptmm	AT90S2333	flash	page_size	1
.ptmm	AT90S2343	flash	page_size	1
.ptmm	AT90S4433	flash	page_size	1
.ptmm	AT90S4434	flash	page_size	1
.ptmm	AT90S8515	flash	page_size	1
.ptmm	AT90S8535	flash	page_size	1

Sooooo, I have changed the avr_has_paged_access() definition to now say

  • Memory is either flash type with page size > 1 or eeprom

@stefanrueger
Copy link
Collaborator Author

@mcuee Your questions:

  1. Equivalence for avr_read() and avr_write(): The conditions in the git mainline are equivalent, save for one more "sanity check" of checking that the memory size is a multiple of the page size (true for all parts).
  2. Correct, no memory beyond eeprom or flash-type will ever be considered paged memory (so as previously)

@MCUdude and @mcuee Thanks for testing! Good find re optiboot: I thought about this and now realise that optiboot will never be able to treat eeprom with page size of 1 (no matter how hard we try) owing to the fact that the stk500v1 implementation sends a word address for eeprom (we had a long discussion re this); this is why you see only every other byte written. That is unrelated to this PR and cannot be helped now that word addresses were (erronously) used for a decade in this vvv old STK500v1 protocol implementation.

Other bootloaders may benefit from being called with paged routines; good to know that wiring can manage without this PR (but then it is large, so can afford to implement both byte-wise read/write and paged read/write, most bootloaders only implement paged r/w!); I know that my (soon to be included) urcock bootloader will be able to work with this PR but that's not the point.

It really is about when to call paged r/w routines. I realise that there is mem->paged (set in avrdude.conf) but this is not set for PDI and UPDI parts nor is it ever set for eeprom (except accidentally for ATtiny43U) so this must be used for something else...

@MCUdude
Copy link
Collaborator

MCUdude commented Oct 7, 2022

Here's the result with an ATmega8535. Writing to EEPROM using the bootloader doesn't work properly...

USBasp read/write EEPROM:

yes "hello, world" | head -c512 | ./avrdude -cusbasp -p atmega8535 -Ueeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions

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

avrdude: Device signature = 0x1e9308 (probably m8535)
avrdude: reading input file <stdin> for eeprom
avrdude: writing 512 bytes eeprom ...

Writing | ################################################## | 100% 5.80s

avrdude: 512 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>

Reading | ################################################## | 100% 0.60s

avrdude: 512 bytes of eeprom verified

avrdude done.  Thank you.


$ echo "read eeprom 0 0x200" | ./avrdude -cusbasp -patmega8535 -t

avrdude: AVR device initialized and ready to accept instructions

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

avrdude: Device signature = 0x1e9308 (probably m8535)
>>> read eeprom 0 0x200 

Reading | ################################################## | 100% 0.60s

0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 0a 68 65 6c  |hello, world hel|
0010  6c 6f 2c 20 77 6f 72 6c  64 0a 68 65 6c 6c 6f 2c  |lo, world hello,|
0020  20 77 6f 72 6c 64 0a 68  65 6c 6c 6f 2c 20 77 6f  | world hello, wo|
0030  72 6c 64 0a 68 65 6c 6c  6f 2c 20 77 6f 72 6c 64  |rld hello, world|
0040  0a 68 65 6c 6c 6f 2c 20  77 6f 72 6c 64 0a 68 65  | hello, world he|
0050  6c 6c 6f 2c 20 77 6f 72  6c 64 0a 68 65 6c 6c 6f  |llo, world hello|
0060  2c 20 77 6f 72 6c 64 0a  68 65 6c 6c 6f 2c 20 77  |, world hello, w|
0070  6f 72 6c 64 0a 68 65 6c  6c 6f 2c 20 77 6f 72 6c  |orld hello, worl|
0080  64 0a 68 65 6c 6c 6f 2c  20 77 6f 72 6c 64 0a 68  |d hello, world h|
0090  65 6c 6c 6f 2c 20 77 6f  72 6c 64 0a 68 65 6c 6c  |ello, world hell|
00a0  6f 2c 20 77 6f 72 6c 64  0a 68 65 6c 6c 6f 2c 20  |o, world hello, |
00b0  77 6f 72 6c 64 0a 68 65  6c 6c 6f 2c 20 77 6f 72  |world hello, wor|
00c0  6c 64 0a 68 65 6c 6c 6f  2c 20 77 6f 72 6c 64 0a  |ld hello, world |
00d0  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 0a 68 65 6c  |hello, world hel|
00e0  6c 6f 2c 20 77 6f 72 6c  64 0a 68 65 6c 6c 6f 2c  |lo, world hello,|
00f0  20 77 6f 72 6c 64 0a 68  65 6c 6c 6f 2c 20 77 6f  | world hello, wo|
0100  72 6c 64 0a 68 65 6c 6c  6f 2c 20 77 6f 72 6c 64  |rld hello, world|
0110  0a 68 65 6c 6c 6f 2c 20  77 6f 72 6c 64 0a 68 65  | hello, world he|
0120  6c 6c 6f 2c 20 77 6f 72  6c 64 0a 68 65 6c 6c 6f  |llo, world hello|
0130  2c 20 77 6f 72 6c 64 0a  68 65 6c 6c 6f 2c 20 77  |, world hello, w|
0140  6f 72 6c 64 0a 68 65 6c  6c 6f 2c 20 77 6f 72 6c  |orld hello, worl|
0150  64 0a 68 65 6c 6c 6f 2c  20 77 6f 72 6c 64 0a 68  |d hello, world h|
0160  65 6c 6c 6f 2c 20 77 6f  72 6c 64 0a 68 65 6c 6c  |ello, world hell|
0170  6f 2c 20 77 6f 72 6c 64  0a 68 65 6c 6c 6f 2c 20  |o, world hello, |
0180  77 6f 72 6c 64 0a 68 65  6c 6c 6f 2c 20 77 6f 72  |world hello, wor|
0190  6c 64 0a 68 65 6c 6c 6f  2c 20 77 6f 72 6c 64 0a  |ld hello, world |
01a0  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 0a 68 65 6c  |hello, world hel|
01b0  6c 6f 2c 20 77 6f 72 6c  64 0a 68 65 6c 6c 6f 2c  |lo, world hello,|
01c0  20 77 6f 72 6c 64 0a 68  65 6c 6c 6f 2c 20 77 6f  | world hello, wo|
01d0  72 6c 64 0a 68 65 6c 6c  6f 2c 20 77 6f 72 6c 64  |rld hello, world|
01e0  0a 68 65 6c 6c 6f 2c 20  77 6f 72 6c 64 0a 68 65  | hello, world he|
01f0  6c 6c 6f 2c 20 77 6f 72  6c 64 0a 68 65 6c 6c 6f  |llo, world hello|


avrdude done.  Thank you.

Optiboot (Arduino) bootloader

$ yes "hello, world" | head -c512 | ./avrdude -carduino -P/dev/cu.usbserial-1410 -p atmega8535 -Ueeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions

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

avrdude: Device signature = 0x1e9308 (probably m8535)
avrdude: reading input file <stdin> for eeprom
avrdude: writing 512 bytes eeprom ...

Writing | ################################################## | 100% 4.28s

avrdude: 512 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>

Reading | ################################################## | 100% 3.02s

avrdude: verification error, first mismatch at byte 0x0000
         0x65 != 0x68
avrdude: verification error; content mismatch

avrdude done.  Thank you.

$ echo "read eeprom 0 0x200" | ./avrdude -carduino -P/dev/cu.usbserial-1410 -p atmega8535 -t

avrdude: AVR device initialized and ready to accept instructions

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

avrdude: Device signature = 0x1e9308 (probably m8535)
>>> read eeprom 0 0x200 

Reading | ################################################## | 100% 3.02s

0000  65 65 6c 6c 2c 2c 77 77  72 72 64 64 68 68 6c 6c  |eell,,wwrrddhhll|
0010  6f 6f 20 20 6f 6f 6c 6c  0a 0a 65 65 6c 6c 2c 2c  |oo  ooll  eell,,|
0020  77 77 72 72 64 64 68 68  6c 6c 6f 6f 20 20 6f 6f  |wwrrddhhlloo  oo|
0030  6c 6c 0a 0a 65 65 6c 6c  2c 2c 77 77 72 72 64 64  |ll  eell,,wwrrdd|
0040  68 68 6c 6c 6f 6f 20 20  6f 6f 6c 6c 0a 0a 65 65  |hhlloo  ooll  ee|
0050  6c 6c 2c 2c 77 77 72 72  64 64 68 68 6c 6c 6f 6f  |ll,,wwrrddhhlloo|
0060  20 20 6f 6f 6c 6c 0a 0a  65 65 6c 6c 2c 2c 77 77  |  ooll  eell,,ww|
0070  72 72 64 64 68 68 6c 6c  6f 6f 20 20 6f 6f 6c 6c  |rrddhhlloo  ooll|
0080  0a 0a 65 65 6c 6c 2c 2c  77 77 72 72 64 64 68 68  |  eell,,wwrrddhh|
0090  6c 6c 6f 6f 20 20 6f 6f  6c 6c 0a 0a 65 65 6c 6c  |lloo  ooll  eell|
00a0  2c 2c 77 77 72 72 64 64  68 68 6c 6c 6f 6f 20 20  |,,wwrrddhhlloo  |
00b0  6f 6f 6c 6c 0a 0a 65 65  6c 6c 2c 2c 77 77 72 72  |ooll  eell,,wwrr|
00c0  64 64 68 68 6c 6c 6f 6f  20 20 6f 6f 6c 6c 0a 0a  |ddhhlloo  ooll  |
00d0  65 65 6c 6c 2c 2c 77 77  72 72 64 64 68 68 6c 6c  |eell,,wwrrddhhll|
00e0  6f 6f 20 20 6f 6f 6c 6c  0a 0a 65 65 6c 6c 2c 2c  |oo  ooll  eell,,|
00f0  77 77 72 72 64 64 68 68  6c 6c 6f 6f 20 20 6f 6f  |wwrrddhhlloo  oo|
0100  6c 6c 0a 0a 65 65 6c 6c  2c 2c 77 77 72 72 64 64  |ll  eell,,wwrrdd|
0110  68 68 6c 6c 6f 6f 20 20  6f 6f 6c 6c 0a 0a 65 65  |hhlloo  ooll  ee|
0120  6c 6c 2c 2c 77 77 72 72  64 64 68 68 6c 6c 6f 6f  |ll,,wwrrddhhlloo|
0130  20 20 6f 6f 6c 6c 0a 0a  65 65 6c 6c 2c 2c 77 77  |  ooll  eell,,ww|
0140  72 72 64 64 68 68 6c 6c  6f 6f 20 20 6f 6f 6c 6c  |rrddhhlloo  ooll|
0150  0a 0a 65 65 6c 6c 2c 2c  77 77 72 72 64 64 68 68  |  eell,,wwrrddhh|
0160  6c 6c 6f 6f 20 20 6f 6f  6c 6c 0a 0a 65 65 6c 6c  |lloo  ooll  eell|
0170  2c 2c 77 77 72 72 64 64  68 68 6c 6c 6f 6f 20 20  |,,wwrrddhhlloo  |
0180  6f 6f 6c 6c 0a 0a 65 65  6c 6c 2c 2c 77 77 72 72  |ooll  eell,,wwrr|
0190  64 64 68 68 6c 6c 6f 6f  20 20 6f 6f 6c 6c 0a 0a  |ddhhlloo  ooll  |
01a0  65 65 6c 6c 2c 2c 77 77  72 72 64 64 68 68 6c 6c  |eell,,wwrrddhhll|
01b0  6f 6f 20 20 6f 6f 6c 6c  0a 0a 65 65 6c 6c 2c 2c  |oo  ooll  eell,,|
01c0  77 77 72 72 64 64 68 68  6c 6c 6f 6f 20 20 6f 6f  |wwrrddhhlloo  oo|
01d0  6c 6c 0a 0a 65 65 6c 6c  2c 2c 77 77 72 72 64 64  |ll  eell,,wwrrdd|
01e0  68 68 6c 6c 6f 6f 20 20  6f 6f 6c 6c 0a 0a 65 65  |hhlloo  ooll  ee|
01f0  6c 6c 2c 2c 77 77 72 72  64 64 68 68 6c 6c 6f 6f  |ll,,wwrrddhhlloo|


avrdude done.  Thank you.

@mcuee
Copy link
Collaborator

mcuee commented Oct 7, 2022

@MCUdude and @mcuee Thanks for testing! Good find re optiboot: I thought about this and now realise that optiboot will never be able to treat eeprom with page size of 1 (no matter how hard we try) owing to the fact that the stk500v1 implementation sends a word address for eeprom (we had a long discussion re this); this is why you see only every other byte written. That is unrelated to this PR and cannot be helped now that word addresses were (erronously) used for a decade in this vvv old STK500v1 protocol implementation.

Thanks for the explanations. Yes it is clear to me why this PR does not work with the optiboot.

@stefanrueger
Copy link
Collaborator Author

@MCUdude Thanks for testing, yes writing EEPROM with the optiboot bootloader and a page_size of 1 doesn't work as optiboot wants, and avrdude -c arduino sends word eeprom addresses. That's a problem outside this PR. Just imagine both had used byte addresses and all would be good.

@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

@mcuee Your questions:

  1. Equivalence for avr_read() and avr_write(): The conditions in the git mainline are equivalent, save for one more "sanity check" of checking that the memory size is a multiple of the page size (true for all parts).
  2. Correct, no memory beyond eeprom or flash-type will ever be considered paged memory (so as previously)

@stefanrueger
So the "sanity check" is not necessary since the cache size is a multiple of the page size. Right?
Take note that user can write flash size less than the page size (say 4 bytes). The user can even write single byte of flash (even though there is argument whether this should even happen in real world or not). But I think with your improved implemenation (already in git main), the cache size will be at least expanded to the page size.

@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

Other bootloaders may benefit from being called with paged routines; good to know that wiring can manage without this PR (but then it is large, so can afford to implement both byte-wise read/write and paged read/write, most bootloaders only implement paged r/w!); I know that my (soon to be included) urcock bootloader will be able to work with this PR but that's not the point.

@stefanrueger
I am thinking that we need to have some improvement in describing the capability of the bootloaders, to see whether they support byte-wise read/write and paged read/write, for Flash or for EEPROM. And maybe other things. Some bootloaders do not even support EEPROM (eg: Arduino's optiboot).

It really is about when to call paged r/w routines. I realise that there is mem->paged (set in avrdude.conf) but this is not set for PDI and UPDI parts nor is it ever set for eeprom (except accidentally for ATtiny43U) so this must be used for something else...

Or shall we just extend avrdude.conf to include such information?

@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

@MCUdude Thanks for testing, yes writing EEPROM with the optiboot bootloader and a page_size of 1 doesn't work as optiboot wants, and avrdude -c arduino sends word eeprom addresses. That's a problem outside this PR. Just imagine both had used byte addresses and all would be good.

@stefanrueger
Just wondering if we can differentiate the terminal mode from normal mode and carry out special treatment in this case as this old stk500v1 bootloader implementation is widely used (eg: by Arduino).

@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

@stefanrueger

I was worried about whether this PR would cause regressions for optiboot. It seems to me there is no regression.
Tested using optiboot big hex file with proper EEPROM support for ATmega328P.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom
>>> dump eeprom
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> write eeprom 0 0xff 0xff 0xff 0xff
>>> write eeprom 0 0xff 0xff 0xff 0xff
avrdude> flush
>>> flush
avrdude_git.exe: synching cache to device... avrdude>
avrdude> quit
>>> quit
PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x10
>>> dump eeprom 0 0x10
0000  ff ff ff ff 71 75 69 63  6b 20 62 72 6f 77 6e 20  |....quick brown |

avrdude> quit
>>> quit

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1 -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x10
>>> dump eeprom 0 0x10
0000  ff ff ff ff 71 75 69 63  6b 20 62 72 6f 77 6e 20  |....quick brown |

avrdude> write eeprom 0x04 0xff 0xff 0xff 0xff
>>> write eeprom 0x04 0xff 0xff 0xff 0xff
avrdude> flush
>>> flush
avrdude_pr1110v1.exe: synching cache to device... avrdude_pr1110v1.exe: stk500_recv(): programmer is not responding
avrdude> quit
>>> quit

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1 -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x10
>>> dump eeprom 0 0x10
0000  ff ff ff ff ff ff ff ff  6b 20 62 72 6f 77 6e 20  |........k brown |

avrdude> quit
>>> quit

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x10
>>> dump eeprom 0 0x10
0000  ff ff ff ff ff ff ff ff  6b 20 62 72 6f 77 6e 20  |........k brown |

avrdude> quit
>>> quit

@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

@stefanrueger

I was worried about whether this PR would cause regressions for optiboot. It seems to me there is no regression. Tested using optiboot big hex file with proper EEPROM support for ATmega328P.

Hmm, I do see a potential problem. Please take note the extra warning stk500_recv(): programmer is not responding from this PR, which does not seem to happen with git main. I am not so sure why this happends.

I've tried multiple times and the result is the same.

Edit: please ignore this one. It seems to be random and I think this is not the problem of this PR.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x20
>>> dump eeprom 0 0x20
0000  ff ff ff ff ff ff ff ff  6b 20 62 72 6f 77 6e 20  |........k brown |
0010  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|

avrdude> quit
>>> quit

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1 -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x20
>>> dump eeprom 0 0x20
avrdude_pr1110v1.exe: stk500_recv(): programmer is not responding
0000  ff ff ff ff ff ff ff ff  6b 20 62 72 6f 77 6e 20  |........k brown |
0010  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|

avrdude> quit
>>> quit


@mcuee
Copy link
Collaborator

mcuee commented Oct 8, 2022

@stefanrueger
A minor thing, I think we need to add a \n to the following message.

avrdude_message(MSG_INFO, "%s: synching cache to device... ", progname);

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1 -p m328p -c arduino -P COM5 -b 115200 -qq -t
avrdude> dump eeprom 0 0x20
>>> dump eeprom 0 0x20
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|

avrdude> write eeprom 0 0xff
>>> write eeprom 0 0xff
avrdude> flush
>>> flush
avrdude_pr1110v1.exe: synching cache to device... avrdude>  
(Note: extra return after the message is better -- I have to manually hit a return 
here myself to get the prompt in the next line) 
avrdude> dump eeprom 0 0x20
>>> dump eeprom 0 0x20
0000  ff 68 65 20 71 75 69 63  6b 20 62 72 6f 77 6e 20  |.he quick brown |
0010  66 6f 78 20 6a 75 6d 70  73 20 6f 76 65 72 20 74  |fox jumps over t|

avrdude> quit
>>> quit

@mcuee
Copy link
Collaborator

mcuee commented Oct 10, 2022

Just figured out how to burn the Optiboot-DX bootloader from DXCore for my AVR128DB48 Curiosity Nano board (need to use UART3). Results seem to be the same. It is better than git main but still both writing and reading will fail.

So all in all, it does not work with optiboot as of now.

@stefanrueger and @MCUdude

If we can differentiate with the different optiboot bootlaoders and use a_div = 1 for these bootloaders, then optiboot can work. I think we can use a_div = 2 for classic AVR and a_div = 1 for UPDI parts. Not so sure about PDI part.

With current code, the optiboot_dx will fail.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1 -c arduino -P COM6 -b 115200 -p avr128db48 
-qq -U eeprom:w:.\hex2\entest.eep && echo OK
avrdude_pr1110v1.exe: verification error, first mismatch at byte 0x0001
                      0x54 != 0x68
avrdude_pr1110v1.exe: verification error; content mismatch

But with a_div = 1, it will work again.

PS C:\work\avr\avrdude_test\avrdude_sr> git diff
diff --git a/src/stk500.c b/src/stk500.c
index 8966747..43e5111 100644
--- a/src/stk500.c
+++ b/src/stk500.c
@@ -793,7 +793,7 @@ static int stk500_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
      * 2000s. Since optiboot, arduino as ISP and others assume a_div = 2,
      * better use that. See https://github.com/avrdudes/avrdude/issues/967
      */
-    a_div = 2;
+    a_div = 1;
   } else {
     return -2;
   }
@@ -888,7 +888,7 @@ static int stk500_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
      * 2000s. Since optiboot, arduino as ISP and others assume a_div = 2,
      * better use that. See https://github.com/avrdudes/avrdude/issues/967
      */
-    a_div = 2;
+    a_div = 1;
   } else {
     return -2;
   }

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1mod -c arduino -P COM6 -b 115200 
-p avr128db48 -qq -U eeprom:w:.\hex2\entest.eep && echo OK
OK

Edit: sorry but I can not reproduce the good write test results any more. It writes nothing now.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1110v1mod -c arduino -P COM6 -b 115200 
-p avr128db48 -qq -U eeprom:w:.\hex2\entest.eep:i && echo OK
avrdude_pr1110v1mod.exe: verification error, first mismatch at byte 0x0000
                         0xaa != 0x54
avrdude_pr1110v1mod.exe: verification error; content mismatch

@stefanrueger
Copy link
Collaborator Author

Thanks all round for testing. Given the potential for regressions that has been uncovered and the limited benefit that this PR shows, I now think it is better to close this PR and the issue it was supposed to solve.

@stefanrueger stefanrueger deleted the pm_spm_eeprom branch October 11, 2022 15:04
@mcuee
Copy link
Collaborator

mcuee commented Oct 12, 2022

Thanks all round for testing. Given the potential for regressions that has been uncovered and the limited benefit that this PR shows, I now think it is better to close this PR and the issue it was supposed to solve.

@stefanrueger
I agree to close this PR as it has issues. However, I am not so sure why you want to close #970 as the issue is still there.

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

Successfully merging this pull request may close these issues.

Cannot access EEPROM on some bootloader/part combinations
3 participants