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

[Alexander Dahl]

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 at 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 at 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