[PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Hei hei,

while working on FPGA support for a new device I discovered debug
logging in some FPGA drivers is still done as in the old days.  Bring
that to what I thougt would be the currently preferred approach.

Notes: Adding those Kconfig symbols in patch 3 is just to be able to
build those two old drivers.

All drivers touched were build tested with sandbox_defconfig and GCC 8
on Debian GNU/Linux 10 (buster).

Lines with other possibly questionable output were not touched, only
what seemed to be designated debug output, and only for FPGA drivers
having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
future improvements.

Changelog:

v2 -> v3:
- Patch introducing FPGA uclass was completely reworked, sent
  independently from this series, and applied already, thus removed
- Because requiring that new FPGA uclass changes, rebased on Michal's
  microblaze branch '20221005'
- Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
  because log framework can add those (enabled by CONFIG_LOGF_FUNC and
  CONFIG_LOGF_LINE)

v1 -> v2:
- Rebased on master
- Added patch to introduce new FPGA uclass in front of the other patches
- Use that new uclass as log category
- Slightly reworded cover letter

Greets
Alex

Cc: Michal Simek <michal.simek@amd.com>

Alexander Dahl (7):
  fpga: altera: Use logging feature instead of FPGA_DEBUG
  fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
  fpga: Add missing Kconfig symbols for old FPGA drivers
  fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
  fpga: spartan2: Use logging feature instead of FPGA_DEBUG
  fpga: spartan3: Use logging feature instead of FPGA_DEBUG
  fpga: virtex2: Use logging feature instead of FPGA_DEBUG

 drivers/fpga/ACEX1K.c   | 37 +++++++++----------
 drivers/fpga/Kconfig    | 12 +++++++
 drivers/fpga/altera.c   | 11 +++---
 drivers/fpga/cyclon2.c  | 38 +++++++++-----------
 drivers/fpga/spartan2.c | 80 +++++++++++++++++++----------------------
 drivers/fpga/spartan3.c | 80 +++++++++++++++++++----------------------
 drivers/fpga/virtex2.c  | 69 ++++++++++++++++-------------------
 7 files changed, 152 insertions(+), 175 deletions(-)


base-commit: 2d8cf392a77815f062446ef441f1078958dc1b2a
-- 
2.30.2

[PATCH v3 1/7] fpga: altera: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
index 10c0475d25..6a4f0cb9bc 100644
--- a/drivers/fpga/altera.c
+++ b/drivers/fpga/altera.c
@@ -7,6 +7,8 @@
  * Rich Ireland, Enterasys Networks, rireland@enterasys.com.
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 /*
  *  Altera FPGA support
  */
@@ -16,9 +18,6 @@
 #include <log.h>
 #include <stratixII.h>
 
-/* Define FPGA_DEBUG to 1 to get debug printf's */
-#define FPGA_DEBUG	0
-
 static const struct altera_fpga {
 	enum altera_family	family;
 	const char		*name;
@@ -106,8 +105,7 @@ int altera_load(Altera_desc *desc, const void *buf, size_t bsize)
 	if (!fpga)
 		return FPGA_FAIL;
 
-	debug_cond(FPGA_DEBUG, "%s: Launching the %s Loader...\n",
-		   __func__, fpga->name);
+	log_debug("Launching the %s Loader...\n", fpga->name);
 	if (fpga->load)
 		return fpga->load(desc, buf, bsize);
 	return 0;
@@ -120,8 +118,7 @@ int altera_dump(Altera_desc *desc, const void *buf, size_t bsize)
 	if (!fpga)
 		return FPGA_FAIL;
 
-	debug_cond(FPGA_DEBUG, "%s: Launching the %s Reader...\n",
-		   __func__, fpga->name);
+	log_debug("Launching the %s Reader...\n", fpga->name);
 	if (fpga->dump)
 		return fpga->dump(desc, buf, bsize);
 	return 0;
-- 
2.30.2

[PATCH v3 2/7] fpga: cyclon2: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/cyclon2.c b/drivers/fpga/cyclon2.c
index 3b008facb8..f264ff8c0e 100644
--- a/drivers/fpga/cyclon2.c
+++ b/drivers/fpga/cyclon2.c
@@ -5,18 +5,14 @@
  * Based on ACE1XK.c
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 #include <common.h>		/* core U-Boot definitions */
+#include <log.h>
 #include <altera.h>
 #include <ACEX1K.h>		/* ACEX device family */
 #include <linux/delay.h>
 
-/* Define FPGA_DEBUG to get debug printf's */
-#ifdef	FPGA_DEBUG
-#define PRINTF(fmt, args...)	printf(fmt, ##args)
-#else
-#define PRINTF(fmt, args...)
-#endif
-
 /* Note: The assumption is that we cannot possibly run fast enough to
  * overrun the device (the Slave Parallel mode can free run at 50MHz).
  * If there is a need to operate slower, define CONFIG_FPGA_DELAY in
@@ -42,7 +38,7 @@ 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", __func__);
+		log_debug("Launching Passive Serial Loader\n");
 		ret_val = CYC2_ps_load(desc, buf, bsize);
 		break;
 
@@ -51,8 +47,7 @@ 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",
-		       __func__);
+		log_debug("Launching Fast Passive Parallel Loader\n");
 		ret_val = CYC2_ps_load(desc, buf, bsize);
 		break;
 
@@ -72,7 +67,7 @@ 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", __func__);
+		log_debug("Launching Passive Serial Dump\n");
 		ret_val = CYC2_ps_dump(desc, buf, bsize);
 		break;
 
@@ -99,22 +94,21 @@ static int CYC2_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 	Altera_CYC2_Passive_Serial_fns *fn = desc->iface_fns;
 	int	ret = 0;
 
-	PRINTF("%s: start with interface functions @ 0x%p\n",
-	       __func__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		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",
-				__func__, &fn, fn, fn->config, fn->status,
-				fn->write, fn->done);
+		log_debug("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",
+			  &fn, fn, fn->config, fn->status,
+			  fn->write, fn->done);
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 		printf("Loading FPGA Device %d...", cookie);
 #endif
-- 
2.30.2

[PATCH v3 3/7] fpga: Add missing Kconfig symbols for old FPGA drivers

[Alexander Dahl]

Those drivers could not be built anymore without those options present.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/Kconfig | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index e2fd16e6d2..813d6a836d 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -27,6 +27,12 @@ config FPGA_STRATIX_V
 	help
 	  Say Y here to enable the Altera Stratix V FPGA specific driver.
 
+config FPGA_ACEX1K
+	bool "Enable Altera ACEX 1K driver"
+	depends on FPGA_ALTERA
+	help
+	  Say Y here to enable the Altera ACEX 1K FPGA specific driver.
+
 config FPGA_CYCLON2
 	bool "Enable Altera FPGA driver for Cyclone II"
 	depends on FPGA_ALTERA
@@ -71,6 +77,12 @@ config FPGA_VERSALPL
 	  Versal. The bitstream will only be generated as PDI for Versal
 	  platform.
 
+config FPGA_SPARTAN2
+	bool "Enable Spartan2 FPGA driver"
+	depends on FPGA_XILINX
+	help
+	  Enable Spartan2 FPGA driver.
+
 config FPGA_SPARTAN3
 	bool "Enable Spartan3 FPGA driver"
 	depends on FPGA_XILINX
-- 
2.30.2

[PATCH v3 4/7] fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/ACEX1K.c b/drivers/fpga/ACEX1K.c
index aca8049c56..a1ff47035b 100644
--- a/drivers/fpga/ACEX1K.c
+++ b/drivers/fpga/ACEX1K.c
@@ -7,18 +7,14 @@
  * Rich Ireland, Enterasys Networks, rireland@enterasys.com.
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 #include <common.h>		/* core U-Boot definitions */
 #include <console.h>
+#include <log.h>
 #include <ACEX1K.h>		/* ACEX device family */
 #include <linux/delay.h>
 
-/* Define FPGA_DEBUG to get debug printf's */
-#ifdef	FPGA_DEBUG
-#define PRINTF(fmt,args...)	printf (fmt ,##args)
-#else
-#define PRINTF(fmt,args...)
-#endif
-
 /* Note: The assumption is that we cannot possibly run fast enough to
  * overrun the device (the Slave Parallel mode can free run at 50MHz).
  * If there is a need to operate slower, define CONFIG_FPGA_DELAY in
@@ -44,7 +40,7 @@ int ACEX1K_load(Altera_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case passive_serial:
-		PRINTF ("%s: Launching Passive Serial Loader\n", __FUNCTION__);
+		log_debug("Launching Passive Serial Loader\n");
 		ret_val = ACEX1K_ps_load (desc, buf, bsize);
 		break;
 
@@ -64,7 +60,7 @@ int ACEX1K_dump(Altera_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case passive_serial:
-		PRINTF ("%s: Launching Passive Serial Dump\n", __FUNCTION__);
+		log_debug("Launching Passive Serial Dump\n");
 		ret_val = ACEX1K_ps_dump (desc, buf, bsize);
 		break;
 
@@ -93,8 +89,7 @@ static int ACEX1K_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 	Altera_ACEX1K_Passive_Serial_fns *fn = desc->iface_fns;
 	int i;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		size_t bytecount = 0;
@@ -102,16 +97,16 @@ static int ACEX1K_ps_load(Altera_desc *desc, const void *buf, size_t bsize)
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
-				"ptr:\t0x%p\n"
-				"struct: 0x%p\n"
-				"config:\t0x%p\n"
-				"status:\t0x%p\n"
-				"clk:\t0x%p\n"
-				"data:\t0x%p\n"
-				"done:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->config, fn->status,
-				fn->clk, fn->data, fn->done);
+		log_debug("Function Table:\n"
+			  "ptr:\t0x%p\n"
+			  "struct: 0x%p\n"
+			  "config:\t0x%p\n"
+			  "status:\t0x%p\n"
+			  "clk:\t0x%p\n"
+			  "data:\t0x%p\n"
+			  "done:\t0x%p\n\n",
+			  &fn, fn, fn->config, fn->status,
+			  fn->clk, fn->data, fn->done);
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 		printf ("Loading FPGA Device %d...", cookie);
 #endif
-- 
2.30.2

[PATCH v3 5/7] fpga: spartan2: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/spartan2.c b/drivers/fpga/spartan2.c
index 47692e3207..20abf881bd 100644
--- a/drivers/fpga/spartan2.c
+++ b/drivers/fpga/spartan2.c
@@ -4,16 +4,12 @@
  * Rich Ireland, Enterasys Networks, rireland@enterasys.com.
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 #include <common.h>		/* core U-Boot definitions */
+#include <log.h>
 #include <spartan2.h>		/* Spartan-II device family */
 
-/* Define FPGA_DEBUG to get debug printf's */
-#ifdef	FPGA_DEBUG
-#define PRINTF(fmt,args...)	printf (fmt ,##args)
-#else
-#define PRINTF(fmt,args...)
-#endif
-
 #undef CONFIG_SYS_FPGA_CHECK_BUSY
 
 /* Note: The assumption is that we cannot possibly run fast enough to
@@ -46,12 +42,12 @@ static int spartan2_load(xilinx_desc *desc, const void *buf, size_t bsize,
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF ("%s: Launching Slave Serial Load\n", __FUNCTION__);
+		log_debug("Launching Slave Serial Load\n");
 		ret_val = spartan2_ss_load(desc, buf, bsize);
 		break;
 
 	case slave_parallel:
-		PRINTF ("%s: Launching Slave Parallel Load\n", __FUNCTION__);
+		log_debug("Launching Slave Parallel Load\n");
 		ret_val = spartan2_sp_load(desc, buf, bsize);
 		break;
 
@@ -69,12 +65,12 @@ static int spartan2_dump(xilinx_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF ("%s: Launching Slave Serial Dump\n", __FUNCTION__);
+		log_debug("Launching Slave Serial Dump\n");
 		ret_val = spartan2_ss_dump(desc, buf, bsize);
 		break;
 
 	case slave_parallel:
-		PRINTF ("%s: Launching Slave Parallel Dump\n", __FUNCTION__);
+		log_debug("Launching Slave Parallel Dump\n");
 		ret_val = spartan2_sp_dump(desc, buf, bsize);
 		break;
 
@@ -100,8 +96,7 @@ static int spartan2_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 	int ret_val = FPGA_FAIL;	/* assume the worst */
 	xilinx_spartan2_slave_parallel_fns *fn = desc->iface_fns;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		size_t bytecount = 0;
@@ -109,24 +104,24 @@ static int spartan2_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
-				"ptr:\t0x%p\n"
-				"struct: 0x%p\n"
-				"pre: 0x%p\n"
-				"pgm:\t0x%p\n"
-				"init:\t0x%p\n"
-				"err:\t0x%p\n"
-				"clk:\t0x%p\n"
-				"cs:\t0x%p\n"
-				"wr:\t0x%p\n"
-				"read data:\t0x%p\n"
-				"write data:\t0x%p\n"
-				"busy:\t0x%p\n"
-				"abort:\t0x%p\n",
-				"post:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->pre, fn->pgm, fn->init, fn->err,
-				fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, fn->busy,
-				fn->abort, fn->post);
+		log_debug("Function Table:\n"
+			  "ptr:\t0x%p\n"
+			  "struct: 0x%p\n"
+			  "pre: 0x%p\n"
+			  "pgm:\t0x%p\n"
+			  "init:\t0x%p\n"
+			  "err:\t0x%p\n"
+			  "clk:\t0x%p\n"
+			  "cs:\t0x%p\n"
+			  "wr:\t0x%p\n"
+			  "read data:\t0x%p\n"
+			  "write data:\t0x%p\n"
+			  "busy:\t0x%p\n"
+			  "abort:\t0x%p\n",
+			  "post:\t0x%p\n\n",
+			  &fn, fn, fn->pre, fn->pgm, fn->init, fn->err,
+			  fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, fn->busy,
+			  fn->abort, fn->post);
 
 		/*
 		 * This code is designed to emulate the "Express Style"
@@ -302,8 +297,7 @@ static int spartan2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize)
 	int i;
 	unsigned char val;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		size_t bytecount = 0;
@@ -311,16 +305,16 @@ static int spartan2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize)
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
-				"ptr:\t0x%p\n"
-				"struct: 0x%p\n"
-				"pgm:\t0x%p\n"
-				"init:\t0x%p\n"
-				"clk:\t0x%p\n"
-				"wr:\t0x%p\n"
-				"done:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->pgm, fn->init,
-				fn->clk, fn->wr, fn->done);
+		log_debug("Function Table:\n"
+			  "ptr:\t0x%p\n"
+			  "struct: 0x%p\n"
+			  "pgm:\t0x%p\n"
+			  "init:\t0x%p\n"
+			  "clk:\t0x%p\n"
+			  "wr:\t0x%p\n"
+			  "done:\t0x%p\n\n",
+			  &fn, fn, fn->pgm, fn->init,
+			  fn->clk, fn->wr, fn->done);
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 		printf ("Loading FPGA Device %d...\n", cookie);
 #endif
-- 
2.30.2

[PATCH v3 6/7] fpga: spartan3: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/spartan3.c b/drivers/fpga/spartan3.c
index 918f6db506..b001ed8b70 100644
--- a/drivers/fpga/spartan3.c
+++ b/drivers/fpga/spartan3.c
@@ -9,16 +9,12 @@
  * on spartan2.c (Rich Ireland, rireland@enterasys.com).
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 #include <common.h>		/* core U-Boot definitions */
+#include <log.h>
 #include <spartan3.h>		/* Spartan-II device family */
 
-/* Define FPGA_DEBUG to get debug printf's */
-#ifdef	FPGA_DEBUG
-#define PRINTF(fmt,args...)	printf (fmt ,##args)
-#else
-#define PRINTF(fmt,args...)
-#endif
-
 #undef CONFIG_SYS_FPGA_CHECK_BUSY
 
 /* Note: The assumption is that we cannot possibly run fast enough to
@@ -51,12 +47,12 @@ static int spartan3_load(xilinx_desc *desc, const void *buf, size_t bsize,
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF ("%s: Launching Slave Serial Load\n", __FUNCTION__);
+		log_debug("Launching Slave Serial Load\n");
 		ret_val = spartan3_ss_load(desc, buf, bsize);
 		break;
 
 	case slave_parallel:
-		PRINTF ("%s: Launching Slave Parallel Load\n", __FUNCTION__);
+		log_debug("Launching Slave Parallel Load\n");
 		ret_val = spartan3_sp_load(desc, buf, bsize);
 		break;
 
@@ -74,12 +70,12 @@ static int spartan3_dump(xilinx_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF ("%s: Launching Slave Serial Dump\n", __FUNCTION__);
+		log_debug("Launching Slave Serial Dump\n");
 		ret_val = spartan3_ss_dump(desc, buf, bsize);
 		break;
 
 	case slave_parallel:
-		PRINTF ("%s: Launching Slave Parallel Dump\n", __FUNCTION__);
+		log_debug("Launching Slave Parallel Dump\n");
 		ret_val = spartan3_sp_dump(desc, buf, bsize);
 		break;
 
@@ -105,8 +101,7 @@ static int spartan3_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 	int ret_val = FPGA_FAIL;	/* assume the worst */
 	xilinx_spartan3_slave_parallel_fns *fn = desc->iface_fns;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		size_t bytecount = 0;
@@ -114,24 +109,24 @@ static int spartan3_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
-				"ptr:\t0x%p\n"
-				"struct: 0x%p\n"
-				"pre: 0x%p\n"
-				"pgm:\t0x%p\n"
-				"init:\t0x%p\n"
-				"err:\t0x%p\n"
-				"clk:\t0x%p\n"
-				"cs:\t0x%p\n"
-				"wr:\t0x%p\n"
-				"read data:\t0x%p\n"
-				"write data:\t0x%p\n"
-				"busy:\t0x%p\n"
-				"abort:\t0x%p\n",
-				"post:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->pre, fn->pgm, fn->init, fn->err,
-				fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, fn->busy,
-				fn->abort, fn->post);
+		log_debug("Function Table:\n"
+			  "ptr:\t0x%p\n"
+			  "struct: 0x%p\n"
+			  "pre: 0x%p\n"
+			  "pgm:\t0x%p\n"
+			  "init:\t0x%p\n"
+			  "err:\t0x%p\n"
+			  "clk:\t0x%p\n"
+			  "cs:\t0x%p\n"
+			  "wr:\t0x%p\n"
+			  "read data:\t0x%p\n"
+			  "write data:\t0x%p\n"
+			  "busy:\t0x%p\n"
+			  "abort:\t0x%p\n",
+			  "post:\t0x%p\n\n",
+			  &fn, fn, fn->pre, fn->pgm, fn->init, fn->err,
+			  fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata, fn->busy,
+			  fn->abort, fn->post);
 
 		/*
 		 * This code is designed to emulate the "Express Style"
@@ -309,8 +304,7 @@ static int spartan3_ss_load(xilinx_desc *desc, const void *buf, size_t bsize)
 	int i;
 	unsigned char val;
 
-	PRINTF ("%s: start with interface functions @ 0x%p\n",
-			__FUNCTION__, fn);
+	log_debug("start with interface functions @ 0x%p\n", fn);
 
 	if (fn) {
 		size_t bytecount = 0;
@@ -318,16 +312,16 @@ static int spartan3_ss_load(xilinx_desc *desc, const void *buf, size_t bsize)
 		int cookie = desc->cookie;	/* make a local copy */
 		unsigned long ts;		/* timestamp */
 
-		PRINTF ("%s: Function Table:\n"
-				"ptr:\t0x%p\n"
-				"struct: 0x%p\n"
-				"pgm:\t0x%p\n"
-				"init:\t0x%p\n"
-				"clk:\t0x%p\n"
-				"wr:\t0x%p\n"
-				"done:\t0x%p\n\n",
-				__FUNCTION__, &fn, fn, fn->pgm, fn->init,
-				fn->clk, fn->wr, fn->done);
+		log_debug("Function Table:\n"
+			  "ptr:\t0x%p\n"
+			  "struct: 0x%p\n"
+			  "pgm:\t0x%p\n"
+			  "init:\t0x%p\n"
+			  "clk:\t0x%p\n"
+			  "wr:\t0x%p\n"
+			  "done:\t0x%p\n\n",
+			  &fn, fn, fn->pgm, fn->init,
+			  fn->clk, fn->wr, fn->done);
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 		printf ("Loading FPGA Device %d...\n", cookie);
 #endif
-- 
2.30.2

[PATCH v3 7/7] fpga: virtex2: Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Instead of using DEBUG or LOG_DEBUG the driver still had its own
definition for debug output.

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

diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index 51b8d31205..760edb6883 100644
--- a/drivers/fpga/virtex2.c
+++ b/drivers/fpga/virtex2.c
@@ -12,21 +12,14 @@
  * on spartan2.c (Rich Ireland, rireland@enterasys.com).
  */
 
+#define LOG_CATEGORY UCLASS_FPGA
+
 #include <common.h>
 #include <console.h>
+#include <log.h>
 #include <virtex2.h>
 #include <linux/delay.h>
 
-#if 0
-#define FPGA_DEBUG
-#endif
-
-#ifdef	FPGA_DEBUG
-#define	PRINTF(fmt, args...)	printf(fmt, ##args)
-#else
-#define PRINTF(fmt, args...)
-#endif
-
 /*
  * If the SelectMap interface can be overrun by the processor, define
  * CONFIG_SYS_FPGA_CHECK_BUSY and/or CONFIG_FPGA_DELAY in the board
@@ -89,12 +82,12 @@ static int virtex2_load(xilinx_desc *desc, const void *buf, size_t bsize,
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF("%s: Launching Slave Serial Load\n", __func__);
+		log_debug("Launching Slave Serial Load\n");
 		ret_val = virtex2_ss_load(desc, buf, bsize);
 		break;
 
 	case slave_selectmap:
-		PRINTF("%s: Launching Slave Parallel Load\n", __func__);
+		log_debug("Launching Slave Parallel Load\n");
 		ret_val = virtex2_ssm_load(desc, buf, bsize);
 		break;
 
@@ -111,12 +104,12 @@ static int virtex2_dump(xilinx_desc *desc, const void *buf, size_t bsize)
 
 	switch (desc->iface) {
 	case slave_serial:
-		PRINTF("%s: Launching Slave Serial Dump\n", __func__);
+		log_debug("Launching Slave Serial Dump\n");
 		ret_val = virtex2_ss_dump(desc, buf, bsize);
 		break;
 
 	case slave_parallel:
-		PRINTF("%s: Launching Slave Parallel Dump\n", __func__);
+		log_debug("Launching Slave Parallel Dump\n");
 		ret_val = virtex2_ssm_dump(desc, buf, bsize);
 		break;
 
@@ -150,8 +143,7 @@ static int virtex2_slave_pre(xilinx_virtex2_slave_fns *fn, int cookie)
 {
 	unsigned long ts;
 
-	PRINTF("%s:%d: Start with interface functions @ 0x%p\n",
-	       __func__, __LINE__, fn);
+	log_debug("Start with interface functions @ 0x%p\n", fn);
 
 	if (!fn) {
 		printf("%s:%d: NULL Interface function table!\n",
@@ -160,25 +152,24 @@ static int virtex2_slave_pre(xilinx_virtex2_slave_fns *fn, int cookie)
 	}
 
 	/* Gotta split this one up (so the stack won't blow??) */
-	PRINTF("%s:%d: Function Table:\n"
-	       "  base   0x%p\n"
-	       "  struct 0x%p\n"
-	       "  pre    0x%p\n"
-	       "  prog   0x%p\n"
-	       "  init   0x%p\n"
-	       "  error  0x%p\n",
-	       __func__, __LINE__,
-	       &fn, fn, fn->pre, fn->pgm, fn->init, fn->err);
-	PRINTF("  clock  0x%p\n"
-	       "  cs     0x%p\n"
-	       "  write  0x%p\n"
-	       "  rdata  0x%p\n"
-	       "  wdata  0x%p\n"
-	       "  busy   0x%p\n"
-	       "  abort  0x%p\n"
-	       "  post   0x%p\n\n",
-	       fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata,
-	       fn->busy, fn->abort, fn->post);
+	log_debug("Function Table:\n"
+		  "  base   0x%p\n"
+		  "  struct 0x%p\n"
+		  "  pre    0x%p\n"
+		  "  prog   0x%p\n"
+		  "  init   0x%p\n"
+		  "  error  0x%p\n",
+		  &fn, fn, fn->pre, fn->pgm, fn->init, fn->err);
+	log_debug("  clock  0x%p\n"
+		  "  cs     0x%p\n"
+		  "  write  0x%p\n"
+		  "  rdata  0x%p\n"
+		  "  wdata  0x%p\n"
+		  "  busy   0x%p\n"
+		  "  abort  0x%p\n"
+		  "  post   0x%p\n\n",
+		  fn->clk, fn->cs, fn->wr, fn->rdata, fn->wdata,
+		  fn->busy, fn->abort, fn->post);
 
 #ifdef CONFIG_SYS_FPGA_PROG_FEEDBACK
 	printf("Initializing FPGA Device %d...\n", cookie);
@@ -330,8 +321,8 @@ static int virtex2_ssm_load(xilinx_desc *desc, const void *buf, size_t bsize)
 #endif
 
 		if ((*fn->done)(cookie) == FPGA_SUCCESS) {
-			PRINTF("%s:%d:done went active early, bytecount = %d\n",
-			       __func__, __LINE__, bytecount);
+			log_debug("done went active early, bytecount = %d\n",
+				  bytecount);
 			break;
 		}
 
@@ -465,8 +456,8 @@ static int virtex2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize)
 #endif
 
 			if ((*fn->done)(cookie) == FPGA_SUCCESS) {
-				PRINTF("%s:%d:done went active early, bytecount = %d\n",
-				       __func__, __LINE__, bytecount);
+				log_debug("done went active early, bytecount = %d\n",
+					  bytecount);
 				break;
 			}
 
-- 
2.30.2

Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[Michal Simek]

Hi,

On 10/5/22 13:44, Alexander Dahl wrote:
> Hei hei,
> 
> while working on FPGA support for a new device I discovered debug
> logging in some FPGA drivers is still done as in the old days.  Bring
> that to what I thougt would be the currently preferred approach.
> 
> Notes: Adding those Kconfig symbols in patch 3 is just to be able to
> build those two old drivers.
> 
> All drivers touched were build tested with sandbox_defconfig and GCC 8
> on Debian GNU/Linux 10 (buster).
> 
> Lines with other possibly questionable output were not touched, only
> what seemed to be designated debug output, and only for FPGA drivers
> having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
> future improvements.
> 
> Changelog:
> 
> v2 -> v3:
> - Patch introducing FPGA uclass was completely reworked, sent
>    independently from this series, and applied already, thus removed
> - Because requiring that new FPGA uclass changes, rebased on Michal's
>    microblaze branch '20221005'
> - Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
>    because log framework can add those (enabled by CONFIG_LOGF_FUNC and
>    CONFIG_LOGF_LINE)
> 
> v1 -> v2:
> - Rebased on master
> - Added patch to introduce new FPGA uclass in front of the other patches
> - Use that new uclass as log category
> - Slightly reworded cover letter
> 
> Greets
> Alex
> 
> Cc: Michal Simek <michal.simek@amd.com>
> 
> Alexander Dahl (7):
>    fpga: altera: Use logging feature instead of FPGA_DEBUG
>    fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
>    fpga: Add missing Kconfig symbols for old FPGA drivers
>    fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
>    fpga: spartan2: Use logging feature instead of FPGA_DEBUG
>    fpga: spartan3: Use logging feature instead of FPGA_DEBUG
>    fpga: virtex2: Use logging feature instead of FPGA_DEBUG
> 
>   drivers/fpga/ACEX1K.c   | 37 +++++++++----------
>   drivers/fpga/Kconfig    | 12 +++++++
>   drivers/fpga/altera.c   | 11 +++---
>   drivers/fpga/cyclon2.c  | 38 +++++++++-----------
>   drivers/fpga/spartan2.c | 80 +++++++++++++++++++----------------------
>   drivers/fpga/spartan3.c | 80 +++++++++++++++++++----------------------
>   drivers/fpga/virtex2.c  | 69 ++++++++++++++++-------------------
>   7 files changed, 152 insertions(+), 175 deletions(-)

I pushed it to CI loop and got failure.

https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/508906

Building current source for 136 boards (64 threads, 1 job per thread)
       m68k:  +   astro_mcf5373l
+In file included from include/linux/printk.h:4,
+                 from include/common.h:20,
+                 from drivers/fpga/spartan3.c:14:
+drivers/fpga/spartan3.c: In function 'spartan3_sp_load':
+drivers/fpga/spartan3.c:112:27: error: too many arguments for format 
[-Werror=format-extra-args]
+  112 |                 log_debug("Function Table:\n"
+      |                           ^~~~~~~~~~~~~~~~~~~
+include/log.h:220:24: note: in definition of macro 'log'
+  220 |                 printf(_fmt, ##_args); \
+      |                        ^~~~
+drivers/fpga/spartan3.c:112:17: note: in expansion of macro 'log_debug'
+      |                 ^~~~~~~~~
+cc1: all warnings being treated as errors
+make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Error 1
+make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2
+make[1]: *** [Makefile:1883: drivers] Error 2

Please fix it up.

Thanks,
Michal

Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Hello Michal,

Am Donnerstag, 6. Oktober 2022, 16:44:29 CEST schrieb Michal Simek:
> Hi,
> 
> On 10/5/22 13:44, Alexander Dahl wrote:
> > Hei hei,
> > 
> > while working on FPGA support for a new device I discovered debug
> > logging in some FPGA drivers is still done as in the old days.  Bring
> > that to what I thougt would be the currently preferred approach.
> > 
> > Notes: Adding those Kconfig symbols in patch 3 is just to be able to
> > build those two old drivers.
> > 
> > All drivers touched were build tested with sandbox_defconfig and GCC 8
> > on Debian GNU/Linux 10 (buster).
> > 
> > Lines with other possibly questionable output were not touched, only
> > what seemed to be designated debug output, and only for FPGA drivers
> > having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
> > future improvements.
> > 
> > Changelog:
> > 
> > v2 -> v3:
> > - Patch introducing FPGA uclass was completely reworked, sent
> > 
> >    independently from this series, and applied already, thus removed
> > 
> > - Because requiring that new FPGA uclass changes, rebased on Michal's
> > 
> >    microblaze branch '20221005'
> > 
> > - Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
> > 
> >    because log framework can add those (enabled by CONFIG_LOGF_FUNC and
> >    CONFIG_LOGF_LINE)
> > 
> > v1 -> v2:
> > - Rebased on master
> > - Added patch to introduce new FPGA uclass in front of the other patches
> > - Use that new uclass as log category
> > - Slightly reworded cover letter
> > 
> > Greets
> > Alex
> > 
> > Cc: Michal Simek <michal.simek@amd.com>
> > 
> > Alexander Dahl (7):
> >    fpga: altera: Use logging feature instead of FPGA_DEBUG
> >    fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
> >    fpga: Add missing Kconfig symbols for old FPGA drivers
> >    fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
> >    fpga: spartan2: Use logging feature instead of FPGA_DEBUG
> >    fpga: spartan3: Use logging feature instead of FPGA_DEBUG
> >    fpga: virtex2: Use logging feature instead of FPGA_DEBUG
> >   
> >   drivers/fpga/ACEX1K.c   | 37 +++++++++----------
> >   drivers/fpga/Kconfig    | 12 +++++++
> >   drivers/fpga/altera.c   | 11 +++---
> >   drivers/fpga/cyclon2.c  | 38 +++++++++-----------
> >   drivers/fpga/spartan2.c | 80 +++++++++++++++++++----------------------
> >   drivers/fpga/spartan3.c | 80 +++++++++++++++++++----------------------
> >   drivers/fpga/virtex2.c  | 69 ++++++++++++++++-------------------
> >   7 files changed, 152 insertions(+), 175 deletions(-)
> 
> I pushed it to CI loop and got failure.
> 
> https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/508906
> 
> Building current source for 136 boards (64 threads, 1 job per thread)
>        m68k:  +   astro_mcf5373l
> +In file included from include/linux/printk.h:4,
> +                 from include/common.h:20,
> +                 from drivers/fpga/spartan3.c:14:
> +drivers/fpga/spartan3.c: In function 'spartan3_sp_load':
> +drivers/fpga/spartan3.c:112:27: error: too many arguments for format
> [-Werror=format-extra-args]
> +  112 |                 log_debug("Function Table:\n"
> +      |                           ^~~~~~~~~~~~~~~~~~~
> +include/log.h:220:24: note: in definition of macro 'log'
> +  220 |                 printf(_fmt, ##_args); \
> +      |                        ^~~~
> +drivers/fpga/spartan3.c:112:17: note: in expansion of macro 'log_debug'
> +      |                 ^~~~~~~~~
> +cc1: all warnings being treated as errors
> +make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Error 1
> +make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2
> +make[1]: *** [Makefile:1883: drivers] Error 2
> 
> Please fix it up.

Not sure if those warnings were present before on the old PRINTF calls, but we 
got them now.  However the underlying problem was there before: putting to 
much things in one printf/log line.  I can go split it up like in 'drivers/
fpga/virtex2.c' already, where you have the following comment:

    /* Gotta split this one up (so the stack won't blow??) */

Not sure however if debug printing all function pointers in those function 
tables has any value at all? Maybe that can just be dropped?

Greets
Alex

Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[Michal Simek]

Hi,

On 10/6/22 16:56, Alexander Dahl wrote:
> Hello Michal,
> 
> Am Donnerstag, 6. Oktober 2022, 16:44:29 CEST schrieb Michal Simek:
>> Hi,
>>
>> On 10/5/22 13:44, Alexander Dahl wrote:
>>> Hei hei,
>>>
>>> while working on FPGA support for a new device I discovered debug
>>> logging in some FPGA drivers is still done as in the old days.  Bring
>>> that to what I thougt would be the currently preferred approach.
>>>
>>> Notes: Adding those Kconfig symbols in patch 3 is just to be able to
>>> build those two old drivers.
>>>
>>> All drivers touched were build tested with sandbox_defconfig and GCC 8
>>> on Debian GNU/Linux 10 (buster).
>>>
>>> Lines with other possibly questionable output were not touched, only
>>> what seemed to be designated debug output, and only for FPGA drivers
>>> having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
>>> future improvements.
>>>
>>> Changelog:
>>>
>>> v2 -> v3:
>>> - Patch introducing FPGA uclass was completely reworked, sent
>>>
>>>     independently from this series, and applied already, thus removed
>>>
>>> - Because requiring that new FPGA uclass changes, rebased on Michal's
>>>
>>>     microblaze branch '20221005'
>>>
>>> - Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
>>>
>>>     because log framework can add those (enabled by CONFIG_LOGF_FUNC and
>>>     CONFIG_LOGF_LINE)
>>>
>>> v1 -> v2:
>>> - Rebased on master
>>> - Added patch to introduce new FPGA uclass in front of the other patches
>>> - Use that new uclass as log category
>>> - Slightly reworded cover letter
>>>
>>> Greets
>>> Alex
>>>
>>> Cc: Michal Simek <michal.simek@amd.com>
>>>
>>> Alexander Dahl (7):
>>>     fpga: altera: Use logging feature instead of FPGA_DEBUG
>>>     fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
>>>     fpga: Add missing Kconfig symbols for old FPGA drivers
>>>     fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
>>>     fpga: spartan2: Use logging feature instead of FPGA_DEBUG
>>>     fpga: spartan3: Use logging feature instead of FPGA_DEBUG
>>>     fpga: virtex2: Use logging feature instead of FPGA_DEBUG
>>>    
>>>    drivers/fpga/ACEX1K.c   | 37 +++++++++----------
>>>    drivers/fpga/Kconfig    | 12 +++++++
>>>    drivers/fpga/altera.c   | 11 +++---
>>>    drivers/fpga/cyclon2.c  | 38 +++++++++-----------
>>>    drivers/fpga/spartan2.c | 80 +++++++++++++++++++----------------------
>>>    drivers/fpga/spartan3.c | 80 +++++++++++++++++++----------------------
>>>    drivers/fpga/virtex2.c  | 69 ++++++++++++++++-------------------
>>>    7 files changed, 152 insertions(+), 175 deletions(-)
>>
>> I pushed it to CI loop and got failure.
>>
>> https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/508906
>>
>> Building current source for 136 boards (64 threads, 1 job per thread)
>>         m68k:  +   astro_mcf5373l
>> +In file included from include/linux/printk.h:4,
>> +                 from include/common.h:20,
>> +                 from drivers/fpga/spartan3.c:14:
>> +drivers/fpga/spartan3.c: In function 'spartan3_sp_load':
>> +drivers/fpga/spartan3.c:112:27: error: too many arguments for format
>> [-Werror=format-extra-args]
>> +  112 |                 log_debug("Function Table:\n"
>> +      |                           ^~~~~~~~~~~~~~~~~~~
>> +include/log.h:220:24: note: in definition of macro 'log'
>> +  220 |                 printf(_fmt, ##_args); \
>> +      |                        ^~~~
>> +drivers/fpga/spartan3.c:112:17: note: in expansion of macro 'log_debug'
>> +      |                 ^~~~~~~~~
>> +cc1: all warnings being treated as errors
>> +make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Error 1
>> +make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2
>> +make[1]: *** [Makefile:1883: drivers] Error 2
>>
>> Please fix it up.
> 
> Not sure if those warnings were present before on the old PRINTF calls, but we
> got them now.  However the underlying problem was there before: putting to
> much things in one printf/log line.  I can go split it up like in 'drivers/
> fpga/virtex2.c' already, where you have the following comment:
> 
>      /* Gotta split this one up (so the stack won't blow??) */
> 
> Not sure however if debug printing all function pointers in those function
> tables has any value at all? Maybe that can just be dropped?

No idea if this is useful or not. But it is there and I would split it as it is 
done in virtex2. Please do it in separate patch with mentioning virtex2 to have 
the same change.

Thanks,
Michal

Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG

[Alexander Dahl]

Hello Michal,

Am Freitag, 7. Oktober 2022, 08:34:15 CEST schrieb Michal Simek:
> Hi,
> 
> On 10/6/22 16:56, Alexander Dahl wrote:
> > Hello Michal,
> > 
> > Am Donnerstag, 6. Oktober 2022, 16:44:29 CEST schrieb Michal Simek:
> >> Hi,
> >> 
> >> On 10/5/22 13:44, Alexander Dahl wrote:
> >>> Hei hei,
> >>> 
> >>> while working on FPGA support for a new device I discovered debug
> >>> logging in some FPGA drivers is still done as in the old days.  Bring
> >>> that to what I thougt would be the currently preferred approach.
> >>> 
> >>> Notes: Adding those Kconfig symbols in patch 3 is just to be able to
> >>> build those two old drivers.
> >>> 
> >>> All drivers touched were build tested with sandbox_defconfig and GCC 8
> >>> on Debian GNU/Linux 10 (buster).
> >>> 
> >>> Lines with other possibly questionable output were not touched, only
> >>> what seemed to be designated debug output, and only for FPGA drivers
> >>> having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
> >>> future improvements.
> >>> 
> >>> Changelog:
> >>> 
> >>> v2 -> v3:
> >>> - Patch introducing FPGA uclass was completely reworked, sent
> >>> 
> >>>     independently from this series, and applied already, thus removed
> >>> 
> >>> - Because requiring that new FPGA uclass changes, rebased on Michal's
> >>> 
> >>>     microblaze branch '20221005'
> >>> 
> >>> - Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
> >>> 
> >>>     because log framework can add those (enabled by CONFIG_LOGF_FUNC and
> >>>     CONFIG_LOGF_LINE)
> >>> 
> >>> v1 -> v2:
> >>> - Rebased on master
> >>> - Added patch to introduce new FPGA uclass in front of the other patches
> >>> - Use that new uclass as log category
> >>> - Slightly reworded cover letter
> >>> 
> >>> Greets
> >>> Alex
> >>> 
> >>> Cc: Michal Simek <michal.simek@amd.com>
> >>> 
> >>> Alexander Dahl (7):
> >>>     fpga: altera: Use logging feature instead of FPGA_DEBUG
> >>>     fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
> >>>     fpga: Add missing Kconfig symbols for old FPGA drivers
> >>>     fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
> >>>     fpga: spartan2: Use logging feature instead of FPGA_DEBUG
> >>>     fpga: spartan3: Use logging feature instead of FPGA_DEBUG
> >>>     fpga: virtex2: Use logging feature instead of FPGA_DEBUG
> >>>    
> >>>    drivers/fpga/ACEX1K.c   | 37 +++++++++----------
> >>>    drivers/fpga/Kconfig    | 12 +++++++
> >>>    drivers/fpga/altera.c   | 11 +++---
> >>>    drivers/fpga/cyclon2.c  | 38 +++++++++-----------
> >>>    drivers/fpga/spartan2.c | 80
> >>>    +++++++++++++++++++----------------------
> >>>    drivers/fpga/spartan3.c | 80
> >>>    +++++++++++++++++++----------------------
> >>>    drivers/fpga/virtex2.c  | 69 ++++++++++++++++-------------------
> >>>    7 files changed, 152 insertions(+), 175 deletions(-)
> >> 
> >> I pushed it to CI loop and got failure.
> >> 
> >> https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/508906
> >> 
> >> Building current source for 136 boards (64 threads, 1 job per thread)
> >> 
> >>         m68k:  +   astro_mcf5373l
> >> 
> >> +In file included from include/linux/printk.h:4,
> >> +                 from include/common.h:20,
> >> +                 from drivers/fpga/spartan3.c:14:
> >> +drivers/fpga/spartan3.c: In function 'spartan3_sp_load':
> >> +drivers/fpga/spartan3.c:112:27: error: too many arguments for format
> >> [-Werror=format-extra-args]
> >> +  112 |                 log_debug("Function Table:\n"
> >> +      |                           ^~~~~~~~~~~~~~~~~~~
> >> +include/log.h:220:24: note: in definition of macro 'log'
> >> +  220 |                 printf(_fmt, ##_args); \
> >> +      |                        ^~~~
> >> +drivers/fpga/spartan3.c:112:17: note: in expansion of macro 'log_debug'
> >> +      |                 ^~~~~~~~~
> >> +cc1: all warnings being treated as errors
> >> +make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Error
> >> 1
> >> +make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2
> >> +make[1]: *** [Makefile:1883: drivers] Error 2
> >> 
> >> Please fix it up.
> > 
> > Not sure if those warnings were present before on the old PRINTF calls,
> > but we got them now.  However the underlying problem was there before:
> > putting to much things in one printf/log line.  I can go split it up like
> > in 'drivers/> 
> > fpga/virtex2.c' already, where you have the following comment:
> >      /* Gotta split this one up (so the stack won't blow??) */
> > 
> > Not sure however if debug printing all function pointers in those function
> > tables has any value at all? Maybe that can just be dropped?
> 
> No idea if this is useful or not. But it is there and I would split it as it
> is done in virtex2. Please do it in separate patch with mentioning virtex2
> to have the same change.

Turned out there was a different cause for those warnings.  
See v4 of the series which I just sent.

Have a nice weekend
Alex