[U-Boot] [PATCH] ubi: Remove flash selection parameter (nor|nand|onenand) from "ubi part"

[U-Boot] [PATCH] ubi: Remove flash selection parameter (nor|nand|onenand) from "ubi part"

[Stefan Roese]

This patch removes the now unnecessary flash type parameter from the
"ubi part" command. Currently the user has to define the type of flash
he will be using UBI on. Example:

=> ubi part nor partition1

With this patch this type parameter is not needed anymore. The user can
now select the partition directly without the flash type paramter.
Example:

=> ubi part partition1

This breaks backward compatibility right now because of the change in the
command syntax. But UBI support is still quite fresh and the advantage of
this new command is syntax big enough for this change. Additionally the
code is much cleaner now.

Signed-off-by: Stefan Roese <sr@denx.de>
CC: Kyungmin Park <kyungmin.park@samsung.com>
---
 common/cmd_ubi.c |   73 +++++++++++++++++++----------------------------------
 1 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 9c17d71..55b9a72 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -34,9 +34,8 @@ static char buffer[80];
 static int ubi_initialized;
 
 struct selected_dev {
-	char dev_name[32];	/* NAND/OneNAND etc */
 	char part_name[80];
-	int type;
+	int selected;
 	int nr;
 	struct mtd_info *mtd_info;
 };
@@ -448,19 +447,24 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	}
 
 	if (strcmp(argv[1], "part") == 0) {
+		char mtd_dev[16];
+		struct mtd_device *dev;
+		struct part_info *part;
+		u8 pnum;
+
 		/* Print current partition */
 		if (argc == 2) {
-			if (ubi_dev.type == DEV_TYPE_NONE) {
+			if (!ubi_dev.selected) {
 				printf("Error, no UBI device/partition selected!\n");
 				return 1;
 			}
 
-			printf("%s Device %d: %s, partition %s\n", ubi_dev.dev_name,
+			printf("Device %d: %s, partition %s\n",
 			       ubi_dev.nr, ubi_dev.mtd_info->name, ubi_dev.part_name);
 			return 0;
 		}
 
-		if (argc < 4) {
+		if (argc < 3) {
 			cmd_usage(cmdtp);
 			return 1;
 		}
@@ -477,54 +481,29 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 		}
 
 		/*
-		 * Check for nor|nand|onenand selection
+		 * Search the mtd device number where this partition
+		 * is located
 		 */
-#if defined(CONFIG_CMD_NAND)
-		if (strcmp(argv[2], "nand") == 0) {
-			strcpy(ubi_dev.dev_name, "NAND");
-			ubi_dev.type = DEV_TYPE_NAND;
-			ubi_dev.mtd_info = &nand_info[ubi_dev.nr];
-		}
-#endif
-#if defined(CONFIG_FLASH_CFI_MTD)
-		if (strcmp(argv[2], "nor") == 0) {
-			char mtd_dev[16];
-			struct mtd_device *dev;
-			struct part_info *part;
-			u8 pnum;
-
-			/*
-			 * Search the mtd device number where this partition
-			 * is located
-			 */
-			if (find_dev_and_part(argv[3], &dev, &pnum, &part)) {
-				printf("Partition %s not found!\n", argv[3]);
-				return 1;
-			}
-			sprintf(mtd_dev, "nor%d", dev->id->num);
-			ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
-			strcpy(ubi_dev.dev_name, "NOR");
-			ubi_dev.type = DEV_TYPE_NOR;
-		}
-#endif
-#if defined(CONFIG_CMD_ONENAND)
-		if (strcmp(argv[2], "onenand") == 0) {
-			strcpy(ubi_dev.dev_name, "OneNAND");
-			ubi_dev.type = DEV_TYPE_ONENAND;
-			ubi_dev.mtd_info = &onenand_mtd;
+		if (find_dev_and_part(argv[2], &dev, &pnum, &part)) {
+			printf("Partition %s not found!\n", argv[2]);
+			return 1;
 		}
-#endif
-
-		if (ubi_dev.type == DEV_TYPE_NONE) {
-			printf("Error, no UBI device/partition selected!\n");
+		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
+		printf("%s: mtd_dev=%s!!!\n", __func__, mtd_dev); // test-only
+		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
+		printf("%s: mtd_info=%p!!!\n", __func__, ubi_dev.mtd_info); // test-only
+                if (IS_ERR(ubi_dev.mtd_info)) {
+			printf("Partition %s not found on device %s!\n", argv[2], mtd_dev);
 			return 1;
 		}
 
-		strcpy(ubi_dev.part_name, argv[3]);
+		ubi_dev.selected = 1;
+
+		strcpy(ubi_dev.part_name, argv[2]);
 		err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name);
 		if (err) {
 			printf("UBI init error %d\n", err);
-			ubi_dev.type = DEV_TYPE_NONE;
+			ubi_dev.selected = 0;
 			return err;
 		}
 
@@ -533,7 +512,7 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 		return 0;
 	}
 
-	if ((strcmp(argv[1], "part") != 0) && (ubi_dev.type == DEV_TYPE_NONE)) {
+	if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
 		printf("Error, no UBI device/partition selected!\n");
 		return 1;
 	}
@@ -617,7 +596,7 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 
 U_BOOT_CMD(ubi, 6, 1, do_ubi,
 	"ubi commands",
-	"part [nand|nor|onenand] [part]"
+	"part [part]"
 		" - Show or set current partition\n"
 	"ubi info [l[ayout]]"
 		" - Display volume and ubi layout information\n"
-- 
1.6.2.4

[U-Boot] [PATCH] ubi: Remove flash selection parameter (nor|nand|onenand) from "ubi part"

[Scott Wood]

On Fri, Apr 24, 2009 at 04:00:19PM +0200, Stefan Roese wrote:
> This patch removes the now unnecessary flash type parameter from the
> "ubi part" command. Currently the user has to define the type of flash
> he will be using UBI on. Example:
> 
> => ubi part nor partition1
> 
> With this patch this type parameter is not needed anymore. The user can
> now select the partition directly without the flash type paramter.
> Example:
> 
> => ubi part partition1
> 
> This breaks backward compatibility right now because of the change in the
> command syntax. But UBI support is still quite fresh and the advantage of
> this new command is syntax big enough for this change. Additionally the
> code is much cleaner now.

Yes, some polymorphism at last. :-)

Now to do the same with cmd_nand/cmd_onenand...  and NOR flash for that
matter.

> +		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
> +		printf("%s: mtd_dev=%s!!!\n", __func__, mtd_dev); // test-only
> +		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
> +		printf("%s: mtd_info=%p!!!\n", __func__, ubi_dev.mtd_info); // test-only

Should these be debug()?

> +                if (IS_ERR(ubi_dev.mtd_info)) {

s/spaces/tabs/

-Scott

[U-Boot] [PATCH] ubi: Remove flash selection parameter (nor|nand|onenand) from "ubi part"

[Stefan Roese]

On Friday 24 April 2009, Scott Wood wrote:
> On Fri, Apr 24, 2009 at 04:00:19PM +0200, Stefan Roese wrote:
> > This patch removes the now unnecessary flash type parameter from the
> > "ubi part" command. Currently the user has to define the type of flash
> > he will be using UBI on. Example:
> >
> > => ubi part nor partition1
> >
> > With this patch this type parameter is not needed anymore. The user can
> > now select the partition directly without the flash type paramter.
> > Example:
> >
> > => ubi part partition1
> >
> > This breaks backward compatibility right now because of the change in the
> > command syntax. But UBI support is still quite fresh and the advantage of
> > this new command is syntax big enough for this change. Additionally the
> > code is much cleaner now.
>
> Yes, some polymorphism at last. :-)
>
> Now to do the same with cmd_nand/cmd_onenand...  and NOR flash for that
> matter.

That's a bigger task...

> > +		sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
> > +		printf("%s: mtd_dev=%s!!!\n", __func__, mtd_dev); // test-only
> > +		ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
> > +		printf("%s: mtd_info=%p!!!\n", __func__, ubi_dev.mtd_info); //
> > test-only
>
> Should these be debug()?

Ups. Thanks for catching.

> > +                if (IS_ERR(ubi_dev.mtd_info)) {
>
> s/spaces/tabs/

OK. Will fix and resubmit.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] ubi: Remove flash selection parameter (nor|nand|onenand) from "ubi part"

[Wolfgang Denk]

Dear Stefan Roese,

In message <1240581619-746-1-git-send-email-sr@denx.de> you wrote:
> This patch removes the now unnecessary flash type parameter from the
> "ubi part" command. Currently the user has to define the type of flash
> he will be using UBI on. Example:
> 
> => ubi part nor partition1
> 
> With this patch this type parameter is not needed anymore. The user can
> now select the partition directly without the flash type paramter.
> Example:
> 
> => ubi part partition1
> 
> This breaks backward compatibility right now because of the change in the
> command syntax. But UBI support is still quite fresh and the advantage of
> this new command is syntax big enough for this change. Additionally the
> code is much cleaner now.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> CC: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  common/cmd_ubi.c |   73 +++++++++++++++++++----------------------------------
>  1 files changed, 26 insertions(+), 47 deletions(-)

Appied, 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
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offence.
          -- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5