Re: [U-Boot] [Patch v3] cmd/gpt: Support gpt command for all devices

11 Oct 2013

Dear Egbert Eich,
...
-----Original Message-----
From: Egbert Eich [mailto:egbert.eich@gmail.com]
Sent: Friday, October 04, 2013 6:53 PM
To: u-boot@lists.denx.de
Cc: Piotr Wilczek; Tom Rini; Egbert Eich; Egbert Eich
Subject: [Patch v3] cmd/gpt: Support gpt command for all devices
From: Egbert Eich eich@suse.com
The gpt command was only implemented for mmc devices. There is no
reason why this command should not be generalized and be applied all
other storage device classes.
This change both simplifies the implementation and eliminates a build
failure for systems that don't support mmcs.
Signed-off-by: Egbert Eich eich@suse.com
Changes for v2:

Coding style cleanup.

Changes for v3:

Removed wrong '&'
Removed unused variable
Fixed argument checking

Spotted by Piotr Wilczek p.wilczek@samsung.com
common/cmd_gpt.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/common/cmd_gpt.c b/common/cmd_gpt.c index a46f5cc..17b1180
100644
--- a/common/cmd_gpt.c
+++ b/common/cmd_gpt.c
@@ -11,7 +11,6 @@
 #include <common.h>
 #include <malloc.h>
 #include <command.h>
-#include <mmc.h>
 #include <part_efi.h>
 #include <exports.h>
 #include <linux/ctype.h>
@@ -122,7 +121,7 @@ static int set_gpt_info(block_dev_desc_t *dev_desc,
   int errno = 0;
   uint64_t size_ll, start_ll;

debug("%s: MMC lba num: 0x%x %d\n", __func__,


debug("%s:  lba num: 0x%x %d\n", __func__,
    (unsigned int)dev_desc->lba, (unsigned int)dev_desc->lba);
if (str_part == NULL)


@@ -235,25 +234,18 @@ err:
   return errno;
 }
-static int gpt_mmc_default(int dev, const char *str_part)
+static int gpt_default(block_dev_desc_t *blk_dev_desc, const char
+*str_part)
 {
   int ret;
   char *str_disk_guid;
   u8 part_count = 0;
   disk_partition_t *partitions = NULL;

struct mmc *mmc = find_mmc_device(dev);


if (mmc == NULL) {

printf("%s: mmc dev %d NOT available\n", __func__, dev);


return CMD_RET_FAILURE;


}

if (!str_part)
  return -1;
/* fill partitions */

ret = set_gpt_info(&mmc->block_dev, str_part,



ret = set_gpt_info(blk_dev_desc, str_part,
	&str_disk_guid, &partitions, &part_count);
if (ret) {
if (ret == -1)

@@ -266,7 +258,7 @@ static int gpt_mmc_default(int dev, const char
*str_part)
   }
/* save partitions layout to disk */

gpt_restore(&mmc->block_dev, str_disk_guid, partitions,

part_count);

gpt_restore(blk_dev_desc, str_disk_guid, partitions, part_count);
free(str_disk_guid);
free(partitions);

@@ -287,27 +279,28 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int
argc, char * const argv[])  {
   int ret = CMD_RET_SUCCESS;
   int dev = 0;

char *pstr;
if (argc < 5)
return CMD_RET_USAGE;
/* command: 'write' */
if ((strcmp(argv[1], "write") == 0) && (argc == 5)) {

/* device: 'mmc' */


if (strcmp(argv[2], "mmc") == 0) {


	/* check if 'dev' is a number */


	for (pstr = argv[3]; *pstr != '\0'; pstr++)


		if (!isdigit(*pstr)) {


			printf("'%s' is not a number\n",


				argv[3]);


			return CMD_RET_USAGE;


		}


	dev = (int)simple_strtoul(argv[3], NULL, 10);


	/* write to mmc */


	if (gpt_mmc_default(dev, argv[4]))


		return CMD_RET_FAILURE;




char *ep;


block_dev_desc_t *blk_dev_desc;



This probably should be at the beginning of the function
...

dev = (int)simple_strtoul(argv[3], &ep, 10);


if (!ep || ep[0] != '\0') {


	printf("'%s' is not a number\n", argv[3]);


	return CMD_RET_USAGE;

}
blk_dev_desc = get_dev(argv[2], dev);


if (!blk_dev_desc) {


	printf("%s: %s dev %d NOT available\n",


	       __func__, argv[2], dev);



I think it is not necessary since the mmc subsystem prints 'MMC Device not
found'.
...

	return CMD_RET_FAILURE;


}



if (gpt_default(blk_dev_desc, argv[4]))


	return CMD_RET_FAILURE;

} else {
return CMD_RET_USAGE;
}

--
1.8.1.4
Except minor comments this patch looks good to me.
I tested it on mmc device (Trats2) and works well.
Tested-by: Piotr Wilczek p.wilczek@samsung.com
Best regards,
Piotr Wilczek

    