RE: [RFC PATCH v4 28/28] board: keymile: common: Use environment to store IVM_* variables.

[Aleksandar Gerasimovski <aleksandar.gerasimovski@hitachienergy.com>]

On Fri, Jun 17, 2022 at 12:31:58AM +0200, Francis Laniel wrote:

> These boards used set_local_var() to store some variables as local shell.
> They then used get_local_var() to retrieve the variables values.
> 
> Instead of using local shell variables, they should use environment 
> ones (like a majority of board).
> So, this patch converts using local variables to environment ones.
> 
> Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> ---
>  board/keymile/common/common.c | 8 ++++----
>  board/keymile/common/ivm.c    | 9 +--------
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/board/keymile/common/common.c 
> b/board/keymile/common/common.c index 3999f48719..72939af36e 100644
> --- a/board/keymile/common/common.c
> +++ b/board/keymile/common/common.c
> @@ -219,7 +219,7 @@ static int do_setboardid(struct cmd_tbl *cmdtp, int flag, int argc,
>  	unsigned char buf[32];
>  	char *p;
>  
> -	p = get_local_var("IVM_BoardId");
> +	p = env_get("IVM_BoardId");
>  	if (!p) {
>  		printf("can't get the IVM_Boardid\n");
>  		return 1;
> @@ -228,7 +228,7 @@ static int do_setboardid(struct cmd_tbl *cmdtp, int flag, int argc,
>  	env_set("boardid", (char *)buf);
>  	printf("set boardid=%s\n", buf);
>  
> -	p = get_local_var("IVM_HWKey");
> +	p = env_get("IVM_HWKey");
>  	if (!p) {
>  		printf("can't get the IVM_HWKey\n");
>  		return 1;
> @@ -272,14 +272,14 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc,
>  	 * first read out the real inventory values, these values are
>  	 * already stored in the local hush variables
>  	 */
> -	p = get_local_var("IVM_BoardId");
> +	p = env_get("IVM_BoardId");
>  	if (!p) {
>  		printf("can't get the IVM_Boardid\n");
>  		return 1;
>  	}
>  	rc = strict_strtoul(p, 16, &ivmbid);
>  
> -	p = get_local_var("IVM_HWKey");
> +	p = env_get("IVM_HWKey");
>  	if (!p) {
>  		printf("can't get the IVM_HWKey\n");
>  		return 1;
> diff --git a/board/keymile/common/ivm.c b/board/keymile/common/ivm.c 
> index 67db0c50f4..e266d7ce81 100644
> --- a/board/keymile/common/ivm.c
> +++ b/board/keymile/common/ivm.c
> @@ -44,14 +44,7 @@ static int ivm_calc_crc(unsigned char *buf, int 
> len)
>  
>  static int ivm_set_value(char *name, char *value)  {
> -	char tempbuf[256];
> -
> -	if (value) {
> -		sprintf(tempbuf, "%s=%s", name, value);
> -		return set_local_var(tempbuf, 0);
> -	}
> -	unset_local_var(name);
> -	return 0;
> +	return env_set(name, value);
>  }
>  
>  static int ivm_get_value(unsigned char *buf, int len, char *name, int 
> off,

Adding in the maintainer, but seems reasonable to me.

Reviewed-by: Tom Rini <trini@konsulko.com>

--
Tom

Reviewed-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@hitachienergy.com>

--
Aleksandar