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

The last write/erase can fail with -c jtag2updi #1661

Closed
stefanrueger opened this issue Feb 5, 2024 · 4 comments · Fixed by #1662
Closed

The last write/erase can fail with -c jtag2updi #1661

stefanrueger opened this issue Feb 5, 2024 · 4 comments · Fixed by #1662
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@stefanrueger
Copy link
Collaborator

stefanrueger commented Feb 5, 2024

Based on a comment my @askn37 and test by @mcuee. Depending where one stands on this this is limitation/bug in the FW and/or a bug in avrdude (not recognising this limitation).

@mcuee
Copy link
Collaborator

mcuee commented Feb 6, 2024

The comment by @askn37
#1649 (comment)

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

  1. 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.
  2. 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.

@mcuee
Copy link
Collaborator

mcuee commented Feb 6, 2024

The initial work-around by @askn37 was to change the test script.
#1649 (comment)

diff --git a/tools/test-avrdude b/tools/test-avrdude
index 85059976..1ad5d23b 100755
--- a/tools/test-avrdude
+++ b/tools/test-avrdude
@@ -326,7 +326,7 @@ for (( p=0; p<$arraylength; p++ )); do
 
       if [ -n "$EE_SIZE" ]; then
         specify="fuse access: set eesave fusebit to delete EEPROM on chip erase"
-        command=(${avrdude[@]} -T '"config eesave=ee*erased"')
+        command=(${avrdude[@]} -T '"config eesave=ee*erased; config eesave"')
         execute "${command[@]}" > $outfile
         sed -e/^config/d -e/"> "/d $outfile > ${outfile}-2
         result [[ ! -s ${outfile}-2  '&&' ! -s $logfile ]]

@mcuee
Copy link
Collaborator

mcuee commented Feb 6, 2024

Later @askn37 provided a patch which fixes the issue.
#1649 (comment)

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 mcuee added the bug Something isn't working label Feb 6, 2024
@mcuee
Copy link
Collaborator

mcuee commented Feb 6, 2024

@stefanrueger has adapted the patch to PR #1662 and I have tested to be good.

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 a pull request may close this issue.

2 participants