Re: [PATCH v2 8/8] fpga: virtex2: Use logging feature instead of FPGA_DEBUG

22 Sep 2022

Hello Michal,
Am Thu, Sep 22, 2022 at 12:30:08PM +0200 schrieb Michal Simek:
...
On 9/21/22 15:22, Alexander Dahl wrote:
...
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 | 37 +++++++++++++++----------------------
  1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index 51b8d31205..fddf8ac4ce 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("%s: Launching Slave Serial Load\n", __func__);

ret_val = virtex2_ss_load(desc, buf, bsize);
break;
case slave_selectmap:


PRINTF("%s: Launching Slave Parallel Load\n", __func__);




log_debug("%s: Launching Slave Parallel Load\n", __func__);

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("%s: Launching Slave Serial Dump\n", __func__);

ret_val = virtex2_ss_dump(desc, buf, bsize);
break;
case slave_parallel:


PRINTF("%s: Launching Slave Parallel Dump\n", __func__);




log_debug("%s: Launching Slave Parallel Dump\n", __func__);

ret_val = virtex2_ssm_dump(desc, buf, bsize);
break;

@@ -150,8 +143,8 @@ 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("%s:%d: Start with interface functions @ 0x%p\n",
  __func__, __LINE__, fn);

if (!fn) {
printf("%s:%d: NULL Interface function table!\n",

@@ -160,7 +153,7 @@ 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"


log_debug("%s:%d: Function Table:\n"
     "  base   0x%p\n"

Above you are also aligning next lines which is not what you do here.
The same issue is visible also in other patches. I think it will be good to
also align it to have proper coding style.
I think I can explain that.  I just fixed checkpatch warnings before
sending, and I got those only for lines changed.  I did only change
next lines when checkpatch complained about __FUNCTION__, which is not
stricly what I intended to change here, but did it along the way.
You're right, it makes sense to align all of them.
Will send a v3.
Greets
Alex
...
Thanks,
Michal

    