* main.c: revert to rev 1159 (doing pgm_display after pgm_open)

* avrpart.[ch]: moved avr_pin_name to pindefs.[ch]
* pgm.c: moved pins_to_str to pindefs.[ch], added initialization of 
          new pin definitions in pgm_new()
* pindefs.[ch]: added moved functions from other files, added a lot of 
          documentation, reformatted files using astyle to have consistent spacing,
          added a new generic check function for pins
* ft245r.c: used new generic pin check function

git-svn-id: svn://svn.savannah.nongnu.org/avrdude/trunk/avrdude@1161 81a1dc3b-b13d-400b-aceb-764788c761c2

ChangeLog

@@ -1,3 +1,14 @@
+2013-05-05  Rene Liebscher <R.Liebscher@gmx.de>
+
+	* main.c: revert to rev 1159 (doing pgm_display after pgm_open)
+	* avrpart.[ch]: moved avr_pin_name to pindefs.[ch]
+	* pgm.c: moved pins_to_str to pindefs.[ch], added initialization of 
+          new pin definitions in pgm_new()
+	* pindefs.[ch]: added moved functions from other files, added a lot of 
+          documentation, reformatted files using astyle to have consistent spacing,
+          added a new generic check function for pins
+	* ft245r.c: used new generic pin check function
+
 2013-05-03  Rene Liebscher <R.Liebscher@gmx.de>
 
 	Create new pin definition data structures to support 0-based pin numbers,

avrpart.c

@@ -595,24 +595,6 @@ static char * reset_disp_str(int r)
 }
 
 
-const char * avr_pin_name(int pinno)
-{
-  switch (pinno) {
-    case PPI_AVR_VCC   : return "VCC";
-    case PPI_AVR_BUFF  : return "BUFF";
-    case PIN_AVR_RESET : return "RESET";
-    case PIN_AVR_SCK   : return "SCK";
-    case PIN_AVR_MOSI  : return "MOSI";
-    case PIN_AVR_MISO  : return "MISO";
-    case PIN_LED_ERR   : return "ERRLED";
-    case PIN_LED_RDY   : return "RDYLED";
-    case PIN_LED_PGM   : return "PGMLED";
-    case PIN_LED_VFY   : return "VFYLED";
-    default : return "<unknown>";
-  }
-}
-
-
 void avr_display(FILE * f, AVRPART * p, const char * prefix, int verbose)
 {
   int i;

avrpart.h

@@ -222,7 +222,6 @@ void      avr_free_part(AVRPART * d);
 AVRPART * locate_part(LISTID parts, char * partdesc);
 AVRPART * locate_part_by_avr910_devcode(LISTID parts, int devcode);
 void avr_display(FILE * f, AVRPART * p, const char * prefix, int verbose);
-const char * avr_pin_name(int);
 
 typedef void (*walk_avrparts_cb)(const char *name, const char *desc,
                                  const char *cfgname, int cfglineno,

ft245r.c

@@ -328,34 +328,6 @@ static int ft245r_read_sig_bytes(PROGRAMMER * pgm, AVRPART * p, AVRMEM * m) {
     return 3;
 }
 
-static int ft245r_check_pins(PROGRAMMER * pgm){
-    static const int pinlist[] = {PIN_AVR_SCK,PIN_AVR_MOSI,PIN_AVR_MISO,PIN_AVR_RESET,PPI_AVR_BUFF};
-    static const pinmask_t valid_pins[PIN_FIELD_SIZE] = { 0x000000ff }; // only lower 8 pins are allowed
-    pinmask_t already_used[PIN_FIELD_SIZE] = {0};
-    int i,j;
-
-    for( i=0; i<sizeof(pinlist)/sizeof(pinlist[0]); i++){
-        for( j=0; j<PIN_FIELD_SIZE; j++){
-            // check if it does not use any non valid pins
-            if(pgm->pin[pinlist[i]].mask[j] & ~valid_pins[j]){
-                fprintf(stderr,
-                    "%s: at least one pin is not a valid pin number\n",
-                    progname);
-                exit(1);
-            }
-            // check if it does not use same pins as other function
-            if(pgm->pin[pinlist[i]].mask[j] & already_used[j]){
-                fprintf(stderr,
-                    "%s: at least one pin is set for multiple functions.\n",
-                    progname);
-                exit(1);
-            }
-            already_used[j] |= pgm->pin[pinlist[i]].mask[j];
-        }
-    }
-    return 0;
-}
-
 /*
  * initialize the AVR device and prepare it to accept commands
  */
@@ -456,12 +428,26 @@ static int ft245r_cmd(PROGRAMMER * pgm, unsigned char cmd[4],
     return 0;
 }
 
+/* lower 8 pins are accepted, they might be also inverted */
+static const struct pindef_t valid_pins = {{0xff},{0xff}} ;
+
+static const struct pin_checklist_t pin_checklist[] = {
+    { PIN_AVR_SCK,  1, &valid_pins},
+    { PIN_AVR_MOSI, 1, &valid_pins},
+    { PIN_AVR_MISO, 1, &valid_pins},
+    { PIN_AVR_RESET,1, &valid_pins},
+    { PPI_AVR_BUFF, 0, &valid_pins},
+};
 
 static int ft245r_open(PROGRAMMER * pgm, char * port) {
     int rv;
     int devnum = -1;
 
-    ft245r_check_pins(pgm);
+    rv = pins_check(pgm,pin_checklist,sizeof(pin_checklist)/sizeof(pin_checklist[0]));
+    if(rv) {
+        pgm->display(pgm, progbuf);
+        return rv;
+    }
 
     strcpy(pgm->port, port);
 
@@ -556,6 +542,7 @@ static int ft245r_open(PROGRAMMER * pgm, char * port) {
 
 
 static void ft245r_close(PROGRAMMER * pgm) {
+    if (handle) {
         // I think the switch to BB mode and back flushes the buffer.
         ftdi_set_bitmode(handle, 0, BITMODE_SYNCBB); // set Synchronous BitBang, all in puts
         ftdi_set_bitmode(handle, 0, BITMODE_RESET); // disable Synchronous BitBang
@@ -564,6 +551,8 @@ static void ft245r_close(PROGRAMMER * pgm) {
         ftdi_usb_close(handle);
         ftdi_deinit (handle);
         free(handle);
+        handle = NULL;
+    }
 }
 
 static void ft245r_display(PROGRAMMER * pgm, const char * p) {
@@ -849,6 +838,8 @@ void ft245r_initpgm(PROGRAMMER * pgm) {
     pgm->paged_load = ft245r_paged_load;
 
     pgm->read_sig_bytes = ft245r_read_sig_bytes;
+
+    handle = NULL;
 }
 
 #else

main.c

@@ -921,12 +921,6 @@ int main(int argc, char * argv [])
     pgm->ispdelay = ispdelay;
   }
 
-  if (verbose) {
-    avr_display(stderr, p, progbuf, verbose);
-    fprintf(stderr, "\n");
-    programmer_display(pgm, progbuf);
-  }
-
   rc = pgm->open(pgm, port);
   if (rc < 0) {
     exitrc = 1;
@@ -957,6 +951,12 @@ int main(int argc, char * argv [])
     goto main_exit;
   }
 
+  if (verbose) {
+    avr_display(stderr, p, progbuf, verbose);
+    fprintf(stderr, "\n");
+    programmer_display(pgm, progbuf);
+  }
+
   if (quell_progress < 2) {
     fprintf(stderr, "\n");
   }

pgm.c

@@ -83,8 +83,10 @@ PROGRAMMER * pgm_new(void)
   pgm->baudrate = 0;
   pgm->initpgm = NULL;
 
-  for (i=0; i<N_PINS; i++)
+  for (i=0; i<N_PINS; i++) {
     pgm->pinno[i] = 0;
+    pin_clear_all(&(pgm->pin[i]));
+  }
 
   /*
    * mandatory functions - these are called without checking to see
@@ -211,34 +213,6 @@ void programmer_display(PROGRAMMER * pgm, const char * p)
   pgm->display(pgm, p);
 }
 
-static char * pins_to_str(const struct pindef_t * const pindef)
-{
-  static char buf[(PIN_MAX+1)*5]; // should be enough for PIN_MAX=255
-  char *p = buf;
-  int n;
-  int pin;
-  const char * fmt;
-
-  buf[0] = 0;
-  for (pin = PIN_MIN; pin <= PIN_MAX; pin++) {
-    int index = pin/PIN_FIELD_ELEMENT_SIZE;
-    int bit = pin%PIN_FIELD_ELEMENT_SIZE;
-    if (pindef->mask[index] & (1 << bit)) {
-      if (pindef->inverse[index] & (1 << bit)) {
-         fmt = (buf[0]==0)?"~%d":",~%d";
-      } else {
-         fmt = (buf[0]==0)?" %d":",%d";
-      }
-      n = sprintf(p, fmt, pin);
-      p += n;
-    }
-  }
-
-  if (buf[0] == 0)
-     return " (not used)";
-
-  return buf;
-}
 
 void pgm_display_generic_mask(PROGRAMMER * pgm, const char * p, unsigned int show)
 {

pindefs.c

@@ -21,6 +21,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include "avrdude.h"
 #include "pindefs.h"
 #include "pgm.h"
 
@@ -34,11 +35,12 @@
 void pin_set_value(struct pindef_t * const pindef, const int pin, const bool inverse) {
 
   pindef->mask[pin / PIN_FIELD_ELEMENT_SIZE] |= 1 << (pin % PIN_FIELD_ELEMENT_SIZE);
-  if (inverse)
+  if(inverse) {
     pindef->inverse[pin / PIN_FIELD_ELEMENT_SIZE] |= (1 << (pin % PIN_FIELD_ELEMENT_SIZE));
-  else
+  } else {
     pindef->inverse[pin / PIN_FIELD_ELEMENT_SIZE] &= ~(1 << (pin % PIN_FIELD_ELEMENT_SIZE));
   }
+}
 
 /**
  * Clear all defined pins in pindef.
@@ -124,3 +126,233 @@ void pgm_fill_old_pins(struct programmer_t * const pgm) {
   pin_fill_old_pinno(&(pgm->pin[PIN_LED_VFY]),  &(pgm->pinno[PIN_LED_VFY]));
 
 }
+
+/**
+ * This function returns a string representation of pins in the mask eg. 1,3,5-7,9,12
+ * Another execution of this function will overwrite the previous result in the static buffer.
+ * Consecutive pin number are representated as start-end.
+ *
+ * @param[in] pinmask the pin mask for which we want the string representation
+ * @returns pointer to a static string.
+ */
+const char * pinmask_to_str(const pinmask_t * const pinmask) {
+  static char buf[(PIN_MAX + 1) * 5]; // should be enough for PIN_MAX=255
+  char *p = buf;
+  int n;
+  int pin;
+  const char * fmt;
+  int start = -1;
+  int end = -1;
+
+  buf[0] = 0;
+  for(pin = PIN_MIN; pin <= PIN_MAX; pin++) {
+    int index = pin / PIN_FIELD_ELEMENT_SIZE;
+    int bit = pin % PIN_FIELD_ELEMENT_SIZE;
+    if(pinmask[index] & (1 << bit)) {
+      bool output = false;
+      if(start == -1) {
+        output = true;
+        start = pin;
+        end = start;
+      } else if(pin == end + 1) {
+        end = pin;
+      } else {
+        if(start != end) {
+          n = sprintf(p, "-%d", end);
+          p += n;
+        }
+        output = true;
+        start = pin;
+        end = start;
+      }
+      if(output) {
+        fmt = (buf[0] == 0) ? "%d" : ",%d";
+        n = sprintf(p, fmt, pin);
+        p += n;
+      }
+    }
+  }
+  if(start != end) {
+    n = sprintf(p, "-%d", end);
+    p += n;
+  }
+
+  if(buf[0] == 0)
+    return  "(no pins)";
+
+  return buf;
+}
+
+
+/**
+ * This function checks all pin of pgm against the constraints given in the checklist.
+ * It checks if 
+ * @li any invalid pins are used
+ * @li valid pins are used inverted when not allowed
+ * @li any pins are used by more than one function
+ * @li any mandatory pin is not set all.
+ *
+ * In case of any error it report the wrong function and the pin numbers.
+ * For verbose >= 2 it also reports the possible correct values.
+ * For verbose >=3 it shows also which pins were ok.
+ *
+ * @param[in] pgm the programmer to check
+ * @param[in] checklist the constraint for the pins
+ * @param[in] size the number of entries in checklist
+ * @returns 0 if all pin definitions are valid, -1 otherwise
+ */
+int pins_check(const struct programmer_t * const pgm, const struct pin_checklist_t * const checklist, const int size) {
+  static const struct pindef_t no_valid_pins = {{0}, {0}}; // default value if check list does not contain anything else
+  int rv = 0; // return value
+  int pinname; // loop counter through pinnames
+  pinmask_t already_used_all[PIN_FIELD_SIZE] = {0}; // collect pin definitions of all pin names for check of double use
+  // loop over all possible pinnames
+  for(pinname = 0; pinname < N_PINS; pinname++) {
+    bool used = false;
+    bool invalid = false;
+    bool inverse = false;
+    int index;
+    int segment;
+    bool mandatory_used = false;
+    pinmask_t invalid_used[PIN_FIELD_SIZE] = {0};
+    pinmask_t inverse_used[PIN_FIELD_SIZE] = {0};
+    pinmask_t already_used[PIN_FIELD_SIZE] = {0};
+    const struct pindef_t * valid_pins = &no_valid_pins;
+    bool is_mandatory = false;
+    bool is_ok = true;
+    //find corresponding check pattern
+    for(index = 0; index < size; index++) {
+      if(checklist[index].pinname == pinname) {
+        valid_pins = checklist[index].valid_pins;
+        is_mandatory = checklist[index].mandatory;
+        break;
+      }
+    }
+
+    for(segment = 0; segment < PIN_FIELD_SIZE; segment++) {
+      // check if for mandatory any pin is defined
+      invalid_used[segment] = pgm->pin[pinname].mask[segment] & ~valid_pins->mask[segment];
+      if(is_mandatory && (0 != (pgm->pin[pinname].mask[segment] & valid_pins->mask[segment]))) {
+        mandatory_used = true;
+      }
+      // check if it does not use any non valid pins
+      invalid_used[segment] = pgm->pin[pinname].mask[segment] & ~valid_pins->mask[segment];
+      if(invalid_used[segment]) {
+        invalid = true;
+      }
+      // check if it does not use any valid pins as inverse if not allowed
+      inverse_used[segment] = pgm->pin[pinname].inverse[segment] & valid_pins->mask[segment] & ~valid_pins->inverse[segment];
+      if(inverse_used[segment]) {
+        inverse = true;
+      }
+      // check if it does not use same pins as other function
+      already_used[segment] = pgm->pin[pinname].mask[segment] & already_used_all[segment];
+      if(already_used[segment]) {
+        used = true;
+      }
+      already_used_all[segment] |= pgm->pin[pinname].mask[segment];
+    }
+    if(invalid) {
+      fprintf(stderr,
+              "%s: %s: Following pins are not valid pins for this function: %s\n",
+              progname, avr_pin_name(pinname), pinmask_to_str(invalid_used));
+      if(verbose >= 2) {
+        fprintf(stderr,
+                "%s: %s: Valid pins for this function are: %s\n",
+                progname, avr_pin_name(pinname), pinmask_to_str(valid_pins->mask));
+      }
+      is_ok = false;
+    }
+    if(inverse) {
+      fprintf(stderr,
+              "%s: %s: Following pins are not usable as inverse pins for this function: %s\n",
+              progname, avr_pin_name(pinname), pinmask_to_str(inverse_used));
+      if(verbose >= 2) {
+        fprintf(stderr,
+                "%s: %s: Valid inverse pins for this function are: %s\n",
+                progname, avr_pin_name(pinname), pinmask_to_str(valid_pins->inverse));
+      }
+      is_ok = false;
+    }
+    if(used) {
+      fprintf(stderr,
+              "%s: %s: Following pins are set for other functions too: %s\n",
+              progname, avr_pin_name(pinname), pinmask_to_str(already_used));
+      is_ok = false;
+    }
+    if(!mandatory_used && is_mandatory && !invalid) {
+      fprintf(stderr,
+              "%s: %s: Mandatory pin is not defined.\n",
+              progname, avr_pin_name(pinname));
+      is_ok = false;
+    }
+    if(!is_ok) {
+      rv = -1;
+    } else if(verbose >= 3) {
+      fprintf(stderr,
+              "%s: %s: Pin is ok.\n",
+              progname, avr_pin_name(pinname));
+
+    }
+  }
+  return rv;
+}
+
+/**
+ * This function returns a string representation of defined pins eg. ~1,2,~4,~5,7
+ * Another execution of this function will overwrite the previous result in the static buffer.
+ *
+ * @param[in] pindef the pin definition for which we want the string representation
+ * @returns pointer to a static string.
+ */
+const char * pins_to_str(const struct pindef_t * const pindef) {
+  static char buf[(PIN_MAX + 1) * 5]; // should be enough for PIN_MAX=255
+  char *p = buf;
+  int n;
+  int pin;
+  const char * fmt;
+
+  buf[0] = 0;
+  for(pin = PIN_MIN; pin <= PIN_MAX; pin++) {
+    int index = pin / PIN_FIELD_ELEMENT_SIZE;
+    int bit = pin % PIN_FIELD_ELEMENT_SIZE;
+    if(pindef->mask[index] & (1 << bit)) {
+      if(pindef->inverse[index] & (1 << bit)) {
+        fmt = (buf[0] == 0) ? "~%d" : ",~%d";
+      } else {
+        fmt = (buf[0] == 0) ? " %d" : ",%d";
+      }
+      n = sprintf(p, fmt, pin);
+      p += n;
+    }
+  }
+
+  if(buf[0] == 0)
+    return " (not used)";
+
+  return buf;
+}
+
+/**
+ * Returns the name of the pin as string.
+ * 
+ * @param pinname the pinname which we want as string.
+ * @returns a string with the pinname, or <unknown> if pinname is invalid.
+ */
+const char * avr_pin_name(int pinname) {
+  switch(pinname) {
+  case PPI_AVR_VCC   : return "VCC";
+  case PPI_AVR_BUFF  : return "BUFF";
+  case PIN_AVR_RESET : return "RESET";
+  case PIN_AVR_SCK   : return "SCK";
+  case PIN_AVR_MOSI  : return "MOSI";
+  case PIN_AVR_MISO  : return "MISO";
+  case PIN_LED_ERR   : return "ERRLED";
+  case PIN_LED_RDY   : return "RDYLED";
+  case PIN_LED_PGM   : return "PGMLED";
+  case PIN_LED_VFY   : return "VFYLED";
+  default : return "<unknown>";
+  }
+}
+
+

pindefs.h

@@ -110,12 +110,86 @@ struct pindef_t {
   pinmask_t inverse[PIN_FIELD_SIZE]; ///< bitfield of inverse/normal usage of used pins
 };
 
+/**
+ * Data structure to define a checklist of valid pins for each function.
+ */
+struct pin_checklist_t {
+  int pinname; ///< logical pinname eg. PIN_AVR_SCK
+  int mandatory; ///< is this a mandatory pin
+  const struct pindef_t* valid_pins; ///< mask defines allowed pins, inverse define is they might be used inverted
+};
+
+/**
+ * Adds a pin in the pin definition as normal or inverse pin.
+ *
+ * @param[out] pindef pin definition to update
+ * @param[in] pin number of pin [0..PIN_MAX]
+ * @param[in] inverse inverse (true) or normal (false) pin
+ */
 void pin_set_value(struct pindef_t * const pindef, const int pin, const bool inverse);
 
+/**
+ * Clear all defined pins in pindef.
+ *
+ * @param[out] pindef pin definition to clear
+ */
 void pin_clear_all(struct pindef_t * const pindef);
 
 struct programmer_t; /* forward declaration */
+
+/**
+ * Convert for given programmer new pin definitions to old pin definitions.
+ *
+ * @param[inout] pgm programmer whose pins shall be converted.
+ */
 void pgm_fill_old_pins(struct programmer_t * const pgm);
 
+/**
+ * This function checks all pin of pgm against the constraints given in the checklist.
+ * It checks if 
+ * @li any invalid pins are used
+ * @li valid pins are used inverted when not allowed
+ * @li any pins are used by more than one function
+ * @li any mandatory pin is not set all.
+ *
+ * In case of any error it report the wrong function and the pin numbers.
+ * For verbose >= 2 it also reports the possible correct values.
+ * For verbose >=3 it shows also which pins were ok.
+ *
+ * @param[in] pgm the programmer to check
+ * @param[in] checklist the constraint for the pins
+ * @param[in] size the number of entries in checklist
+ * @returns 0 if all pin definitions are valid, -1 otherwise
+ */
+int pins_check(const struct programmer_t * const pgm, const struct pin_checklist_t * const checklist, const int size);
+
+/**
+ * Returns the name of the pin as string.
+ * 
+ * @param pinname the pinname which we want as string.
+ * @returns a string with the pinname, or <unknown> if pinname is invalid.
+ */
+const char * avr_pin_name(int pinname);
+
+/**
+ * This function returns a string representation of defined pins eg. ~1,2,~4,~5,7
+ * Another execution of this function will overwrite the previous result in the static buffer.
+ *
+ * @param[in] pindef the pin definition for which we want the string representation
+ * @returns pointer to a static string.
+ */
+const char * pins_to_str(const struct pindef_t * const pindef);
+
+/**
+ * This function returns a string representation of pins in the mask eg. 1,3,5-7,9,12
+ * Another execution of this function will overwrite the previous result in the static buffer.
+ * Consecutive pin number are representated as start-end.
+ *
+ * @param[in] pinmask the pin mask for which we want the string representation
+ * @returns pointer to a static string.
+ */
+const char * pinmask_to_str(const pinmask_t * const pinmask);
+
+
 #endif