[PATCH v4 00/10] Use logging feature instead of FPGA_DEBUG

[PATCH v4 00/10] 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 1 is just to be able to
build two of the 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:

v3 -> v4:
- Reordered patches, Kconfig patch comes first now (made it easier to
  build and test the series step by step)
- Added three patches fixing printf compiler warnings first, before
  changing to the new logging framework (so CI should not fail anymore
  with -Werror)

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 (10):
  fpga: Add missing Kconfig symbols for old FPGA drivers
  fpga: spartan2: Fix printf arguments warning
  fpga: spartan3: Fix printf arguments warning
  fpga: virtex2: Fix printf format string warnings
  fpga: altera: Use logging feature instead of FPGA_DEBUG
  fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
  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 v4 01/10] 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 v4 02/10] fpga: spartan2: Fix printf arguments warning

[Alexander Dahl]

That extra comma messes up format arguments.
Warning appears if built with FPGA_DEBUG defined:

      CC      drivers/fpga/spartan2.o
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan2.c: In function ‘spartan2_sp_load’:
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan2.c:112:11: warning: too many arguments for format [-Wformat-extra-args]
       PRINTF ("%s: Function Table:\n"
               ^~~~~~~~~~~~~~~~~~~~~~~
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan2.c:12:37: note: in definition of macro ‘PRINTF’
     #define PRINTF(fmt,args...) printf (fmt ,##args)
                                         ^~~
      CC      drivers/fpga/spartan3.o
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c: In function ‘spartan3_sp_load’:
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c:117:11: warning: too many arguments for format [-Wformat-extra-args]
       PRINTF ("%s: Function Table:\n"
               ^~~~~~~~~~~~~~~~~~~~~~~
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c:17:37: note: in definition of macro ‘PRINTF’
     #define PRINTF(fmt,args...) printf (fmt ,##args)
                                         ^~~

Fixes: e221174377d7 ("Initial revision")
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/spartan2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/spartan2.c b/drivers/fpga/spartan2.c
index 47692e3207..3817ad8bb7 100644
--- a/drivers/fpga/spartan2.c
+++ b/drivers/fpga/spartan2.c
@@ -122,7 +122,7 @@ static int spartan2_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 				"read data:\t0x%p\n"
 				"write data:\t0x%p\n"
 				"busy:\t0x%p\n"
-				"abort:\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,
-- 
2.30.2

[PATCH v4 03/10] fpga: spartan3: Fix printf arguments warning

[Alexander Dahl]

The additional comma messes up the arguments.
Warning appears if built with FPGA_DEBUG defined:

      CC      drivers/fpga/spartan3.o
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c: In function ‘spartan3_sp_load’:
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c:118:11: warning: too many arguments for format [-Wformat-extra-args]
       PRINTF ("%s: Function Table:\n"
               ^~~~~~~~~~~~~~~~~~~~~~~
    /mnt/data/adahl/src/u-boot/drivers/fpga/spartan3.c:18:37: note: in definition of macro ‘PRINTF’
     #define PRINTF(fmt,args...) printf (fmt ,##args)
                                         ^~~

Fixes: 875c78934ee2 ("Add Xilinx Spartan3 family FPGA support Patch by Kurt Stremerch, 14 February 2005")
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/fpga/spartan3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/spartan3.c b/drivers/fpga/spartan3.c
index 918f6db506..641216ad5c 100644
--- a/drivers/fpga/spartan3.c
+++ b/drivers/fpga/spartan3.c
@@ -127,7 +127,7 @@ static int spartan3_sp_load(xilinx_desc *desc, const void *buf, size_t bsize)
 				"read data:\t0x%p\n"
 				"write data:\t0x%p\n"
 				"busy:\t0x%p\n"
-				"abort:\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,
-- 
2.30.2

[PATCH v4 04/10] fpga: virtex2: Fix printf format string warnings

[Alexander Dahl]

Warning appears if built with FPGA_DEBUG defined:

  CC      drivers/fpga/virtex2.o
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c: In function ‘virtex2_ssm_load’:
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c:333:11: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]
    PRINTF("%s:%d:done went active early, bytecount = %d\n",
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           __func__, __LINE__, bytecount);
                               ~~~~~~~~~
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c:25:37: note: in definition of macro ‘PRINTF’
 #define PRINTF(fmt, args...) printf(fmt, ##args)
                                     ^~~
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c: In function ‘virtex2_ss_load’:
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c:468:12: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]
     PRINTF("%s:%d:done went active early, bytecount = %d\n",
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __func__, __LINE__, bytecount);
                                ~~~~~~~~~
/mnt/data/adahl/src/u-boot/drivers/fpga/virtex2.c:25:37: note: in definition of macro ‘PRINTF’
 #define PRINTF(fmt, args...) printf(fmt, ##args)
                                     ^~~

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

diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index 51b8d31205..e769ceebc1 100644
--- a/drivers/fpga/virtex2.c
+++ b/drivers/fpga/virtex2.c
@@ -330,7 +330,7 @@ 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",
+			PRINTF("%s:%d:done went active early, bytecount = %zu\n",
 			       __func__, __LINE__, bytecount);
 			break;
 		}
@@ -465,7 +465,7 @@ 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",
+				PRINTF("%s:%d:done went active early, bytecount = %zu\n",
 				       __func__, __LINE__, bytecount);
 				break;
 			}
-- 
2.30.2

[PATCH v4 05/10] 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 v4 06/10] 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 v4 07/10] 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 v4 08/10] 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 3817ad8bb7..f72dfdec94 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 v4 09/10] 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 641216ad5c..b7a063a95f 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 v4 10/10] 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 e769ceebc1..0d536f0d04 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 = %zu\n",
-			       __func__, __LINE__, bytecount);
+			log_debug("done went active early, bytecount = %zu\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 = %zu\n",
-				       __func__, __LINE__, bytecount);
+				log_debug("done went active early, bytecount = %zu\n",
+					  bytecount);
 				break;
 			}
 
-- 
2.30.2

Re: [PATCH v4 00/10] Use logging feature instead of FPGA_DEBUG

[Michal Simek]

On 10/7/22 14:19, 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 1 is just to be able to
> build two of the 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:
> 
> v3 -> v4:
> - Reordered patches, Kconfig patch comes first now (made it easier to
>    build and test the series step by step)
> - Added three patches fixing printf compiler warnings first, before
>    changing to the new logging framework (so CI should not fail anymore
>    with -Werror)
> 
> 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 (10):
>    fpga: Add missing Kconfig symbols for old FPGA drivers
>    fpga: spartan2: Fix printf arguments warning
>    fpga: spartan3: Fix printf arguments warning
>    fpga: virtex2: Fix printf format string warnings
>    fpga: altera: Use logging feature instead of FPGA_DEBUG
>    fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
>    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

Applied.
M