[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi,

we had some discussions about the FPGA subsystem some days (and also 
a couple of months) before on this list. I have also some local improvements 
for the FPGA subsystem.

Before posting a couple of clean patches I dare to post a single patch just to
get an ok for the different topics. When all points are clear, I will split my patch
into one patch per per topic.

This is what I like to get into U-Boot:

1) Replace the CONFIG_FPGA bit mask by U-Boot like configuration options:

CONFIG_FPGA
	- enable FPGA subsystem

CONFIG_FPGA_<vendor>
	- enable support for specific chip vendors 
	(ALTERA, XILINX)

CONFIG_FPGA_<family>
	- enable support for FPGA family 
	(SPARTAN2, SPARTAN3, VIRTEX2, CYCLONE2, ACEX1K, ACEX)

This means when you have an Xilinx Spartan3 FPGA on your board you 
typically have these lines in your board configuration:

#define CONFIG_FPGA
#define CONFIG_FPGA_XILINX
#define CONFIG_FPGA_SPARTAN3

(TODO: add this to the README file)

2) Fix FPGA support for some boards that will get broken through the 
above change (GEN860T, ...)

3) Remove bit swapping in fpga_loadbitstream().

4) Use AND-operation for checking MSB of bytes instead of sign check for 
Spartan2/3 FPGAs in slave serial code.

5) Add post() and pre() callback for Spartan2/3 FPGAs inslave serial mode.

6) Add some new FPGA types.

Matthias


diff --git a/board/gen860t/fpga.c b/board/gen860t/fpga.c
index 2ba7e0e..5997584 100644
--- a/board/gen860t/fpga.c
+++ b/board/gen860t/fpga.c
@@ -34,7 +34,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if (CONFIG_FPGA)
+#ifdef CONFIG_FPGA
 
 #if 0
 #define GEN860T_FPGA_DEBUG
diff --git a/board/gen860t/gen860t.c b/board/gen860t/gen860t.c
index d448f9f..70341f8 100644
--- a/board/gen860t/gen860t.c
+++ b/board/gen860t/gen860t.c
@@ -254,7 +254,7 @@ int misc_init_r (void)
 	mii_init ();
 #endif
 
-#if (CONFIG_FPGA)
+#ifdef CONFIG_FPGA
 	gen860t_init_fpga ();
 #endif
 	return 0;
diff --git a/common/ACEX1K.c b/common/ACEX1K.c
index 2a421e2..76dc166 100644
--- a/common/ACEX1K.c
+++ b/common/ACEX1K.c
@@ -28,7 +28,7 @@
 #include <common.h>		/* core U-Boot definitions */
 #include <ACEX1K.h>		/* ACEX device family */
 
-#if (CONFIG_FPGA & (CFG_ALTERA | CFG_ACEX1K))
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ALTERA) && defined(CONFIG_FPGA_ACEX1K)
 
 /* Define FPGA_DEBUG to get debug printf's */
 #ifdef	FPGA_DEBUG
@@ -363,4 +363,4 @@ static int ACEX1K_ps_reloc (Altera_desc * desc, ulong reloc_offset)
 
 }
 
-#endif /* (CONFIG_FPGA & (CFG_ALTERA | CFG_ACEX1K)) */
+#endif /* CONFIG_FPGA && CONFIG_FPGA_ALTERA && CONFIG_FPGA_ACEX1K */
diff --git a/common/altera.c b/common/altera.c
index 06e8a95..eb874e4 100644
--- a/common/altera.c
+++ b/common/altera.c
@@ -40,7 +40,7 @@
 #define PRINTF(fmt,args...)
 #endif
 
-#if (CONFIG_FPGA & CFG_FPGA_ALTERA)
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ALTERA)
 
 /* Local Static Functions */
 static int altera_validate (Altera_desc * desc, char *fn);
@@ -56,11 +56,11 @@ int altera_load( Altera_desc *desc, void *buf, size_t bsize )
 		switch (desc->family) {
 		case Altera_ACEX1K:
 		case Altera_CYC2:
-#if (CONFIG_FPGA & CFG_ACEX1K)
+#ifdef CONFIG_FPGA_ACEX1K
 			PRINTF ("%s: Launching the ACEX1K Loader...\n",
 					__FUNCTION__);
 			ret_val = ACEX1K_load (desc, buf, bsize);
-#elif (CONFIG_FPGA & CFG_CYCLON2)
+#elif defined CONFIG_FPGA_CYCLON2
 			PRINTF ("%s: Launching the CYCLON II Loader...\n",
 					__FUNCTION__);
 			ret_val = CYC2_load (desc, buf, bsize);
@@ -88,7 +88,7 @@ int altera_dump( Altera_desc *desc, void *buf, size_t bsize )
 	} else {
 		switch (desc->family) {
 		case Altera_ACEX1K:
-#if (CONFIG_FPGA & CFG_ACEX)
+#ifdef CONFIG_FPGA_ACEX
 			PRINTF ("%s: Launching the ACEX1K Reader...\n",
 					__FUNCTION__);
 			ret_val = ACEX1K_dump (desc, buf, bsize);
@@ -156,9 +156,9 @@ int altera_info( Altera_desc *desc )
 			switch (desc->family) {
 			case Altera_ACEX1K:
 			case Altera_CYC2:
-#if (CONFIG_FPGA & CFG_ACEX1K)
+#ifdef CONFIG_FPGA_ACEX1K
 				ACEX1K_info (desc);
-#elif (CONFIG_FPGA & CFG_CYCLON2)
+#elif defined CONFIG_FPGA_CYCLON2
 				CYC2_info (desc);
 #else
 				/* just in case */
@@ -192,7 +192,7 @@ int altera_reloc( Altera_desc *desc, ulong reloc_offset)
 	} else {
 		switch (desc->family) {
 		case Altera_ACEX1K:
-#if (CONFIG_FPGA & CFG_ACEX1K)
+#ifdef CONFIG_FPGA_ACEX1K
 			ret_val = ACEX1K_reloc (desc, reloc_offset);
 #else
 			printf ("%s: No support for ACEX devices.\n",
@@ -200,7 +200,7 @@ int altera_reloc( Altera_desc *desc, ulong reloc_offset)
 #endif
 			break;
 		case Altera_CYC2:
-#if (CONFIG_FPGA & CFG_CYCLON2)
+#ifdef CONFIG_FPGA_CYCLON2
 			ret_val = CYC2_reloc (desc, reloc_offset);
 #else
 			printf ("%s: No support for CYCLON II devices.\n",
@@ -249,4 +249,4 @@ static int altera_validate (Altera_desc * desc, char *fn)
 
 /* ------------------------------------------------------------------------- */
 
-#endif /* CONFIG_FPGA & CFG_FPGA_ALTERA */
+#endif /* CONFIG_FPGA & CONFIG_FPGA_ALTERA */
diff --git a/common/cmd_fpga.c b/common/cmd_fpga.c
index cce23ad..3813140 100644
--- a/common/cmd_fpga.c
+++ b/common/cmd_fpga.c
@@ -60,14 +60,11 @@ static int fpga_get_op (char *opstr);
 /* Convert bitstream data and load into the fpga */
 int fpga_loadbitstream(unsigned long dev, char* fpgadata, size_t size)
 {
-#if (CONFIG_FPGA & CFG_FPGA_XILINX)
+#ifdef CONFIG_FPGA_XILINX
 	unsigned int length;
-	unsigned char* swapdata;
 	unsigned int swapsize;
 	char buffer[80];
-	unsigned char *ptr;
 	unsigned char *dataptr;
-	unsigned char data;
 	unsigned int i;
 	int rc;
 
@@ -142,42 +139,10 @@ int fpga_loadbitstream(unsigned long dev, char* fpgadata, size_t size)
 	           ((unsigned int) *(dataptr+1) <<16) +
 	           ((unsigned int) *(dataptr+2) <<8 ) +
 	           ((unsigned int) *(dataptr+3)     ) ;
-	dataptr+=4;
+	dataptr += 4;
 	printf("  bytes in bitstream = %d\n", swapsize);
 
-	/* check consistency of length obtained */
-	if (swapsize >= size) {
-		printf("%s: Could not find right length of data in bitstream\n",
-			__FUNCTION__);
-		return FPGA_FAIL;
-	}
-
-	/* allocate memory */
-	swapdata = (unsigned char *)malloc(swapsize);
-	if (swapdata == NULL) {
-		printf("%s: Could not allocate %d bytes memory !\n",
-			__FUNCTION__, swapsize);
-		return FPGA_FAIL;
-	}
-
-	/* read data into memory and swap bits */
-	ptr = swapdata;
-	for (i = 0; i < swapsize; i++) {
-		data = 0x00;
-		data |= (*dataptr & 0x01) << 7;
-		data |= (*dataptr & 0x02) << 5;
-		data |= (*dataptr & 0x04) << 3;
-		data |= (*dataptr & 0x08) << 1;
-		data |= (*dataptr & 0x10) >> 1;
-		data |= (*dataptr & 0x20) >> 3;
-		data |= (*dataptr & 0x40) >> 5;
-		data |= (*dataptr & 0x80) >> 7;
-		*ptr++ = data;
-		dataptr++;
-	}
-
-	rc = fpga_load(dev, swapdata, swapsize);
-	free(swapdata);
+	rc = fpga_load(dev, dataptr, swapsize);
 	return rc;
 #else
 	printf("Bitstream support only for Xilinx devices\n");
diff --git a/common/cyclon2.c b/common/cyclon2.c
index dce13b5..06f5e8a 100644
--- a/common/cyclon2.c
+++ b/common/cyclon2.c
@@ -27,7 +27,7 @@
 #include <altera.h>
 #include <ACEX1K.h>		/* ACEX device family */
 
-#if (CONFIG_FPGA & (CFG_ALTERA | CFG_CYCLON2))
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ALTERA) && defined(CONFIG_FPGA_CYCLON2)
 
 /* Define FPGA_DEBUG to get debug printf's */
 #ifdef	FPGA_DEBUG
@@ -302,4 +302,4 @@ static int CYC2_ps_reloc (Altera_desc * desc, ulong reloc_offset)
 	return ret_val;
 }
 
-#endif /* (CONFIG_FPGA & (CFG_ALTERA | CFG_CYCLON2)) */
+#endif /* CONFIG_FPGA && CONFIG_FPGA_ALTERA && CONFIG_FPGA_CYCLON2 */
diff --git a/common/fpga.c b/common/fpga.c
index 2eff239..e4073be 100644
--- a/common/fpga.c
+++ b/common/fpga.c
@@ -67,14 +67,11 @@ static int fpga_dev_info( int devnum );
 static void fpga_no_sup( char *fn, char *msg )
 {
 	if ( fn && msg ) {
-		printf( "%s: No support for %s.  CONFIG_FPGA defined as 0x%x.\n",
-				fn, msg, CONFIG_FPGA );
+		printf( "%s: No support for %s.\n", fn, msg);
 	} else if ( msg ) {
-		printf( "No support for %s. CONFIG_FPGA defined as 0x%x.\n",
-				msg, CONFIG_FPGA );
+		printf( "No support for %s.\n", msg);
 	} else {
-		printf( "No FPGA suport!  CONFIG_FPGA defined as 0x%x.\n",
-				CONFIG_FPGA );
+		printf( "No FPGA suport!\n");
 	}
 }
 
@@ -112,11 +109,6 @@ static __attribute__((__const__)) fpga_desc * __attribute__((__const__)) fpga_va
 		printf( "%s: Null buffer.\n", fn );
 		return (fpga_desc * const)NULL;
 	}
-	if ( !bsize ) {
-		printf( "%s: Null buffer size.\n", fn );
-		return (fpga_desc * const)NULL;
-	}
-
 	return desc;
 }
 
@@ -135,7 +127,7 @@ static int fpga_dev_info( int devnum )
 
 		switch ( desc->devtype ) {
 		case fpga_xilinx:
-#if CONFIG_FPGA & CFG_FPGA_XILINX
+#ifdef CONFIG_FPGA_XILINX
 			printf( "Xilinx Device\nDescriptor @ 0x%p\n", desc );
 			ret_val = xilinx_info( desc->devdesc );
 #else
@@ -143,7 +135,7 @@ static int fpga_dev_info( int devnum )
 #endif
 			break;
 		case fpga_altera:
-#if CONFIG_FPGA & CFG_FPGA_ALTERA
+#ifdef CONFIG_FPGA_ALTERA
 			printf( "Altera Device\nDescriptor @ 0x%p\n", desc );
 			ret_val = altera_info( desc->devdesc );
 #else
@@ -175,14 +167,14 @@ int fpga_reloc( fpga_type devtype, void *desc, ulong reloc_off )
 
 	switch ( devtype ) {
 	case fpga_xilinx:
-#if CONFIG_FPGA & CFG_FPGA_XILINX
+#ifdef CONFIG_FPGA_XILINX
 		ret_val = xilinx_reloc( desc, reloc_off );
 #else
 		fpga_no_sup( (char *)__FUNCTION__, "Xilinx devices" );
 #endif
 		break;
 	case fpga_altera:
-#if CONFIG_FPGA & CFG_FPGA_ALTERA
+#ifdef CONFIG_FPGA_ALTERA
 		ret_val = altera_reloc( desc, reloc_off );
 #else
 		fpga_no_sup( (char *)__FUNCTION__, "Altera devices" );
@@ -268,14 +260,14 @@ int fpga_load( int devnum, void *buf, size_t bsize )
 	if ( desc ) {
 		switch ( desc->devtype ) {
 		case fpga_xilinx:
-#if CONFIG_FPGA & CFG_FPGA_XILINX
+#ifdef CONFIG_FPGA_XILINX
 			ret_val = xilinx_load( desc->devdesc, buf, bsize );
 #else
 			fpga_no_sup( (char *)__FUNCTION__, "Xilinx devices" );
 #endif
 			break;
 		case fpga_altera:
-#if CONFIG_FPGA & CFG_FPGA_ALTERA
+#ifdef CONFIG_FPGA_ALTERA
 			ret_val = altera_load( desc->devdesc, buf, bsize );
 #else
 			fpga_no_sup( (char *)__FUNCTION__, "Altera devices" );
@@ -301,14 +293,14 @@ int fpga_dump( int devnum, void *buf, size_t bsize )
 	if ( desc ) {
 		switch ( desc->devtype ) {
 		case fpga_xilinx:
-#if CONFIG_FPGA & CFG_FPGA_XILINX
+#ifdef CONFIG_FPGA_XILINX
 			ret_val = xilinx_dump( desc->devdesc, buf, bsize );
 #else
 			fpga_no_sup( (char *)__FUNCTION__, "Xilinx devices" );
 #endif
 			break;
 		case fpga_altera:
-#if CONFIG_FPGA & CFG_FPGA_ALTERA
+#ifdef CONFIG_FPGA_ALTERA
 			ret_val = altera_dump( desc->devdesc, buf, bsize );
 #else
 			fpga_no_sup( (char *)__FUNCTION__, "Altera devices" );
diff --git a/common/spartan2.c b/common/spartan2.c
index 0fb23b6..6d1ed96 100644
--- a/common/spartan2.c
+++ b/common/spartan2.c
@@ -25,7 +25,7 @@
 #include <common.h>		/* core U-Boot definitions */
 #include <spartan2.h>		/* Spartan-II device family */
 
-#if (CONFIG_FPGA & (CFG_XILINX | CFG_SPARTAN2))
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_SPARTAN2)
 
 /* Define FPGA_DEBUG to get debug printf's */
 #ifdef	FPGA_DEBUG
@@ -441,7 +441,7 @@ static int Spartan2_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 	int ret_val = FPGA_FAIL;	/* assume the worst */
 	Xilinx_Spartan2_Slave_Serial_fns *fn = desc->iface_fns;
 	int i;
-	char  val;
+	unsigned char  val;
 
 	PRINTF ("%s: start with interface functions @ 0x%p\n",
 			__FUNCTION__, fn);
@@ -516,7 +516,7 @@ static int Spartan2_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 				(*fn->clk) (FALSE, TRUE, cookie);
 				CONFIG_FPGA_DELAY ();
 				/* Write data */
-				(*fn->wr) ((val < 0), TRUE, cookie);
+				(*fn->wr) ((val & 0x80), TRUE, cookie);
 				CONFIG_FPGA_DELAY ();
 				/* Assert the clock */
 				(*fn->clk) (TRUE, TRUE, cookie);
@@ -561,6 +561,13 @@ static int Spartan2_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 		}
 		putc ('\n');			/* terminate the dotted line */
 
+		/*
+		 * Run the post configuration function if there is one.
+		 */
+		if (*fn->post) {
+			(*fn->post) (cookie);
+		}
+
 #ifdef CFG_FPGA_PROG_FEEDBACK
 		if (ret_val == FPGA_SUCCESS) {
 			puts ("Done.\n");
@@ -615,8 +622,10 @@ static int Spartan2_ss_reloc (Xilinx_desc * desc, ulong reloc_offset)
 			PRINTF ("%s: Relocating descriptor at 0x%p\n", __FUNCTION__,
 					desc);
 
-			addr = (ulong) (fn->pre) + reloc_offset;
-			fn_r->pre = (Xilinx_pre_fn) addr;
+			if (fn->pre) {
+				addr = (ulong) (fn->pre) + reloc_offset;
+				fn_r->pre = (Xilinx_pre_fn) addr;
+			}
 
 			addr = (ulong) (fn->pgm) + reloc_offset;
 			fn_r->pgm = (Xilinx_pgm_fn) addr;
@@ -633,6 +642,11 @@ static int Spartan2_ss_reloc (Xilinx_desc * desc, ulong reloc_offset)
 			addr = (ulong) (fn->wr) + reloc_offset;
 			fn_r->wr = (Xilinx_wr_fn) addr;
 
+			if (fn->post) {
+				addr = (ulong) (fn->post) + reloc_offset;
+				fn_r->post = (Xilinx_post_fn) addr;
+			}
+
 			fn_r->relocated = TRUE;
 
 		} else {
diff --git a/common/spartan3.c b/common/spartan3.c
index c0f2b05..4fe3e89 100644
--- a/common/spartan3.c
+++ b/common/spartan3.c
@@ -30,7 +30,7 @@
 #include <common.h>		/* core U-Boot definitions */
 #include <spartan3.h>		/* Spartan-II device family */
 
-#if (CONFIG_FPGA & (CFG_XILINX | CFG_SPARTAN3))
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_SPARTAN3)
 
 /* Define FPGA_DEBUG to get debug printf's */
 #ifdef	FPGA_DEBUG
@@ -446,7 +446,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 	int ret_val = FPGA_FAIL;	/* assume the worst */
 	Xilinx_Spartan3_Slave_Serial_fns *fn = desc->iface_fns;
 	int i;
-	char  val;
+	unsigned char val;
 
 	PRINTF ("%s: start with interface functions @ 0x%p\n",
 			__FUNCTION__, fn);
@@ -514,6 +514,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 				puts ("** CRC error during FPGA load.\n");
 				return (FPGA_FAIL);
 			}
+
 			val = data [bytecount ++];
 			i = 8;
 			do {
@@ -521,7 +522,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 				(*fn->clk) (FALSE, TRUE, cookie);
 				CONFIG_FPGA_DELAY ();
 				/* Write data */
-				(*fn->wr) ((val < 0), TRUE, cookie);
+				(*fn->wr) ((val & 0x80), TRUE, cookie);
 				CONFIG_FPGA_DELAY ();
 				/* Assert the clock */
 				(*fn->clk) (TRUE, TRUE, cookie);
@@ -566,6 +567,13 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 		}
 		putc ('\n');			/* terminate the dotted line */
 
+		/*
+		 * Run the post configuration function if there is one.
+		 */
+		if (*fn->post) {
+			(*fn->post) (cookie);
+		}
+
 #ifdef CFG_FPGA_PROG_FEEDBACK
 		if (ret_val == FPGA_SUCCESS) {
 			puts ("Done.\n");
@@ -620,8 +628,10 @@ static int Spartan3_ss_reloc (Xilinx_desc * desc, ulong reloc_offset)
 			PRINTF ("%s: Relocating descriptor at 0x%p\n", __FUNCTION__,
 					desc);
 
-			addr = (ulong) (fn->pre) + reloc_offset;
-			fn_r->pre = (Xilinx_pre_fn) addr;
+			if (fn->pre) {
+				addr = (ulong) (fn->pre) + reloc_offset;
+				fn_r->pre = (Xilinx_pre_fn) addr;
+			}
 
 			addr = (ulong) (fn->pgm) + reloc_offset;
 			fn_r->pgm = (Xilinx_pgm_fn) addr;
@@ -638,6 +648,11 @@ static int Spartan3_ss_reloc (Xilinx_desc * desc, ulong reloc_offset)
 			addr = (ulong) (fn->wr) + reloc_offset;
 			fn_r->wr = (Xilinx_wr_fn) addr;
 
+			if (fn->post) {
+				addr = (ulong) (fn->post) + reloc_offset;
+				fn_r->post = (Xilinx_post_fn) addr;
+			}
+
 			fn_r->relocated = TRUE;
 
 		} else {
diff --git a/common/virtex2.c b/common/virtex2.c
index b5dc366..1283ff6 100644
--- a/common/virtex2.c
+++ b/common/virtex2.c
@@ -31,7 +31,7 @@
 #include <common.h>
 #include <virtex2.h>
 
-#if (CONFIG_FPGA & (CFG_XILINX | CFG_VIRTEX2))
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_VIRTEX2)
 
 #if 0
 #define FPGA_DEBUG
diff --git a/common/xilinx.c b/common/xilinx.c
index e03e78c..2a7e9f4 100644
--- a/common/xilinx.c
+++ b/common/xilinx.c
@@ -32,7 +32,7 @@
 #include <spartan2.h>
 #include <spartan3.h>
 
-#if (CONFIG_FPGA & CFG_FPGA_XILINX)
+#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_XILINX)
 
 #if 0
 #define FPGA_DEBUG
@@ -59,7 +59,7 @@ int xilinx_load (Xilinx_desc * desc, void *buf, size_t bsize)
 	} else
 		switch (desc->family) {
 		case Xilinx_Spartan2:
-#if (CONFIG_FPGA & CFG_SPARTAN2)
+#ifdef CONFIG_FPGA_SPARTAN2
 			PRINTF ("%s: Launching the Spartan-II Loader...\n",
 					__FUNCTION__);
 			ret_val = Spartan2_load (desc, buf, bsize);
@@ -69,7 +69,7 @@ int xilinx_load (Xilinx_desc * desc, void *buf, size_t bsize)
 #endif
 			break;
 		case Xilinx_Spartan3:
-#if (CONFIG_FPGA & CFG_SPARTAN3)
+#ifdef CONFIG_FPGA_SPARTAN3
 			PRINTF ("%s: Launching the Spartan-III Loader...\n",
 					__FUNCTION__);
 			ret_val = Spartan3_load (desc, buf, bsize);
@@ -79,7 +79,7 @@ int xilinx_load (Xilinx_desc * desc, void *buf, size_t bsize)
 #endif
 			break;
 		case Xilinx_Virtex2:
-#if (CONFIG_FPGA & CFG_VIRTEX2)
+#ifdef CONFIG_FPGA_VIRTEX2
 			PRINTF ("%s: Launching the Virtex-II Loader...\n",
 					__FUNCTION__);
 			ret_val = Virtex2_load (desc, buf, bsize);
@@ -106,7 +106,7 @@ int xilinx_dump (Xilinx_desc * desc, void *buf, size_t bsize)
 	} else
 		switch (desc->family) {
 		case Xilinx_Spartan2:
-#if (CONFIG_FPGA & CFG_SPARTAN2)
+#ifdef CONFIG_FPGA_SPARTAN2
 			PRINTF ("%s: Launching the Spartan-II Reader...\n",
 					__FUNCTION__);
 			ret_val = Spartan2_dump (desc, buf, bsize);
@@ -116,7 +116,7 @@ int xilinx_dump (Xilinx_desc * desc, void *buf, size_t bsize)
 #endif
 			break;
 		case Xilinx_Spartan3:
-#if (CONFIG_FPGA & CFG_SPARTAN3)
+#ifdef CONFIG_FPGA_SPARTAN3
 			PRINTF ("%s: Launching the Spartan-III Reader...\n",
 					__FUNCTION__);
 			ret_val = Spartan3_dump (desc, buf, bsize);
@@ -126,7 +126,7 @@ int xilinx_dump (Xilinx_desc * desc, void *buf, size_t bsize)
 #endif
 			break;
 		case Xilinx_Virtex2:
-#if (CONFIG_FPGA & CFG_VIRTEX2)
+#ifdef CONFIG_FPGA_VIRTEX2
 			PRINTF ("%s: Launching the Virtex-II Reader...\n",
 					__FUNCTION__);
 			ret_val = Virtex2_dump (desc, buf, bsize);
@@ -198,7 +198,7 @@ int xilinx_info (Xilinx_desc * desc)
 			printf ("Device Function Table @ 0x%p\n", desc->iface_fns);
 			switch (desc->family) {
 			case Xilinx_Spartan2:
-#if (CONFIG_FPGA & CFG_SPARTAN2)
+#ifdef CONFIG_FPGA_SPARTAN2
 				Spartan2_info (desc);
 #else
 				/* just in case */
@@ -207,7 +207,7 @@ int xilinx_info (Xilinx_desc * desc)
 #endif
 				break;
 			case Xilinx_Spartan3:
-#if (CONFIG_FPGA & CFG_SPARTAN3)
+#ifdef CONFIG_FPGA_SPARTAN3
 				Spartan3_info (desc);
 #else
 				/* just in case */
@@ -216,7 +216,7 @@ int xilinx_info (Xilinx_desc * desc)
 #endif
 				break;
 			case Xilinx_Virtex2:
-#if (CONFIG_FPGA & CFG_VIRTEX2)
+#ifdef CONFIG_FPGA_VIRTEX2
 				Virtex2_info (desc);
 #else
 				/* just in case */
@@ -249,7 +249,7 @@ int xilinx_reloc (Xilinx_desc * desc, ulong reloc_offset)
 	} else
 		switch (desc->family) {
 		case Xilinx_Spartan2:
-#if (CONFIG_FPGA & CFG_SPARTAN2)
+#ifdef CONFIG_FPGA_SPARTAN2
 			ret_val = Spartan2_reloc (desc, reloc_offset);
 #else
 			printf ("%s: No support for Spartan-II devices.\n",
@@ -257,7 +257,7 @@ int xilinx_reloc (Xilinx_desc * desc, ulong reloc_offset)
 #endif
 			break;
 		case Xilinx_Spartan3:
-#if (CONFIG_FPGA & CFG_SPARTAN3)
+#ifdef CONFIG_FPGA_SPARTAN3
 			ret_val = Spartan3_reloc (desc, reloc_offset);
 #else
 			printf ("%s: No support for Spartan-III devices.\n",
@@ -265,7 +265,7 @@ int xilinx_reloc (Xilinx_desc * desc, ulong reloc_offset)
 #endif
 			break;
 		case Xilinx_Virtex2:
-#if (CONFIG_FPGA & CFG_VIRTEX2)
+#ifdef CONFIG_FPGA_VIRTEX2
 			ret_val = Virtex2_reloc (desc, reloc_offset);
 #else
 			printf ("%s: No support for Virtex-II devices.\n",
@@ -308,4 +308,4 @@ static int xilinx_validate (Xilinx_desc * desc, char *fn)
 	return ret_val;
 }
 
-#endif							/* CONFIG_FPGA & CFG_FPGA_XILINX */
+#endif	/* CONFIG_FPGA && CONFIG_FPGA_XILINX */
diff --git a/include/configs/GEN860T.h b/include/configs/GEN860T.h
index bfbf3a8..3eb3131 100644
--- a/include/configs/GEN860T.h
+++ b/include/configs/GEN860T.h
@@ -273,7 +273,9 @@
  * Virtex2 FPGA configuration support
  */
 #define CONFIG_FPGA_COUNT		1
-#define CONFIG_FPGA				CFG_XILINX_VIRTEX2
+#define CONFIG_FPGA
+#define CONFIG_FPGA_XILINX
+#define CONFIG_FPGA_VIRTEX2
 #define CFG_FPGA_PROG_FEEDBACK
 
 
diff --git a/include/configs/M54455EVB.h b/include/configs/M54455EVB.h
index 6f4859c..487d2fc 100644
--- a/include/configs/M54455EVB.h
+++ b/include/configs/M54455EVB.h
@@ -190,7 +190,9 @@
 
 /* FPGA - Spartan 2 */
 /* experiment
-#define CONFIG_FPGA		CFG_SPARTAN3
+#define CONFIG_FPGA
+#define CONFIG_FPGA_XILINX
+#define CONFIG_FPGA_SPARTAN3
 #define CONFIG_FPGA_COUNT	1
 #define CFG_FPGA_PROG_FEEDBACK
 #define CFG_FPGA_CHECK_CTRLC
diff --git a/include/configs/alpr.h b/include/configs/alpr.h
index aff9823..bd32f6f 100644
--- a/include/configs/alpr.h
+++ b/include/configs/alpr.h
@@ -296,7 +296,10 @@
 /*-----------------------------------------------------------------------
  * FPGA stuff
  *-----------------------------------------------------------------------*/
-#define CONFIG_FPGA             CFG_ALTERA_CYCLON2
+#define CONFIG_FPGA
+#define CONFIG_FPGA_ALTERA
+#define CONFIG_FPGA_CYCLON2
+
 #define CFG_FPGA_CHECK_CTRLC
 #define CFG_FPGA_PROG_FEEDBACK
 #define CONFIG_FPGA_COUNT       1		/* Ich habe 2 ... aber in
diff --git a/include/spartan2.h b/include/spartan2.h
index d2e81e3..bd159e1 100644
--- a/include/spartan2.h
+++ b/include/spartan2.h
@@ -58,6 +58,7 @@ typedef struct {
 	Xilinx_init_fn	init;
 	Xilinx_done_fn	done;
 	Xilinx_wr_fn	wr;
+	Xilinx_post_fn	post;
 	int           	relocated;
 } Xilinx_Spartan2_Slave_Serial_fns;
 
@@ -69,6 +70,7 @@ typedef struct {
 #define XILINX_XC2S50_SIZE  	559232/8
 #define XILINX_XC2S100_SIZE 	781248/8
 #define XILINX_XC2S150_SIZE 	1040128/8
+#define XILINX_XC2S200_SIZE 	1335872/8
 
 /* Spartan-IIE (1.8V) */
 #define XILINX_XC2S50E_SIZE     630048/8
@@ -95,6 +97,9 @@ typedef struct {
 #define XILINX_XC2S150_DESC(iface, fn_table, cookie) \
 { Xilinx_Spartan2, iface, XILINX_XC2S150_SIZE, fn_table, cookie }
 
+#define XILINX_XC2S200_DESC(iface, fn_table, cookie) \
+{ Xilinx_Spartan2, iface, XILINX_XC2S200_SIZE, fn_table, cookie }
+
 #define XILINX_XC2S50E_DESC(iface, fn_table, cookie) \
 { Xilinx_Spartan2, iface, XILINX_XC2S50E_SIZE, fn_table, cookie }
 
diff --git a/include/spartan3.h b/include/spartan3.h
index 65a3f5a..c203eeb 100644
--- a/include/spartan3.h
+++ b/include/spartan3.h
@@ -58,6 +58,7 @@ typedef struct {
 	Xilinx_init_fn	init;
 	Xilinx_done_fn	done;
 	Xilinx_wr_fn	wr;
+	Xilinx_post_fn	post;
 	int           	relocated;
 } Xilinx_Spartan3_Slave_Serial_fns;
 
@@ -80,9 +81,12 @@ typedef struct {
 #define	XILINX_XC3S1200E_SIZE	3841184/8
 #define	XILINX_XC3S1600E_SIZE	5969696/8
 
+/* Spartan-IIIE (1.2V) */
+#define XILINX_XC3S1200E_SIZE  	3841184/8
+
 /* Descriptor Macros
  *********************************************************************/
-/* Spartan-II devices */
+/* Spartan-III devices */
 #define XILINX_XC3S50_DESC(iface, fn_table, cookie) \
 { Xilinx_Spartan3, iface, XILINX_XC3S50_SIZE, fn_table, cookie }
 
@@ -124,4 +128,9 @@ typedef struct {
 #define XILINX_XC3S1600E_DESC(iface, fn_table, cookie) \
 { Xilinx_Spartan3, iface, XILINX_XC3S1600E_SIZE, fn_table, cookie }
 
+
+/* Spartan-IIIE devices */
+#define XILINX_XC3S1200E_DESC(iface, fn_table, cookie) \
+{ Xilinx_Spartan3, iface, XILINX_XC3S1200E_SIZE, fn_table, cookie }
+
 #endif /* _SPARTAN3_H_ */
diff --git a/include/xilinx.h b/include/xilinx.h
index 3704e1d..95ebe3d 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -31,11 +31,11 @@
  *********************************************************************/
 #define CFG_SPARTAN2 			CFG_FPGA_DEV( 0x1 )
 #define CFG_VIRTEX_E 			CFG_FPGA_DEV( 0x2 )
-#define CFG_VIRTEX2	 			CFG_FPGA_DEV( 0x4 )
+#define CFG_VIRTEX2			CFG_FPGA_DEV( 0x4 )
 #define CFG_SPARTAN3 			CFG_FPGA_DEV( 0x8 )
 #define CFG_XILINX_SPARTAN2 	(CFG_FPGA_XILINX | CFG_SPARTAN2)
 #define CFG_XILINX_VIRTEX_E 	(CFG_FPGA_XILINX | CFG_VIRTEX_E)
-#define CFG_XILINX_VIRTEX2	 	(CFG_FPGA_XILINX | CFG_VIRTEX2)
+#define CFG_XILINX_VIRTEX2	(CFG_FPGA_XILINX | CFG_VIRTEX2)
 #define CFG_XILINX_SPARTAN3 	(CFG_FPGA_XILINX | CFG_SPARTAN3)
 /* XXX - Add new models here */

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Grant Likely]

On 11/11/07, Matthias Fuchs <matthias.fuchs@esd-electronics.com> wrote:
> Hi,
>
> we had some discussions about the FPGA subsystem some days (and also
> a couple of months) before on this list. I have also some local improvements
> for the FPGA subsystem.
>
> Before posting a couple of clean patches I dare to post a single patch just to
> get an ok for the different topics. When all points are clear, I will split my patch
> into one patch per per topic.
>
> This is what I like to get into U-Boot:
>
> 1) Replace the CONFIG_FPGA bit mask by U-Boot like configuration options:
>
> CONFIG_FPGA
>         - enable FPGA subsystem

This should be automatically selected by CONFIG_FPGA_<vendor>; for now
it's probably okay to have it done manually in the board config file;
but the move to Kconfig will allow for easy dependency checking.

>
> CONFIG_FPGA_<vendor>
>         - enable support for specific chip vendors
>         (ALTERA, XILINX)

Similarly, this should be automatically selected by CONFIG_FPGA_<family>

>
> CONFIG_FPGA_<family>
>         - enable support for FPGA family
>         (SPARTAN2, SPARTAN3, VIRTEX2, CYCLONE2, ACEX1K, ACEX)
>
> This means when you have an Xilinx Spartan3 FPGA on your board you
> typically have these lines in your board configuration:
>
> #define CONFIG_FPGA
> #define CONFIG_FPGA_XILINX
> #define CONFIG_FPGA_SPARTAN3
>
> (TODO: add this to the README file)
>
> 2) Fix FPGA support for some boards that will get broken through the
> above change (GEN860T, ...)
>
> 3) Remove bit swapping in fpga_loadbitstream().
>
> 4) Use AND-operation for checking MSB of bytes instead of sign check for
> Spartan2/3 FPGAs in slave serial code.
>
> 5) Add post() and pre() callback for Spartan2/3 FPGAs inslave serial mode.
>
> 6) Add some new FPGA types.
>
> Matthias

Sounds good to me.

>
>
> diff --git a/board/gen860t/fpga.c b/board/gen860t/fpga.c
> index 2ba7e0e..5997584 100644
> --- a/board/gen860t/fpga.c
> +++ b/board/gen860t/fpga.c
> @@ -34,7 +34,7 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#if (CONFIG_FPGA)
> +#ifdef CONFIG_FPGA

#if defined(CONFIG_FPGA) is preferred.

Otherwise, this looks like the right direction to me.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[w.wegner at astro-kom.de]

Hi,

On 11 Nov 2007 at 17:45, Matthias Fuchs wrote:

> Hi,
> 
> we had some discussions about the FPGA subsystem some days (and also 
> a couple of months) before on this list. I have also some local improvements 
> for the FPGA subsystem.
> 
> Before posting a couple of clean patches I dare to post a single patch just to
> get an ok for the different topics. When all points are clear, I will split my patch
> into one patch per per topic.

I have one wish on my list:

Would it be possible to have an optional "block write" function for the FPGA?

While I appreciate the current approach of single bit functions for the FPGA
to be very convenient for board bring-up, it is somewhat slow on the larger
FPGAs (with something like 1.5 MByte bitstream size).

An additional block write function that - if present - replaces the internal
(generic) programming algorithm would give a clean way to use the existing
FPGA infrastructure (commands, image handling, pre- and post-configuration)
and switch to a fast load for production use.

Only then, you could take advantage of buses like SPI for FPGA load, too.

(I hacked something like this in my local u-boot, and the speed-up for
FPGA loading was significant, at least 3 times. And I still did not
implement the SPI path because I did not have the time yet...)

Best regards,
Wolfgang

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi,

On Monday 12 November 2007 10:15, w.wegner at astro-kom.de wrote:
> I have one wish on my list:
... you haven't seen my wishlist :-)
> 
> Would it be possible to have an optional "block write" function for the FPGA?
yes.
> 
> While I appreciate the current approach of single bit functions for the FPGA
> to be very convenient for board bring-up, it is somewhat slow on the larger
> FPGAs (with something like 1.5 MByte bitstream size).
> 
> An additional block write function that - if present - replaces the internal
> (generic) programming algorithm would give a clean way to use the existing
> FPGA infrastructure (commands, image handling, pre- and post-configuration)
> and switch to a fast load for production use.
> 
> Only then, you could take advantage of buses like SPI for FPGA load, too.
I was in a similar situation some time before. Our PMC440 board (patches have
been posted yesterday) uses a Spartan3E FPGA that is programmed in slave serial 
mode. I started with download time about 13 seconds (!!!). The main reason 
is the extensive use of callbacks by the FPGA subsystem. Then updated the boot 
code to use U-Boot's slave parallel implementation to boot the FPGA still in
slave serial mode. My write-data-byte function just shifts out the byte bit by bit.
This improved the download time to about 3 seconds. The last step was to
enable the cache for the 440 CPU. This speeds things up to an acceptable level.

But I agree, a block write function would be cleaner.
> 
> (I hacked something like this in my local u-boot, and the speed-up for
> FPGA loading was significant, at least 3 times. And I still did not
> implement the SPI path because I did not have the time yet...)
If you already have something it might be a good idea to share your work
with us. I think this is independent from what my patch does at the moment.

Matthias

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[w.wegner at astro-kom.de]

Hi,

On 12 Nov 2007 at 10:45, Matthias Fuchs wrote:

> > I have one wish on my list:
> ... you haven't seen my wishlist :-)

:-))

> I was in a similar situation some time before. Our PMC440 board (patches have
> been posted yesterday) uses a Spartan3E FPGA that is programmed in slave serial 
> mode. I started with download time about 13 seconds (!!!). The main reason 
> is the extensive use of callbacks by the FPGA subsystem. Then updated the boot 
> code to use U-Boot's slave parallel implementation to boot the FPGA still in
> slave serial mode. My write-data-byte function just shifts out the byte bit by bit.
> This improved the download time to about 3 seconds. The last step was to
> enable the cache for the 440 CPU. This speeds things up to an acceptable level.

I currently have around 3 seconds for a partially filled XC3S4000 on my
MCF5373L (240 MHz) with cache enabled. This is almost below my pain
limit, which is why I did not yet take the effort to debug the SPI load any
further.

> If you already have something it might be a good idea to share your work
> with us. I think this is independent from what my patch does at the moment.

The usual problem: it is a bit difficult to extract only this as a clean patch...

I will see what I can do, but it can take some days.

> Matthias

Regards,
Wolfgang

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Jerry Van Baren]

Matthias Fuchs wrote:
> Hi,
> 
> we had some discussions about the FPGA subsystem some days (and also 
> a couple of months) before on this list. I have also some local improvements 
> for the FPGA subsystem.

[snip]

> Matthias

Hi Matthias,

[massive snip]

> diff --git a/common/spartan3.c b/common/spartan3.c
> index c0f2b05..4fe3e89 100644
> --- a/common/spartan3.c
> +++ b/common/spartan3.c
> @@ -30,7 +30,7 @@
>  #include <common.h>		/* core U-Boot definitions */
>  #include <spartan3.h>		/* Spartan-II device family */
>  
> -#if (CONFIG_FPGA & (CFG_XILINX | CFG_SPARTAN3))
> +#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_SPARTAN3)
>  
>  /* Define FPGA_DEBUG to get debug printf's */
>  #ifdef	FPGA_DEBUG
> @@ -446,7 +446,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
>  	int ret_val = FPGA_FAIL;	/* assume the worst */
>  	Xilinx_Spartan3_Slave_Serial_fns *fn = desc->iface_fns;
>  	int i;
> -	char  val;
> +	unsigned char val;

Trivia: This change should not be necessary since you fixed the 
conditional (below) to do a bit-wise & 0x80 rather than a signed compare 
that has portability and aesthetic problems.

>  	PRINTF ("%s: start with interface functions @ 0x%p\n",
>  			__FUNCTION__, fn);
> @@ -514,6 +514,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
>  				puts ("** CRC error during FPGA load.\n");
>  				return (FPGA_FAIL);
>  			}
> +
>  			val = data [bytecount ++];
>  			i = 8;
>  			do {
> @@ -521,7 +522,7 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
>  				(*fn->clk) (FALSE, TRUE, cookie);
>  				CONFIG_FPGA_DELAY ();
>  				/* Write data */
> -				(*fn->wr) ((val < 0), TRUE, cookie);
> +				(*fn->wr) ((val & 0x80), TRUE, cookie);

Thank you, that makes my aesthetics radar much happier!  :-)  Now it 
should be immaterial whether val is signed or unsigned.

Painting the bike shed blue,
gvb

[snip]

Ref:
<http://www.freebsd.org/doc/en_US.ISO8859-1/books/faq/misc.html#BIKESHED-PAINTING>

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi Jerry,

On Monday 12 November 2007 14:24, Jerry Van Baren wrote:
> > -	char  val;
> > +	unsigned char val;
> 
> Trivia: This change should not be necessary since you fixed the 
> conditional (below) to do a bit-wise & 0x80 rather than a signed compare 
> that has portability and aesthetic problems.
yes, of course.

> >  			val = data [bytecount ++];
But since the right value is an unsigned char, I like the left value
to be of the same type.

> Painting the bike shed blue,
We should ask the others that are involved ins discussion if blue might
bring disadvantages :-)

Matthias

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Jerry Van Baren]

Matthias Fuchs wrote:
> Hi Jerry,
> 
> On Monday 12 November 2007 14:24, Jerry Van Baren wrote:
>>> -	char  val;
>>> +	unsigned char val;
>> Trivia: This change should not be necessary since you fixed the 
>> conditional (below) to do a bit-wise & 0x80 rather than a signed compare 
>> that has portability and aesthetic problems.
> yes, of course.
> 
>>>  			val = data [bytecount ++];
> But since the right value is an unsigned char, I like the left value
> to be of the same type.
> 
>> Painting the bike shed blue,
> We should ask the others that are involved ins discussion if blue might
> bring disadvantages :-)
> 
> Matthias

Oops, yes, you are right.  My jaundiced eye read *signed* char instead 
of unsigned.

Sorry for the noise,
gvb

(feeling blue...)

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Bruce_Leonard at selinc.com]

Matthias,

Matthias Fuchs <matthias.fuchs@esd-electronics.com> wrote on 11/11/2007 
08:45:02 AM:

< snip > 

> 5) Add post() and pre() callback for Spartan2/3 FPGAs inslave serial 
mode.

Only comment I have is to point out that one of the things we talked about 
last summer.  The Spartan 3 code in particular (and I think all of the 
Xilinx/Altera code in general) makes the pre()/post() function calls 
optional.  However, the relocation code doesn't check to see if the 
pre()/post() functions exist and therefore incorrectly "relocates" them 
even if they don't exist.  This causes problems later because they now 
appear to exist since they no longer have a NULL address.  One of the 
things we talked about was makeing the FPGA relocation code smart enough 
to detect NULL addresses for these functions and not relocate them.

Now it may not be an issue anymore.  I made the changes to the code, but 
when stepping through 1.3.0-rc3 of the code, I noticed that gd->reloc_off 
= 0 which ment that the relocation_offset passed to the FPGA relocation 
code was zero and therefore nothing actually got relocated.  So a NULL 
pointer is still NULL when zero is added to it.  Now, I'm not sure why 
gd->reloc_off = 0, it may be my setup, it may be I'm broken, it may be 
that it's no longer used for some reason I don't know about.  But it seems 
to me that the FPGA relocation code should still deal with non-existant 
pre()/post() functions, just in case gd->reloc_off is not zero in someone 
else's circumstances.

My 2 cents worth and thanks for making these changes.  I'm looking forward 
to them all.

Bruce
 

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Grant Likely]

On 11/12/07, Bruce_Leonard at selinc.com <Bruce_Leonard@selinc.com> wrote:
> Matthias,
>
> Matthias Fuchs <matthias.fuchs@esd-electronics.com> wrote on 11/11/2007
> 08:45:02 AM:
>
> < snip >
>
> > 5) Add post() and pre() callback for Spartan2/3 FPGAs inslave serial
> mode.
>
> Only comment I have is to point out that one of the things we talked about
> last summer.  The Spartan 3 code in particular (and I think all of the
> Xilinx/Altera code in general) makes the pre()/post() function calls
> optional.  However, the relocation code doesn't check to see if the
> pre()/post() functions exist and therefore incorrectly "relocates" them
> even if they don't exist.  This causes problems later because they now
> appear to exist since they no longer have a NULL address.  One of the
> things we talked about was makeing the FPGA relocation code smart enough
> to detect NULL addresses for these functions and not relocate them.
>
> Now it may not be an issue anymore.  I made the changes to the code, but
> when stepping through 1.3.0-rc3 of the code, I noticed that gd->reloc_off
> = 0 which ment that the relocation_offset passed to the FPGA relocation
> code was zero and therefore nothing actually got relocated.  So a NULL
> pointer is still NULL when zero is added to it.  Now, I'm not sure why
> gd->reloc_off = 0, it may be my setup, it may be I'm broken, it may be
> that it's no longer used for some reason I don't know about.  But it seems
> to me that the FPGA relocation code should still deal with non-existant
> pre()/post() functions, just in case gd->reloc_off is not zero in someone
> else's circumstances.

Manual relocation shouldn't be necessary *at all*.  The fact that we
have it is due to the u-boot C environment being broken. I've fixed it
for many of the powerpc ports, which is why reloc_off is now zero for
those boards.  Unfortunately, the fix only works with some versions of
GCC, so the fix might need to be temporarily backed out.  :-(

Regardless, fixing the FPGA manual reloc code is trivial and should
definitely be done.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi,

On Monday 12 November 2007 23:55, Bruce_Leonard at selinc.com wrote:
> Matthias,
> 
> Matthias Fuchs <matthias.fuchs@esd-electronics.com> wrote on 11/11/2007 
> 08:45:02 AM:
> 
> < snip > 
> 
> > 5) Add post() and pre() callback for Spartan2/3 FPGAs inslave serial 
> mode.
> 
> Only comment I have is to point out that one of the things we talked about 
> last summer.  The Spartan 3 code in particular (and I think all of the 
> Xilinx/Altera code in general) makes the pre()/post() function calls 
> optional.  However, the relocation code doesn't check to see if the 
> pre()/post() functions exist and therefore incorrectly "relocates" them 
> even if they don't exist.  This causes problems later because they now 
> appear to exist since they no longer have a NULL address.  One of the 
> things we talked about was makeing the FPGA relocation code smart enough 
> to detect NULL addresses for these functions and not relocate them.
I forgot to mention this in my 'RFC' email. But if you take a look at my
patches, you will see that this has been implemented. I must say that I only
implemented it for the pre and post functions. It may be helpful for others
as well but for pre and post it is most obvious for me.

Perhaps you guys can give a little ack reply to my five FPGA patches. I did
not see any no-go comment on any of them, uuh.

Matthias

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Bruce_Leonard at selinc.com]

> 
> Perhaps you guys can give a little ack reply to my five FPGA patches. I 
did
> not see any no-go comment on any of them, uuh.
> 
> Matthias

Sorry, in my haste and stupidity I failed to read your entire original 
post and see that you had patches :(.

ACK everything with the following question:


-                rc = fpga_load(dev, swapdata, swapsize);
-                free(swapdata);
+                rc = fpga_load(dev, dataptr, swapsize);
                 return rc;

I see you're using the size pulled from the BIT file rather than the size 
passed into the parameter (which IMOHO is the right way to do it), but you 
left the name of the variable as 'swapsize' which isn't really relevant 
anymore since there's no swapping going on.  I don't care since I know 
what's going on, but it might be cleaner for future generations to rename 
it to something more descriptive of what it really is now.  Also, on a 'I 
REALLY don't care' note, how much work do you think it would be to remove 
the requirement of having a size on the command line for this operation? 
Since the 'size' parameter is never used, I'd like to see it gone.  If 
it's too much work, I can do it sometime when I'm bored ;).  Just a 
thought.

Thanks for the work on this, saved me from having to embarrass myself by 
getting patch after patch rejected 'cause I is stupid.

Bruce

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi Bruce,

On Tuesday 13 November 2007 19:15, Bruce_Leonard at selinc.com wrote:
> > 
> > Perhaps you guys can give a little ack reply to my five FPGA patches. I 
> did
> > not see any no-go comment on any of them, uuh.
> > 
> > Matthias
> 
> Sorry, in my haste and stupidity I failed to read your entire original 
> post and see that you had patches :(.
> 
> ACK everything with the following question:
> 
> 
> -                rc = fpga_load(dev, swapdata, swapsize);
> -                free(swapdata);
> +                rc = fpga_load(dev, dataptr, swapsize);
>                  return rc;
> 
> I see you're using the size pulled from the BIT file rather than the size 
> passed into the parameter (which IMOHO is the right way to do it), but you 
> left the name of the variable as 'swapsize' which isn't really relevant 
> anymore since there's no swapping going on.  I don't care since I know 
I put this on my list.

> what's going on, but it might be cleaner for future generations to rename 
> it to something more descriptive of what it really is now.  Also, on a 'I 
> REALLY don't care' note, how much work do you think it would be to remove 
> the requirement of having a size on the command line for this operation? 
> Since the 'size' parameter is never used, I'd like to see it gone.  If 
> it's too much work, I can do it sometime when I'm bored ;).  Just a 
> thought.
You are right. You do not need to pass the size parameter to the 'fpga loadb'
command at all. Only 'fpga load' still needs it.

Matthias

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[w.wegner at astro-kom.de]

Hi,

sorry for the delay... I finally cleaned up my xilinx spartan3 code,
and here is a proposal for an optional block write (originally called
fast write).

The observation is that using callbacks for every single data/clock
line change takes very long, doing this in a monolithic function is
much faster; additionally, you could take advantage of some
hardware (e.g. SPI, although I did not get this to work yet) to
do the actual load.

Best regards,
Wolfgang

PS: sorry I can not provide a better patch at the moment, git-diff
without any arguments prints out an empty diff for any file present
in the tree, and I do not know what I did to cause this.


diff --git a/common/spartan3.c b/common/spartan3.c
old mode 100644
new mode 100755
index f7c4f8c..a386006
--- a/common/spartan3.c
+++ b/common/spartan3.c
@@ -40,7 +40,7 @@
 #endif
 
 #undef CFG_FPGA_CHECK_BUSY
-#undef CFG_FPGA_PROG_FEEDBACK
+// #undef CFG_FPGA_PROG_FEEDBACK
 
 /* Note: The assumption is that we cannot possibly run fast enough to
  * overrun the device (the Slave Parallel mode can free run at 50MHz).
@@ -505,35 +505,39 @@ static int Spartan3_ss_load (Xilinx_desc * desc, void *buf, size_t bsize)
 			}
 		} while ((*fn->init) (cookie));
 
-		/* Load the data */
-		while (bytecount < bsize) {
-
-			/* Xilinx detects an error if INIT goes low (active)
-			   while DONE is low (inactive) */
-			if ((*fn->done) (cookie) == 0 && (*fn->init) (cookie)) {
-				puts ("** CRC error during FPGA load.\n");
-				return (FPGA_FAIL);
-			}
-			val = data [bytecount ++];
-			i = 8;
-			do {
-				/* Deassert the clock */
-				(*fn->clk) (FALSE, TRUE, cookie);
-				CONFIG_FPGA_DELAY ();
-				/* Write data */
-				(*fn->wr) ((val & 0x80), TRUE, cookie);
-				CONFIG_FPGA_DELAY ();
-				/* Assert the clock */
-				(*fn->clk) (TRUE, TRUE, cookie);
-				CONFIG_FPGA_DELAY ();
-				val <<= 1;
-				i --;
-			} while (i > 0);
+		if(*fn->fwr)
+			(*fn->fwr) (data, bsize, TRUE, cookie);
+		else {
+			/* Load the data */
+			while (bytecount < bsize) {
+
+				/* Xilinx detects an error if INIT goes low (active)
+				   while DONE is low (inactive) */
+				if ((*fn->done) (cookie) == 0 && (*fn->init) (cookie)) {
+					puts ("** CRC error during FPGA load.\n");
+					return (FPGA_FAIL);
+				}
+				val = data [bytecount ++];
+				i = 8;
+				do {
+					/* Deassert the clock */
+					(*fn->clk) (FALSE, TRUE, cookie);
+					CONFIG_FPGA_DELAY ();
+					/* Write data */
+					(*fn->wr) ((val & 0x80), TRUE, cookie);
+					CONFIG_FPGA_DELAY ();
+					/* Assert the clock */
+					(*fn->clk) (TRUE, TRUE, cookie);
+					CONFIG_FPGA_DELAY ();
+					val <<= 1;
+					i --;
+				} while (i > 0);
 
 #ifdef CFG_FPGA_PROG_FEEDBACK
-			if (bytecount % (bsize / 40) == 0)
-				putc ('.');		/* let them know we are alive */
+				if (bytecount % (bsize / 40) == 0)
+					putc ('.');		/* let them know we are alive */
 #endif
+			}
 		}
 
 		CONFIG_FPGA_DELAY ();
@@ -638,6 +642,11 @@ static int Spartan3_ss_reloc (Xilinx_desc * desc, ulong reloc_offset)
 			addr = (ulong) (fn->wr) + reloc_offset;
 			fn_r->wr = (Xilinx_wr_fn) addr;
 
+			if(*fn->fwr) {
+				addr = (ulong) (fn->fwr) + reloc_offset;
+				fn_r->fwr = (Xilinx_fastwr_fn) addr;
+			}
+
 			fn_r->relocated = TRUE;
 
 		} else {
diff --git a/include/spartan3.h b/include/spartan3.h
old mode 100644
new mode 100755
index 65a3f5a..11735e7
--- a/include/spartan3.h
+++ b/include/spartan3.h
@@ -58,6 +58,8 @@ typedef struct {
 	Xilinx_init_fn	init;
 	Xilinx_done_fn	done;
 	Xilinx_wr_fn	wr;
+// for block write (fast_write):
+	Xilinx_fastwr_fn fwr;
 	int           	relocated;
 } Xilinx_Spartan3_Slave_Serial_fns;
 

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi Wolfgang,

On Monday 10 December 2007 13:04, you wrote:
> Hi,
> 
> The observation is that using callbacks for every single data/clock
> line change takes very long, doing this in a monolithic function is
> much faster; additionally, you could take advantage of some
> hardware (e.g. SPI, although I did not get this to work yet) to
> do the actual load.

The new fwr callback makes sense to me. I agree that using the callbacks for every signal
change might be very slow. I used the the slave parallel code to load an fpga in slave serial
mode some time before. This reduces the callbacks a lot and booting was about 4 times faster.
I used the byte write callback to shift out 8 bits in this case.

I hope that my FPGA patches will get it into U-Boot when the next merge windows opens.
Perhaps you can provide a patch on top of that afterwards.

Matthias

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[w.wegner at astro-kom.de]

Hi Matthias,

On 11 Dec 2007 at 18:00, Matthias Fuchs wrote:

> 
> The new fwr callback makes sense to me. I agree that using the callbacks for every signal
> change might be very slow. I used the the slave parallel code to load an fpga in slave serial
> mode some time before. This reduces the callbacks a lot and booting was about 4 times faster.
> I used the byte write callback to shift out 8 bits in this case.
> 
> I hope that my FPGA patches will get it into U-Boot when the next merge windows opens.
> Perhaps you can provide a patch on top of that afterwards.

I will try to, as meanwhile my U-Boot environment settled down a bit, this should
not be too much work.

Would such a "selective patch" like the last one be OK, then I do not have to investigate
why I get all the empty diff lines from the new tree...?

> Matthias

Regards,
Wolfgang

[U-Boot-Users] RFC: Some improvements for the FPGA subsystem

[Matthias Fuchs]

Hi Wolfgang,

On Wednesday 12 December 2007 11:44, w.wegner at astro-kom.de wrote:
> Hi Matthias,
> 
> On 11 Dec 2007 at 18:00, Matthias Fuchs wrote:
> 
> > 
> > The new fwr callback makes sense to me. I agree that using the callbacks for every signal
> > change might be very slow. I used the the slave parallel code to load an fpga in slave serial
> > mode some time before. This reduces the callbacks a lot and booting was about 4 times faster.
> > I used the byte write callback to shift out 8 bits in this case.
> > 
> > I hope that my FPGA patches will get it into U-Boot when the next merge windows opens.
> > Perhaps you can provide a patch on top of that afterwards.
> 
> I will try to, as meanwhile my U-Boot environment settled down a bit, this should
> not be too much work.
> 
> Would such a "selective patch" like the last one be OK, then I do not have to investigate
> why I get all the empty diff lines from the new tree...?
I do not have to apply your patch. But I expect that wd will only except clean 'splipping' patches.

So it would be better to fix up your tree so that you can generate a patch as it is expected.
If nothing helps, check out a clean U-boot tree, modify it and try it again. 
It should be easier to fix your toolchain (we the help of others) than to get unclean patches
accepted. 

Matthias