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

[Niel Fourie]

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 
Cc: Holger Brunck 
Cc: Heiko Schocher 
Cc: Priyanka Jain 
---
  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, );
  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

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80  Email: lu...@denx.de

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

[Stefan Roese]

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 
Cc: Holger Brunck 
Cc: Heiko Schocher 
Cc: Priyanka Jain 
---
  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, );
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.

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

[Niel Fourie]

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 
Cc: Holger Brunck 
Cc: Heiko Schocher 
Cc: Priyanka Jain 
---
 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, );
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;
-- 
2.29.2