diff mbox series

[2/3] keymile: common: Fix pram variable radix

Message ID 20210108105310.299135-3-lusus@denx.de
State Superseded
Delegated to: Priyanka Jain
Headers show
Series PowerPC: keymile: Add support for kmcent2 board | expand

Commit Message

Niel Fourie Jan. 8, 2021, 10:53 a.m. UTC
In set_km_env() the pram variable was set to an hexadecimal value,
while initr_mem() expects an unsigned decimal. Set the pram variable
to an unsigned decimal instead.

Signed-off-by: Niel Fourie <lusus@denx.de>
Cc: Holger Brunck <holger.brunck@hitachi-powergrids.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
---
 board/keymile/common/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Roese Jan. 19, 2021, 3:13 p.m. UTC | #1
Hi Niel,

On 08.01.21 11:53, Niel Fourie wrote:
> In set_km_env() the pram variable was set to an hexadecimal value,
> while initr_mem() expects an unsigned decimal. Set the pram variable
> to an unsigned decimal instead.
> 
> Signed-off-by: Niel Fourie <lusus@denx.de>
> Cc: Holger Brunck <holger.brunck@hitachi-powergrids.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> ---
>   board/keymile/common/common.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
> index 03c7ce9da7..106787efd5 100644
> --- a/board/keymile/common/common.c
> +++ b/board/keymile/common/common.c
> @@ -60,7 +60,7 @@ int set_km_env(void)
>   		strict_strtoul(p, 16, &rootfssize);
>   	pram = (rootfssize + CONFIG_KM_RESERVED_PRAM + CONFIG_KM_PHRAM +
>   		CONFIG_KM_PNVRAM) / 0x400;
> -	sprintf((char *)buf, "0x%x", pram);
> +	sprintf((char *)buf, "%u", pram);
>   	env_set("pram", (char *)buf);

Why don't you switch to using a different env_set_foo() API instead?
Like env_set_ulong() in this case? Or env_set_hex() in the HEX case.
This could also be done for some other env_set cases in this file as
well to reduce code size and complexity.
Niel Fourie Jan. 21, 2021, 12:13 p.m. UTC | #2
Hi Stefan

On 19/01/2021 16:13, Stefan Roese wrote:
> Hi Niel,
> 
> On 08.01.21 11:53, Niel Fourie wrote:
>> In set_km_env() the pram variable was set to an hexadecimal value,
>> while initr_mem() expects an unsigned decimal. Set the pram variable
>> to an unsigned decimal instead.
>>
>> Signed-off-by: Niel Fourie <lusus@denx.de>
>> Cc: Holger Brunck <holger.brunck@hitachi-powergrids.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Priyanka Jain <priyanka.jain@nxp.com>
>> ---
>>   board/keymile/common/common.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/board/keymile/common/common.c 
>> b/board/keymile/common/common.c
>> index 03c7ce9da7..106787efd5 100644
>> --- a/board/keymile/common/common.c
>> +++ b/board/keymile/common/common.c
>> @@ -60,7 +60,7 @@ int set_km_env(void)
>>           strict_strtoul(p, 16, &rootfssize);
>>       pram = (rootfssize + CONFIG_KM_RESERVED_PRAM + CONFIG_KM_PHRAM +
>>           CONFIG_KM_PNVRAM) / 0x400;
>> -    sprintf((char *)buf, "0x%x", pram);
>> +    sprintf((char *)buf, "%u", pram);
>>       env_set("pram", (char *)buf);
> 
> Why don't you switch to using a different env_set_foo() API instead?
> Like env_set_ulong() in this case? Or env_set_hex() in the HEX case.
> This could also be done for some other env_set cases in this file as
> well to reduce code size and complexity.

Thank you for the suggestion, I realised I was simply trying to change 
as little as possible.

I replaced all instances of sprintf()/env_set() for numerical values in 
this file to env_set_hex()/env_set_ulong() instead.

I also considered use env_get_hex() where applicable, but it uses 
simple_strtoul() instead of strict_strtoul(), therefore changing the 
behaviour, so I left it as-is instead.

Best regards,
Niel Fourie
diff mbox series

Patch

diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
index 03c7ce9da7..106787efd5 100644
--- a/board/keymile/common/common.c
+++ b/board/keymile/common/common.c
@@ -60,7 +60,7 @@  int set_km_env(void)
 		strict_strtoul(p, 16, &rootfssize);
 	pram = (rootfssize + CONFIG_KM_RESERVED_PRAM + CONFIG_KM_PHRAM +
 		CONFIG_KM_PNVRAM) / 0x400;
-	sprintf((char *)buf, "0x%x", pram);
+	sprintf((char *)buf, "%u", pram);
 	env_set("pram", (char *)buf);
 
 	varaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM - CONFIG_KM_PHRAM;