[U-Boot] [RFC PATCH v2 6/8] cmd: mtdparts: update command to support GPT over MTD
Simon Glass
sjg at chromium.org
Thu Dec 1 03:21:23 CET 2016
Hi Patrick,
On 30 November 2016 at 04:01, Patrick Delaunay
<patrick.delaunay73 at gmail.com> wrote:
> add new subcommand :
> mtdparts gpt <mtd-dev> [<GUID>]
>
> extract mtd partition from GPT header present in MTD device
>> mtdparts gpt nand0
>> mtdparts gpt nor0
>
> extract mtd partitions only for some GUID
>> mtdparts gpt nand0 data
>> mtdparts gpt nor0 0FC63DAF-8483-4772-8E79-3D69D8477DE4
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay73 at gmail.com>
> ---
>
> Changes in v2: None
>
> cmd/mtdparts.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 119 insertions(+), 2 deletions(-)
>
Reviewed-by: Simon Glass <sjg at chromium.org>
But please see nits below.
> diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
> index b9b160d..035a7ec 100644
> --- a/cmd/mtdparts.c
> +++ b/cmd/mtdparts.c
> @@ -121,6 +121,8 @@ extern void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> static const char *mtdids_default = MTDIDS_DEFAULT;
> static const char *mtdparts_default = MTDPARTS_DEFAULT;
>
> +#define PART_ADD_DESC_MAXLEN 64
> +
This change should really be in a new patch, and it would be good to
have a comment as to what this is.
> /* copies of last seen 'mtdids', 'mtdparts' and 'partition' env variables */
> #define MTDIDS_MAXLEN 128
> #define MTDPARTS_MAXLEN 512
> @@ -1891,6 +1893,90 @@ static struct part_info* mtd_part_info(struct mtd_device *dev, unsigned int part
> return NULL;
> }
>
> +#ifdef CONFIG_EFI_PARTITION_MTD
> +/**
> + * extract MTD info from GPT information
> + *
> + * @param device string to select GPT device
> + * @param p_guid_filter guid for filtering add
> + *
> + * @return 0 on success, 1 otherwise
I think it would be better to return a useful error number from
errno.h and have the caller turn it into CMD_RET_FAILURE.
> + */
> +
> +static int mtdparts_gpt(char *device, char *p_guid_filter)
> +{
> + char tmpbuf[PART_ADD_DESC_MAXLEN];
> + u8 type, num;
> + struct mtdids *id;
> + int part_id;
> + struct mtd_info *mtd;
> + struct mtd_device *dev;
> + struct mtd_device *dev_tmp;
> + disk_partition_t info;
> + struct part_info *p;
> +
> + if (mtd_id_parse(device, NULL, &type, &num) != 0) {
> + printf("invalid MTD device %s\n", device);
> + return 1;
> + }
> +
> + /* this may be the first run, initialize lists if needed
> + and make sure we are in sync with env variables */
> + mtdparts_init();
> + /* don't treat error (mtdparts can be empty) */
> +
> + id = id_find(type, num);
> + if (id == NULL) {
> + printf("no such device %s defined in mtdids variable\n",
> + device);
> + return 1;
> + }
> +
> + if (get_mtd_info(type, num, &mtd) || (mtd == NULL)) {
> + printf("no such MTD device %s\n", device);
> + return 1;
> + }
> +
> + for (part_id = 1; part_id <= GPT_ENTRY_NUMBERS; part_id++) {
> + if (part_get_info_efi_mtd(mtd, part_id, &info))
> + break; /* Stop for first non valid partition */
> +
> +#ifdef CONFIG_PARTITION_TYPE_GUID
> + if (p_guid_filter &&
> + strcmp(p_guid_filter, info.type_guid))
> + continue;
> +#endif
> +
> + sprintf(tmpbuf, "%s:0x%llx at 0x%llx(%s)",
Can you use snprintf() to be safe?
> + id->mtd_id,
> + (u64)(info.size * info.blksz),
> + (u64)(info.start * info.blksz),
> + info.name);
> + debug("add tmpbuf: %s\n", tmpbuf);
> +
> + if ((device_parse(tmpbuf, NULL, &dev) != 0) || (!dev))
> + return 1;
> +
> + debug("+ %s\t%d\t%s\n", MTD_DEV_TYPE(type), num,
> + id->mtd_id);
> +
> + p = list_entry(dev->parts.next, struct part_info, link);
> +
> + dev_tmp = device_find(type, num);
> +
> + if (dev_tmp == NULL)
> + device_add(dev);
> + else if (part_add(dev_tmp, p) != 0)
> + return 1;
> + }
> + if (generate_mtdparts_save(last_parts, MTDPARTS_MAXLEN) != 0) {
> + printf("generated mtdparts too long, resetting to null\n");
> + return 1;
> + }
> + return 0;
> +}
> +#endif
> +
> /***************************************************/
> /* U-Boot commands */
> /***************************************************/
> @@ -1966,6 +2052,30 @@ static int do_mtdparts(cmd_tbl_t *cmdtp, int flag, int argc,
> }
> }
>
> +#ifdef CONFIG_EFI_PARTITION_MTD
> + /* mtdparts gpt <mtd-dev> [GUID] */
> + if ((argc == 3 || argc == 4) && (strncmp(argv[1], "gpt", 3) == 0)) {
> +#ifdef CONFIG_PARTITION_TYPE_GUID
> + char guid_str[UUID_STR_LEN + 1];
> + char *p_guid_filter = NULL;
> +#endif
> +
> + if (argc == 4) {
> +#ifdef CONFIG_PARTITION_TYPE_GUID
> + p_guid_filter = guid_str;
> + if (uuid_guid_parse_str(argv[3], guid_str)) {
> + printf("invalid type gui %s\n", argv[3]);
> + return 1;
> + }
> + debug("filtered type gui %s (%s)\n", argv[3], guid_str);
> +#else
> + puts("type GUI not support\n");
printf()
Should this be GUID?
> +#endif
> + }
> +
> + return mtdparts_gpt(argv[2], p_guid_filter);
> + }
> +#endif
> /* make sure we are in sync with env variables */
> if (mtdparts_init() != 0)
> return 1;
> @@ -1977,7 +2087,6 @@ static int do_mtdparts(cmd_tbl_t *cmdtp, int flag, int argc,
>
> /* mtdparts add <mtd-dev> <size>[@<offset>] <name> [ro] */
> if (((argc == 5) || (argc == 6)) && (strncmp(argv[1], "add", 3) == 0)) {
> -#define PART_ADD_DESC_MAXLEN 64
> char tmpbuf[PART_ADD_DESC_MAXLEN];
> #if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> struct mtd_info *mtd;
> @@ -2083,6 +2192,10 @@ static char mtdparts_help_text[] =
> "mtdparts add.spread <mtd-dev> <size>[@<offset>] [<name>] [ro]\n"
> " - add partition, padding size by skipping bad blocks\n"
> #endif
> +#ifdef CONFIG_EFI_PARTITION_MTD
> + "mtdparts gpt <mtd-dev> [<GUID>]\n"
> + " - add partitions for device from gpt, filtered by GUID type\n"
> +#endif
> "mtdparts default\n"
> " - reset partition table to defaults\n"
> #if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> @@ -2112,7 +2225,11 @@ static char mtdparts_help_text[] =
> "<size> := standard linux memsize OR '-' to denote all remaining space\n"
> "<offset> := partition start offset within the device\n"
> "<name> := '(' NAME ')'\n"
> - "<ro-flag> := when set to 'ro' makes partition read-only (not used, passed to kernel)";
> + "<ro-flag> := when set to 'ro' makes partition read-only (not used, passed to kernel)"
> +#ifdef CONFIG_EFI_PARTITION_MTD
> + "<GUID> := partition guid"
> +#endif
> + ;
> #endif
>
> U_BOOT_CMD(
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list