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

-U writes sometimes more bytes to flash/memory than input file has #1050

Closed
stefanrueger opened this issue Aug 1, 2022 · 10 comments · Fixed by #1188
Closed

-U writes sometimes more bytes to flash/memory than input file has #1050

stefanrueger opened this issue Aug 1, 2022 · 10 comments · Fixed by #1188
Assignees
Labels
bug Something isn't working

Comments

@stefanrueger
Copy link
Collaborator

Problem. -U writes sometimes more bytes to flash/memory than input file has. This happens when

  • The programmer provides a paged write routine and
  • The the input file leaves memory pages partially filled and
  • The memory does not look like an NOR-memory, which is true for
    • EEPROM in general
    • For XMEGA flash (because AVRDUDE uses a pgm->page_erase() before pgm->page_write())
    • For is_bootloader programmers that carry out SPM erase before a page write

Solution. The culprit is avr_write() that uses paged write routines when possible and assumes all memories look like NOR-memory (for which writing 0xff is a NOP). For now, I suggest always reading back the unset bytes in partially filled memory pages before the paged write. This is not necessary for flash that looks like NOR-memory, but there is (currently) no way of knowing which programmer/part combination makes flash look like NOR-memory or not: Sometimes bootloaders pretend to be following a certain protocol, but don't (think optiboot and -c arduino or Trinket/Gemma that pose as -c usbtiny). The small delay for read-back of partially filled memory pages is a price worth paying to ensure AVRDUDE does not alter memory outside the input file.

Example. Use your own favourite programmer instead of ... and try below. Notice the echo -n H input should write one byte, but four bytes are written as evidenced by the three 0xff at addresses 1..3.

$ yes 'hello, world!!!' | head -2 | avrdude -qqp m328p -c ... -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | avrdude -qqp m328p -c ... -t
avrdude> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

$ echo -n H | avrdude -qp m328p -c ... -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file "<stdin>"
avrdude: writing eeprom (1 bytes):
avrdude: 1 bytes of eeprom written
avrdude: verifying eeprom memory against -:
avrdude: 0 bytes of eeprom verified

avrdude done.  Thank you.

$ echo d ee 0 32 | avrdude -qqp m328p -c ... -t
avrdude> d ee 0 32
0000  48 ff ff ff 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |H...o, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
@mcuee mcuee added the bug Something isn't working label Aug 2, 2022
@mcuee
Copy link
Collaborator

mcuee commented Aug 2, 2022

Yes, this is a problem. Tested with ATmega2560 and usbasp (with PR #1051 to address the stdin verification issue).

MINGW64 /c/work/avr/avrdude_test/avrdude_sr/build_mingw64_nt-10.0-19043/src
$ yes 'hello, world!!!' | head -2 | ./avrdude -qqp m2560 -c usbasp -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | ./avrdude -qqp m2560 -c usbasp -t
avrdude> >>> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

$ echo -n H | ./avrdude -qp m2560 -c usbasp -U eeprom:w:-:r

avrdude.exe: AVR device initialized and ready to accept instructions
avrdude.exe: Device signature = 0x1e9801 (probably m2560)
avrdude.exe: reading input file <stdin> for eeprom
             with 1 byte in 1 section within [0, 0]
             using 1 page and 7 pad bytes
avrdude.exe: writing 1 byte eeprom ...
avrdude.exe: 1 byte of eeprom written
avrdude.exe: verifying eeprom memory against <stdin>
avrdude.exe: 1 byte of eeprom verified

avrdude.exe done.  Thank you.

$ echo d ee 0 32 | ./avrdude -qqp m2560 -c usbasp -t
avrdude> >>> d ee 0 32
0000  48 ff ff ff ff ff ff ff  6f 72 6c 64 21 21 21 0a  |H.......orld!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

avrdude>

@mcuee
Copy link
Collaborator

mcuee commented Aug 2, 2022

The small delay for read-back of partially filled memory pages is a price worth paying to ensure AVRDUDE does not alter memory outside the input file.

I agree.

@mcuee
Copy link
Collaborator

mcuee commented Aug 28, 2022

@stefanrueger
For Flash, this only applies to the terminal mode, right? Or it also applies to the normal mode when -D is specified? The normal -U command without -D will anyway eraze the Flash.

If it is just applicable for terminal mode, then the penalty is small. Same probably for the case of -D -U since you only need to care for the last page to write.

Since you have the other idea of implementing is_bootloader, then probably this becomes a non-issue for flash. Is that correct understanding?
.

@mcuee
Copy link
Collaborator

mcuee commented Oct 16, 2022

@stefanrueger
I think this has been fixed in #1106, right?

Sorry not yet. This issue is still there in CLI mode.

$ yes 'hello, world!!!' | head -2 | ./avrdude -qqp m328p -c usbasp -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | ./avrdude -qqp m328p -c usbasp -t
avrdude> >>> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

$ echo -n H | ./avrdude -qp m328p -c usbasp -U eeprom:w:-:r

avrdude.exe: AVR device initialized and ready to accept instructions
avrdude.exe: Device signature = 0x1e950f (probably m328p)
avrdude.exe: reading input file <stdin> for eeprom
avrdude.exe: writing 1 byte eeprom ...
avrdude.exe: 1 byte of eeprom written
avrdude.exe: verifying eeprom memory against <stdin>
avrdude.exe: 1 byte of eeprom verified

avrdude.exe done.  Thank you.

$ echo d ee 0 32 | ./avrdude -qqp m328p -c usbasp -t
avrdude> >>> d ee 0 32
0000  48 ff ff ff 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |H...o, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

@mcuee
Copy link
Collaborator

mcuee commented Oct 16, 2022

Terminal mode is okay.

$ yes 'hello, world!!!' | head -2 | ./avrdude -qqp m328p -c usbasp -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | ./avrdude -qqp m328p -c usbasp -t
avrdude> >>> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

$ echo "w ee 0 'H'" | ./avrdude -qqp m328p -c usbasp -t
avrdude> >>> w ee 0 'H'
avrdude> avrdude.exe: synching cache to device... done

$ echo d ee 0 32 | ./avrdude -qqp m328p -c usbasp -t
avrdude> >>> d ee 0 32
0000  48 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |Hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

@stefanrueger
Copy link
Collaborator Author

stefanrueger commented Oct 17, 2022

Yes, this Issue is only for -U (the regular write) and not the terminal (-t).

It normally affects EEPROM (which might be saved on a chip erase when the fuses are set accordingly), but also flash, eg, with -c arduino and optiboot that simply has decided to not implement chip erase (and where it does not matter whether you specify -e or -D) or other bootloaders that implement chip erase but where the user specifies -D. Ever noticed that every sketch that is uploaded with arduino/optiboot will be padded with 0xff to the end of the last page? If you upload a 1026 byte long sketch you get 126 extra 0xff bytes on a m328p (page size 128). Also if the input file had holes these too would be filled with 0xff rather than left alone. This should also affect xmega flash programming (I never tried, but the logic as I read it in the avrdude source indicates that -U also fills holes with 0xff and pads unfinished pages with 0xff when writing flash).

@mcuee
Copy link
Collaborator

mcuee commented Oct 17, 2022

Yes, this Issue is only for -U (the regular write) and not the terminal (-t).

It normally affects EEPROM (which might be saved on a chip erase when the fuses are set accordingly), but also flash, eg, with -c arduino and optiboot that simply has decided to not implement chip erase (and where it does not matter whether you specify -e or -D) or other bootloaders that implement chip erase but where the user specifies -D. Ever noticed that every sketch that is uploaded with arduino/optiboot will be padded with 0xff to the end of the last page? If you upload a 1026 byte long sketch you get 126 extra 0xff bytes on a m328p (page size 128). Also if the input file had holes these too would be filled with 0xff rather than left alone. This should also affect xmega flash programming (I never tried, but the logic as I read it in the avrdude source indicates that -U also fills holes with 0xff and pads unfinished pages with 0xff when writing flash).

Indeed. Just tested it with an Arduino Uno clone. I can see that the four bytes 0xaa 0x55 0xaa 0x55 was overwritten. For the purpose of Arduino, this may not be a real issue for Flash though.

C:\work\avr\avrdude_test\avrdude_bin> echo "dump flash 0x3f0 0x10" | .\avrdude_git -c arduino -P COM6 -b 115200 
-p m328p -qq -t
avrdude> >>> dump flash 0x3f0 0x10
03f0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|

avrdude>
C:\work\avr\avrdude_test\avrdude_bin> echo "write flash 0x3f0 0xaa 0x55 0xaa 0x55" | .\avrdude_git -c arduino 
-P COM6 -b 115200 -qq -p m328p -t
avrdude> >>> write flash 0x3f0 0xaa 0x55 0xaa 0x55
avrdude> avrdude_git.exe: synching cache to device... done
C:\work\avr\avrdude_test\avrdude_bin> echo "dump flash 0x3f0 0x10" | .\avrdude_git -c arduino -P COM6 -b 115200 
-p m328p -qq -t
avrdude> >>> dump flash 0x3f0 0x10
03f0  aa 55 aa 55 ff ff ff ff  ff ff ff ff ff ff ff ff  |.U.U............|

avrdude>
C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_git -c arduino -P COM6 -b 115200 -D -U .\Blin_Uno.ino.hex 
-p m328p

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

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

avrdude_git.exe: Device signature = 0x1e950f (probably m328p)
avrdude_git.exe: reading input file .\Blin_Uno.ino.hex for flash
avrdude_git.exe: writing 924 bytes flash ...

Writing | ################################################## | 100% 0.22s

avrdude_git.exe: 924 bytes of flash written
avrdude_git.exe: verifying flash memory against .\Blin_Uno.ino.hex

Reading | ################################################## | 100% 0.16s

avrdude_git.exe: 924 bytes of flash verified

avrdude_git.exe done.  Thank you.

C:\work\avr\avrdude_test\avrdude_bin> echo "dump flash 0x300 0x100" | .\avrdude_git -c arduino -P COM6
 -b 115200 -p m328p -qq -t
avrdude> >>> dump flash 0x300 0x100
0300  80 91 b1 00 84 60 80 93  b1 00 80 91 b0 00 81 60  |.....`.........`|
0310  80 93 b0 00 80 91 7a 00  84 60 80 93 7a 00 80 91  |......z..`..z...|
0320  7a 00 82 60 80 93 7a 00  80 91 7a 00 81 60 80 93  |z..`..z...z..`..|
0330  7a 00 80 91 7a 00 80 68  80 93 7a 00 10 92 c1 00  |z...z..h..z.....|
0340  ed e9 f0 e0 24 91 e9 e8  f0 e0 84 91 88 23 99 f0  |....$........#..|
0350  90 e0 88 0f 99 1f fc 01  e8 59 ff 4f a5 91 b4 91  |.........Y.O....|
0360  fc 01 ee 58 ff 4f 85 91  94 91 8f b7 f8 94 ec 91  |...X.O..........|
0370  e2 2b ec 93 8f bf c0 e0  d0 e0 81 e0 0e 94 70 00  |.+............p.|
0380  0e 94 dd 00 80 e0 0e 94  70 00 0e 94 dd 00 20 97  |........p..... .|
0390  a1 f3 0e 94 00 00 f1 cf  f8 94 ff cf ff ff ff ff  |................|
03a0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
03b0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
03c0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
03d0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
03e0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
03f0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|

avrdude>

@mcuee
Copy link
Collaborator

mcuee commented Nov 24, 2022

Without PR #1188, using git main, the issue is there.

$  yes 'hello, world!!!' | head -2 | ./avrdude -qqp t88 -c usbasp -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | ./avrdude -qqp t88 -c usbasp -t
avrdude> d ee 0 32
>>> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

avrdude>
avrdude>

$ echo -n H | ./avrdude -qp t88 -c usbasp -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9311 (probably t88)
avrdude: reading input file <stdin> for eeprom
         with 1 byte in 1 section within [0, 0]
         using 1 page and 3 pad bytes
avrdude: writing 1 byte eeprom ...
avrdude: 1 byte of eeprom written
avrdude: verifying eeprom memory against <stdin>
avrdude: 1 byte of eeprom verified

avrdude done.  Thank you.

$ echo d ee 0 32 | ./avrdude -qqp t88 -c usbasp -t
avrdude> d ee 0 32
>>> d ee 0 32
0000  48 ff ff ff 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |H...o, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

avrdude>
avrdude>

@mcuee
Copy link
Collaborator

mcuee commented Nov 24, 2022

With PR #1188.

@stefanrueger
Somehow PR#1188 does not fix the issue.

$  yes 'hello, world!!!' | head -2 | ./avrdude_pr1188 -qqp t88 -c usbasp -U eeprom:w:-:r && echo OK
OK

$ echo d ee 0 32 | ./avrdude_pr1188 -qqp t88 -c usbasp -t
avrdude> d ee 0 32
>>> d ee 0 32
0000  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

avrdude>
avrdude>

$ echo -n H | ./avrdude_pr1188 -qp t88 -c usbasp -U eeprom:w:-:r

avrdude_pr1188: AVR device initialized and ready to accept instructions
avrdude_pr1188: device signature = 0x1e9311 (probably t88)
avrdude_pr1188: reading input file <stdin> for eeprom
                with 1 byte in 1 section within [0, 0]
                using 1 page and 3 pad bytes
avrdude_pr1188: writing 1 byte eeprom ...
avrdude_pr1188: 1 byte of eeprom written
avrdude_pr1188: verifying eeprom memory against <stdin>
avrdude_pr1188: 1 byte of eeprom verified

avrdude_pr1188 done.  Thank you.


$ echo d ee 0 32 | ./avrdude_pr1188 -qqp t88 -c usbasp -t
avrdude> d ee 0 32
>>> d ee 0 32
0000  48 ff ff ff 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |H...o, world!!! |
0010  68 65 6c 6c 6f 2c 20 77  6f 72 6c 64 21 21 21 0a  |hello, world!!! |

avrdude>
avrdude>

$ ./avrdude_pr1188
Usage: avrdude_pr1188 [options]
Options:
  -p <partno>                Specify AVR device
  -b <baudrate>              Override RS-232 baud rate
  -B <bitclock>              Specify bit clock period (us)
  -C <config-file>           Specify location of configuration file
  -c <programmer>            Specify programmer type
  -A                         Disable trailing-0xff removal from file and AVR read
  -D                         Disable auto erase for flash memory; implies -A
  -i <delay>                 ISP Clock Delay [in microseconds]
  -P <port>                  Specify connection port
  -F                         Override invalid signature or initialisation check
  -e                         Perform a chip erase
  -O                         Perform RC oscillator calibration (see AVR053)
  -U <memtype>:r|w|v:<filename>[:format]
                             Memory operation specification
                             Multiple -U options are allowed, each request
                             is performed in the order specified
  -n                         Do not write anything to the device
  -V                         Do not verify
  -t                         Enter terminal mode
  -E <exitspec>[,<exitspec>] List programmer exit specifications
  -x <extended_param>        Pass <extended_param> to programmer
  -v                         Verbose output; -v -v for more
  -q                         Quell progress output; -q -q for less
  -l logfile                 Use logfile rather than stderr for diagnostics
  -?                         Display this usage

avrdude version 7.0-20221122 (b925b51), URL: <https://github.com/avrdudes/avrdude>

@mcuee
Copy link
Collaborator

mcuee commented Nov 24, 2022

The updated version is okay now, tested with -c usbasp and -c arduino.

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

Successfully merging a pull request may close this issue.

2 participants