Hi,

I have here now a patch which introduces the new pin definition format into the programmer structure.

At the moment the config parser writes the new definitions and all bitbanging programmers call in their initpgm function a function which creates the corresponding values in the old pinno elements of the programmers structure.

So all existing code should run as usual and we can start to convert one programmer after another. If it uses the new format, we can remove then the call to the converter function, and if the last programmer removed it, we can also remove the old data field.

May be you want have a look in the patch if there are any obvious problems. Otherwise I will check in it the next days.

I think all changes should be done before releasing version 6 as we have changed already the config file format. When changing now the pin numbers for some programmers from 1-based to 0-based this will result in nobody having an old version 5 config file lying around which would be parsed without the need to manually correcting it. (Also the new programmers which would be 0-based (avrftdi,ftdi_syncbb,linuxgpio) would all be new in version 6.)


Kind regards

René Liebscher


On 05.01.2013 17:35, Hannes Weisbach wrote:
You have the same behaviour for ftdi based prgrammers, where you have to
write 4 when you actually mean ADBUS3.

I would prefer a much more general approach to this problem.

You have to remember that there are not only single pins but also list
of pins are possible for vcc and buf pin definitions. And these define
their pins as 1<< pin1 | 1 << pin2 | ... . So setting the default to -1
would set there all bits if there is no definition in the conf-file.

I would like to have the pin definition as follows:

typedef struct programmer_t {
...
  unsigned int pinno[N_PINS];
...
}

changed to

typedef struct programmer_t {
...
  pin_def_t pinno[N_PINS];
...
}

with pin_def_t defined as:

#define N_MAX_PINS 32 /* or 256 if GPIO is compiled in */

struct {
    uint32_t mask   [N_MAX_PINS/32];
    uint32_t inverse[N_MAX_PINS/32];
} pin_def_t;

Then you have a single pin defined as:
mask = { 0b0001000, .... }
inverse = { 0b0000000, .... }

an inverted pin as:

mask = { 0b0001000, .... }
inverse = { 0b0001000, .... }

pin lists as:

mask = { 0b0010110, .... }
inverse = { 0b0000000, .... }

and it would even be possible to define a mixed inverted pin list as
wished in bug #37727 Add support for LM3S811 dev board as a programmer

mask = { 0b0010110, .... }
inverse = { 0b0010010, .... }

However you had to change the code for the following programmer types
avrftdi, ftdi_syncbb, par and serbb. (These are the only ones which
currently use pin definitions in the config file.)

I would also propose to change then pin numbers of ftdi based
programmers to 0 based numbers in the config file. But leave them for
parallel and serial port 1-based as they correspond to real pins at
their connectors.
I think it's a good idea and would provide the necessary changes to avrftdi, if 
this proposal is generally accepted and you provide the definitions.

Best regards,
Hannes

Index: pindefs.c
===================================================================
--- pindefs.c	(Revision 0)
+++ pindefs.c	(Arbeitskopie)
@@ -0,0 +1,98 @@
+/*
+ * avrdude - A Downloader/Uploader for AVR device programmers
+ * Copyright (C) 2000-2004  Brian S. Dean <b...@bsdhome.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* $Id: pindefs.h 1132 2013-01-09 19:23:30Z rliebscher $ */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include "pindefs.h"
+#include "pgm.h"
+
+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)
+    pindef->inverse[pin/PIN_FIELD_ELEMENT_SIZE] |=  (1 << (pin % PIN_FIELD_ELEMENT_SIZE));
+  else
+    pindef->inverse[pin/PIN_FIELD_ELEMENT_SIZE] &= ~(1 << (pin % PIN_FIELD_ELEMENT_SIZE));
+}
+
+void pin_clear_all(struct pindef_t * const pindef){
+    memset(pindef,0,sizeof(struct pindef_t));
+}
+
+/* Convert new pin definition to old pin number */
+static void pin_fill_old_pinno(const struct pindef_t * const pindef, unsigned int * const pinno){
+    bool found = false;
+    int i;
+    for (i=0;i<PIN_MAX;i++){
+      if (pindef->mask[i/PIN_FIELD_ELEMENT_SIZE] & (1 << (i % PIN_FIELD_ELEMENT_SIZE))){
+        if(found){
+	    fprintf(stderr,"Multiple pins found\n"); //TODO
+	    exit(1);
+	}
+	found = true;
+	*pinno = i;
+	if (pindef->inverse[i/PIN_FIELD_ELEMENT_SIZE] & (1 << (i % PIN_FIELD_ELEMENT_SIZE))){
+	    *pinno |= PIN_INVERSE;
+	}
+      }
+    }
+}
+
+/* Convert new pin definition to old pinlist, does not support mixed inverted/non-inverted pin */
+static void pin_fill_old_pinlist(const struct pindef_t * const pindef, unsigned int * const pinno){
+    int i;
+
+    for (i=0;i<PIN_FIELD_SIZE;i++){
+      if(i == 0) {
+	if ((pindef->mask[i] & ~PIN_MASK) != 0){
+	    fprintf(stderr,"Pins of higher index than max field size for old pinno found\n");
+	    exit(1);
+	}
+	if (pindef->mask[i] == pindef->inverse[i]) { /* all set bits in mask are set in inverse */
+	    *pinno = pindef->mask[i];
+	    *pinno |= PIN_INVERSE;
+	} else if (pindef->mask[i] == ((~pindef->inverse[i]) & pindef->mask[i])) { /* all set bits in mask are cleared in inverse */
+	    *pinno = pindef->mask[i];
+	} else {
+	    fprintf(stderr,"pins have different polarity set\n");
+	    exit(1);
+        }
+      } else if (pindef->mask[i] != 0){
+	    fprintf(stderr,"Pins have higher number than fit in old format\n");
+	    exit(1);
+      }
+    }
+}
+
+void pgm_fill_old_pins(struct programmer_t * const pgm) {
+
+  pin_fill_old_pinlist(&(pgm->pin[PPI_AVR_VCC]),  &(pgm->pinno[PPI_AVR_VCC]));
+  pin_fill_old_pinlist(&(pgm->pin[PPI_AVR_BUFF]), &(pgm->pinno[PPI_AVR_BUFF]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_AVR_RESET]),&(pgm->pinno[PIN_AVR_RESET]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_AVR_SCK]),  &(pgm->pinno[PIN_AVR_SCK]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_AVR_MOSI]), &(pgm->pinno[PIN_AVR_MOSI]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_AVR_MISO]), &(pgm->pinno[PIN_AVR_MISO]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_LED_ERR]),  &(pgm->pinno[PIN_LED_ERR]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_LED_RDY]),  &(pgm->pinno[PIN_LED_RDY]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_LED_PGM]),  &(pgm->pinno[PIN_LED_PGM]));
+  pin_fill_old_pinno(  &(pgm->pin[PIN_LED_VFY]),  &(pgm->pinno[PIN_LED_VFY]));
+
+}
\ Kein Zeilenumbruch am Ende der Datei
Index: pindefs.h
===================================================================
--- pindefs.h	(Revision 1144)
+++ pindefs.h	(Arbeitskopie)
@@ -23,6 +23,17 @@
 
 #include <limits.h>
 
+/* lets try to select at least 32 bits */
+#ifdef HAVE_SDTINT_H
+#include <stdint.h>
+typedef uint32_t pinmask_t;
+#else
+typedef unsigned long pinmask_t;
+#endif
+
+#include <stdbool.h>
+
+
 enum {
   PPI_AVR_VCC=1,
   PPI_AVR_BUFF,
@@ -36,9 +47,32 @@
   PIN_LED_VFY,
   N_PINS
 };
+
 #define PIN_MASK    (UINT_MAX>>1)
 #define PIN_INVERSE (~(PIN_MASK))	/* flag for inverted pin in serbb */
-#define PIN_MIN     1   /* smallest allowed pin number */
+#define PIN_MIN     0   /* smallest allowed pin number */
+#define PIN_MAX     31  /* largest allowed pin number */
+
+#ifdef HAVE_LINUX_GPIO
+/* Embedded systems might have a lot more gpio than only 0-31 */
+#undef PIN_MAX
 #define PIN_MAX     255 /* largest allowed pin number */
+#endif
 
+#define PIN_FIELD_ELEMENT_SIZE (sizeof(pinmask_t) * 8)
+#define PIN_FIELD_SIZE ((PIN_MAX + PIN_FIELD_ELEMENT_SIZE)/PIN_FIELD_ELEMENT_SIZE)
+
+struct pindef_t {
+    pinmask_t mask[PIN_FIELD_SIZE];
+    pinmask_t inverse[PIN_FIELD_SIZE];
+};
+
+void pin_set_value(struct pindef_t * const pindef, const int pin, const bool inverse);
+
+void pin_clear_all(struct pindef_t * const pindef);
+
+struct programmer_t; /* forward declaration */
+void pgm_fill_old_pins(struct programmer_t * const pgm);
+
 #endif
+
Index: serbb_win32.c
===================================================================
--- serbb_win32.c	(Revision 1144)
+++ serbb_win32.c	(Arbeitskopie)
@@ -350,6 +350,8 @@
 {
   strcpy(pgm->type, "SERBB");
 
+  pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
   pgm->rdy_led        = bitbang_rdy_led;
   pgm->err_led        = bitbang_err_led;
   pgm->pgm_led        = bitbang_pgm_led;
Index: Makefile.am
===================================================================
--- Makefile.am	(Revision 1144)
+++ Makefile.am	(Arbeitskopie)
@@ -138,6 +138,7 @@
 	pgm_type.h \
 	pickit2.c \
 	pickit2.h \
+	pindefs.c \
 	pindefs.h \
 	ppi.c \
 	ppi.h \
Index: buspirate.c
===================================================================
--- buspirate.c	(Revision 1144)
+++ buspirate.c	(Arbeitskopie)
@@ -1283,6 +1283,8 @@
 {
 	strcpy(pgm->type, "BusPirate_BB");
 
+	pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
 	pgm->display        = buspirate_dummy_6;
 
 	/* BusPirate itself related methods */
Index: par.c
===================================================================
--- par.c	(Revision 1144)
+++ par.c	(Arbeitskopie)
@@ -359,6 +359,8 @@
 {
   strcpy(pgm->type, "PPI");
 
+  pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
   pgm->exit_vcc = EXIT_VCC_UNSPEC;
   pgm->exit_reset = EXIT_RESET_UNSPEC;
   pgm->exit_datahigh = EXIT_DATAHIGH_UNSPEC;
Index: config_gram.y
===================================================================
--- config_gram.y	(Revision 1144)
+++ config_gram.y	(Arbeitskopie)
@@ -288,6 +288,8 @@
         pgm_free(existing_prog);
       }
       PUSH(programmers, current_prog);
+//      pgm_fill_old_pins(current_prog); // TODO to be removed if old pin data no longer needed
+//      pgm_display_generic(current_prog, id);
       current_prog = NULL;
     }
 ;
@@ -538,7 +540,7 @@
   |
   TKN_TILDE TKN_NUMBER { assign_pin(pin_name, $2, 1); }
   |
-  /* empty */ { current_prog->pinno[pin_name] = 0; }
+  /* empty */ { pin_clear_all(&(current_prog->pin[pin_name])); }
 ;
 
 pin_list:
@@ -548,9 +550,7 @@
   |
   TKN_TILDE TKN_LEFT_PAREN num_list TKN_RIGHT_PAREN { assign_pin_list(1); }
   |
-  /* empty */ {
-    current_prog->pinno[pin_name] = 0;
-  }
+  /* empty */ { pin_clear_all(&(current_prog->pin[pin_name])); }
 ;
 
 prog_parm_pins:
@@ -1353,10 +1353,8 @@
             progname, lineno, infile, PIN_MIN, PIN_MAX);
     exit(1);
   }
-  if (invert)
-    value |= PIN_INVERSE;
 
-  current_prog->pinno[pinno] = value;
+  pin_set_value(&(current_prog->pin[pinno]), value, invert);
 
   return 0;
 }
@@ -1370,9 +1368,15 @@
   while (lsize(number_list)) {
     t = lrmv_n(number_list, 1);
     pin = t->value.number;
-    current_prog->pinno[pin_name] |= (1 << pin);
-    if (invert)
-      current_prog->pinno[pin_name] |= PIN_INVERSE;
+    if ((pin < PIN_MIN) || (pin > PIN_MAX)) {
+      fprintf(stderr, 
+            "%s: error at line %d of %s: pin must be in the "
+            "range %d-%d\n",
+            progname, lineno, infile, PIN_MIN, PIN_MAX);
+      exit(1);
+      /* TODO clear list and free tokens if no exit is done */
+    }
+    pin_set_value(&(current_prog->pin[pin_name]), pin, invert);
     free_token(t);
   }
 
Index: ft245r.c
===================================================================
--- ft245r.c	(Revision 1144)
+++ ft245r.c	(Arbeitskopie)
@@ -806,6 +806,8 @@
 void ft245r_initpgm(PROGRAMMER * pgm) {
     strcpy(pgm->type, "ftdi_syncbb");
 
+    pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
     /*
      * mandatory functions
      */
Index: avrftdi.c
===================================================================
--- avrftdi.c	(Revision 1144)
+++ avrftdi.c	(Arbeitskopie)
@@ -1259,6 +1259,8 @@
 
 	strcpy(pgm->type, "avrftdi");
 
+	pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
 	/*
 	 * mandatory functions
 	 */
Index: serbb_posix.c
===================================================================
--- serbb_posix.c	(Revision 1144)
+++ serbb_posix.c	(Arbeitskopie)
@@ -289,6 +289,8 @@
 {
   strcpy(pgm->type, "SERBB");
 
+  pgm_fill_old_pins(pgm); // TODO to be removed if old pin data no longer needed
+
   pgm->rdy_led        = bitbang_rdy_led;
   pgm->err_led        = bitbang_err_led;
   pgm->pgm_led        = bitbang_pgm_led;
Index: pgm.c
===================================================================
--- pgm.c	(Revision 1144)
+++ pgm.c	(Arbeitskopie)
@@ -220,17 +220,17 @@
   if ((pmask & PIN_MASK) == 0)
      return " (not used)";
 
-  buf[0] = ' ';
+  buf[0] = (pmask & PIN_INVERSE)?'~':' ';
   buf[1] = 0;
+  if (pmask & PIN_INVERSE) strcat(buf, "(");
   for (pin = 0; pin <= 17; pin++) {
     if (pmask & (1 << pin)) {
       sprintf(b2, "%d", pin);
       if (buf[1] != 0)
-        strcat(buf, ",");
       strcat(buf, b2);
     }
   }
-
+  if (pmask & PIN_INVERSE) strcat(buf, ")");
   return buf;
 }
 
Index: pgm.h
===================================================================
--- pgm.h	(Revision 1144)
+++ pgm.h	(Arbeitskopie)
@@ -68,6 +68,7 @@
   char port[PGM_PORTLEN];
   void (*initpgm)(struct programmer_t * pgm);
   unsigned int pinno[N_PINS];
+  struct pindef_t pin[N_PINS];
   exit_vcc_t exit_vcc;
   exit_reset_t exit_reset;
   exit_datahigh_t exit_datahigh;
_______________________________________________
avrdude-dev mailing list
avrdude-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/avrdude-dev

Reply via email to