[U-Boot] [PATCH v2 0/5] fpga: collected patches

[U-Boot] [PATCH v2 0/5] fpga: collected patches

[Alexander Dahl]

Hei hei,

after some Feedback from Michal Simek, I made a v2 of this patch
series, and based it onto v2019.07-rc4 now.

The patch number 2 is new, fixing all kinds of checkpatch warnings
in 'drivers/fpga/cyclon2.c' before fixing the indentation in that
file in patch number 3 (which was part of v1 already).

I dropped the last patch and replaced it with a new one based on the
suggestion made by Michal.

Greets
Alex

Alexander Dahl (5):
  fpga: altera: Add some more device sizes
  fpga: altera: cyclon2: Fix most checkpatch warnings
  fpga: altera: cyclon2: Fix indentation
  fpga: altera: cyclon2: Check function pointer before calling
  cmd: fpga: Change return value to avoid printing usage text

 drivers/fpga/cyclon2.c | 101 +++++++++++++++++++++++++------------------------
 include/ACEX1K.h       |  10 +++++
 include/fpga.h         |   2 +-
 3 files changed, 62 insertions(+), 51 deletions(-)

-- 
2.11.0

[U-Boot] [PATCH v2 1/5] fpga: altera: Add some more device sizes

[Alexander Dahl]

There seems to be only one place, where this is checked against:
`altera_validate()`. It should be non zero. Otherwise it is only used to
display it, so it probably does not really matter at the moment. But we
had the datasheet open anyway …

Sizes in datasheet are bit counts, display here is in bytes.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 include/ACEX1K.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/ACEX1K.h b/include/ACEX1K.h
index 9814bba284..7c5253c66c 100644
--- a/include/ACEX1K.h
+++ b/include/ACEX1K.h
@@ -60,6 +60,16 @@ typedef struct {
 #define Altera_EP2C35_SIZE	883905
 #define Altera_EP3C5_SIZE	368011		/* .rbf size in bytes */
 
+#define ALTERA_EP4CE6_SIZE	368011		/* 2944088 Bits */
+#define ALTERA_EP4CE10_SIZE	368011		/* 2944088 Bits */
+#define ALTERA_EP4CE15_SIZE	510856		/* 4086848 Bits */
+#define ALTERA_EP4CE22_SIZE	718569		/* 5748552 Bits */
+#define ALTERA_EP4CE30_SIZE	1191788		/* 9534304 Bits */
+#define ALTERA_EP4CE40_SIZE	1191788		/* 9534304 Bits */
+#define ALTERA_EP4CE55_SIZE	1861195		/* 14889560 Bits */
+#define ALTERA_EP4CE75_SIZE	2495719		/* 19965752 Bits */
+#define ALTERA_EP4CE115_SIZE	3571462		/* 28571696 Bits */
+
 /* Descriptor Macros
  *********************************************************************/
 /* ACEX1K devices */
-- 
2.11.0

[U-Boot] [PATCH v2 2/5] fpga: altera: cyclon2: Fix most checkpatch warnings

[Alexander Dahl]

Nothing special, but done before further cleanup.

* spacing
* braces
* __FUNCTION__ → __func__

Suggested-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/cyclon2.c | 81 ++++++++++++++++++++++++--------------------------
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/fpga/cyclon2.c b/drivers/fpga/cyclon2.c
index 6755956e85..99a64d437c 100644
--- a/drivers/fpga/cyclon2.c
+++ b/drivers/fpga/cyclon2.c
@@ -11,9 +11,9 @@
 
 /* Define FPGA_DEBUG to get debug printf's */
 #ifdef	FPGA_DEBUG
-#define PRINTF(fmt,args...)	printf (fmt ,##args)
+#define PRINTF(fmt, args...)	printf(fmt, ##args)
 #else
-#define PRINTF(fmt,args...)
+#define PRINTF(fmt, args...)
 #endif
 
 /* Note: The assumption is that we cannot possibly run fast enough to
@@ -26,7 +26,7 @@
 #endif
 
 #ifndef CONFIG_SYS_FPGA_WAIT
-#define CONFIG_SYS_FPGA_WAIT CONFIG_SYS_HZ/10		/* 100 ms */
+#define CONFIG_SYS_FPGA_WAIT CONFIG_SYS_HZ / 10		/* 100 ms */
 #endif
 
 static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize);
@@ -41,8 +41,8 @@ int CYC2_load(Altera_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case passive_serial:
-		PRINTF ("%s: Launching Passive Serial Loader\n", __FUNCTION__);
-		ret_val = CYC2_ps_load (desc, buf, bsize);
+		PRINTF("%s: Launching Passive Serial Loader\n", __func__);
+		ret_val = CYC2_ps_load(desc, buf, bsize);
 		break;
 
 	case fast_passive_parallel:
@@ -50,16 +50,16 @@ int CYC2_load(Altera_desc *desc, const void *buf, size_t bsize)
 		 * done in the write() callback. Use the existing PS load
 		 * function for FPP, too.
 		 */
-		PRINTF ("%s: Launching Fast Passive Parallel Loader\n",
-		      __FUNCTION__);
+		PRINTF("%s: Launching Fast Passive Parallel Loader\n",
+		       __func__);
 		ret_val = CYC2_ps_load(desc, buf, bsize);
 		break;
 
 		/* Add new interface types here */
 
 	default:
-		printf ("%s: Unsupported interface type, %d\n",
-				__FUNCTION__, desc->iface);
+		printf("%s: Unsupported interface type, %d\n",
+		       __func__, desc->iface);
 	}
 
 	return ret_val;
@@ -71,59 +71,58 @@ int CYC2_dump(Altera_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case passive_serial:
-		PRINTF ("%s: Launching Passive Serial Dump\n", __FUNCTION__);
-		ret_val = CYC2_ps_dump (desc, buf, bsize);
+		PRINTF("%s: Launching Passive Serial Dump\n", __func__);
+		ret_val = CYC2_ps_dump(desc, buf, bsize);
 		break;
 
 		/* Add new interface types here */
 
 	default:
-		printf ("%s: Unsupported interface type, %d\n",
-				__FUNCTION__, desc->iface);
+		printf("%s: Unsupported interface type, %d\n",
+		       __func__, desc->iface);
 	}
 
 	return ret_val;
 }
 
-int CYC2_info( Altera_desc *desc )
+int CYC2_info(Altera_desc *desc)
 {
 	return FPGA_SUCCESS;
 }
 
 /* ------------------------------------------------------------------------- */
-/* CYCLON2 Passive Serial Generic Implementation                                  */
+/* CYCLON2 Passive Serial Generic Implementation                             */
 static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 {
 	int ret_val = FPGA_FAIL;	/* assume the worst */
 	Altera_CYC2_Passive_Serial_fns *fn = desc->iface_fns;
 	int	ret = 0;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	PRINTF("%s: start with interface functions @ 0x%p\n",
+	       __func__, fn);
 
 	if (fn) {
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
+		PRINTF("%s: Function Table:\n"
 				"ptr:\t0x%p\n"
 				"struct: 0x%p\n"
 				"config:\t0x%p\n"
 				"status:\t0x%p\n"
 				"write:\t0x%p\n"
 				"done:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->config, fn->status,
+				__func__, &fn, fn, fn->config, fn->status,
 				fn->write, fn->done);
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
-		printf ("Loading FPGA Device %d...", cookie);
+		printf("Loading FPGA Device %d...", cookie);
 #endif
 
 		/*
 		 * Run the pre configuration function if there is one.
 		 */
-		if (*fn->pre) {
+		if (*fn->pre)
 			(*fn->pre) (cookie);
-		}
 
 		/* Establish the initial state */
 		(*fn->config) (false, true, cookie);	/* De-assert nCONFIG */
@@ -133,22 +132,23 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 		udelay(2);		/* T_cfg > 2us	*/
 
 		/* Wait for nSTATUS to be asserted */
-		ts = get_timer (0);		/* get current time */
+		ts = get_timer(0);		/* get current time */
 		do {
-			CONFIG_FPGA_DELAY ();
-			if (get_timer (ts) > CONFIG_SYS_FPGA_WAIT) {	/* check the time */
-				puts ("** Timeout waiting for STATUS to go high.\n");
+			CONFIG_FPGA_DELAY();
+			if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT) {
+				/* check the time */
+				puts("** Timeout waiting for STATUS to go high.\n");
 				(*fn->abort) (cookie);
 				return FPGA_FAIL;
 			}
 		} while (!(*fn->status) (cookie));
 
 		/* Get ready for the burn */
-		CONFIG_FPGA_DELAY ();
+		CONFIG_FPGA_DELAY();
 
 		ret = (*fn->write) (buf, bsize, true, cookie);
 		if (ret) {
-			puts ("** Write failed.\n");
+			puts("** Write failed.\n");
 			(*fn->abort) (cookie);
 			return FPGA_FAIL;
 		}
@@ -156,20 +156,20 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 		puts(" OK? ...");
 #endif
 
-		CONFIG_FPGA_DELAY ();
+		CONFIG_FPGA_DELAY();
 
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
-		putc (' ');			/* terminate the dotted line */
+		putc(' ');			/* terminate the dotted line */
 #endif
 
 	/*
 	 * Checking FPGA's CONF_DONE signal - correctly booted ?
 	 */
 
-	if ( ! (*fn->done) (cookie) ) {
-		puts ("** Booting failed! CONF_DONE is still deasserted.\n");
+	if (!(*fn->done) (cookie)) {
+		puts("** Booting failed! CONF_DONE is still deasserted.\n");
 		(*fn->abort) (cookie);
-		return (FPGA_FAIL);
+		return FPGA_FAIL;
 	}
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 	puts(" OK\n");
@@ -178,17 +178,15 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 	ret_val = FPGA_SUCCESS;
 
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
-	if (ret_val == FPGA_SUCCESS) {
-		puts ("Done.\n");
-	}
-	else {
-		puts ("Fail.\n");
-	}
+	if (ret_val == FPGA_SUCCESS)
+		puts("Done.\n");
+	else
+		puts("Fail.\n");
 #endif
 	(*fn->post) (cookie);
 
 	} else {
-		printf ("%s: NULL Interface function table!\n", __FUNCTION__);
+		printf("%s: NULL Interface function table!\n", __func__);
 	}
 
 	return ret_val;
@@ -198,7 +196,6 @@ static int CYC2_ps_dump(Altera_desc *desc, const void *buf, size_t bsize)
 {
 	/* Readback is only available through the Slave Parallel and         */
 	/* boundary-scan interfaces.                                         */
-	printf ("%s: Passive Serial Dumping is unavailable\n",
-			__FUNCTION__);
+	printf("%s: Passive Serial Dumping is unavailable\n", __func__);
 	return FPGA_FAIL;
 }
-- 
2.11.0

[U-Boot] [PATCH v2 3/5] fpga: altera: cyclon2: Fix indentation

[Alexander Dahl]

Some code parts stood too far left …

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/cyclon2.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/fpga/cyclon2.c b/drivers/fpga/cyclon2.c
index 99a64d437c..ab979b017a 100644
--- a/drivers/fpga/cyclon2.c
+++ b/drivers/fpga/cyclon2.c
@@ -162,28 +162,28 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 		putc(' ');			/* terminate the dotted line */
 #endif
 
-	/*
-	 * Checking FPGA's CONF_DONE signal - correctly booted ?
-	 */
-
-	if (!(*fn->done) (cookie)) {
-		puts("** Booting failed! CONF_DONE is still deasserted.\n");
-		(*fn->abort) (cookie);
-		return FPGA_FAIL;
-	}
+		/*
+		 * Checking FPGA's CONF_DONE signal - correctly booted ?
+		 */
+
+		if (!(*fn->done) (cookie)) {
+			puts("** Booting failed! CONF_DONE is still deasserted.\n");
+			(*fn->abort) (cookie);
+			return FPGA_FAIL;
+		}
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
-	puts(" OK\n");
+		puts(" OK\n");
 #endif
 
-	ret_val = FPGA_SUCCESS;
+		ret_val = FPGA_SUCCESS;
 
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
-	if (ret_val == FPGA_SUCCESS)
-		puts("Done.\n");
-	else
-		puts("Fail.\n");
+		if (ret_val == FPGA_SUCCESS)
+			puts("Done.\n");
+		else
+			puts("Fail.\n");
 #endif
-	(*fn->post) (cookie);
+		(*fn->post) (cookie);
 
 	} else {
 		printf("%s: NULL Interface function table!\n", __func__);
-- 
2.11.0

[U-Boot] [PATCH v2 4/5] fpga: altera: cyclon2: Check function pointer before calling

[Alexander Dahl]

As already done for the 'pre' function, a check is added to not follow a
NULL pointer, if somebody has not assigned a 'post' function.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/cyclon2.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/cyclon2.c b/drivers/fpga/cyclon2.c
index ab979b017a..c929cd2cc5 100644
--- a/drivers/fpga/cyclon2.c
+++ b/drivers/fpga/cyclon2.c
@@ -183,8 +183,12 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 		else
 			puts("Fail.\n");
 #endif
-		(*fn->post) (cookie);
 
+		/*
+		 * Run the post configuration function if there is one.
+		 */
+		if (*fn->post)
+			(*fn->post) (cookie);
 	} else {
 		printf("%s: NULL Interface function table!\n", __func__);
 	}
-- 
2.11.0

[U-Boot] [PATCH v2 5/5] cmd: fpga: Change return value to avoid printing usage text

[Alexander Dahl]

In cmd/fpga.c the commands should return enum command_ret_t, e.g.
CMD_RET_USAGE, CMD_RET_SUCCESS, or CMD_RET_FAILURE. What they actually
do is passing a return value from different 'fpga_' functions.

Passing on a return value of -1 from a called function leads to printing
out usage text. In case of actually correct usage with correctly
specified parameters but some fail at runtime printing out that usage
text is distracting.

The reason is most 'fpga_' functions return either FPGA_SUCCESS or
FPGA_FAIL, the latter was equal to -1 which is the same value as
CMD_RET_USAGE. So just passing on FPGA_FAIL lead to printing out usage.

We should only return CMD_RET_USAGE in cases, where the user sent wrong
input. Every other case should return CMD_RET_SUCCESS or
CMD_RET_FAILURE, and not simply pass an error code.

Simply changing FPGA_FAIL from -1 to 1 gets the job done.

Suggested-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 include/fpga.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/fpga.h b/include/fpga.h
index 51de5c55f8..ec5144334d 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -15,7 +15,7 @@
 
 /* fpga_xxxx function return value definitions */
 #define FPGA_SUCCESS		0
-#define FPGA_FAIL		-1
+#define FPGA_FAIL		1
 
 /* device numbers must be non-negative */
 #define FPGA_INVALID_DEVICE	-1
-- 
2.11.0

[U-Boot] [PATCH v2 0/5] fpga: collected patches

[Michal Simek]

Hi,

On 28. 06. 19 14:41, Alexander Dahl wrote:
> Hei hei,
> 
> after some Feedback from Michal Simek, I made a v2 of this patch
> series, and based it onto v2019.07-rc4 now.
> 
> The patch number 2 is new, fixing all kinds of checkpatch warnings
> in 'drivers/fpga/cyclon2.c' before fixing the indentation in that
> file in patch number 3 (which was part of v1 already).
> 
> I dropped the last patch and replaced it with a new one based on the
> suggestion made by Michal.
> 
> Greets
> Alex
> 
> Alexander Dahl (5):
>   fpga: altera: Add some more device sizes
>   fpga: altera: cyclon2: Fix most checkpatch warnings
>   fpga: altera: cyclon2: Fix indentation
>   fpga: altera: cyclon2: Check function pointer before calling
>   cmd: fpga: Change return value to avoid printing usage text
> 
>  drivers/fpga/cyclon2.c | 101 +++++++++++++++++++++++++------------------------
>  include/ACEX1K.h       |  10 +++++
>  include/fpga.h         |   2 +-
>  3 files changed, 62 insertions(+), 51 deletions(-)
> 

Applied all.
M