Merge pull request #1758 from stefanrueger/mmt-avr910

Make programmer avr910 libavrdude ready

src/avr910.c

@@ -50,23 +50,40 @@ 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))
 
-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));
+// 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) {
+  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);
+  pgm->cookie = NULL;
 }
 
 
@@ -76,14 +93,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);
 }
 
 
@@ -95,11 +105,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;
+    pmsg_error("protocol error for command: %s\n", errmsg);
+    return -1;
   }
+
   return 0;
 }
 
@@ -108,7 +119,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,22 +133,19 @@ 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");
 }
 
 
-/*
- * 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);
 }
 
 
@@ -156,22 +164,22 @@ 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);
+  EI(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);
-  avr910_recv(pgm, sw, sizeof(sw));
+  EI(avr910_send(pgm, "V", 1));
+  EI(avr910_recv(pgm, sw, sizeof(sw)));
 
-  avr910_send(pgm, "v", 1);
-  avr910_recv(pgm, hw, sizeof(hw));
+  EI(avr910_send(pgm, "v", 1));
+  EI(avr910_recv(pgm, hw, sizeof(hw)));
 
   /* Get the programmer type (serial or parallel). Expect serial. */
 
-  avr910_send(pgm, "p", 1);
-  avr910_recv(pgm, &type, 1);
+  EI(avr910_send(pgm, "p", 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]);
@@ -179,20 +187,20 @@ static int avr910_initialize(const PROGRAMMER *pgm, const AVRPART *p) {
 
   /* See if programmer supports autoincrement of address. */
 
-  avr910_send(pgm, "a", 1);
-  avr910_recv(pgm, &PDATA(pgm)->has_auto_incr_addr, 1);
+  EI(avr910_send(pgm, "a", 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");
 
   /* Check support for buffered memory access, ignore if not available */
 
   if (PDATA(pgm)->test_blockmode == 1) {
-    avr910_send(pgm, "b", 1);
-    avr910_recv(pgm, &c, 1);
+    EI(avr910_send(pgm, "b", 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",
@@ -211,11 +219,11 @@ 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) {
-      avr910_recv(pgm, &c, 1);
+    while(1) {
+      EI(avr910_recv(pgm, &c, 1));
       if (devtype_1st == 0)
 	devtype_1st = c;
       if (c == 0)
@@ -251,28 +259,22 @@ 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);
-  avr910_vfy_cmd_sent(pgm, "select device");
+  EI(avr910_send(pgm, buf, 2));
+  if(avr910_vfy_cmd_sent(pgm, "select device") < 0)
+    return -1;
 
   pmsg_notice("avr910_devcode selected: 0x%02x\n", (unsigned) buf[1]);
 
-  avr910_enter_prog_mode(pgm);
-
-  return 0;
+  return pgm->program_enable(pgm, p);
 }
 
 
 static void avr910_disable(const PROGRAMMER *pgm) {
-  /* Do nothing. */
-
-  return;
+  avr910_leave_prog_mode(pgm);
 }
 
 
 static void avr910_enable(PROGRAMMER *pgm, const AVRPART *p) {
-  /* Do nothing. */
-
-  return;
 }
 
 
@@ -293,8 +295,8 @@ static int avr910_cmd(const PROGRAMMER *pgm, const unsigned char *cmd,
   buf[3] = cmd[2];
   buf[4] = cmd[3];
 
-  avr910_send (pgm, buf, 5);
-  avr910_recv (pgm, buf, 2);
+  EI(avr910_send(pgm, buf, 5));
+  EI(avr910_recv(pgm, buf, 2));
 
   res[0] = 0x00;                /* Dummy value */
   res[1] = cmd[0];
@@ -337,7 +339,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);
@@ -350,32 +352,22 @@ 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;
   pinfo.serialinfo.cflags = SERIAL_8N1;
-  if (serial_open(port, pinfo, &pgm->fd)==-1) {
+  if(serial_open(port, pinfo, &pgm->fd) < 0)
     return -1;
-  }
 
-  /*
-   * drain any extraneous input
-   */
-  avr910_drain (pgm, 0);
+  (void) avr910_drain (pgm, 0);
 	
   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;
 }
@@ -393,7 +385,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");
 }
 
@@ -405,18 +397,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;
-  }
-  else if (mem_is_eeprom(m)) {
+    PDATA(pgm)->ctype = 0;    // Invalidate read cache
+  } else if (mem_is_eeprom(m)) {
     cmd[0] = 'D';
-  }
-  else {
+  } else {
     return avr_write_byte_default(pgm, p, m, addr, value);
   }
 
@@ -424,10 +414,8 @@ static int avr910_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
 
   avr910_set_addr(pgm, addr);
 
-  avr910_send(pgm, cmd, sizeof(cmd));
-  avr910_vfy_cmd_sent(pgm, "write byte");
-
-  return 0;
+  EI(avr910_send(pgm, cmd, sizeof(cmd)));
+  return avr910_vfy_cmd_sent(pgm, "write byte");
 }
 
 
@@ -436,19 +424,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);
 
-  avr910_send(pgm, "R", 1);
+  EI(avr910_send(pgm, "R", 1));
+  EI(avr910_recv(pgm, buf, sizeof(buf)));
 
-  /* 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];
-  }
+  *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;
 }
@@ -458,8 +447,8 @@ 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);
-  avr910_recv(pgm, (char *)value, 1);
+  EI(avr910_send(pgm, "d", 1));
+  EI(avr910_recv(pgm, (char *) value, 1));
 
   return 0;
 }
@@ -491,6 +480,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);
 
@@ -498,8 +489,9 @@ 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));
-    avr910_vfy_cmd_sent(pgm, "write byte");
+    EI(avr910_send(pgm, buf, sizeof(buf)));
+    if(avr910_vfy_cmd_sent(pgm, "write byte") < 0)
+      return -1;
 
     addr++;
     page_bytes--;
@@ -508,8 +500,9 @@ 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);
-      avr910_vfy_cmd_sent(pgm, "flush page");
+      EI(avr910_send(pgm, "m", 1));
+      if(avr910_vfy_cmd_sent(pgm, "flush page") < 0)
+        return -1;
 
       page_wr_cmd_pending = 0;
       usleep(m->max_write_delay);
@@ -530,12 +523,13 @@ 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);
-    avr910_vfy_cmd_sent(pgm, "flush final page");
+    EI(avr910_send(pgm, "m", 1));
+    if(avr910_vfy_cmd_sent(pgm, "flush final page") < 0)
+      return -1;
     usleep(m->max_write_delay);
   }
 
-  return addr;
+  return n_bytes;
 }
 
 
@@ -553,18 +547,18 @@ 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));
-    avr910_vfy_cmd_sent(pgm, "write byte");
+    EI(avr910_send(pgm, cmd, sizeof(cmd)));
+    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;
 }
 
 
@@ -572,59 +566,56 @@ 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 = mmt_malloc(4 + blocksize);
 
-    cmd = malloc(4 + blocksize);
-    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) {
+      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;
 
-      avr910_send(pgm, cmd, 4 + blocksize);
-      avr910_vfy_cmd_sent(pgm, "write block");
+      if(avr910_send(pgm, cmd, 4 + blocksize) < 0 ||
+         avr910_vfy_cmd_sent(pgm, "write block") < 0) {
+        mmt_free(cmd);
+        return -1;
+      }
 
       addr += blocksize;
-    } /* while */
-    free(cmd);
-
-    rval = addr;
+    }
+    mmt_free(cmd);
   }
-  return rval;
+
+  return n_bytes;
 }
 
 
@@ -633,31 +624,27 @@ 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 */
     int blocksize = PDATA(pgm)->buffersize;
 
     cmd[0] = 'g';
-    cmd[3] = toupper((int)(m->desc[0]));
-
-    avr910_set_addr(pgm, addr / rd_size);
+    cmd[3] = isee? 'E': 'F';
 
     while (addr < max_addr) {
       if (max_addr - addr < (unsigned int) blocksize)
@@ -666,41 +653,30 @@ 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);
-      avr910_recv(pgm, (char *)&m->buf[addr], blocksize);
+      EI(avr910_send(pgm, cmd, 4));
+      EI(avr910_recv(pgm, (char *) &m->buf[addr], blocksize));
 
       addr += blocksize;
     }
-
-    rval = addr;
   } else {
-
-    avr910_set_addr(pgm, addr / rd_size);
-
     while (addr < max_addr) {
-      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. */
-        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_send(pgm, cmd, 1));
+      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
+        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. */
@@ -713,8 +689,8 @@ static int avr910_read_sig_bytes(const PROGRAMMER *pgm, const AVRPART *p, const
     return -1;
   }
 
-  avr910_send(pgm, "s", 1);
-  avr910_recv(pgm, (char *)m->buf, 3);
+  EI(avr910_send(pgm, "s", 1));
+  EI(avr910_recv(pgm, (char *) m->buf, 3));
   /* Returned signature has wrong order. */
   tmp = m->buf[2];
   m->buf[2] = m->buf[0];