Dear Egbert Eich,

> -----Original Message-----
> From: Egbert Eich [mailto:[email protected]]
> Sent: Friday, October 04, 2013 6:53 PM
> To: [email protected]
> Cc: Piotr Wilczek; Tom Rini; Egbert Eich; Egbert Eich
> Subject: [Patch v3] cmd/gpt: Support gpt command for all devices
> 
> From: Egbert Eich <[email protected]>
> 
> 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 <[email protected]>
> ---
> Changes for v2:
>    - Coding style cleanup.
> Changes for v3:
>    - Removed wrong '&'
>    - Removed unused variable
>    - Fixed argument checking
>    Spotted by Piotr Wilczek <[email protected]>
> 
>  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 <[email protected]>

Best regards,
Piotr Wilczek


_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to