[U-Boot] [review request] cmd_bmp.c: added standard subcommand handling

[Frans Meulenbroeks]

---
Hi,

This is not a formal patch but a review request.
Do people feel this patch is desirable and ok?

and should I keep it like this, or should I merge do_bmp_info and bmp_info into one func
(and similarly for do_bmp_display and bmp_display)

and please ignore the superfluous int x = 0, y = 0; line in do_bmp_info

Frans
---
 common/cmd_bmp.c |   82 +++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index 74ab24c..cb98fc9 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -90,46 +90,78 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
 }
 #endif
 
+static cmd_tbl_t cmd_bmp_sub[] = {
+	U_BOOT_CMD_MKENT(info, 3, 0, do_bmp_info, "", ""),
+	U_BOOT_CMD_MKENT(display, 5, 0, do_bmp_display, "", ""),
+};
 
-/*
- * Subroutine:  do_bmp
- *
- * Description: Handler for 'bmp' command..
- *
- * Inputs:	argv[1] contains the subcommand
- *
- * Return:      None
- *
- */
-int do_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+static int do_bmp_info(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong addr;
+	int x = 0, y = 0;
+
+	switch (argc) {
+	case 1:		/* use load_addr as default address */
+		addr = load_addr;
+		break;
+	case 2:		/* use argument */
+		addr = simple_strtoul(argv[1], NULL, 16);
+		break;
+	default:
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	return (bmp_info(addr));
+}
+
+static int do_bmp_display(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 {
 	ulong addr;
 	int x = 0, y = 0;
 
 	switch (argc) {
-	case 2:		/* use load_addr as default address */
+	case 1:		/* use load_addr as default address */
 		addr = load_addr;
 		break;
-	case 3:		/* use argument */
-		addr = simple_strtoul(argv[2], NULL, 16);
+	case 2:		/* use argument */
+		addr = simple_strtoul(argv[1], NULL, 16);
 		break;
-	case 5:
-		addr = simple_strtoul(argv[2], NULL, 16);
-	        x = simple_strtoul(argv[3], NULL, 10);
-	        y = simple_strtoul(argv[4], NULL, 10);
+	case 4:
+		addr = simple_strtoul(argv[1], NULL, 16);
+	        x = simple_strtoul(argv[2], NULL, 10);
+	        y = simple_strtoul(argv[3], NULL, 10);
 	        break;
 	default:
 		cmd_usage(cmdtp);
 		return 1;
 	}
 
-	/* Allow for short names
-	 * Adjust length if more sub-commands get added
-	 */
-	if (strncmp(argv[1],"info",1) == 0) {
-		return (bmp_info(addr));
-	} else if (strncmp(argv[1],"display",1) == 0) {
-	    return (bmp_display(addr, x, y));
+	 return (bmp_display(addr, x, y));
+}
+
+/*
+ * Subroutine:  do_bmp
+ *
+ * Description: Handler for 'bmp' command..
+ *
+ * Inputs:	argv[1] contains the subcommand
+ *
+ * Return:      None
+ *
+ */
+static int do_bmp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	cmd_tbl_t *c;
+
+	/* Strip off leading 'bmp' command argument */
+	argc--;
+	argv++;
+
+	c = find_cmd_tbl(argv[0], &cmd_i2c_sub[0], ARRAY_SIZE(cmd_i2c_sub));
+
+	if (c) {
+		return  c->cmd(cmdtp, flag, argc, argv);
 	} else {
 		cmd_usage(cmdtp);
 		return 1;
-- 
1.6.4.2