diff mbox series

[U-Boot,RFC,03/15] cmd: fpga: Move fpga_get_op to avoid local function declaration

Message ID 21316b64cb24813d9beab2f25f1b77522e186b0b.1531926958.git.michal.simek@xilinx.com
State RFC
Delegated to: Michal Simek
Headers show
Series cmd: fpga: Fix fpga command handling and add some fpga tests | expand

Commit Message

Michal Simek July 18, 2018, 3:16 p.m. UTC 
Move fpga_get_op() to top of file to remove local function declaration
and also remove useless retyping.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 cmd/fpga.c | 85 ++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

Comments

Simon Glass July 19, 2018, 1:32 a.m. UTC | #1 
On 18 July 2018 at 09:16, Michal Simek <michal.simek@xilinx.com> wrote:
> Move fpga_get_op() to top of file to remove local function declaration
> and also remove useless retyping.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  cmd/fpga.c | 85 ++++++++++++++++++++++++++++++--------------------------------
>  1 file changed, 41 insertions(+), 44 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

But can this not use a sub-command array?

- Simon
Michal Simek July 19, 2018, 6:55 a.m. UTC | #2 
On 19.7.2018 03:32, Simon Glass wrote:
> On 18 July 2018 at 09:16, Michal Simek <michal.simek@xilinx.com> wrote:
>> Move fpga_get_op() to top of file to remove local function declaration
>> and also remove useless retyping.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  cmd/fpga.c | 85 ++++++++++++++++++++++++++++++--------------------------------
>>  1 file changed, 41 insertions(+), 44 deletions(-)
>>
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> But can this not use a sub-command array?

As you see the whole series is coming to that direction.
I just needed to do some preparation steps before doing that.

Thanks,
Michal
Simon Glass July 19, 2018, 3:21 p.m. UTC | #3 
Hi Michal,

On 19 July 2018 at 00:55, Michal Simek <michal.simek@xilinx.com> wrote:
> On 19.7.2018 03:32, Simon Glass wrote:
>> On 18 July 2018 at 09:16, Michal Simek <michal.simek@xilinx.com> wrote:
>>> Move fpga_get_op() to top of file to remove local function declaration
>>> and also remove useless retyping.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>  cmd/fpga.c | 85 ++++++++++++++++++++++++++++++--------------------------------
>>>  1 file changed, 41 insertions(+), 44 deletions(-)
>>>
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> But can this not use a sub-command array?
>
> As you see the whole series is coming to that direction.
> I just needed to do some preparation steps before doing that.

Yes I t see that, thanks.

- Simon
diff mbox series

Patch

diff --git a/cmd/fpga.c b/cmd/fpga.c
index abe683720285..de8505e9d4c8 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -13,9 +13,6 @@ 
 #include <fs.h>
 #include <malloc.h>
 
-/* Local functions */
-static int fpga_get_op(char *opstr);
-
 /* Local defines */
 enum {
 	FPGA_NONE = -1,
@@ -30,6 +27,46 @@  enum {
 	FPGA_LOADS,
 };
 
+/*
+ * Map op to supported operations.  We don't use a table since we
+ * would just have to relocate it from flash anyway.
+ */
+static int fpga_get_op(char *opstr)
+{
+	int op = FPGA_NONE;
+
+	if (!strcmp("info", opstr))
+		op = FPGA_INFO;
+	else if (!strcmp("loadb", opstr))
+		op = FPGA_LOADB;
+	else if (!strcmp("load", opstr))
+		op = FPGA_LOAD;
+#if defined(CONFIG_CMD_FPGA_LOADP)
+	else if (!strcmp("loadp", opstr))
+		op = FPGA_LOADP;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADBP)
+	else if (!strcmp("loadbp", opstr))
+		op = FPGA_LOADBP;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+	else if (!strcmp("loadfs", opstr))
+		op = FPGA_LOADFS;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADMK)
+	else if (!strcmp("loadmk", opstr))
+		op = FPGA_LOADMK;
+#endif
+	else if (!strcmp("dump", opstr))
+		op = FPGA_DUMP;
+#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+	else if (!strcmp("loads", opstr))
+		op = FPGA_LOADS;
+#endif
+
+	return op;
+}
+
 /* ------------------------------------------------------------------------- */
 /* command form:
  *   fpga <op> <device number> <data addr> <datasize>
@@ -71,7 +108,7 @@  int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		return CMD_RET_USAGE;
 	}
 
-	op = (int)fpga_get_op(argv[1]);
+	op = fpga_get_op(argv[1]);
 
 	switch (op) {
 	case FPGA_NONE:
@@ -326,46 +363,6 @@  int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	return rc;
 }
 
-/*
- * Map op to supported operations.  We don't use a table since we
- * would just have to relocate it from flash anyway.
- */
-static int fpga_get_op(char *opstr)
-{
-	int op = FPGA_NONE;
-
-	if (!strcmp("info", opstr))
-		op = FPGA_INFO;
-	else if (!strcmp("loadb", opstr))
-		op = FPGA_LOADB;
-	else if (!strcmp("load", opstr))
-		op = FPGA_LOAD;
-#if defined(CONFIG_CMD_FPGA_LOADP)
-	else if (!strcmp("loadp", opstr))
-		op = FPGA_LOADP;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADBP)
-	else if (!strcmp("loadbp", opstr))
-		op = FPGA_LOADBP;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	else if (!strcmp("loadfs", opstr))
-		op = FPGA_LOADFS;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADMK)
-	else if (!strcmp("loadmk", opstr))
-		op = FPGA_LOADMK;
-#endif
-	else if (!strcmp("dump", opstr))
-		op = FPGA_DUMP;
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	else if (!strcmp("loads", opstr))
-		op = FPGA_LOADS;
-#endif
-
-	return op;
-}
-
 #if defined(CONFIG_CMD_FPGA_LOADFS) || defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 U_BOOT_CMD(fpga, 9, 1, do_fpga,
 #else