Hi,

I added some more functions for working with the new pin definitions. Especially there is now a generic function for checking for valid pin definitions to be used by any bitbanging programmer.

It is currently only used in ftdi_syncbb (search for pins_check in ft245r.c), but might also used in avrftdi and others.

For avrftdi I have attached an example how to it (after you have changed the pin numbering there to be 0-based). Also one could remove then the pin validation code from the add_pins function.


Kind regards

René


On 04.05.2013 11:33, René Liebscher wrote:
Hi,

as I already mentioned I checked in now the new pin definitions data structures.

All existing bitbang programmer should work as before as the convert the new data into the existing data fields. (See their initpgm functions.)

For ftdi_syncbb I use now the new format internally, so it uses now the pin numbers 0-based. Also it should now support the buff pins and inverted pins for all its supported pins. (This needs to be tested as my hardware has no need for a buff pin and any inverter.) Also there are currently more exit() in the code than I really want, but this will be changed next.

I guess avrftdi could be adapted very easy by changing the add_pins and set_pins functions (add_pin and set_pin might be removed). Another place to change is the pin setup in avrftdi_open. (Maybe looking at ft245r.c and its usage of the SET_BITS_0 macro and its check_pins function might help. )

BTW: I don't like the fact that in avrftdi the default pins are set if MOSI,MISO,SCK, or RESET does not fit the expectations. If some one has really connected the pins as he describes in the config file, than we should rather exit because of wrong connections, instead to change it by ourself and possible destroy the connected hardware. (Maybe he could still use synchronous bitbanging using ftdi_syncbb in this configuration.)

Also the add_pin(s) and set_pin(s) functions seem to use different return code for errors (1 vs. -1). Maybe we should introduce some generic defines in avrdude.h so it is really clear what the value for OK is, what values for error (any negative number? so you can code the reason in it?)


Kind regards

René


On 29.04.2013 20:51, René Liebscher wrote:
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



_______________________________________________
avrdude-dev mailing list
avrdude-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/avrdude-dev

Index: avrftdi.c
===================================================================
--- avrftdi.c	(Revision 1161)
+++ avrftdi.c	(Arbeitskopie)
@@ -775,6 +775,56 @@
 	
 	//pdata->pin_direction = (0x3 | (1 << (pgm->pinno[PIN_AVR_RESET] - 1)));
 
+/* SCK/MOSI/MISO are fixed and not invertable?*/
+static const struct pindef_t valid_pins_SCK = {{0x01},{0x00}} ;
+static const struct pindef_t valid_pins_MOSI = {{0x02},{0x00}} ;
+static const struct pindef_t valid_pins_MISO = {{0x04},{0x00}} ;
+/* value for 8/12/16 bit wide interface for other pins */
+static const struct pindef_t valid_pins_others_8bit = {{0xf8},{0xf8}} ;
+static const struct pindef_t valid_pins_others_12bit = {{0xff8},{0xff8}} ;
+static const struct pindef_t valid_pins_others_16bit = {{0xfff8},{0xfff8}} ;
+
+static const struct pin_checklist_t pin_checklist_8bit[] = {
+	{ PIN_AVR_SCK,  1, &valid_pins_SCK},
+	{ PIN_AVR_MOSI, 1, &valid_pins_MOSI},
+	{ PIN_AVR_MISO, 1, &valid_pins_MISO},
+	{ PIN_AVR_RESET,1, &valid_pins_others_8bit},
+	{ PPI_AVR_VCC,  0, &valid_pins_others_8bit},
+	{ PPI_AVR_BUFF, 0, &valid_pins_others_8bit},
+	{ PIN_LED_ERR,  0, &valid_pins_others_8bit},
+	{ PIN_LED_RDY,  0, &valid_pins_others_8bit},
+	{ PIN_LED_PGM,  0, &valid_pins_others_8bit},
+	{ PIN_LED_VFY,  0, &valid_pins_others_8bit},
+};
+
+static const struct pin_checklist_t pin_checklist_12bit[] = {
+	{ PIN_AVR_SCK,  1, &valid_pins_SCK},
+	{ PIN_AVR_MOSI, 1, &valid_pins_MOSI},
+	{ PIN_AVR_MISO, 1, &valid_pins_MISO},
+	{ PIN_AVR_RESET,1, &valid_pins_others_12bit},
+	{ PPI_AVR_VCC,  0, &valid_pins_others_12bit},
+	{ PPI_AVR_BUFF, 0, &valid_pins_others_12bit},
+	{ PIN_LED_ERR,  0, &valid_pins_others_12bit},
+	{ PIN_LED_RDY,  0, &valid_pins_others_12bit},
+	{ PIN_LED_PGM,  0, &valid_pins_others_12bit},
+	{ PIN_LED_VFY,  0, &valid_pins_others_12bit},
+};
+
+static const struct pin_checklist_t pin_checklist_16bit[] = {
+	{ PIN_AVR_SCK,  1, &valid_pins_SCK},
+	{ PIN_AVR_MOSI, 1, &valid_pins_MOSI},
+	{ PIN_AVR_MISO, 1, &valid_pins_MISO},
+	{ PIN_AVR_RESET,1, &valid_pins_others_16bit},
+	{ PPI_AVR_VCC,  0, &valid_pins_others_16bit},
+	{ PPI_AVR_BUFF, 0, &valid_pins_others_16bit},
+	{ PIN_LED_ERR,  0, &valid_pins_others_16bit},
+	{ PIN_LED_RDY,  0, &valid_pins_others_16bit},
+	{ PIN_LED_PGM,  0, &valid_pins_others_16bit},
+	{ PIN_LED_VFY,  0, &valid_pins_others_16bit},
+};
+
+const struct pin_checklist_t * pin_checklist = pin_checklist_8bit;
+
 	/* set pin limit depending on chip type */
 	switch(pdata->ftdic->type) {
 		case TYPE_AM:
@@ -786,14 +836,17 @@
 		case TYPE_2232C:
 			pdata->pin_limit = 11;
 			pdata->rx_buffer_size = 384;
+			pin_checklist = pin_checklist_12bit;
 			break;
 		case TYPE_2232H:
 			pdata->pin_limit = 15;
 			pdata->rx_buffer_size = 4096;
+			pin_checklist = pin_checklist_16bit;
 			break;
 		case TYPE_232H:
 			pdata->pin_limit = 15;
 			pdata->rx_buffer_size = 1024;
+			pin_checklist = pin_checklist_16bit;
 			break;
 		case TYPE_4232H:
 			pdata->pin_limit = 7;
@@ -806,7 +859,10 @@
 			pdata->rx_buffer_size = pdata->ftdic->max_packet_size;
 			break;
 	}
-	
+
+	/* assumes all checklists above have same number of entries */
+	if (pins_check(pgm, pin_checklist,sizeof(pin_checklist_8bit)/sizeof(pin_checklist_8bit[0]))) return -1;
+
 	/* add SCK, MOSI and RESET as output pins - MISO needs no configuration */
 	if (add_pin(pgm, PIN_AVR_SCK)) return -1;
 	if (add_pin(pgm, PIN_AVR_MOSI)) return -1;
_______________________________________________
avrdude-dev mailing list
avrdude-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/avrdude-dev

Reply via email to