From: Pieter Van Trappen <pieter.van.trap...@cern.ch>

In cmd/fpga.c, change some `debug` calls to `log_err` for important
user feedback and use CMD_RET_FAILURE in favor of CMD_RET_USAGE due to
its long output which hides the actual, useful return message. Change
the remaining `debug` calls to `log_debug`. Remove all 'fpga:' and
__func__ strings as log_* has this covered.

For `do_fpga_loads`, move up the `do_fpga_check_params` call for more
consistent command output; use a constant instead of multiple '5' use.

In drivers/fpga/zynq*.c, change 'up to' to 'above' which corrects this
confusing/wrong statement.

Signed-off-by: Pieter Van Trappen <pieter.van.trap...@cern.ch>
---
 cmd/fpga.c              | 99 +++++++++++++++++++++--------------------
 drivers/fpga/zynqmppl.c |  4 +-
 drivers/fpga/zynqpl.c   |  4 +-
 3 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 9dc7b63db5d..9cc51f04698 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -28,7 +28,7 @@ static long do_fpga_get_device(char *arg)
        if (dev == FPGA_INVALID_DEVICE && arg)
                dev = simple_strtol(arg, NULL, 16);
 
-       debug("%s: device = %ld\n", __func__, dev);
+       log_debug("device = %ld\n", dev);
 
        return dev;
 }
@@ -40,26 +40,26 @@ static int do_fpga_check_params(long *dev, long *fpga_data, 
size_t *data_size,
        size_t local_data_size;
        long local_fpga_data;
 
-       debug("%s %d, %d\n", __func__, argc, cmdtp->maxargs);
+       log_debug("%d, %d\n", argc, cmdtp->maxargs);
 
        if (argc != cmdtp->maxargs) {
-               debug("fpga: incorrect parameters passed\n");
-               return CMD_RET_USAGE;
+               log_err("Incorrect number of parameters passed\n");
+               return CMD_RET_FAILURE;
        }
 
        *dev = do_fpga_get_device(argv[0]);
 
        local_fpga_data = simple_strtol(argv[1], NULL, 16);
        if (!local_fpga_data) {
-               debug("fpga: zero fpga_data address\n");
-               return CMD_RET_USAGE;
+               log_err("Zero fpga_data address\n");
+               return CMD_RET_FAILURE;
        }
        *fpga_data = local_fpga_data;
 
        local_data_size = hextoul(argv[2], NULL);
        if (!local_data_size) {
-               debug("fpga: zero size\n");
-               return CMD_RET_USAGE;
+               log_err("Zero size\n");
+               return CMD_RET_FAILURE;
        }
        *data_size = local_data_size;
 
@@ -70,51 +70,52 @@ static int do_fpga_check_params(long *dev, long *fpga_data, 
size_t *data_size,
 static int do_fpga_loads(struct cmd_tbl *cmdtp, int flag, int argc,
                         char *const argv[])
 {
+       struct fpga_secure_info fpga_sec_info;
+       const int pos_userkey = 5;
        size_t data_size = 0;
        long fpga_data, dev;
        int ret;
-       struct fpga_secure_info fpga_sec_info;
 
        memset(&fpga_sec_info, 0, sizeof(fpga_sec_info));
 
-       if (argc < 5) {
-               debug("fpga: incorrect parameters passed\n");
-               return CMD_RET_USAGE;
+       if (argc < pos_userkey) {
+               log_err("Too few parameters passed\n");
+               return CMD_RET_FAILURE;
        }
 
-       if (argc == 6)
+       if (argc == pos_userkey + 1)
                fpga_sec_info.userkey_addr = (u8 *)(uintptr_t)
-                                             simple_strtoull(argv[5],
+                                             simple_strtoull(argv[pos_userkey],
                                                              NULL, 16);
        else
                /*
                 * If 6th parameter is not passed then do_fpga_check_params
                 * will get 5 instead of expected 6 which means that function
-                * return CMD_RET_USAGE. Increase number of params +1 to pass
+                * return CMD_RET_FAILURE. Increase number of params +1 to pass
                 * this.
                 */
                argc++;
 
+       ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+                                  cmdtp, argc, argv);
+       if (ret)
+               return ret;
+
        fpga_sec_info.encflag = (u8)hextoul(argv[4], NULL);
        fpga_sec_info.authflag = (u8)hextoul(argv[3], NULL);
 
        if (fpga_sec_info.authflag >= FPGA_NO_ENC_OR_NO_AUTH &&
            fpga_sec_info.encflag >= FPGA_NO_ENC_OR_NO_AUTH) {
-               debug("fpga: Use <fpga load> for NonSecure bitstream\n");
-               return CMD_RET_USAGE;
+               log_err("Use <fpga load> for NonSecure bitstream\n");
+               return CMD_RET_FAILURE;
        }
 
        if (fpga_sec_info.encflag == FPGA_ENC_USR_KEY &&
            !fpga_sec_info.userkey_addr) {
-               debug("fpga: User key not provided\n");
-               return CMD_RET_USAGE;
+               log_err("User key not provided\n");
+               return CMD_RET_FAILURE;
        }
 
-       ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
-                                  cmdtp, argc, argv);
-       if (ret)
-               return ret;
-
        return fpga_loads(dev, (void *)fpga_data, data_size, &fpga_sec_info);
 }
 #endif
@@ -245,23 +246,23 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int 
flag, int argc,
        ulong dev = do_fpga_get_device(argv[0]);
        char *datastr = env_get("fpgadata");
 
-       debug("fpga: argc %x, dev %lx, datastr %s\n", argc, dev, datastr);
+       log_debug("argc %x, dev %lx, datastr %s\n", argc, dev, datastr);
 
        if (dev == FPGA_INVALID_DEVICE) {
-               debug("fpga: Invalid fpga device\n");
-               return CMD_RET_USAGE;
+               log_err("Invalid fpga device\n");
+               return CMD_RET_FAILURE;
        }
 
        if (argc == 0 && !datastr) {
-               debug("fpga: No datastr passed\n");
-               return CMD_RET_USAGE;
+               log_err("No datastr passed\n");
+               return CMD_RET_FAILURE;
        }
 
        if (argc == 2) {
                datastr = argv[1];
-               debug("fpga: Full command with two args\n");
+               log_debug("Full command with two args\n");
        } else if (argc == 1 && !datastr) {
-               debug("fpga: Dev is setup - fpgadata passed\n");
+               log_debug("Dev is setup - fpgadata passed\n");
                datastr = argv[0];
        }
 
@@ -269,20 +270,20 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int 
flag, int argc,
        if (fit_parse_subimage(datastr, (ulong)fpga_data,
                               &fit_addr, &fit_uname)) {
                fpga_data = (void *)fit_addr;
-               debug("*  fpga: subimage '%s' from FIT image ",
-                     fit_uname);
-               debug("at 0x%08lx\n", fit_addr);
+               log_debug("*  fpga: subimage '%s' from FIT image ",
+                         fit_uname);
+               log_debug("at 0x%08lx\n", fit_addr);
        } else
 #endif
        {
                fpga_data = (void *)hextoul(datastr, NULL);
-               debug("*  fpga: cmdline image address = 0x%08lx\n",
-                     (ulong)fpga_data);
+               log_debug("*  fpga: cmdline image address = 0x%08lx\n",
+                         (ulong)fpga_data);
        }
-       debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
+       log_debug("fpga_data = 0x%lx\n", (ulong)fpga_data);
        if (!fpga_data) {
-               puts("Zero fpga_data address\n");
-               return CMD_RET_USAGE;
+               log_err("Zero fpga_data address\n");
+               return CMD_RET_FAILURE;
        }
 
        switch (genimg_get_format(fpga_data)) {
@@ -303,13 +304,13 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int 
flag, int argc,
 
                        if (gunzip((void *)data, ~0UL, (void *)image_buf,
                                   &image_size) != 0) {
-                               puts("GUNZIP: error\n");
+                               log_err("Gunzip error\n");
                                return CMD_RET_FAILURE;
                        }
                        data_size = image_size;
 #else
-                       puts("Gunzip image is not supported\n");
-                       return 1;
+                       log_err("Gunzip image is not supported\n");
+                       return CMD_RET_FAILURE;
 #endif
                } else {
                        data = (ulong)image_get_data(hdr);
@@ -327,12 +328,12 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int 
flag, int argc,
                const void *fit_data;
 
                if (!fit_uname) {
-                       puts("No FIT subimage unit name\n");
+                       log_err("No FIT subimage unit name\n");
                        return CMD_RET_FAILURE;
                }
 
                if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
-                       puts("Bad FIT image format\n");
+                       log_err("Bad FIT image format\n");
                        return CMD_RET_FAILURE;
                }
 
@@ -348,7 +349,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, 
int argc,
        }
 #endif
        default:
-               puts("** Unknown image type\n");
+               log_err("Unknown image type\n");
                return CMD_RET_FAILURE;
        }
 }
@@ -390,16 +391,16 @@ static int do_fpga_wrapper(struct cmd_tbl *cmdtp, int 
flag, int argc,
        fpga_cmd = find_cmd_tbl(argv[1], fpga_commands,
                                ARRAY_SIZE(fpga_commands));
        if (!fpga_cmd) {
-               debug("fpga: non existing command\n");
-               return CMD_RET_USAGE;
+               log_err("Non existing command\n");
+               return CMD_RET_FAILURE;
        }
 
        argc -= 2;
        argv += 2;
 
        if (argc > fpga_cmd->maxargs) {
-               debug("fpga: more parameters passed\n");
-               return CMD_RET_USAGE;
+               log_err("Too many parameters passed\n");
+               return CMD_RET_FAILURE;
        }
 
        ret = fpga_cmd->cmd(fpga_cmd, flag, argc, argv);
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 2b62bbbe3cf..1199b249e36 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -191,8 +191,8 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, 
const void *buf,
        }
 
        if ((ulong)buf < SZ_1M) {
-               printf("%s: Bitstream has to be placed up to 1MB (%px)\n",
-                      __func__, buf);
+               log_err("Bitstream has to be placed above 1MB (%px)\n",
+                       buf);
                return FPGA_FAIL;
        }
 
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
index 3e86d854a01..5a37a33b0a7 100644
--- a/drivers/fpga/zynqpl.c
+++ b/drivers/fpga/zynqpl.c
@@ -360,8 +360,8 @@ static int zynq_validate_bitstream(xilinx_desc *desc, const 
void *buf,
        }
 
        if ((u32)buf < SZ_1M) {
-               printf("%s: Bitstream has to be placed up to 1MB (%x)\n",
-                      __func__, (u32)buf);
+               log_err("Bitstream has to be placed above 1MB (%x)\n",
+                       (u32)buf);
                return FPGA_FAIL;
        }
 
-- 
2.48.1

Reply via email to