PICKit 2 ATmega2560 Flash Reading Problem

[mcuee]

With the help from @stefanrueger, now I am testing more programmers with the nice test hex file from him. After testing with FT2232H and FT232R, now I am testing with PICkit 2 and I found the same problem as well.

Programming the ATmega2560 has no issues but reading got problems at 0x21000.

• https://github.com/avrdudes/avrdude/issues/995#issuecomment-1159588797

  I expect most programmers to be able to read/write/verify the first 128k of an ATmega256 correctly as the load extended address is only needed above 128k (0x20000). However, as soon as you start to write files that cross that 0x20000 boundary, any programmer that is unable to handle 24-bit word addresses will get into trouble one way or the other.

• https://github.com/avrdudes/avrdude/pull/998#issuecomment-1160804556

C:\work\avr\binary\avrdude-7.0_bin64> .\avrdude -qqp m2560 -c pickit2 
-U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude.exe: verification error, first mismatch at byte 0x21000
             0xff != 0x48
avrdude.exe: verification error; content mismatch
C:\work\avr\binary\avrdude-7.0_bin64> .\avrdude -qqp m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude.exe: verification error, first mismatch at byte 0x21000
             0xff != 0x48
avrdude.exe: verification error; content mismatch
C:\work\avr\binary\avrdude-7.0_bin64> .\avrdude -qqp m2560 -c usbasp 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

[mcuee]

Write codes:

https://github.com/avrdudes/avrdude/blob/main/src/pickit2.c#L592

    // use the "load extended address" command, if available
    lext = mem->op[AVR_OP_LOAD_EXT_ADDR];
    if (lext != NULL)
    {
        avr_set_bits(lext, cmd);
        avr_set_addr(lext, cmd, addr);
    }

Read codes are slightly different. https://github.com/avrdudes/avrdude/blob/main/src/pickit2.c#L487

    OPCODE *readop = 0, *lext = mem->op[AVR_OP_LOAD_EXT_ADDR];
    uint8_t data = 0, cmd[SPI_MAX_CHUNK], res[SPI_MAX_CHUNK];
    unsigned int addr_base;
    unsigned int max_addr = addr + n_bytes;

    pgm->pgm_led(pgm, ON);

[stefanrueger]

Good find, @mcuee!

Your hunch is right: I predict if you replace the following two lines https://github.com/avrdudes/avrdude/blob/64ee4858fd5e4a2ce65cf3c280274674c4320de5/src/pickit2.c#L496-L497

with

if(lext)

the problem will go away, but reads will be slower. (The code incorrectly assumes linear reads without holes.)

You can then move the if(lext) { ... } block before the for() loop, and reads will be vvv close to previous speed. This works b/c AVRDUDE paged reads are only called in AVRDUDE for actual memory pages (page_size === mem->page_size == n_bytes) where addr is at memory page boundaries.

@mcuee Would be fab if you were able to test the suggested changes and, once confirmed working, create a PR.

[stefanrueger]

Given the problems of paged access functions in quite a few programmers, I plan to review all of them with a particular emphasis on checking ATmega2560 support. Makes sense to centralise a cache for the load extended address high byte of the 24-bit address as this allows speeding up the byte-wise access in avr.c. See also https://github.com/avrdudes/avrdude/issues/995#issuecomment-1174454683

Only problem: How to test? I only have an old usbtiny programmer, so cannot test (most of)

• avr910.c
• avrftdi.c
• avrftdi_tpi.c
• buspirate.c
• butterfly.c
• flip1.c
• flip2.c
• ft245r.c
• jtag3.c
• jtagmkI.c
• jtagmkII.c
• micronucleus.c
• pickit2.c
• serialupdi.c
• stk500.c
• stk500v2.c
• teensy.c
• usbasp.c
• usbtiny.c

[mcuee]

Only problem: How to test? I only have an old usbtiny programmer, so cannot test (most of)

I should be able to test quite some of them since I have the following programmer and ATmega2560 based Arduino Mega2560 clones.

1) avrftdi -- FT2232H based JTAGkey2 2) ft245r -- FT232R based programmer (arduino-ft232r) 3) PICKit 2 4) usbasp -- Atmega8A or ATmega88 based 5) usbtinyisp 6) JTAGmkii -- AVR Dragon

In another month I should have two more programmers available. 1) stk500v2 (not so sure about v1): stk500v2 compatible programmer and AVRISP mkII compatible programmer.

I have other tools which may not be relevant in this case but may be useful in other tests. 1) Microchip AVR128DB48 CURIOSITY NANO 2) Microchip ATMEGA328PB XPLAINED MINI 3) Arduino Nano Every official board with ATmega4809 4) Arduino Leonardo official board with ATmega32U4 5) Arduino Nano ATmega4808 clone 6) Quite a number of Arduino Uno and Nano clones 7) One ATmega32A based board running Mighty Core. 8) Quite a few ATtiny88 based boards which can run micronucleus bootloader

[mcuee]

Your hunch is right: I predict if you replace the following two lines

https://github.com/avrdudes/avrdude/blob/64ee4858fd5e4a2ce65cf3c280274674c4320de5/src/pickit2.c#L496-L497

with

if(lext)

the problem will go away, but reads will be slower. (The code incorrectly assumes linear reads without holes.)

Somehow it does not work and failed at 0x11000.

/c/work/avr/avrdude_test/avrdude
$ git diff
diff --git a/src/pickit2.c b/src/pickit2.c
index 9e4be94..a1dd140 100644
--- a/src/pickit2.c
+++ b/src/pickit2.c
@@ -493,9 +493,10 @@ static int  pickit2_paged_load(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem,

     for (addr_base = addr; addr_base < max_addr; )
     {
-        if ((addr_base == 0 || (addr_base % /*ext_address_boundary*/ 65536) == 0)
-                && lext != NULL)
-        {
+//        if ((addr_base == 0 || (addr_base % /*ext_address_boundary*/ 65536) == 0)
+//                && lext != NULL)
+        if(lext)
+               {
             memset(cmd, 0, sizeof(cmd));

             avr_set_bits(lext, cmd);

Test results:

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004.exe -qqp m2560 -c pickit2 
-U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude_issue1004.exe: verification error, first mismatch at byte 0x11000
                       0x48 != 0x42
avrdude_issue1004.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004.exe -qqp m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude_issue1004.exe: verification error, first mismatch at byte 0x11000
                       0x48 != 0x42
avrdude_issue1004.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004.exe -qqp m2560 -c usbasp 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

Same problem if I use usbasp to program the hex file and then use PICKit 2 to verify.

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004.exe -qqp m2560 -c usbasp
 -U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004.exe -qqp m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude_issue1004.exe: verification error, first mismatch at byte 0x11000
                       0x48 != 0x42
avrdude_issue1004.exe: verification error; content mismatch

[stefanrueger]

Somehow it does not work and failed at 0x11000.

Aha, incrementing high byte too early: pickit2.c also forgot to use a word address for the load extended command. Changing

https://github.com/avrdudes/avrdude/blob/64ee4858fd5e4a2ce65cf3c280274674c4320de5/src/pickit2.c#L502

to avr_set_addr(lext, cmd, addr_base/2); in addition to previous changes should do the trick.

Fingers crossed there are not more problems!

[MCUdude]

I actually own two PICkit2 programmers; one clone and one genuine. I've never used them with Avrdude though. Can they be used out of the box, or do they need custom firmware running on them to work with AVRs?

[mcuee]

@MCUdude

; one clone and one genuine. I've never used them with Avrdude though. Can they be used out of the box, or do they need custom firmware running on them to work with AVRs?

It should work out of the box. I have two genuine PICkit 2 (I bought one myself and the other one was courtesy of Microchip many years ago).

[mcuee]

Reference: I myself do not use the VDD pin from PICkit 2 but rather power up the AVR separately. https://www.instructables.com/Arduino-Pickit2-As-Programmer/

AVR - PICKit2 (pin):

RST - VPP/MCLR (1) VDD - VDD Target (2) -- optional if AVR self powered GND - GND (3) MISO - PGD (4) SCLK - PDC (5) MOSI - AUX (6)

[MCUdude]

Thanks @mcuee!

I got a few more things to figure out though (macOS)...

$ ./avrdude -cpickit2 -patmega328p
Could not claim interface. Error code -13, Permission denied
You may need to run avrdude as root or set up correct usb port permissions.
pickit2_write_report failed (ec -2). No such file or directory
avrdude: initialization failed, rc=-1
         Double check connections and try again, or use -F to override
         this check.

avrdude done.  Thank you.

[mcuee]

@MCUdude Ah, you are hitting into issue #883. You may have to wait for that to happen first for it to work under macOS.

• https://github.com/avrdudes/avrdude/issues/883

Moving to HIDAPI is recommended. Ref: https://github.com/libusb/libusb/wiki/FAQ#does-libusb-support-usb-hid-devices

[mcuee]

Somehow it does not work and failed at 0x11000.

Aha, incrementing high byte too early: pickit2.c also forgot to use a word address for the load extended command. Changing

https://github.com/avrdudes/avrdude/blob/64ee4858fd5e4a2ce65cf3c280274674c4320de5/src/pickit2.c#L502

to avr_set_addr(lext, cmd, addr_base/2); in addition to previous changes should do the trick.

Fingers crossed there are not more problems!

@stefanrueger Great. Now everything is okay.

/c/work/avr/avrdude_test/avrdude
$ git diff
diff --git a/src/pickit2.c b/src/pickit2.c
index 9e4be94..8309a28 100644
--- a/src/pickit2.c
+++ b/src/pickit2.c
@@ -493,13 +493,12 @@ static int  pickit2_paged_load(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem,

     for (addr_base = addr; addr_base < max_addr; )
     {
-        if ((addr_base == 0 || (addr_base % /*ext_address_boundary*/ 65536) == 0)
-                && lext != NULL)
-        {
+        if(lext)
+               {
             memset(cmd, 0, sizeof(cmd));

             avr_set_bits(lext, cmd);
-            avr_set_addr(lext, cmd, addr_base);
+            avr_set_addr(lext, cmd, addr_base/2);
             pgm->cmd(pgm, cmd, res);
         }

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v1.exe -qqp m2560 -c pickit2 
-U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v1.exe -qqp m2560 -c pickit2
 -U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v1.exe -qqp m2560 -c usbasp 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

[stefanrueger]

Great. Now everything is okay.

@mcuee Brill! One more test, if you can: please move the if(lext) { ... } block up a few lines just outside the for loop. This should still work but be considerably faster. Thanks

[mcuee]

Brill! One more test, if you can: please move the if(lext) { ... } block up a few lines just outside the for loop. This should still work but be considerably faster. Thanks

@stefanrueger I think there is a problem here, since addr_base has not been initialized.

/c/work/avr/avrdude_test/avrdude
$ git diff
diff --git a/src/pickit2.c b/src/pickit2.c
index 9e4be94..2216da6 100644
--- a/src/pickit2.c
+++ b/src/pickit2.c
@@ -491,17 +491,16 @@ static int  pickit2_paged_load(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem,

     pgm->pgm_led(pgm, ON);

+    if(lext)
+       {
+        memset(cmd, 0, sizeof(cmd));
+        avr_set_bits(lext, cmd);
+        avr_set_addr(lext, cmd, addr_base/2);
+        pgm->cmd(pgm, cmd, res);
+    }
+
     for (addr_base = addr; addr_base < max_addr; )
     {
-        if ((addr_base == 0 || (addr_base % /*ext_address_boundary*/ 65536) == 0)
-                && lext != NULL)
-        {
-            memset(cmd, 0, sizeof(cmd));
-
-            avr_set_bits(lext, cmd);
-            avr_set_addr(lext, cmd, addr_base);
-            pgm->cmd(pgm, cmd, res);
-        }

         // bytes to send in the next packet -- not necessary as pickit2_spi() handles breaking up
         // the data into packets -- but we need to keep transfers frequent so that we can update the

$ cmake -G"MSYS Makefiles" -D CMAKE_BUILD_TYPE=RelWithDebInfo -B build_mingw64
-- Configuration summary:
-- ----------------------
-- DO HAVE    libelf
-- DO HAVE    libusb
-- DO HAVE    libusb_1_0
-- DO HAVE    libhidapi
-- DON'T HAVE libftdi
-- DO HAVE    libftdi1
-- DISABLED   doc
-- DISABLED   parport
-- DISABLED   linuxgpio
-- DISABLED   linuxspi
-- ----------------------
-- Configuring done
-- Generating done
-- Build files have been written to: C:/work/avr/avrdude_test/avrdude/build_mingw64

$ cmake --build build_mingw64
Consolidate compiler generated dependencies of target libavrdude
[  1%] Building C object src/CMakeFiles/libavrdude.dir/pickit2.c.obj
C:/work/avr/avrdude_test/avrdude/src/pickit2.c: In function 'pickit2_paged_load':
C:/work/avr/avrdude_test/avrdude/src/pickit2.c:498:42: warning: 'addr_base' may be used uninitialized [-Wmaybe-uninitialized]
  498 |         avr_set_addr(lext, cmd, addr_base/2);
      |                                 ~~~~~~~~~^~
[  3%] Linking C static library libavrdude.a
[ 91%] Built target libavrdude
Scanning dependencies of target avrdude
Consolidate compiler generated dependencies of target avrdude
[ 93%] Linking C executable avrdude.exe
[100%] Built target avrdude

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v2.exe -qqp m2560 -c pickit2 
-U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude_issue1004v2.exe: verification error, first mismatch at byte 0x21000
                         0xff != 0x48
avrdude_issue1004v2.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v2.exe -qqp m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
avrdude_issue1004v2.exe: verification error, first mismatch at byte 0x21000
                         0xff != 0x48
avrdude_issue1004v2.exe: verification error; content mismatch

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v2.exe -qqp m2560 -c usbasp 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

[stefanrueger]

Great catch, addr_base/2 needs replacing with addr/2 when moved out of the for loop.

[mcuee]

Great catch, addr_base/2 needs replacing with addr/2 when moved out of the for loop.

Great. Now everything works fine. Verification is slightly faster than before.

$ git diff
diff --git a/src/pickit2.c b/src/pickit2.c
index 9e4be94..de4fc0a 100644
--- a/src/pickit2.c
+++ b/src/pickit2.c
@@ -491,17 +491,16 @@ static int  pickit2_paged_load(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem,

     pgm->pgm_led(pgm, ON);

+    if(lext)
+       {
+        memset(cmd, 0, sizeof(cmd));
+        avr_set_bits(lext, cmd);
+        avr_set_addr(lext, cmd, addr/2);
+        pgm->cmd(pgm, cmd, res);
+    }
+
     for (addr_base = addr; addr_base < max_addr; )
     {
-        if ((addr_base == 0 || (addr_base % /*ext_address_boundary*/ 65536) == 0)
-                && lext != NULL)
-        {
-            memset(cmd, 0, sizeof(cmd));
-
-            avr_set_bits(lext, cmd);
-            avr_set_addr(lext, cmd, addr_base);
-            pgm->cmd(pgm, cmd, res);
-        }

         // bytes to send in the next packet -- not necessary as pickit2_spi() handles breaking up
         // the data into packets -- but we need to keep transfers frequent so that we can update the

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v3.exe -qqp m2560 -c pickit2 
-U flash:w:.\blink-mega2560_lext-test.hex:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v3.exe -qqp m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i && echo OK
OK

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v3.exe -p m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i

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

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

avrdude_issue1004v3.exe: Device signature = 0x1e9801 (probably m2560)
avrdude_issue1004v3.exe: verifying flash memory against .\blink-mega2560_lext-test.hex:

Reading | ################################################## | 100% 0.74s

avrdude_issue1004v3.exe: 200814 bytes of flash verified

avrdude_issue1004v3.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v1.exe -p m2560 -c pickit2 
-U flash:v:.\blink-mega2560_lext-test.hex:i

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

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

avrdude_issue1004v1.exe: Device signature = 0x1e9801 (probably m2560)
avrdude_issue1004v1.exe: verifying flash memory against .\blink-mega2560_lext-test.hex:

Reading | ################################################## | 100% 0.87s

avrdude_issue1004v1.exe: 200814 bytes of flash verified

avrdude_issue1004v1.exe done.  Thank you.

[mcuee]

Reading back the full flash size is quite a bit faster as well: 180.45 seconds vs 213.79 seconds.

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v3.exe -p m2560 -c pickit2 
-U flash:r:.\readback_m2560_v3.hex:i

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

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

avrdude_issue1004v3.exe: Device signature = 0x1e9801 (probably m2560)
avrdude_issue1004v3.exe: reading flash memory:

Reading | ################################################## | 100% 180.45s

avrdude_issue1004v3.exe: writing output file ".\readback_m2560_v3.hex"

avrdude_issue1004v3.exe done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude-7.0_bin64> .\avrdude_issue1004v1.exe -p m2560 -c pickit2 
-U flash:r:.\readback_m2560_v1.hex:i

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

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

avrdude_issue1004v1.exe: Device signature = 0x1e9801 (probably m2560)
avrdude_issue1004v1.exe: reading flash memory:

Reading | ################################################## | 100% 213.79s

avrdude_issue1004v1.exe: writing output file ".\readback_m2560_v1.hex"

avrdude_issue1004v1.exe done.  Thank you.

[mcuee]

@stefanrueger Now that everything is good, please help to create a PR since the fix comes from you.

[stefanrueger]

Fixed with PR #1023