search

avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
759 stars 139 forks source link

Test script issues with Arduino Nano Every under macOS #1649

Closed MCUdude closed 10 months ago

MCUdude commented 10 months ago

While testing #1647, I stumbled across this issue. I'm getting a sed rename() error, and the eesave fuse bit is still set to 1, which means that the EEPROM is preserved, even though it shouldn't be.

$ ./test-avrdude -d 0 -p "-c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101" -v
Testing avrdude version 7.2-20230720
Prepare "-c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101" and press 'enter' or 'space' to continue. Press any other key to skip
sed: rename(): No such file or directory
❌   3.452 s: fuse access: clear, set and read eesave fuse bit (failed command below)
$ avrdude -qq -c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101 -l /tmp/test-avrdude.log.xcLt9q -T "config eesave=0; config eesave=1; config eesave"
config eesave=eex_preserved # 1
sed: rename(): No such file or directory
✅   3.431 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   3.551 s: chip erase
✅  10.358 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   4.256 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   3.523 s: eeprom check whether programmer can flip 0s to 1s
✅   3.541 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   3.649 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   4.398 s: chip erase and spot check flash is actually erased
❌   3.417 s: spot check eeprom is erased, too (failed command below)
$ avrdude -qq -c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101 -l /tmp/test-avrdude.log.xcLt9q -Ueeprom:v:./test_files/holes_eeprom_0xff_256B.hex
avrdude warning: verification mismatch
        device 0x50 != input 0xff at addr 0x0022 (error)
avrdude error: verification mismatch
✅   3.601 s: usersig -T/-U write/read random_data_64B.bin

One or more AVRDUDE "-c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101" tests failed. Do you want to retry this particular test? (y/n): n

$ avrdude -c nanoevery -p atmega4809 -r -P /dev/cu.usbmodem14101 -T "config eesave"
avrdude: touching serial port /dev/cu.usbmodem14101 at 1200 baud
avrdude: waiting for new port... using same port /dev/cu.usbmodem14101
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9651 (probably m4809)

avrdude: processing -T config eesave
config eesave=eex_preserved # 1

avrdude done.  Thank you.
mcuee commented 10 months ago

https://github.com/avrdudes/avrdude/issues/1649 discovered two issues. PR #1657 by @stefanrueger fixes the script error issue.

Hopefully @askn37 can help to fix the fuse writing issue in another PR.

mcuee commented 10 months ago

https://github.com/arduino/ArduinoCore-megaavr/blob/5e639ee40afa693354d3d056ba7fb795a8948c11/firmwares/MuxTO/MuxTO.ino#L199-L202

      case JTAG2::CMND_LEAVE_PROGMODE:
        JTAG2::leave_progmode();
        updi_mode_end = millis();
        break;

A delay is required before JTAG2::leave_progmode() to avoid FUSE write loss. Enough time for UPDI to read one byte of memory.

So this one will be a simple PR, right?

I think it may be good to have two PRs to address the two different issues.

mcuee commented 10 months ago

AVR Dragon and nanoevery alread have a recently inserted sleep in _close() of jtagmkII.c:

On NanoEvery, that delay bypasses the next check in the firmware. In other words, UPDI mode cannot be exited until 500ms has passed after passing through CMND_LEAVE_PROGMODE.

https://github.com/arduino/ArduinoCore-megaavr/blob/5e639ee40afa693354d3d056ba7fb795a8948c11/firmwares/MuxTO/MuxTO.ino#L168-L173

    // updi_mode cannot last more than 1 minute; in that case, break forcibly
    if ((updi_mode_end != 0 && (millis() - updi_mode_end) > 500) || ((millis() - updi_mode_start) > 60000)) {
      updi_mode = false;
      baudrate = -1;
      return;
    }

I am sorry but I do not quite understand this issue. Please help to elaborate. Thanks.

Or maybe @stefanrueger can chime in as well. Thanks.

mcuee commented 10 months ago

As far as using the test script is concerned, the problem only occurs in one place: writing FUSE (and disconnecting without verifying), so would it be enough to implement just one taint flag?

I think the idea is not just to deal with the current test script, but rather the underlying issues of avrdude when dealing with different jtag2updi implementations. But if the changes are too complicated, maybe we can deal with the easier one first for avrdude 7.3 release.

askn37 commented 10 months ago

A very simple implementation of the taint_write flag.

diff --git a/src/jtagmkII.c b/src/jtagmkII.c
index 0aecb7ce..7ad73c84 100644
--- a/src/jtagmkII.c
+++ b/src/jtagmkII.c
@@ -87,6 +87,9 @@ struct pdata
   /* Major firmware version (needed for Xmega programming) */
   unsigned int fwver;

+  /* Remember if the last execution was a memory write */
+  int taint_write;
+
 #define FLAGS32_INIT_SMC      1 // Part will undergo chip erase
 #define FLAGS32_WRITE         2 // At least one write operation specified
   // Couple of flag bits for AVR32 programming
@@ -142,6 +145,7 @@ static int jtagmkII_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const A
                                 unsigned int addr, unsigned int n_bytes);
 static unsigned char jtagmkII_mtype(const PROGRAMMER *pgm, const AVRPART *p, unsigned long addr);
 static unsigned int jtagmkII_memaddr(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr);
+static int jtagmkII_updi_term_keep_alive(const PROGRAMMER *pgm, const AVRPART *p_unused);

 // AVR32
 #define ERROR_SAB 0xFFFFFFFF
@@ -850,6 +854,7 @@ static int jtagmkII_chip_erase(const PROGRAMMER *pgm, const AVRPART *p) {
   if (!(p->prog_modes & (PM_PDI | PM_UPDI)))
       pgm->initialize(pgm, p);

+  PDATA(pgm)->taint_write = 1;
   return 0;
 }

@@ -1105,6 +1110,13 @@ static int jtagmkII_program_disable(const PROGRAMMER *pgm) {
   if (!PDATA(pgm)->prog_enabled)
     return 0;

+  // Delay early termination to ensure memory rewrite operations.
+  if (PDATA(pgm)->taint_write) {
+    pmsg_notice2("jtagmkII_program_disable(): Delay after writing: ");
+    jtagmkII_updi_term_keep_alive(pgm, /* unused */ (AVRPART*)NULL);
+    jtagmkII_updi_term_keep_alive(pgm, /* unused */ (AVRPART*)NULL);
+  }
+
   buf[0] = CMND_LEAVE_PROGMODE;
   pmsg_notice2("jtagmkII_program_disable(): "
     "Sending leave progmode command: ");
@@ -1128,6 +1140,7 @@ static int jtagmkII_program_disable(const PROGRAMMER *pgm) {
     return -1;
   }

+  PDATA(pgm)->taint_write  = 0;
   PDATA(pgm)->prog_enabled = 0;
   (void)jtagmkII_reset(pgm, 0x01);

@@ -2012,6 +2025,7 @@ static int jtagmkII_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const A
   free(cmd);
   serial_recv_timeout = otimeout;

+  PDATA(pgm)->taint_write = 1;
   return n_bytes;
 }

@@ -2102,6 +2116,7 @@ static int jtagmkII_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AV
   }
   serial_recv_timeout = otimeout;

+  PDATA(pgm)->taint_write = 0;
   return n_bytes;
 }

@@ -2290,6 +2305,7 @@ retry:
     *value = resp[1];

   free(resp);
+  PDATA(pgm)->taint_write = 0;
   return 0;

 fail:
@@ -2400,6 +2416,7 @@ retry:
   }

   free(resp);
+  PDATA(pgm)->taint_write = 1;
   return 0;

 fail:

This is still very experimental. I can't reproduce the problem with my nanoevery, but I can assure you it was harmless.

To create a delay, reuse jtagmkII_updi_term_keep_alive and issue a GET_SYNC. I still don't know if once is enough or twice is not enough. You can replace this with usleep if you know the delay time you want.

For now, I'm avoiding using commands that read memory, as I need to store many parameters in cookies when writing to memory.

You may also need to update the taint_write flag elsewhere. Currently, it is only inserted where it is deemed necessary.

mcuee commented 10 months ago

@askn37

Thanks a lot. Your patch works well. I apply your patch on top of PR #1657 and here are the results.

mcuee@mcuees-Mac-mini tools % git diff
diff --git a/src/jtagmkII.c b/src/jtagmkII.c
index 0aecb7ce..7ad73c84 100644
--- a/src/jtagmkII.c
+++ b/src/jtagmkII.c
@@ -87,6 +87,9 @@ struct pdata
   /* Major firmware version (needed for Xmega programming) */
   unsigned int fwver;

+  /* Remember if the last execution was a memory write */
+  int taint_write;
+
 #define FLAGS32_INIT_SMC      1 // Part will undergo chip erase
 #define FLAGS32_WRITE         2 // At least one write operation specified
   // Couple of flag bits for AVR32 programming
@@ -142,6 +145,7 @@ static int jtagmkII_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const A
                                 unsigned int addr, unsigned int n_bytes);
 static unsigned char jtagmkII_mtype(const PROGRAMMER *pgm, const AVRPART *p, unsigned long addr);
 static unsigned int jtagmkII_memaddr(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr);
+static int jtagmkII_updi_term_keep_alive(const PROGRAMMER *pgm, const AVRPART *p_unused);

 // AVR32
 #define ERROR_SAB 0xFFFFFFFF
@@ -850,6 +854,7 @@ static int jtagmkII_chip_erase(const PROGRAMMER *pgm, const AVRPART *p) {
   if (!(p->prog_modes & (PM_PDI | PM_UPDI)))
       pgm->initialize(pgm, p);

+  PDATA(pgm)->taint_write = 1;
   return 0;
 }

@@ -1105,6 +1110,13 @@ static int jtagmkII_program_disable(const PROGRAMMER *pgm) {
   if (!PDATA(pgm)->prog_enabled)
     return 0;

+  // Delay early termination to ensure memory rewrite operations.
+  if (PDATA(pgm)->taint_write) {
+    pmsg_notice2("jtagmkII_program_disable(): Delay after writing: ");
+    jtagmkII_updi_term_keep_alive(pgm, /* unused */ (AVRPART*)NULL);
+    jtagmkII_updi_term_keep_alive(pgm, /* unused */ (AVRPART*)NULL);
+  }
+
   buf[0] = CMND_LEAVE_PROGMODE;
   pmsg_notice2("jtagmkII_program_disable(): "
     "Sending leave progmode command: ");
@@ -1128,6 +1140,7 @@ static int jtagmkII_program_disable(const PROGRAMMER *pgm) {
     return -1;
   }

+  PDATA(pgm)->taint_write  = 0;
   PDATA(pgm)->prog_enabled = 0;
   (void)jtagmkII_reset(pgm, 0x01);

@@ -2012,6 +2025,7 @@ static int jtagmkII_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const A
   free(cmd);
   serial_recv_timeout = otimeout;

+  PDATA(pgm)->taint_write = 1;
   return n_bytes;
 }

@@ -2102,6 +2116,7 @@ static int jtagmkII_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AV
   }
   serial_recv_timeout = otimeout;

+  PDATA(pgm)->taint_write = 0;
   return n_bytes;
 }

@@ -2290,6 +2305,7 @@ retry:
     *value = resp[1];

   free(resp);
+  PDATA(pgm)->taint_write = 0;
   return 0;

 fail:
@@ -2400,6 +2416,7 @@ retry:
   }

   free(resp);
+  PDATA(pgm)->taint_write = 1;
   return 0;

 fail:

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 2 -p "-c nanoevery -p m4809 -P usb:2341:0058 -r" 
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   3.438 s: fuse access: clear, set and read eesave fuse bit
✅   3.468 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   3.621 s: chip erase
✅  10.536 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   4.417 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   3.612 s: eeprom check whether programmer can flip 0s to 1s
✅   3.600 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   3.713 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   4.564 s: chip erase and spot check flash is actually erased
✅   3.492 s: spot check eeprom is erased, too
✅   3.612 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 0 -p "-c nanoevery -p m4809 -P usb:2341:0058 -r" 
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   3.443 s: fuse access: clear, set and read eesave fuse bit
✅   3.449 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   3.604 s: chip erase
✅  10.539 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   4.396 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   3.569 s: eeprom check whether programmer can flip 0s to 1s
✅   3.560 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   3.699 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   4.539 s: chip erase and spot check flash is actually erased
✅   3.459 s: spot check eeprom is erased, too
✅   3.605 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 0 -p "-c nanoevery -p m4809 -P usb:2341:0058 -r" 
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   3.435 s: fuse access: clear, set and read eesave fuse bit
✅   3.449 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   3.588 s: chip erase
✅  10.542 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   4.401 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   3.576 s: eeprom check whether programmer can flip 0s to 1s
✅   3.573 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   3.701 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   4.561 s: chip erase and spot check flash is actually erased
✅   3.472 s: spot check eeprom is erased, too
✅   3.611 s: usersig -T/-U write/read random_data_64B.bin
askn37 commented 10 months ago

I am sorry but I do not quite understand this issue. Please help to elaborate. Thanks

The updi_mode flag controls the operational state of ATSAMD11, which is responsible for programming. If false, behaves as USB serial and monitors 1200bps touch. If true, behaves as JTAG2UPDI variant.

Except for physical reset, the condition for changing this flag from true to false is that at least 500ms has passed since the execution of CMND_LEAVE_PROGMODE (that is, the "jtagmkII_program_disable(): Send Leave progmode command:" part of AVRDUDE)

Therefore, AVRDUDE requires a 500ms delay to re-enable her 1200bps touch.

mcuee commented 10 months ago

@askn37

I think you can create a PR based on your current patch. I am not good at programming so I will not be a good reviewer. But @stefanrueger is an excellent reviewer for codes.

I am not a good reviewer for more complex code changes but I am a pretty good tester for the projects I am working on. :-)

Reference: an example from libusb project https://github.com/libusb/libusb/pull/1406#issuecomment-1918633626

askn37 commented 10 months ago

This is the same idea as chanting a mantra before shutting down an old *UNIX machine. That was the landing spot.

sync; sync; sync
mcuee commented 10 months ago

This is the same idea as chanting a mantra before shutting down an old *UNIX machine. That was the landing spot.

sync; sync; sync

Haha, still the same for new Linux/BSD machines. :-)

I used old things like on the dumb terminals of DEC Ultrix (but MicroVMS/Vax VMS more often), Sun OS and Sun Solaris in the university times. In those cases, I did not get the chance to see "sync; sync; sync".

But for Linux, I played with old Linux distro back in 1998 on a DEC 486 machine and then later Intel Pentium 350/Celeron 400 PC from 1998 to 2002. Then I became a real Linux user in 2005 with Ubuntu 5.04 (still mainly using Ubuntu/Debian Linux now even though I play with other Linux distros like Fedora or Arch). I am OS neutural so I use Windows/macOS/Linux and play with BSDs and other alternative OS as part of testing for various open source projects like libusb, avrdude and OpenOCD.

mcuee commented 10 months ago

In my opinion, the original JTAG2UPDI has the following limitations:

  • The last operation performed before ending the session must not be a write or erase. That operation may be lost. Many users may not be aware of this, as this is typically a write-verify pair. Adding a delay after the session ends does not solve the problem. This also applies to Nano Every.
  • Any process that manipulates the status LED after a series of sessions has completed conflicts with starting a new session. The built-in timer malfunctions and times out, making it impossible to continue. To avoid this, a 1-2 second delay is required after the previous session ends. This is not the case with Nano Every, but it has a similar timer clearing process, so adding a short delay is recommended.

@askn37

Wow, you really know the ins-and-outs of jtag2updi. Just wondering if you can provide a PR for avrdude to fix the issues with the various jtag2updi implementations out there.

  1. official jtag2updi implementation
  2. Arduino Nano Every
  3. Nano 4808, usually using CH340 and an 8051 MCU (this one seems to be pretty okay)

FYI, no issues with Nano 4808 with git main (no need any patches).

mcuee@mcuees-Mac-mini tools % pwd
/Users/mcuee/build/avr/avrdude_test/avrdude_main/tools

mcuee@mcuees-Mac-mini tools % ./test-avrdude -v -d 0 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440"     
Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.862 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
✅   0.864 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.014 s: chip erase
✅   4.812 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.329 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.957 s: eeprom check whether programmer can flip 0s to 1s
✅   0.953 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.030 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.492 s: chip erase and spot check flash is actually erased
✅   0.891 s: spot check eeprom is erased, too
✅   0.992 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -v -d 1 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" 
Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.860 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
✅   0.889 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.030 s: chip erase
✅   4.827 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.348 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.974 s: eeprom check whether programmer can flip 0s to 1s
✅   0.967 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.036 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.508 s: chip erase and spot check flash is actually erased
✅   0.906 s: spot check eeprom is erased, too
✅   1.004 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -v -d 0 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" 
Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.859 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
✅   0.868 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.014 s: chip erase
✅   4.811 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.335 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.959 s: eeprom check whether programmer can flip 0s to 1s
✅   0.961 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.029 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.488 s: chip erase and spot check flash is actually erased
✅   0.891 s: spot check eeprom is erased, too
✅   0.988 s: usersig -T/-U write/read random_data_64B.bin
mcuee commented 10 months ago

@askn37

And no regressions with your patch with Nano 4808.

mcuee@mcuees-Mac-mini tools % pwd                                                                                                  
/Users/mcuee/build/avr/avrdude_test/avrdude_sr/tools
mcuee@mcuees-Mac-mini tools % ls                                                                                                   
atdf-to-avrdude.xslt   avrdude_issue1649.conf get-dw-params.xsl      get-stk600-devices.xsl
avrdude.conf           bootloader-hash        get-hv-params.xsl      test-avrdude
avrdude_issue1649      build-mingw32.sh       get-stk600-cards.xsl   test_files

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 0 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.855 s: fuse access: clear, set and read eesave fuse bit
✅   0.869 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.018 s: chip erase
✅   4.805 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.331 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.955 s: eeprom check whether programmer can flip 0s to 1s
✅   0.957 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.028 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.486 s: chip erase and spot check flash is actually erased
✅   0.887 s: spot check eeprom is erased, too
✅   0.986 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 0 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.854 s: fuse access: clear, set and read eesave fuse bit
✅   0.870 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.016 s: chip erase
✅   4.804 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.330 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.956 s: eeprom check whether programmer can flip 0s to 1s
✅   0.957 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.027 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.485 s: chip erase and spot check flash is actually erased
✅   0.888 s: spot check eeprom is erased, too
✅   0.987 s: usersig -T/-U write/read random_data_64B.bin

mcuee@mcuees-Mac-mini tools % ./test-avrdude -e "./avrdude_issue1649" -v -d 0 -p "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c jtag2updi -p m4808 -P /dev/cu.usbserial-22440" and press 'enter' or 'space' to continue. Press any other key to skip
✅   0.856 s: fuse access: clear, set and read eesave fuse bit
✅   0.868 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   1.016 s: chip erase
✅   4.815 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   2.332 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   0.960 s: eeprom check whether programmer can flip 0s to 1s
✅   0.964 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   1.034 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   2.494 s: chip erase and spot check flash is actually erased
✅   0.888 s: spot check eeprom is erased, too
✅   0.986 s: usersig -T/-U write/read random_data_64B.bin
mcuee commented 10 months ago

I will need to check with FreeBSD 14.0 again. It may have similar issues.

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 0 -v -p "-c nanoevery -p m4809 -P /dev/ttyU0 -r"
Testing avrdude version 7.2-20240131 (5684b85f)
Prepare "-c nanoevery -p m4809 -P /dev/ttyU0 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.388 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
❌   9.412 s: fuse access: set eesave fusebit to delete EEPROM on chip erase (failed command below)
$ avrdude -qq -c nanoevery -p m4809 -P /dev/ttyU0 -r -l /tmp/test-avrdude.log.3UJFNB -T "config eesave=ee*erased"
avrdude warning: attempt 1 of 10: sign-on command: status -1
✅  14.648 s: chip erase
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅  22.086 s: flash -U write/verify holes_rjmp_loops_49152B.hex
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   5.801 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.531 s: eeprom check whether programmer can flip 0s to 1s
✅   4.539 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.653 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.953 s: chip erase and spot check flash is actually erased
✅  14.576 s: spot check eeprom is erased, too
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   4.564 s: usersig -T/-U write/read random_data_64B.bin

One or more AVRDUDE "-c nanoevery -p m4809 -P /dev/ttyU0 -r" tests failed. Do you want to retry this particular test? (y/n): n

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 1 -v -p "-c nanoevery -p m4809 -P /dev/ttyU0 -r"
Testing avrdude version 7.2-20240131 (5684b85f)
Prepare "-c nanoevery -p m4809 -P /dev/ttyU0 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.395 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
❌   9.440 s: fuse access: set eesave fusebit to delete EEPROM on chip erase (failed command below)
$ avrdude -qq -c nanoevery -p m4809 -P /dev/ttyU0 -r -l /tmp/test-avrdude.log.Ckafeb -T "config eesave=ee*erased"
avrdude warning: attempt 1 of 10: sign-on command: status -1
✅  14.568 s: chip erase
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅  22.050 s: flash -U write/verify holes_rjmp_loops_49152B.hex
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   5.786 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.530 s: eeprom check whether programmer can flip 0s to 1s
✅   4.542 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.664 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.936 s: chip erase and spot check flash is actually erased
✅  14.436 s: spot check eeprom is erased, too
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   4.572 s: usersig -T/-U write/read random_data_64B.bin

One or more AVRDUDE "-c nanoevery -p m4809 -P /dev/ttyU0 -r" tests failed. Do you want to retry this particular test? (y/n): n

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 2 -v -p "-c nanoevery -p m4809 -P /dev/ttyU0 -r"
Testing avrdude version 7.2-20240131 (5684b85f)
Prepare "-c nanoevery -p m4809 -P /dev/ttyU0 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.385 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
❌   9.442 s: fuse access: set eesave fusebit to delete EEPROM on chip erase (failed command below)
$ avrdude -qq -c nanoevery -p m4809 -P /dev/ttyU0 -r -l /tmp/test-avrdude.log.G9RA99 -T "config eesave=ee*erased"
avrdude warning: attempt 1 of 10: sign-on command: status -1
✅  14.571 s: chip erase
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅  22.149 s: flash -U write/verify holes_rjmp_loops_49152B.hex
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   5.787 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.524 s: eeprom check whether programmer can flip 0s to 1s
✅   4.541 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.646 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.972 s: chip erase and spot check flash is actually erased
✅  14.542 s: spot check eeprom is erased, too
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   4.571 s: usersig -T/-U write/read random_data_64B.bin

One or more AVRDUDE "-c nanoevery -p m4809 -P /dev/ttyU0 -r" tests failed. Do you want to retry this particular test? (y/n): n

Today somehow I can not get it to work well. I will try tomorrow. I may need to try on a physical machine (not VM).

askn37 commented 10 months ago

Today somehow I can not get it to work well. I will try tomorrow. I may need to try on a physical machine (not VM).

Common to USB Serial where Virtual PC is involved. Handshake communication using small packets is often unpleasant.

In this case, communication retries by jtagmkII_getsync will occur here and there, and a warning on line 650 will be displayed, but the retries will resolve the problem and the actual reading and writing will succeed. I'm concerned about the failure of the first FUSE change.

stefanrueger commented 10 months ago

@askn37 Thanks for your input and patch. Please could you create a PR for your idea. I prefer reviewing PRs rather than patches, but based on your idea, I was wondering

askn37 commented 10 months ago

@askn37 Thanks for your input and patch. Please could you create a PR for your idea. I prefer reviewing PRs rather than patches, but based on your idea, I was wondering

  • Is it sufficient to read any memory rather than the memory that was last written? If so, it might be an idea to read the signature rather than calling keep_alive(). This is a genuine question and I have no preference either way, just wondering what would be better.
  • I ever so slightly prefer int recently_written; // Most recent operation was an erase or a write (less drastic than taint 😄)
  • I may have missed this, but would it be OK/necessary to set PDATA(pgm)->recently_written = 0; for the _read_byte() routine?

In this experimental patch, the AVRPART argument is not passed to program_disable, so we wrote code that does not require the AVRPART argument. To overcome this limitation, it may be desirable to add a new callback to main.c that can insert some cleanup processing before calling program_disable.

The renaming of taint_write makes sense. We recommend that you give your variables more descriptive names.

stefanrueger commented 10 months ago

@askn37 main.c is a bit counter-intuitive with respect to the order of pgm-> functions: https://github.com/avrdudes/avrdude/blob/571a8063b9258cf6d361a02abd4387e55dc28af8/src/main.c#L1736-L1748

jtagmkII.c does not utilise _powerdown(). However, if it did then the code in _disable() would no longer be able to contact the programmer, so at best could sleep. I like the idea of a new callback function, say finish_programming and insert

  if(pgm->finish_programming)
    pgm->finish_programming(pgm, p);

into main.c at line 1737.

mcuee commented 10 months ago

I need to reboot my Proxmox PVE host to get the FreeBSD 14 VM working.

Under FreeBSD, with git main, the second test failed but the remaining tests passed. With the patch (plus PR #1657), all are good. So the patch seems to fix the issue for FreeBSD as well.

git main:

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 1 -v -p "-c nanoevery -p m4809 -P usb:2341:0058 -r"

Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.388 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
❌   9.446 s: fuse access: set eesave fusebit to delete EEPROM on chip erase (failed command below)
$ avrdude -qq -c nanoevery -p m4809 -P usb:2341:0058 -r -l /tmp/test-avrdude.log.HGn9RC -T "config eesave=ee*erased"
avrdude warning: attempt 1 of 10: sign-on command: status -1
✅  14.600 s: chip erase
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅  22.155 s: flash -U write/verify holes_rjmp_loops_49152B.hex
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   5.802 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.524 s: eeprom check whether programmer can flip 0s to 1s
✅   4.541 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.663 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.982 s: chip erase and spot check flash is actually erased
✅  14.419 s: spot check eeprom is erased, too
avrdude warning: attempt 1 of 10: sign-on command: status -1
avrdude warning: attempt 2 of 10: sign-on command: status -1
✅   4.570 s: usersig -T/-U write/read random_data_64B.bin

One or more AVRDUDE "-c nanoevery -p m4809 -P usb:2341:0058 -r" tests failed. Do you want to retry this particular test? (y/n): n

PR #1657 + patch by @askn37 -- all good now.

mcuee@freebsd14vmn100:~/build/avrdude_sr/tools $ sudo ./test-avrdude -e "./avrdude_issue1649" -d 2 -v -p "-c nanoevery -
p m4809 -P usb:2341:0058 -r"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.415 s: fuse access: clear, set and read eesave fuse bit
✅   4.393 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.532 s: chip erase
✅  12.045 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.767 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.501 s: eeprom check whether programmer can flip 0s to 1s
✅   4.530 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.643 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.952 s: chip erase and spot check flash is actually erased
✅   4.392 s: spot check eeprom is erased, too
✅   4.566 s: usersig -T/-U write/read random_data_64B.bin

mcuee@freebsd14vmn100:~/build/avrdude_sr/tools $ sudo ./test-avrdude -e "./avrdude_issue1649" -d 2 -v -p "-c nanoevery -p m4809 -P usb:2341:0058 -r"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.413 s: fuse access: clear, set and read eesave fuse bit
✅   4.413 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.532 s: chip erase
✅  12.050 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.792 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.518 s: eeprom check whether programmer can flip 0s to 1s
✅   4.552 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.652 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.973 s: chip erase and spot check flash is actually erased
✅   4.393 s: spot check eeprom is erased, too
✅   4.522 s: usersig -T/-U write/read random_data_64B.bin

mcuee@freebsd14vmn100:~/build/avrdude_sr/tools $ sudo ./test-avrdude -e "./avrdude_issue1649" -d 1 -v -p "-c nanoevery -
p m4809 -P usb:2341:0058 -r"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.403 s: fuse access: clear, set and read eesave fuse bit
✅   4.376 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.520 s: chip erase
✅  12.000 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.808 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.546 s: eeprom check whether programmer can flip 0s to 1s
✅   4.516 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.644 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.962 s: chip erase and spot check flash is actually erased
✅   4.400 s: spot check eeprom is erased, too
✅   4.566 s: usersig -T/-U write/read random_data_64B.bin

mcuee@freebsd14vmn100:~/build/avrdude_sr/tools $ sudo ./test-avrdude -e "./avrdude_issue1649" -d 0 -v -p "-c nanoevery -
p m4809 -P usb:2341:0058 -r"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.386 s: fuse access: clear, set and read eesave fuse bit
✅   4.399 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.504 s: chip erase
✅  12.030 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.781 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.506 s: eeprom check whether programmer can flip 0s to 1s
✅   4.494 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.635 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.951 s: chip erase and spot check flash is actually erased
✅   4.417 s: spot check eeprom is erased, too
✅   4.544 s: usersig -T/-U write/read random_data_64B.bin

mcuee@freebsd14vmn100:~/build/avrdude_sr/tools $ sudo ./test-avrdude -e "./avrdude_issue1649" -d 0 -v -p "-c nanoevery -p m4809 -P usb:2341:0058 -r"
Testing ./avrdude_issue1649 version 7.2-20240204 (28589ca6)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.406 s: fuse access: clear, set and read eesave fuse bit
✅   4.383 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.498 s: chip erase
✅  11.997 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.801 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.502 s: eeprom check whether programmer can flip 0s to 1s
✅   4.526 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.662 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.954 s: chip erase and spot check flash is actually erased
✅   4.416 s: spot check eeprom is erased, too
✅   4.562 s: usersig -T/-U write/read random_data_64B.bin
stefanrueger commented 10 months ago

Under FreeBSD, with git main, the second test failed but the remaining tests passed.

You already use -v to see what caused the failure. If it is just warnings, then the displayed ❌ failure is a false alarm. You can make test-avrdude ignore warnings by injecting -q into the test like so

$ ./test-avrdude -d 1 -v -p "-c nanoevery -q -p m4809 -P usb:2341:0058 -r"

This -q increases the quell level to 3 for this test, which suppresses warning messages.

mcuee commented 10 months ago

Under FreeBSD, with git main, the second test failed but the remaining tests passed.

You already use -v to see what caused the failure. If it is just warnings, then the displayed ❌ failure is a false alarm. You can make test-avrdude ignore warnings by injecting -q into the test like so

$ ./test-avrdude -d 1 -v -p "-c nanoevery -q -p m4809 -P usb:2341:0058 -r"

This -q increases the quell level to 3 for this test, which suppresses warning messages.

Ah, I see. Thanks for the tip. I was thinking how come the rest of the test passed if step 2 failed. Now I see it is just a warning.

Anyway, the patch does not cause regressions under FreeBSD, so it is still a good thing.

And indeed git main is okay under FreeBSD, the warning sometimes happens and sometimes does not happen.

For example, now it does not happen.

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 2 -v -p "-c nanoevery -p m4809 -P usb:2341:0058 -r"
Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.402 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
✅   4.396 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.555 s: chip erase
✅  12.000 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.765 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.540 s: eeprom check whether programmer can flip 0s to 1s
✅   4.525 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.676 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.972 s: chip erase and spot check flash is actually erased
✅   4.393 s: spot check eeprom is erased, too
✅   4.541 s: usersig -T/-U write/read random_data_64B.bin

mcuee@freebsd14vmn100:~/build/avrdude/tools $ sudo ./test-avrdude -d 0 -v -p "-c nanoevery -p m4809 -P usb:2341:0058 -r"
Testing avrdude version 7.2-20240201 (571a8063)
Prepare "-c nanoevery -p m4809 -P usb:2341:0058 -r" and press 'enter' or 'space' to continue. Press any other key to skip
✅   4.394 s: fuse access: clear, set and read eesave fuse bit
config eesave=eex_preserved # 1
✅   4.376 s: fuse access: set eesave fusebit to delete EEPROM on chip erase
✅   4.531 s: chip erase
✅  12.014 s: flash -U write/verify holes_rjmp_loops_49152B.hex
✅   5.792 s: flash -T write/verify holes_rjmp_loops_49152B.hex
✅   4.512 s: eeprom check whether programmer can flip 0s to 1s
✅   4.549 s: eeprom -U write/verify holes_pack_my_box_256B.hex
✅   4.628 s: eeprom -T write/verify holes_{the_five_boxing_wizards,pack_my_box}_256B.hex
✅   5.929 s: chip erase and spot check flash is actually erased
✅   4.425 s: spot check eeprom is erased, too
✅   4.566 s: usersig -T/-U write/read random_data_64B.bin
stefanrueger commented 10 months ago

@askn37 It would be great if you could

Thank you!

stefanrueger commented 10 months ago

@askn37 On second thoughts, I created a PR based on your technique to facilitate v7.3 release soon.