From 47e46fc97cdfada210d10ef69ec620173041abe3 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 18:54:47 +0100
Subject: [PATCH 01/13] Check return value of avr910_send() calls for errors

---
 src/avr910.c | 66 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 600cc249..4a8a09dc 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -54,6 +54,24 @@ struct pdata
 
 #define PDATA(pgm) ((struct pdata *)(pgm->cookie))
 
+// Print error and return when command failed
+#define EI(x) do {                 \
+  int Eret = (x);                  \
+  if(Eret < 0) {                   \
+    pmsg_error("%s failed\n", #x); \
+    return -1;                     \
+  }                                \
+} while(0)
+
+#define EV(x) do {                 \
+  int Eret = (x);                  \
+  if(Eret < 0) {                   \
+    pmsg_error("%s failed\n", #x); \
+    return;                        \
+  }                                \
+} while(0)
+
+
 static void avr910_setup(PROGRAMMER * pgm)
 {
   if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) {
@@ -108,7 +126,7 @@ static int avr910_vfy_cmd_sent(const PROGRAMMER *pgm, char *errmsg) {
  * issue the 'chip erase' command to the AVR device
  */
 static int avr910_chip_erase(const PROGRAMMER *pgm, const AVRPART *p) {
-  avr910_send(pgm, "e", 1);
+  EI(avr910_send(pgm, "e", 1));
   if (avr910_vfy_cmd_sent(pgm, "chip erase") < 0)
     return -1;
 
@@ -122,13 +140,13 @@ static int avr910_chip_erase(const PROGRAMMER *pgm, const AVRPART *p) {
 
 
 static int avr910_enter_prog_mode(const PROGRAMMER *pgm) {
-  avr910_send(pgm, "P", 1);
+  EI(avr910_send(pgm, "P", 1));
   return avr910_vfy_cmd_sent(pgm, "enter prog mode");
 }
 
 
 static int avr910_leave_prog_mode(const PROGRAMMER *pgm) {
-  avr910_send(pgm, "L", 1);
+  EI(avr910_send(pgm, "L", 1));
   return avr910_vfy_cmd_sent(pgm, "leave prog mode");
 }
 
@@ -156,21 +174,21 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
   /* Get the programmer identifier. Programmer returns exactly 7 chars
      _without_ the null.*/
 
-  avr910_send(pgm, "S", 1);
+  EI(avr910_send(pgm, "S", 1));
   memset (id, 0, sizeof(id));
   avr910_recv(pgm, id, sizeof(id)-1);
 
   /* Get the HW and SW versions to see if the programmer is present. */
 
-  avr910_send(pgm, "V", 1);
+  EI(avr910_send(pgm, "V", 1));
   avr910_recv(pgm, sw, sizeof(sw));
 
-  avr910_send(pgm, "v", 1);
+  EI(avr910_send(pgm, "v", 1));
   avr910_recv(pgm, hw, sizeof(hw));
 
   /* Get the programmer type (serial or parallel). Expect serial. */
 
-  avr910_send(pgm, "p", 1);
+  EI(avr910_send(pgm, "p", 1));
   avr910_recv(pgm, &type, 1);
 
   msg_notice("Programmer id    = %s; type = %c\n", id, type);
@@ -179,7 +197,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
   /* See if programmer supports autoincrement of address. */
 
-  avr910_send(pgm, "a", 1);
+  EI(avr910_send(pgm, "a", 1));
   avr910_recv(pgm, &PDATA(pgm)->has_auto_incr_addr, 1);
   if (PDATA(pgm)->has_auto_incr_addr == 'Y')
       msg_notice("programmer supports auto addr increment\n");
@@ -187,7 +205,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
   /* Check support for buffered memory access, ignore if not available */
 
   if (PDATA(pgm)->test_blockmode == 1) {
-    avr910_send(pgm, "b", 1);
+    EI(avr910_send(pgm, "b", 1));
     avr910_recv(pgm, &c, 1);
     if (c == 'Y') {
       avr910_recv(pgm, &c, 1);
@@ -211,7 +229,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
     /* Get list of devices that the programmer supports. */
 
-    avr910_send(pgm, "t", 1);
+    EI(avr910_send(pgm, "t", 1));
     msg_notice2("\nProgrammer supports the following devices:\n");
     devtype_1st = 0;
     while (1) {
@@ -251,7 +269,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
   buf[0] = 'T';
   /* buf[1] has been set up above */
 
-  avr910_send(pgm, buf, 2);
+  EI(avr910_send(pgm, buf, 2));
   avr910_vfy_cmd_sent(pgm, "select device");
 
   pmsg_notice("avr910_devcode selected: 0x%02x\n", (unsigned) buf[1]);
@@ -293,7 +311,7 @@ static int avr910_cmd(const PROGRAMMER *pgm, const unsigned char *cmd,
   buf[3] = cmd[2];
   buf[4] = cmd[3];
 
-  avr910_send (pgm, buf, 5);
+  EI(avr910_send(pgm, buf, 5));
   avr910_recv (pgm, buf, 2);
 
   res[0] = 0x00;                /* Dummy value */
@@ -393,7 +411,7 @@ static void avr910_set_addr(const PROGRAMMER *pgm, unsigned long addr) {
   cmd[1] = (addr >> 8) & 0xff;
   cmd[2] = addr & 0xff;
   
-  avr910_send(pgm, cmd, sizeof(cmd));
+  EV(avr910_send(pgm, cmd, sizeof(cmd)));
   avr910_vfy_cmd_sent(pgm, "set addr");
 }
 
@@ -424,7 +442,7 @@ static int avr910_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
 
   avr910_set_addr(pgm, addr);
 
-  avr910_send(pgm, cmd, sizeof(cmd));
+  EI(avr910_send(pgm, cmd, sizeof(cmd)));
   avr910_vfy_cmd_sent(pgm, "write byte");
 
   return 0;
@@ -438,7 +456,7 @@ static int avr910_read_byte_flash(const PROGRAMMER *pgm, const AVRPART *p, const
 
   avr910_set_addr(pgm, addr >> 1);
 
-  avr910_send(pgm, "R", 1);
+  EI(avr910_send(pgm, "R", 1));
 
   /* Read back the program mem word (MSB first) */
   avr910_recv(pgm, buf, sizeof(buf));
@@ -458,7 +476,7 @@ static int avr910_read_byte_eeprom(const PROGRAMMER *pgm, const AVRPART *p, cons
                                    unsigned long addr, unsigned char * value)
 {
   avr910_set_addr(pgm, addr);
-  avr910_send(pgm, "d", 1);
+  EI(avr910_send(pgm, "d", 1));
   avr910_recv(pgm, (char *)value, 1);
 
   return 0;
@@ -498,7 +516,7 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
     page_wr_cmd_pending = 1;
     buf[0] = cmd[addr & 0x01];
     buf[1] = m->buf[addr];
-    avr910_send(pgm, buf, sizeof(buf));
+    EI(avr910_send(pgm, buf, sizeof(buf)));
     avr910_vfy_cmd_sent(pgm, "write byte");
 
     addr++;
@@ -508,7 +526,7 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
       /* Send the "Issue Page Write" if we have sent a whole page. */
 
       avr910_set_addr(pgm, page_addr>>1);
-      avr910_send(pgm, "m", 1);
+      EI(avr910_send(pgm, "m", 1));
       avr910_vfy_cmd_sent(pgm, "flush page");
 
       page_wr_cmd_pending = 0;
@@ -530,7 +548,7 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
 
   if (page_wr_cmd_pending) {
     avr910_set_addr(pgm, page_addr>>1);
-    avr910_send(pgm, "m", 1);
+    EI(avr910_send(pgm, "m", 1));
     avr910_vfy_cmd_sent(pgm, "flush final page");
     usleep(m->max_write_delay);
   }
@@ -553,7 +571,7 @@ static int avr910_paged_write_eeprom(const PROGRAMMER *pgm, const AVRPART *p,
 
   while (addr < max_addr) {
     cmd[1] = m->buf[addr];
-    avr910_send(pgm, cmd, sizeof(cmd));
+    EI(avr910_send(pgm, cmd, sizeof(cmd)));
     avr910_vfy_cmd_sent(pgm, "write byte");
     usleep(m->max_write_delay);
 
@@ -615,7 +633,7 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
       cmd[1] = (blocksize >> 8) & 0xff;
       cmd[2] = blocksize & 0xff;
 
-      avr910_send(pgm, cmd, 4 + blocksize);
+      EI(avr910_send(pgm, cmd, 4 + blocksize));
       avr910_vfy_cmd_sent(pgm, "write block");
 
       addr += blocksize;
@@ -666,7 +684,7 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
       cmd[1] = (blocksize >> 8) & 0xff;
       cmd[2] = blocksize & 0xff;
 
-      avr910_send(pgm, cmd, 4);
+      EI(avr910_send(pgm, cmd, 4));
       avr910_recv(pgm, (char *)&m->buf[addr], blocksize);
 
       addr += blocksize;
@@ -678,7 +696,7 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
     avr910_set_addr(pgm, addr / rd_size);
 
     while (addr < max_addr) {
-      avr910_send(pgm, cmd, 1);
+      EI(avr910_send(pgm, cmd, 1));
       if (rd_size == 2) {
         /* The 'R' command returns two bytes, MSB first, we need to put the data
            into the memory buffer LSB first. */
@@ -713,7 +731,7 @@ static int avr910_read_sig_bytes(const PROGRAMMER *pgm, const AVRPART *p, const
     return -1;
   }
 
-  avr910_send(pgm, "s", 1);
+  EI(avr910_send(pgm, "s", 1));
   avr910_recv(pgm, (char *)m->buf, 3);
   /* Returned signature has wrong order. */
   tmp = m->buf[2];

From 70901f95d8665bbd735d6ae218dd9f5b359c5d9b Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 19:17:42 +0100
Subject: [PATCH 02/13] Check return value of avr910_recv() calls for errors

---
 src/avr910.c | 56 ++++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 35 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 4a8a09dc..e5694dd0 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -94,14 +94,7 @@ static int avr910_send(const PROGRAMMER *pgm, char *buf, size_t len) {
 
 
 static int avr910_recv(const PROGRAMMER *pgm, char *buf, size_t len) {
-  int rv;
-
-  rv = serial_recv(&pgm->fd, (unsigned char *)buf, len);
-  if (rv < 0) {
-    pmsg_error("programmer is not responding\n");
-    return 1;
-  }
-  return 0;
+  return serial_recv(&pgm->fd, (unsigned char *) buf, len);
 }
 
 
@@ -113,11 +106,12 @@ static int avr910_drain(const PROGRAMMER *pgm, int display) {
 static int avr910_vfy_cmd_sent(const PROGRAMMER *pgm, char *errmsg) {
   char c;
 
-  avr910_recv(pgm, &c, 1);
+  EI(avr910_recv(pgm, &c, 1));
   if (c != '\r') {
     pmsg_error("programmer did not respond to command: %s\n", errmsg);
     return 1;
   }
+
   return 0;
 }
 
@@ -176,20 +170,20 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
   EI(avr910_send(pgm, "S", 1));
   memset (id, 0, sizeof(id));
-  avr910_recv(pgm, id, sizeof(id)-1);
+  EI(avr910_recv(pgm, id, sizeof(id)-1));
 
   /* Get the HW and SW versions to see if the programmer is present. */
 
   EI(avr910_send(pgm, "V", 1));
-  avr910_recv(pgm, sw, sizeof(sw));
+  EI(avr910_recv(pgm, sw, sizeof(sw)));
 
   EI(avr910_send(pgm, "v", 1));
-  avr910_recv(pgm, hw, sizeof(hw));
+  EI(avr910_recv(pgm, hw, sizeof(hw)));
 
   /* Get the programmer type (serial or parallel). Expect serial. */
 
   EI(avr910_send(pgm, "p", 1));
-  avr910_recv(pgm, &type, 1);
+  EI(avr910_recv(pgm, &type, 1));
 
   msg_notice("Programmer id    = %s; type = %c\n", id, type);
   msg_notice("Software version = %c.%c; ", sw[0], sw[1]);
@@ -198,7 +192,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
   /* See if programmer supports autoincrement of address. */
 
   EI(avr910_send(pgm, "a", 1));
-  avr910_recv(pgm, &PDATA(pgm)->has_auto_incr_addr, 1);
+  EI(avr910_recv(pgm, &PDATA(pgm)->has_auto_incr_addr, 1));
   if (PDATA(pgm)->has_auto_incr_addr == 'Y')
       msg_notice("programmer supports auto addr increment\n");
 
@@ -206,11 +200,11 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
   if (PDATA(pgm)->test_blockmode == 1) {
     EI(avr910_send(pgm, "b", 1));
-    avr910_recv(pgm, &c, 1);
+    EI(avr910_recv(pgm, &c, 1));
     if (c == 'Y') {
-      avr910_recv(pgm, &c, 1);
+      EI(avr910_recv(pgm, &c, 1));
       PDATA(pgm)->buffersize = (unsigned int)(unsigned char)c<<8;
-      avr910_recv(pgm, &c, 1);
+      EI(avr910_recv(pgm, &c, 1));
       PDATA(pgm)->buffersize += (unsigned int)(unsigned char)c;
       msg_notice("programmer supports buffered memory access with "
                       "buffersize = %u bytes\n",
@@ -232,8 +226,8 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
     EI(avr910_send(pgm, "t", 1));
     msg_notice2("\nProgrammer supports the following devices:\n");
     devtype_1st = 0;
-    while (1) {
-      avr910_recv(pgm, &c, 1);
+    while(1) {
+      EI(avr910_recv(pgm, &c, 1));
       if (devtype_1st == 0)
 	devtype_1st = c;
       if (c == 0)
@@ -312,7 +306,7 @@ static int avr910_cmd(const PROGRAMMER *pgm, const unsigned char *cmd,
   buf[4] = cmd[3];
 
   EI(avr910_send(pgm, buf, 5));
-  avr910_recv (pgm, buf, 2);
+  EI(avr910_recv(pgm, buf, 2));
 
   res[0] = 0x00;                /* Dummy value */
   res[1] = cmd[0];
@@ -457,16 +451,8 @@ static int avr910_read_byte_flash(const PROGRAMMER *pgm, const AVRPART *p, const
   avr910_set_addr(pgm, addr >> 1);
 
   EI(avr910_send(pgm, "R", 1));
-
-  /* Read back the program mem word (MSB first) */
-  avr910_recv(pgm, buf, sizeof(buf));
-
-  if ((addr & 0x01) == 0) {
-    *value = buf[1];
-  }
-  else {
-    *value = buf[0];
-  }
+  EI(avr910_recv(pgm, buf, sizeof(buf)));
+  *value = buf[(addr & 1) ^ 1]; // MSB in buffer first
 
   return 0;
 }
@@ -477,7 +463,7 @@ static int avr910_read_byte_eeprom(const PROGRAMMER *pgm, const AVRPART *p, cons
 {
   avr910_set_addr(pgm, addr);
   EI(avr910_send(pgm, "d", 1));
-  avr910_recv(pgm, (char *)value, 1);
+  EI(avr910_recv(pgm, (char *) value, 1));
 
   return 0;
 }
@@ -685,7 +671,7 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
       cmd[2] = blocksize & 0xff;
 
       EI(avr910_send(pgm, cmd, 4));
-      avr910_recv(pgm, (char *)&m->buf[addr], blocksize);
+      EI(avr910_recv(pgm, (char *) &m->buf[addr], blocksize));
 
       addr += blocksize;
     }
@@ -700,12 +686,12 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
       if (rd_size == 2) {
         /* The 'R' command returns two bytes, MSB first, we need to put the data
            into the memory buffer LSB first. */
-        avr910_recv(pgm, buf, 2);
+        EI(avr910_recv(pgm, buf, 2));
         m->buf[addr]   = buf[1];  /* LSB */
         m->buf[addr + 1] = buf[0];  /* MSB */
       }
       else {
-        avr910_recv(pgm, (char *)&m->buf[addr], 1);
+        EI(avr910_recv(pgm, (char *) &m->buf[addr], 1));
       }
 
       addr += rd_size;
@@ -732,7 +718,7 @@ static int avr910_read_sig_bytes(const PROGRAMMER *pgm, const AVRPART *p, const
   }
 
   EI(avr910_send(pgm, "s", 1));
-  avr910_recv(pgm, (char *)m->buf, 3);
+  EI(avr910_recv(pgm, (char *) m->buf, 3));
   /* Returned signature has wrong order. */
   tmp = m->buf[2];
   m->buf[2] = m->buf[0];

From bf37d340029a366dbf1d820d1cdc140430c12498 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 19:20:17 +0100
Subject: [PATCH 03/13] Explicitly ignore error checks for avr910_drain()

---
 src/avr910.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index e5694dd0..3a5747b1 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -362,12 +362,9 @@ static int avr910_parseextparms(const PROGRAMMER *pgm, const LISTID extparms) {
 
 static int avr910_open(PROGRAMMER *pgm, const char *port) {
   union pinfo pinfo;
-  /*
-   *  If baudrate was not specified use 19.200 Baud
-   */
-  if(pgm->baudrate == 0) {
+
+  if(pgm->baudrate == 0)
     pgm->baudrate = 19200;
-  }
 
   pgm->port = port;
   pinfo.serialinfo.baud = pgm->baudrate;
@@ -376,10 +373,7 @@ static int avr910_open(PROGRAMMER *pgm, const char *port) {
     return -1;
   }
 
-  /*
-   * drain any extraneous input
-   */
-  avr910_drain (pgm, 0);
+  (void) avr910_drain (pgm, 0);
 	
   return 0;
 }

From 89473ccf17b498d13239f9e97d7992e00d436ff9 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 19:21:47 +0100
Subject: [PATCH 04/13] Change error message and return value of
 avr910_vfy_cmd_sent()

---
 src/avr910.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 3a5747b1..bb5d8589 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -108,8 +108,8 @@ static int avr910_vfy_cmd_sent(const PROGRAMMER *pgm, char *errmsg) {
 
   EI(avr910_recv(pgm, &c, 1));
   if (c != '\r') {
-    pmsg_error("programmer did not respond to command: %s\n", errmsg);
-    return 1;
+    pmsg_error("protocol error for command: %s\n", errmsg);
+    return -1;
   }
 
   return 0;

From 04956f0dc7a312d6d0b1851acb061e3f551eee64 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 19:24:46 +0100
Subject: [PATCH 05/13] Provide meaningful program_enable() method

---
 src/avr910.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index bb5d8589..f2c36477 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -145,11 +145,8 @@ static int avr910_leave_prog_mode(const PROGRAMMER *pgm) {
 }
 
 
-/*
- * issue the 'program enable' command to the AVR device
- */
 static int avr910_program_enable(const PROGRAMMER *pgm, const AVRPART *p) {
-  return -1;
+  return avr910_enter_prog_mode(pgm);
 }
 
 
@@ -268,9 +265,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
   pmsg_notice("avr910_devcode selected: 0x%02x\n", (unsigned) buf[1]);
 
-  avr910_enter_prog_mode(pgm);
-
-  return 0;
+  return pgm->program_enable(pgm, p);
 }
 
 

From 9ed905367165886b7bab8780df77774521e409d1 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 19:28:34 +0100
Subject: [PATCH 06/13] Provide meaningful pgm->disable() method

It is customary that pgm->disable() leaves programming mode; as such it is
the closing part of pgm->program_enable()
---
 src/avr910.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index f2c36477..48acb640 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -270,9 +270,7 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
 
 static void avr910_disable(const PROGRAMMER *pgm) {
-  /* Do nothing. */
-
-  return;
+  avr910_leave_prog_mode(pgm);
 }
 
 
@@ -373,10 +371,7 @@ static int avr910_open(PROGRAMMER *pgm, const char *port) {
   return 0;
 }
 
-static void avr910_close(PROGRAMMER * pgm)
-{
-  avr910_leave_prog_mode(pgm);
-
+static void avr910_close(PROGRAMMER *pgm) {
   serial_close(&pgm->fd);
   pgm->fd.ifd = -1;
 }

From 45b4f7ab0910529de01a796453edd99790f73bcb Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 20:31:49 +0100
Subject: [PATCH 07/13] Provide PDATA cache for avr910_read_byte_flash()

---
 src/avr910.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 48acb640..cc39fb0a 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -50,6 +50,10 @@ struct pdata
   unsigned int buffersize;
   unsigned char test_blockmode;
   unsigned char use_blockmode;
+
+  int ctype;                    // Cache one byte for flash
+  unsigned char cvalue;
+  unsigned long caddr;
 };
 
 #define PDATA(pgm) ((struct pdata *)(pgm->cookie))
@@ -275,9 +279,6 @@ static void avr910_disable(const PROGRAMMER *pgm) {
 
 
 static void avr910_enable(PROGRAMMER *pgm, const AVRPART *p) {
-  /* Do nothing. */
-
-  return;
 }
 
 
@@ -408,6 +409,7 @@ static int avr910_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
     }
 
     addr >>= 1;
+    PDATA(pgm)->ctype = 0;    // Invalidate read cache
   }
   else if (mem_is_eeprom(m)) {
     cmd[0] = 'D';
@@ -432,11 +434,20 @@ static int avr910_read_byte_flash(const PROGRAMMER *pgm, const AVRPART *p, const
 {
   char buf[2];
 
+  if(PDATA(pgm)->ctype == 'F' && PDATA(pgm)->caddr == addr) {
+    *value = PDATA(pgm)->cvalue;
+    return 0;
+  }
+
   avr910_set_addr(pgm, addr >> 1);
 
   EI(avr910_send(pgm, "R", 1));
   EI(avr910_recv(pgm, buf, sizeof(buf)));
+
   *value = buf[(addr & 1) ^ 1]; // MSB in buffer first
+  PDATA(pgm)->ctype = 'F';
+  PDATA(pgm)->cvalue = buf[addr & 1];
+  PDATA(pgm)->caddr = addr ^ 1;
 
   return 0;
 }
@@ -479,6 +490,8 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
   int page_bytes = page_size;
   int page_wr_cmd_pending = 0;
 
+  PDATA(pgm)->ctype = 0;    // Invalidate read cache
+
   page_addr = addr;
   avr910_set_addr(pgm, addr>>1);
 

From 924c9b0a383d2025ac9ca2259c58222ac3ec9725 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 20:47:25 +0100
Subject: [PATCH 08/13] Rework butterfly_paged_write()

  - Return correct value (number of bytes written)
  - Do not use m->desc for memory type: use mem_is_...(m)
---
 src/avr910.c | 66 +++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index cc39fb0a..7b528379 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -265,7 +265,8 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
   /* buf[1] has been set up above */
 
   EI(avr910_send(pgm, buf, 2));
-  avr910_vfy_cmd_sent(pgm, "select device");
+  if(avr910_vfy_cmd_sent(pgm, "select device") < 0)
+    return -1;
 
   pmsg_notice("avr910_devcode selected: 0x%02x\n", (unsigned) buf[1]);
 
@@ -423,9 +424,7 @@ static int avr910_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
   avr910_set_addr(pgm, addr);
 
   EI(avr910_send(pgm, cmd, sizeof(cmd)));
-  avr910_vfy_cmd_sent(pgm, "write byte");
-
-  return 0;
+  return avr910_vfy_cmd_sent(pgm, "write byte");
 }
 
 
@@ -500,7 +499,8 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
     buf[0] = cmd[addr & 0x01];
     buf[1] = m->buf[addr];
     EI(avr910_send(pgm, buf, sizeof(buf)));
-    avr910_vfy_cmd_sent(pgm, "write byte");
+    if(avr910_vfy_cmd_sent(pgm, "write byte") < 0)
+      return -1;
 
     addr++;
     page_bytes--;
@@ -510,7 +510,8 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
 
       avr910_set_addr(pgm, page_addr>>1);
       EI(avr910_send(pgm, "m", 1));
-      avr910_vfy_cmd_sent(pgm, "flush page");
+      if(avr910_vfy_cmd_sent(pgm, "flush page") < 0)
+        return -1;
 
       page_wr_cmd_pending = 0;
       usleep(m->max_write_delay);
@@ -532,11 +533,12 @@ static int avr910_paged_write_flash(const PROGRAMMER *pgm, const AVRPART *p, con
   if (page_wr_cmd_pending) {
     avr910_set_addr(pgm, page_addr>>1);
     EI(avr910_send(pgm, "m", 1));
-    avr910_vfy_cmd_sent(pgm, "flush final page");
+    if(avr910_vfy_cmd_sent(pgm, "flush final page") < 0)
+      return -1;
     usleep(m->max_write_delay);
   }
 
-  return addr;
+  return n_bytes;
 }
 
 
@@ -555,17 +557,17 @@ static int avr910_paged_write_eeprom(const PROGRAMMER *pgm, const AVRPART *p,
   while (addr < max_addr) {
     cmd[1] = m->buf[addr];
     EI(avr910_send(pgm, cmd, sizeof(cmd)));
-    avr910_vfy_cmd_sent(pgm, "write byte");
+    if(avr910_vfy_cmd_sent(pgm, "write byte") < 0)
+      return -1;
     usleep(m->max_write_delay);
 
     addr++;
 
-    if (PDATA(pgm)->has_auto_incr_addr != 'Y') {
+    if (PDATA(pgm)->has_auto_incr_addr != 'Y')
       avr910_set_addr(pgm, addr);
-    }
   }
 
-  return addr;
+  return n_bytes;
 }
 
 
@@ -573,34 +575,30 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
                               unsigned int page_size,
                               unsigned int addr, unsigned int n_bytes)
 {
-  int rval = 0;
+  int isee = mem_is_eeprom(m);
+
   if (PDATA(pgm)->use_blockmode == 0) {
-    if (mem_is_flash(m)) {
-      rval = avr910_paged_write_flash(pgm, p, m, page_size, addr, n_bytes);
-    } else if (mem_is_eeprom(m)) {
-      rval = avr910_paged_write_eeprom(pgm, p, m, page_size, addr, n_bytes);
-    } else {
-      rval = -2;
-    }
+    if(mem_is_flash(m))
+      return avr910_paged_write_flash(pgm, p, m, page_size, addr, n_bytes);
+    if(isee)
+      return avr910_paged_write_eeprom(pgm, p, m, page_size, addr, n_bytes);
+    return -2;
   }
 
   if (PDATA(pgm)->use_blockmode == 1) {
     unsigned int max_addr = addr + n_bytes;
     char *cmd;
     unsigned int blocksize = PDATA(pgm)->buffersize;
-    int wr_size;
 
-    if (!mem_is_flash(m) && !mem_is_eeprom(m))
+    if(!mem_is_flash(m) && !isee)
       return -2;
 
-    if (m->desc[0] == 'e') {
-      blocksize = 1;		/* Write to eeprom single bytes only */
-      wr_size = 1;
-    } else {
-      wr_size = 2;
-    }
+    if(isee)
+      blocksize = 1;            // Write single bytes only to EEPROM
+    else
+      PDATA(pgm)->ctype = 0;    // Invalidate read cache
 
-    avr910_set_addr(pgm, addr / wr_size);
+    avr910_set_addr(pgm, isee? addr: addr>>1);
 
     cmd = malloc(4 + blocksize);
     if (!cmd) return -1;
@@ -617,15 +615,15 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
       cmd[2] = blocksize & 0xff;
 
       EI(avr910_send(pgm, cmd, 4 + blocksize));
-      avr910_vfy_cmd_sent(pgm, "write block");
+      if(avr910_vfy_cmd_sent(pgm, "write block") < 0)
+        return -1;
 
       addr += blocksize;
-    } /* while */
+    }
     free(cmd);
-
-    rval = addr;
   }
-  return rval;
+
+  return n_bytes;
 }
 
 

From f5881e9cd22bba5f925deb8e734d8bb87be04943 Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 20:56:22 +0100
Subject: [PATCH 09/13] Rework butterfly_paged_load()

  - Return correct value (number of bytes loaded)
  - Do not use m->desc for memory type: use mem_is_...(m)
---
 src/avr910.c | 59 ++++++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 39 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 7b528379..9a972d1a 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -364,9 +364,8 @@ static int avr910_open(PROGRAMMER *pgm, const char *port) {
   pgm->port = port;
   pinfo.serialinfo.baud = pgm->baudrate;
   pinfo.serialinfo.cflags = SERIAL_8N1;
-  if (serial_open(port, pinfo, &pgm->fd)==-1) {
+  if(serial_open(port, pinfo, &pgm->fd) < 0)
     return -1;
-  }
 
   (void) avr910_drain (pgm, 0);
 	
@@ -403,19 +402,16 @@ static int avr910_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
 
   if (mem_is_flash(m)) {
     if (addr & 0x01) {
-      cmd[0] = 'C';             /* Write Program Mem high byte */
-    }
-    else {
+      cmd[0] = 'C';           // Write program mem high byte
+    } else {
       cmd[0] = 'c';
     }
 
     addr >>= 1;
     PDATA(pgm)->ctype = 0;    // Invalidate read cache
-  }
-  else if (mem_is_eeprom(m)) {
+  } else if (mem_is_eeprom(m)) {
     cmd[0] = 'D';
-  }
-  else {
+  } else {
     return avr_write_byte_default(pgm, p, m, addr, value);
   }
 
@@ -632,22 +628,20 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
                              unsigned int addr, unsigned int n_bytes)
 {
   char cmd[4];
-  int rd_size;
   unsigned int max_addr;
   char buf[2];
-  int rval=0;
+  int isee = mem_is_eeprom(m);
 
   max_addr = addr + n_bytes;
 
-  if (mem_is_flash(m)) {
+  if(mem_is_flash(m))
     cmd[0] = 'R';
-    rd_size = 2;                /* read two bytes per addr */
-  } else if (mem_is_eeprom(m)) {
+  else if(isee)
     cmd[0] = 'd';
-    rd_size = 1;
-  } else {
+  else
     return -2;
-  }
+
+  avr910_set_addr(pgm, isee? addr: addr>>1);
 
   if (PDATA(pgm)->use_blockmode) {
     /* use buffered mode */
@@ -656,8 +650,6 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
     cmd[0] = 'g';
     cmd[3] = toupper((int)(m->desc[0]));
 
-    avr910_set_addr(pgm, addr / rd_size);
-
     while (addr < max_addr) {
       if (max_addr - addr < (unsigned int) blocksize)
         blocksize = max_addr - addr;
@@ -670,36 +662,25 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
 
       addr += blocksize;
     }
-
-    rval = addr;
   } else {
-
-    avr910_set_addr(pgm, addr / rd_size);
-
     while (addr < max_addr) {
       EI(avr910_send(pgm, cmd, 1));
-      if (rd_size == 2) {
-        /* The 'R' command returns two bytes, MSB first, we need to put the data
-           into the memory buffer LSB first. */
+      if(!isee) {
+        // The 'R' command returns two bytes, MSB first, ie, reverse data
         EI(avr910_recv(pgm, buf, 2));
-        m->buf[addr]   = buf[1];  /* LSB */
-        m->buf[addr + 1] = buf[0];  /* MSB */
-      }
-      else {
+        m->buf[addr]   = buf[1];  // LSB
+        m->buf[addr+1] = buf[0];  // MSB
+      } else
         EI(avr910_recv(pgm, (char *) &m->buf[addr], 1));
-      }
 
-      addr += rd_size;
+      addr += isee? 1: 2;
 
-      if (PDATA(pgm)->has_auto_incr_addr != 'Y') {
-        avr910_set_addr(pgm, addr / rd_size);
-      }
+      if (PDATA(pgm)->has_auto_incr_addr != 'Y')
+        avr910_set_addr(pgm, isee? addr: addr>>1);
     }
-
-    rval = addr;
   }
 
-  return rval;
+  return n_bytes;
 }
 
 /* Signature byte reads are always 3 bytes. */

From 7e3854ff2f70044f31bee9556a2baddc5937dd3a Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 20:58:57 +0100
Subject: [PATCH 10/13] Do not use m->desc for memory type: use mem_is_...(m)

---
 src/avr910.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 9a972d1a..929f36b0 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -600,7 +600,7 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
     if (!cmd) return -1;
      
     cmd[0] = 'B';
-    cmd[3] = toupper((int)(m->desc[0]));
+    cmd[3] = isee? 'E': 'F';
 
     while (addr < max_addr) {
       if ((max_addr - addr) < blocksize) {
@@ -648,7 +648,7 @@ static int avr910_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
     int blocksize = PDATA(pgm)->buffersize;
 
     cmd[0] = 'g';
-    cmd[3] = toupper((int)(m->desc[0]));
+    cmd[3] = isee? 'E': 'F';
 
     while (addr < max_addr) {
       if (max_addr - addr < (unsigned int) blocksize)

From 68d8023b6cf402deb997bfe0ac8e8fd7bd2e07ca Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Thu, 18 Apr 2024 21:03:30 +0100
Subject: [PATCH 11/13] Utilise magic memory tree interface for avr910.c

---
 src/avr910.c  | 27 +++++++++++----------------
 src/avrdude.h |  1 +
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index 929f36b0..bd46ddfd 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -76,19 +76,13 @@ struct pdata
 } while(0)
 
 
-static void avr910_setup(PROGRAMMER * pgm)
-{
-  if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) {
-    pmsg_error("out of memory allocating private data\n");
-    exit(1);
-  }
-  memset(pgm->cookie, 0, sizeof(struct pdata));
+static void avr910_setup(PROGRAMMER * pgm) {
+  pgm->cookie = mmt_malloc(sizeof(struct pdata));
   PDATA(pgm)->test_blockmode = 1;
 }
 
-static void avr910_teardown(PROGRAMMER * pgm)
-{
-  free(pgm->cookie);
+static void avr910_teardown(PROGRAMMER * pgm) {
+  mmt_free(pgm->cookie);
 }
 
 
@@ -596,9 +590,8 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
 
     avr910_set_addr(pgm, isee? addr: addr>>1);
 
-    cmd = malloc(4 + blocksize);
-    if (!cmd) return -1;
-     
+    cmd = mmt_malloc(4 + blocksize);
+
     cmd[0] = 'B';
     cmd[3] = isee? 'E': 'F';
 
@@ -610,13 +603,15 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
       cmd[1] = (blocksize >> 8) & 0xff;
       cmd[2] = blocksize & 0xff;
 
-      EI(avr910_send(pgm, cmd, 4 + blocksize));
-      if(avr910_vfy_cmd_sent(pgm, "write block") < 0)
+      if(avr910_send(pgm, cmd, 4 + blocksize) < 0 ||
+         avr910_vfy_cmd_sent(pgm, "write block") < 0) {
+        mmt_free(cmd);
         return -1;
+      }
 
       addr += blocksize;
     }
-    free(cmd);
+    mmt_free(cmd);
   }
 
   return n_bytes;
diff --git a/src/avrdude.h b/src/avrdude.h
index c679a654..6b777d30 100644
--- a/src/avrdude.h
+++ b/src/avrdude.h
@@ -44,6 +44,7 @@ extern const char *pgmid;    // Programmer -c string
 #define mmt_strdup(s) cfg_strdup(__func__, s)
 #define mmt_malloc(n) cfg_malloc(__func__, n)
 #define mmt_realloc(p, n) cfg_realloc(__func__, p, n)
+#define mmt_free(p) free(p)
 
 int avrdude_message2(FILE *fp, int lno, const char *file, const char *func, int msgmode, int msglvl, const char *format, ...);
 

From 1538f3f81154bc45a3346a000633bb0013086e6d Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Mon, 22 Apr 2024 15:58:27 +0100
Subject: [PATCH 12/13] Return LIBAVRDUDE_EXIT instead of exit(0)

---
 src/avr910.c     | 6 +++---
 src/libavrdude.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/avr910.c b/src/avr910.c
index bd46ddfd..80178cfd 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -338,7 +338,7 @@ static int avr910_parseextparms(const PROGRAMMER *pgm, const LISTID extparms) {
       msg_error("  -xdevcode=<arg> Override device code\n");
       msg_error("  -xno_blockmode  Disable default checking for block transfer capability\n");
       msg_error("  -xhelp          Show this help menu and exit\n");
-      exit(0);
+      return LIBAVRDUDE_EXIT;
     }
 
     pmsg_error("invalid extended parameter '%s'\n", extended_param);
@@ -596,9 +596,9 @@ static int avr910_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVR
     cmd[3] = isee? 'E': 'F';
 
     while (addr < max_addr) {
-      if ((max_addr - addr) < blocksize) {
+      if ((max_addr - addr) < blocksize)
         blocksize = max_addr - addr;
-      };
+
       memcpy(&cmd[4], &m->buf[addr], blocksize);
       cmd[1] = (blocksize >> 8) & 0xff;
       cmd[2] = blocksize & 0xff;
diff --git a/src/libavrdude.h b/src/libavrdude.h
index 453b22a0..60cf4ebf 100644
--- a/src/libavrdude.h
+++ b/src/libavrdude.h
@@ -59,6 +59,7 @@ typedef uint32_t pinmask_t;
 #define LIBAVRDUDE_NOTSUPPORTED (-2) // operation not supported
 #define LIBAVRDUDE_SOFTFAIL (-3) // returned, eg, by avr_signature() if caller
                                  // might proceed with chip erase
+#define LIBAVRDUDE_EXIT (-4)     // End all operations in this session
 
 /* formerly lists.h */
 

From 7cc31eb778cab3968a4080f2ce03ffb7bb26041e Mon Sep 17 00:00:00 2001
From: Stefan Rueger <stefan.rueger@urclocks.com>
Date: Mon, 22 Apr 2024 15:59:26 +0100
Subject: [PATCH 13/13] Render double teardown() harmless in avr910.c

---
 src/avr910.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/avr910.c b/src/avr910.c
index 80178cfd..1cc28f18 100644
--- a/src/avr910.c
+++ b/src/avr910.c
@@ -83,6 +83,7 @@ static void avr910_setup(PROGRAMMER * pgm) {
 
 static void avr910_teardown(PROGRAMMER * pgm) {
   mmt_free(pgm->cookie);
+  pgm->cookie = NULL;
 }