Patchwork [U-Boot,10/30] arm/km: add addbootcount environment variable

login
register
mail settings
Submitter Valentin Longchamp
Date April 8, 2011, 2:24 p.m.
Message ID <ab89ca576a2ad8bcde3b08039a71a9a7695a7eef.1302272395.git.valentin.longchamp@keymile.com>
Download mbox | patch
Permalink /patch/90349/
State Changes Requested
Headers show

Comments

Valentin Longchamp - April 8, 2011, 2:24 p.m.
From: Holger Brunck <holger.brunck@keymile.com>

This environment variable is used to set the bootcount address
for the kernel.

Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
Acked-by: Heiko Schocher <hs@denx.de>
cc: Wolfgang Denk <wd@denx.de>
cc: Detlev Zundel <dzu@denx.de>
cc: Valentin Longchamp <valentin.longchamp@keymile.com>
Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
 board/keymile/common/common.c |    7 +++++++
 include/configs/km_arm.h      |    3 +++
 2 files changed, 10 insertions(+), 0 deletions(-)
Wolfgang Denk - April 30, 2011, 8:09 a.m.
Dear Valentin Longchamp,

In message <ab89ca576a2ad8bcde3b08039a71a9a7695a7eef.1302272395.git.valentin.longchamp@keymile.com> you wrote:
> From: Holger Brunck <holger.brunck@keymile.com>
> 
> This environment variable is used to set the bootcount address
> for the kernel.

"addbootcount" reads to me as "add something to the boot counter". I
do not expect that this has anything to do with an address. Please use
something like "bootcount_addr" (or "bootcnt_addr" or similar)
instead.

> @@ -106,6 +106,13 @@ int set_km_env(void)
>  	varaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM - CONFIG_KM_PHRAM;
>  	sprintf((char *)buf, "0x%x", varaddr);
>  	setenv("varaddr", (char *)buf);
> +
> +#ifdef BOOTCOUNT_ADDR
> +	unsigned int bootcountaddr;
> +	bootcountaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM;
> +	sprintf((char *)buf, "0x%x", bootcountaddr);
> +	setenv("bootcountaddr", (char *)buf);
> +#endif

NAK. We don't allow declarations in the middle of the code.

>  }
>  
> diff --git a/include/configs/km_arm.h b/include/configs/km_arm.h
> index 70113d4..89f9d35 100644
> --- a/include/configs/km_arm.h
> +++ b/include/configs/km_arm.h
> @@ -64,6 +64,9 @@
>  #define CONFIG_KM_KERNEL_ADDR	0x2000000	/* 4096KBytes */
>  
>  #define CONFIG_KM_DEF_ENV_CPU						\
> +	"addbootcount="							\
> +		"setenv bootargs ${bootargs} "				\
> +		"bootcountaddr=${bootcountaddr}\0"			\

Argh.  Not I see what you mean.  Please fix the description,it is
completely misleading.

Best regards,

Wolfgang Denk
Holger Brunck - May 2, 2011, 8:22 a.m.
Hello,

On 04/30/2011 10:09 AM, Wolfgang Denk wrote:
> Dear Valentin Longchamp,
> 
> In message <ab89ca576a2ad8bcde3b08039a71a9a7695a7eef.1302272395.git.valentin.longchamp@keymile.com> you wrote:
>> From: Holger Brunck <holger.brunck@keymile.com>
>>
>> This environment variable is used to set the bootcount address
>> for the kernel.
> 
> "addbootcount" reads to me as "add something to the boot counter". I
> do not expect that this has anything to do with an address. Please use
> something like "bootcount_addr" (or "bootcnt_addr" or similar)
> instead.
> 
>> @@ -106,6 +106,13 @@ int set_km_env(void)
>>  	varaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM - CONFIG_KM_PHRAM;
>>  	sprintf((char *)buf, "0x%x", varaddr);
>>  	setenv("varaddr", (char *)buf);
>> +
>> +#ifdef BOOTCOUNT_ADDR
>> +	unsigned int bootcountaddr;
>> +	bootcountaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM;
>> +	sprintf((char *)buf, "0x%x", bootcountaddr);
>> +	setenv("bootcountaddr", (char *)buf);
>> +#endif
> 
> NAK. We don't allow declarations in the middle of the code.
> 

Ok.

>>  }
>>  
>> diff --git a/include/configs/km_arm.h b/include/configs/km_arm.h
>> index 70113d4..89f9d35 100644
>> --- a/include/configs/km_arm.h
>> +++ b/include/configs/km_arm.h
>> @@ -64,6 +64,9 @@
>>  #define CONFIG_KM_KERNEL_ADDR	0x2000000	/* 4096KBytes */
>>  
>>  #define CONFIG_KM_DEF_ENV_CPU						\
>> +	"addbootcount="							\
>> +		"setenv bootargs ${bootargs} "				\
>> +		"bootcountaddr=${bootcountaddr}\0"			\
> 
> Argh.  Not I see what you mean.  Please fix the description,it is
> completely misleading.
> 

Ok, I will adapt this. Thanks for reviewing.

Best regards
Holger Brunck

Patch

diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
index 3a7980d..9ec0022 100644
--- a/board/keymile/common/common.c
+++ b/board/keymile/common/common.c
@@ -106,6 +106,13 @@  int set_km_env(void)
 	varaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM - CONFIG_KM_PHRAM;
 	sprintf((char *)buf, "0x%x", varaddr);
 	setenv("varaddr", (char *)buf);
+
+#ifdef BOOTCOUNT_ADDR
+	unsigned int bootcountaddr;
+	bootcountaddr = gd->ram_size - CONFIG_KM_RESERVED_PRAM;
+	sprintf((char *)buf, "0x%x", bootcountaddr);
+	setenv("bootcountaddr", (char *)buf);
+#endif
 	return 0;
 }
 
diff --git a/include/configs/km_arm.h b/include/configs/km_arm.h
index 70113d4..89f9d35 100644
--- a/include/configs/km_arm.h
+++ b/include/configs/km_arm.h
@@ -64,6 +64,9 @@ 
 #define CONFIG_KM_KERNEL_ADDR	0x2000000	/* 4096KBytes */
 
 #define CONFIG_KM_DEF_ENV_CPU						\
+	"addbootcount="							\
+		"setenv bootargs ${bootargs} "				\
+		"bootcountaddr=${bootcountaddr}\0"			\
 	"addmtdparts=setenv bootargs ${bootargs} ${mtdparts}\0"		\
 	"boot=bootm ${actual_kernel_addr} - -\0"			\
 	"cramfsloadfdt=echo \\\\c\0"					\