[PATCH v4 1/1] cmd: gpt: add eMMC and GPT support

[Thirupathaiah Annapureddy <thiruan@linux.microsoft.com>]

On 7/23/2020 5:00 AM, Rayagonda Kokatanur wrote:
> From: Corneliu Doban <cdoban@broadcom.com>
> 
> Add eMMC and GPT support.
> - GPT partition list and command to create the GPT added to u-boot
>   environment
> - eMMC boot commands added to u-boot environment
> - new gpt commands (enumarate and setenv) that are used by broadcom
>   update scripts and boot commands
> - eMMC specific u-boot configurations with environment saved in eMMC
>   and GPT support
> 
> Signed-off-by: Corneliu Doban <cdoban@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> ---
> Changes from v3:
>  -Address review comments from Simon Glass,
>   Return -ve number instead of 1 upon failure,
>   Use shorter variable name,
>   Modified code to avoid buffer overflow,
>   Use if (!strcmp(...)) instead of if (strcmp(...) == 0)
> 
> Changes from v2:
>  -Address review comments from Simon Glass,
>   Check for return value of part_driver_get_count(),
>   Don't check return value of part_driver_get(),
>   Rewrite part_driver_get() and rename to part_driver_get_first(),
>   Use env_set_ulong() whereever applicable, 
> 
>  -Address review comments from Michael Nazzareno Trimarchi,
>   Add new function to set all env vriables,
> 
> Changes from v1:
>  -Address review comments from Simon Glass,
>   Correct function comments,
>   Check for return value,
>   Add helper function in part.h
> 
>  cmd/gpt.c      | 161 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/part.h |  29 +++++++++
>  2 files changed, 190 insertions(+)
> 
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index df759416c8..2626992e59 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c

> +
> +/**
> + * gpt_setenv_part_variables() - setup partition environmental variables
> + *
> + * Setup the gpt_partition_name, gpt_partition_entry, gpt_partition_addr
> + * and gpt_partition_size environment variables.
> + *
> + * @pinfo: pointer to disk partition
> + * @i: partition entry
> + *
> + * @Return: '0' on success and -ENOENT on failure
> + */
> +static int gpt_setenv_part_variables(struct disk_partition *pinfo, int i)
> +{
> +	int ret;
> +
> +	ret = env_set_ulong("gpt_partition_addr", pinfo->start);
> +	if (ret)
> +		goto fail;
You should use env_set_hex() instead of env_set_ulong() for following reasons:
1. commands (ex: mmc read/write etc.) that use gpt_partition_addr and
gpt_partition_size expect them to be in hex values.
2. maintain compatibility with existing usage in the upstream code base
https://gitlab.denx.de/u-boot/u-boot/-/blob/master/include/configs/bcm_ns3.h
ex:
	"gpt setenv mmc ${sd_device_number} ${fit_image};"\
	"mmc read ${fit_image_loadaddr} ${gpt_partition_addr} "\

> +
> +	ret = env_set_ulong("gpt_partition_size", pinfo->size);
> +	if (ret)
> +		goto fail;
You should use env_set_hex() instead of env_set_ulong() for same above reasons.

Best Regards,
Thiru