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 <a...@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, rirel...@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
