[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[Frans Meulenbroeks]

On the fly also fixed the following things:
- write help talked about a parameter oob, but that one was not used, so
  removed it from the help message.
- the test command also allowed a force subcommand but didn't use it.
  eliminated the code.
- do_onenand made static
- do_onenand contained
	int blocksize;
	...
	mtd = &onenand_mtd;
	this = mtd->priv;
	blocksize = (1 << this->erase_shift); 
  As blocksize was not used the last two statements were unneeded so
  removed them.
  The first statement (mtd = ....) assigns to a global. Not sure if it
  is needed, and since I could not test this, left the line for now

Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
---
 common/cmd_onenand.c |  307 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 192 insertions(+), 115 deletions(-)

diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
index 565257c..2646ae9 100644
--- a/common/cmd_onenand.c
+++ b/common/cmd_onenand.c
@@ -330,154 +330,231 @@ static int onenand_dump(struct mtd_info *mtd, ulong off, int only_oob)
 	return 0;
 }
 
-int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+static int do_onenand_info(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 {
-	struct onenand_chip *this;
-	int blocksize;
+	printf("%s\n", mtd->name);
+	return 0;
+}
+
+static int do_onenand_bad(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong ofs;
+
+	mtd = &onenand_mtd;
+	/* Currently only one OneNAND device is supported */
+	printf("\nDevice %d bad blocks:\n", 0);
+	for (ofs = 0; ofs < mtd->size; ofs += mtd->erasesize) {
+		if (mtd->block_isbad(mtd, ofs))
+			printf("  %08x\n", (u32)ofs);
+	}
+
+	return 0;
+}
+
+static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	char *s;
+	int oob = 0;
 	ulong addr, ofs;
-	size_t len, retlen = 0;
+	size_t len;
 	int ret = 0;
-	char *cmd, *s;
+	size_t retlen = 0;
 
-	mtd = &onenand_mtd;
-	this = mtd->priv;
-	blocksize = (1 << this->erase_shift);
+	if (argc < 3)
+	{
+		cmd_usage(cmdtp);
+		return 1;
+	}
 
-	cmd = argv[1];
+	s = strchr(argv[0], '.');
+	if ((s != NULL) && (!strcmp(s, ".oob")))
+		oob = 1;
 
-	switch (argc) {
-	case 0:
-	case 1:
-		goto usage;
+	addr = (ulong)simple_strtoul(argv[1], NULL, 16);
 
-	case 2:
-		if (strcmp(cmd, "info") == 0) {
-			printf("%s\n", mtd->name);
-			return 0;
-		}
+	printf("\nOneNAND read: ");
+	if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+		return 1;
 
-		if (strcmp(cmd, "bad") == 0) {
-			/* Currently only one OneNAND device is supported */
-			printf("\nDevice %d bad blocks:\n", 0);
-			for (ofs = 0; ofs < mtd->size; ofs += mtd->erasesize) {
-				if (mtd->block_isbad(mtd, ofs))
-					printf("  %08x\n", (u32)ofs);
-			}
+	ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob);
 
-			return 0;
-		}
+	printf(" %d bytes read: %s\n", retlen, ret ? "ERROR" : "OK");
 
-	default:
-		/* At least 4 args */
+	return ret == 0 ? 0 : 1;
+}
 
-		/*
-		 * Syntax is:
-		 *   0       1     2       3    4
-		 *   onenand erase [force] [off size]
-		 */
-		if ((strcmp(cmd, "erase") == 0) || (strcmp(cmd, "test") == 0)) {
-			int force = argc > 2 && !strcmp("force", argv[2]);
-			int o = force ? 3 : 2;
-			int erase;
+static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong addr, ofs;
+	size_t len;
+	int ret = 0;
+	size_t retlen = 0;
 
-			erase = strcmp(cmd, "erase") == 0; /* 1 = erase, 0 = test */
-			printf("\nOneNAND %s: ", erase ? "erase" : "test");
+	if (argc < 3)
+	{
+		cmd_usage(cmdtp);
+		return 1;
+	}
 
-			/* skip first two or three arguments, look for offset and size */
-			if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0)
-				return 1;
+	addr = (ulong)simple_strtoul(argv[1], NULL, 16);
+
+	printf("\nOneNAND write: ");
+	if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+		return 1;
 
-			if (erase)
-				ret = onenand_block_erase(ofs, len, force);
-			else
-				ret = onenand_block_test(ofs, len);
+	ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr);
 
-			printf("%s\n", ret ? "ERROR" : "OK");
+	printf(" %d bytes written: %s\n", retlen, ret ? "ERROR" : "OK");
 
-			return ret == 0 ? 0 : 1;
+	return ret == 0 ? 0 : 1;
+}
+
+static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong ofs;
+	int ret = 0;
+	size_t len;
+	int force;
+
+	/*
+	 * Syntax is:
+	 *   0       1     2       3    4
+	 *   onenand erase [force] [off size]
+	 */
+	argc--;
+	argv++;
+	if (argc)
+	{
+		if (!strcmp("force", argv[0]))
+		{
+			force = 1;
+			argc--;
+			argv++;
 		}
+	}
+	printf("\nOneNAND erase: ");
 
-		if (strncmp(cmd, "read", 4) == 0 || strncmp(cmd, "write", 5) == 0) {
-			int read;
-			int oob = 0;
+	/* skip first two or three arguments, look for offset and size */
+	if (arg_off_size(argc, argv, &ofs, &len) != 0)
+		return 1;
 
-			if (argc < 4)
-				goto usage;
+	ret = onenand_block_erase(ofs, len, force);
 
-			addr = (ulong)simple_strtoul(argv[2], NULL, 16);
+	printf("%s\n", ret ? "ERROR" : "OK");
 
-			read = strncmp(cmd, "read", 4) == 0; /* 1 = read, 0 = write */
-			printf("\nOneNAND %s: ", read ? "read" : "write");
-			if (arg_off_size(argc - 3, argv + 3, &ofs, &len) != 0)
-				return 1;
+	return ret == 0 ? 0 : 1;
+}
 
-			s = strchr(cmd, '.');
-			if ((s != NULL) && (!strcmp(s, ".oob")))
-				oob = 1;
+static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong ofs;
+	int ret = 0;
+	size_t len;
 
-			if (read) {
-				ret = onenand_block_read(ofs, len, &retlen,
-							 (u8 *)addr, oob);
-			} else {
-				ret = onenand_block_write(ofs, len, &retlen,
-							  (u8 *)addr);
-			}
+	/*
+	 * Syntax is:
+	 *   0       1     2       3    4
+	 *   onenand test [force] [off size]
+	 */
 
-			printf(" %d bytes %s: %s\n", retlen,
-			       read ? "read" : "written", ret ? "ERROR" : "OK");
+	printf("\nOneNAND test: ");
 
-			return ret == 0 ? 0 : 1;
-		}
+	/* skip first two or three arguments, look for offset and size */
+	if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0)
+		return 1;
 
-		if (strcmp(cmd, "markbad") == 0) {
-			argc -= 2;
-			argv += 2;
-
-			if (argc <= 0)
-				goto usage;
-
-			while (argc > 0) {
-				addr = simple_strtoul(*argv, NULL, 16);
-
-				if (mtd->block_markbad(mtd, addr)) {
-					printf("block 0x%08lx NOT marked "
-						"as bad! ERROR %d\n",
-						addr, ret);
-					ret = 1;
-				} else {
-					printf("block 0x%08lx successfully "
-						"marked as bad\n",
-						addr);
-				}
-				--argc;
-				++argv;
-			}
-			return ret;
-		}
+	ret = onenand_block_test(ofs, len);
 
-		if (strncmp(cmd, "dump", 4) == 0) {
-			if (argc < 3)
-				goto usage;
+	printf("%s\n", ret ? "ERROR" : "OK");
 
-			s = strchr(cmd, '.');
-			ofs = (int)simple_strtoul(argv[2], NULL, 16);
+	return ret == 0 ? 0 : 1;
+}
 
-			if (s != NULL && strcmp(s, ".oob") == 0)
-				ret = onenand_dump(mtd, ofs, 1);
-			else
-				ret = onenand_dump(mtd, ofs, 0);
+static int do_onenand_dump(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	ulong ofs;
+	int ret = 0;
+	char *s;
 
-			return ret == 0 ? 1 : 0;
-		}
+	if (argc < 2)
+	{
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	s = strchr(argv[0], '.');
+	ofs = (int)simple_strtoul(argv[1], NULL, 16);
+
+	if (s != NULL && strcmp(s, ".oob") == 0)
+		ret = onenand_dump(mtd, ofs, 1);
+	else
+		ret = onenand_dump(mtd, ofs, 0);
 
-		break;
+	return ret == 0 ? 1 : 0;
+}
+
+static int do_onenand_markbad(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	int ret = 0;
+	ulong addr;
+
+	argc -= 2;
+	argv += 2;
+
+	if (argc <= 0)
+	{
+		cmd_usage(cmdtp);
+		return 1;
 	}
 
-	return 0;
+	while (argc > 0) {
+		addr = simple_strtoul(*argv, NULL, 16);
+
+		if (mtd->block_markbad(mtd, addr)) {
+			printf("block 0x%08lx NOT marked "
+				"as bad! ERROR %d\n",
+				addr, ret);
+			ret = 1;
+		} else {
+			printf("block 0x%08lx successfully "
+				"marked as bad\n",
+				addr);
+		}
+		--argc;
+		++argv;
+	}
+	return ret;
+}
+
+static cmd_tbl_t cmd_onenand_sub[] = {
+	U_BOOT_CMD_MKENT(info, 1, 0, do_onenand_info, "", ""),
+	U_BOOT_CMD_MKENT(bad, 1, 0, do_onenand_bad, "", ""),
+	U_BOOT_CMD_MKENT(read, 4, 0, do_onenand_read, "", ""),
+	U_BOOT_CMD_MKENT(write, 4, 0, do_onenand_write, "", ""),
+	U_BOOT_CMD_MKENT(erase, 3, 0, do_onenand_erase, "", ""),
+	U_BOOT_CMD_MKENT(test, 3, 0, do_onenand_test, "", ""),
+	U_BOOT_CMD_MKENT(dump, 2, 0, do_onenand_dump, "", ""),
+	U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""),
+};
+
+static int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	cmd_tbl_t *c;
+
+	mtd = &onenand_mtd;
+
+	/* Strip off leading 'onenand' command argument */
+	argc--;
+	argv++;
 
-usage:
-	cmd_usage(cmdtp);
-	return 1;
+	c = find_cmd_tbl(argv[0], &cmd_onenand_sub[0], ARRAY_SIZE(cmd_onenand_sub));
+
+	if (c) {
+		return  c->cmd(cmdtp, flag, argc, argv);
+	} else {
+		cmd_usage(cmdtp);
+		return 1;
+	}
 }
 
 U_BOOT_CMD(
@@ -486,7 +563,7 @@ U_BOOT_CMD(
 	"info - show available OneNAND devices\n"
 	"onenand bad - show bad blocks\n"
 	"onenand read[.oob] addr off size\n"
-	"onenand write[.oob] addr off size\n"
+	"onenand write addr off size\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
 	"onenand erase [force] [off size] - erase 'size' bytes from\n"
-- 
1.6.4.2

[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[Frans Meulenbroeks]

Forgot this disclaimer, thought I added it, sry.

NOTE:
This code is untested as I have no board with onenand support.
However, it compiles properly (tested by building opap3_evm, which uses this).
The changes I did are purely splitting do_onenand in separate
functions and add the subcommand parsing (which itself was taken from
i2c, on which I tested it before).
So I do not expect any serious issues, but I would suggest testing
this before integrating it.

Best regards, Frans

[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[Detlev Zundel]

Hi Frans,

> Forgot this disclaimer, thought I added it, sry.
>
> NOTE:
> This code is untested as I have no board with onenand support.
> However, it compiles properly (tested by building opap3_evm, which uses this).
> The changes I did are purely splitting do_onenand in separate
> functions and add the subcommand parsing (which itself was taken from
> i2c, on which I tested it before).
> So I do not expect any serious issues, but I would suggest testing
> this before integrating it.

Why don't you just add the maintainer of omap3_evm board (as I did) on
CC to your mail, so he knows that his testing would be greatly
appreciated?

Thanks for the effort!
  Detlev

-- 
European  efforts to impose  state systems in  conquered  territories are the
source of most conflicts underway right now, after the collapse of the formal
colonial system.  Europe's own  favorite sport of  mutual slaughter had to be
called off  in 1945,  when it was  realized  that the  next time the game was
played would be the last.                                     -- Noam Chomsky
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[Detlev Zundel]

Hi Frans,

> 2010/3/29 Detlev Zundel <dzu@denx.de>:
>> Hi Frans,
>>
>>> Forgot this disclaimer, thought I added it, sry.
>>>
>>> NOTE:
>>> This code is untested as I have no board with onenand support.
>>> However, it compiles properly (tested by building opap3_evm, which uses this).
>>> The changes I did are purely splitting do_onenand in separate
>>> functions and add the subcommand parsing (which itself was taken from
>>> i2c, on which I tested it before).
>>> So I do not expect any serious issues, but I would suggest testing
>>> this before integrating it.
>>
>> Why don't you just add the maintainer of omap3_evm board (as I did) on
>> CC to your mail, so he knows that his testing would be greatly
>> appreciated?
>>
> Will try to do so next time :-)
> Actually there are more boards which utilize onenand. I just picked
> this one as I have an omap3 board (and actually only discovered when I
> wanted to start testing that it did not have onenand, but just regular
> nand)

Hm ok, then maybe add someone else from the list of boards using it, as
the e-mail of the omap3_evm maintainer "Manikandan Pillai
<mani.pillai@ti.com>" bounces.

I added Nishanth Menon from ti on CC, maybe he can tell us the
whereabouts of Manikanden.  Will someone else from ti be maintaining
this board?  If so, then a patch to MAINTAINERS is welcome :)

Cheers
  Detlev

-- 
Those who would trade safety for freedom deserve neither.
           -- Thomas Jefferson
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] cmd_onenand.c: moved to standard subcommand handling

[Wolfgang Denk]

Dear Frans Meulenbroeks,

In message <1269706476-20496-1-git-send-email-fransmeulenbroeks@gmail.com> you wrote:
> On the fly also fixed the following things:
> - write help talked about a parameter oob, but that one was not used, so
>   removed it from the help message.
> - the test command also allowed a force subcommand but didn't use it.
>   eliminated the code.
> - do_onenand made static
> - do_onenand contained
> 	int blocksize;
> 	...
> 	mtd = &onenand_mtd;
> 	this = mtd->priv;
> 	blocksize = (1 << this->erase_shift); 
>   As blocksize was not used the last two statements were unneeded so
>   removed them.
>   The first statement (mtd = ....) assigns to a global. Not sure if it
>   is needed, and since I could not test this, left the line for now
> 
> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> ---
>  common/cmd_onenand.c |  307 +++++++++++++++++++++++++++++++-------------------
>  1 files changed, 192 insertions(+), 115 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Life would be so much easier if we could  just  look  at  the  source
code.                                                   -- Dave Olson