[U-Boot] [PATCH] ARM: Consolidate bootcount_{store|load}

This patch consolidates bootcount_{store|load} for ARM by implementing a common version in arch/arm/lib/bootcount.c. This code is now used by all ARM variants that currently have these functions implemented.
Also supports two different bootcount versions:
a) Use 2 separate words (2 * 32bit) to store the bootcounter b) Use only 1 word (2 * 16bit) to store the bootcounter
The latter was already used by AT91.
Signed-off-by: Anatolij Gustschin agust@denx.de --- arch/arm/cpu/arm926ejs/at91/cpu.c | 34 ------------------------- arch/arm/cpu/ixp/cpu.c | 22 ---------------- arch/arm/lib/Makefile | 1 + arch/arm/lib/bootcount.c | 49 +++++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 56 deletions(-) create mode 100644 arch/arm/lib/bootcount.c
diff --git a/arch/arm/cpu/arm926ejs/at91/cpu.c b/arch/arm/cpu/arm926ejs/at91/cpu.c index 141a7d1..1094f8c 100644 --- a/arch/arm/cpu/arm926ejs/at91/cpu.c +++ b/arch/arm/cpu/arm926ejs/at91/cpu.c @@ -35,13 +35,6 @@ #define CONFIG_SYS_AT91_MAIN_CLOCK 0 #endif
-/* - * The at91sam9260 has 4 GPBR (0-3), we'll use the last one, nr 3, - * to keep track of the bootcount. - */ -#define AT91_GPBR_BOOTCOUNT_REGISTER 3 -#define AT91_BOOTCOUNT_ADDRESS (AT91_GPBR + 4*AT91_GPBR_BOOTCOUNT_REGISTER) - int arch_cpu_init(void) { return at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK); @@ -63,30 +56,3 @@ int print_cpuinfo(void) return 0; } #endif - -#ifdef CONFIG_BOOTCOUNT_LIMIT -/* - * Just as the mpc5xxx, we combine the BOOTCOUNT_MAGIC and boocount - * in one 32-bit register. This is done, as the AT91SAM9260 only has - * 4 GPBR. - */ -void bootcount_store (ulong a) -{ - volatile ulong *save_addr = - (volatile ulong *)(AT91_BASE_SYS + AT91_BOOTCOUNT_ADDRESS); - - *save_addr = (BOOTCOUNT_MAGIC & 0xffff0000) | (a & 0x0000ffff); -} - -ulong bootcount_load (void) -{ - volatile ulong *save_addr = - (volatile ulong *)(AT91_BASE_SYS + AT91_BOOTCOUNT_ADDRESS); - - if ((*save_addr & 0xffff0000) != (BOOTCOUNT_MAGIC & 0xffff0000)) - return 0; - else - return (*save_addr & 0x0000ffff); -} - -#endif /* CONFIG_BOOTCOUNT_LIMIT */ diff --git a/arch/arm/cpu/ixp/cpu.c b/arch/arm/cpu/ixp/cpu.c index ce275e5..b35b1cd 100644 --- a/arch/arm/cpu/ixp/cpu.c +++ b/arch/arm/cpu/ixp/cpu.c @@ -112,28 +112,6 @@ void pci_init(void) } */
-#ifdef CONFIG_BOOTCOUNT_LIMIT - -void bootcount_store (ulong a) -{ - volatile ulong *save_addr = (volatile ulong *)(CONFIG_SYS_BOOTCOUNT_ADDR); - - save_addr[0] = a; - save_addr[1] = BOOTCOUNT_MAGIC; -} - -ulong bootcount_load (void) -{ - volatile ulong *save_addr = (volatile ulong *)(CONFIG_SYS_BOOTCOUNT_ADDR); - - if (save_addr[1] != BOOTCOUNT_MAGIC) - return 0; - else - return save_addr[0]; -} - -#endif /* CONFIG_BOOTCOUNT_LIMIT */ - int cpu_eth_init(bd_t *bis) { #ifdef CONFIG_IXP4XX_NPE diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 0293348..4d26fe7 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -44,6 +44,7 @@ COBJS-y += cache-cp15.o endif COBJS-y += interrupts.o COBJS-y += reset.o +COBJS-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount.o
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \ $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) diff --git a/arch/arm/lib/bootcount.c b/arch/arm/lib/bootcount.c new file mode 100644 index 0000000..22f96bd --- /dev/null +++ b/arch/arm/lib/bootcount.c @@ -0,0 +1,49 @@ +#include <common.h> + +#ifdef CONFIG_AT91SAM9260 +#include <asm/arch/hardware.h> + +/* + * The at91sam9260 has 4 GPBR (0-3), we'll use the last one, nr 3, + * to keep track of the bootcount. + */ +#define AT91_GPBR_BOOTCOUNT_REGISTER 3 +#define AT91_BOOTCOUNT_ADDRESS (AT91_GPBR + 4 * AT91_GPBR_BOOTCOUNT_REGISTER) +#define CONFIG_SYS_BOOTCOUNT_ADDR (AT91_BASE_SYS + AT91_BOOTCOUNT_ADDRESS) +/* + * Just as the mpc5xxx, we combine the BOOTCOUNT_MAGIC and bootcount + * in one 32-bit register. This is done, as the AT91SAM9260 only has + * 4 GPBR. + */ +#define CONFIG_SYS_BOOTCOUNT_SINGLEWORD + +#endif /* CONFIG_AT91SAM9260 */ + +void bootcount_store(ulong a) +{ + volatile ulong *addr = (volatile ulong *)(CONFIG_SYS_BOOTCOUNT_ADDR); + +#if defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD) + addr[0] = (BOOTCOUNT_MAGIC & 0xffff0000) | (a & 0x0000ffff); +#else + addr[0] = a; + addr[1] = BOOTCOUNT_MAGIC; +#endif +} + +ulong bootcount_load(void) +{ + volatile ulong *addr = (volatile ulong *)(CONFIG_SYS_BOOTCOUNT_ADDR); + +#if defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD) + if ((addr[0] & 0xffff0000) == (BOOTCOUNT_MAGIC & 0xffff0000)) + return addr[0] & 0x0000ffff; + else + return 0; +#else + if (addr[1] == BOOTCOUNT_MAGIC) + return addr[0]; + else + return 0; +#endif +}

Dear Anatolij Gustschin,
The latter was already used by AT91.
Looking at your patch I just noticed that the AT91 version is not up to date. It won't work for AT91SAM9XE. I had a better, struct access variant in my tree but somehow due to rebasing I must have lost it...
I'll reconstruct it and send you the source snippets to access the bootcount with struct and readl/writel using the defines in at91_gpbr.h ?
It was something like
at91_gpbr_t *gpbr = (at91_gpbr_t *) AT91_GPR_BASE;
value = readl(&gpbr->reg[AT91_GPBR_INDEX_BOOTCOUNT]);
writel(value, &gpbr->reg[AT91_GPBR_INDEX_BOOTCOUNT]);
I don't think we should keep using the *(volatile *) access methods here, but if, the address calculation would have to be:
+#include <asm/arch/at91_gpbr.h>
-#define AT91_GPBR_BOOTCOUNT_REGISTER 3 -#define AT91_BOOTCOUNT_ADDRESS (AT91_GPBR + 4 * AT91_GPBR_BOOTCOUNT_REGISTER) -#define CONFIG_SYS_BOOTCOUNT_ADDR (AT91_BASE_SYS + AT91_BOOTCOUNT_ADDRESS) +#define CONFIG_SYS_BOOTCOUNT_ADDR (AT91_GPBR_BASE + 4 * AT91_GPBR_INDEX_BOOTCOUNT)
Looking at this, I think its not really helpful to extract the bootcount access from at91 / cpu.c ... to a common arm file.
Reinhard

Dear Anatolij Gustschin,
This patch consolidates bootcount_{store|load} for ARM by implementing a common version in arch/arm/lib/bootcount.c. This code is now used by all ARM variants that currently have these functions implemented.
Also supports two different bootcount versions:
a) Use 2 separate words (2 * 32bit) to store the bootcounter b) Use only 1 word (2 * 16bit) to store the bootcounter
The latter was already used by AT91.
More specific: only AT91SAM9260. There are many more AT91 SoCs that might want to use bootcount in the future!
The more I think about it, the less sense it makes to move bootcount access to arm/lib:
Handling bootcount is very SoC (maybe even board, if the SoC has no nonvolatile storage) and definitely not ARM specific.
Moving it from ARM-SoC specific files to a common ARM-lib file will cause many conditional compiles there.
The arch/powerpc/lib/bootcount.c is an example how the arm/lib/bootcount.c is going to look like in the long run.
Reinhard
PS: your bootcount.c misses the GPL header...

Dear Reinhard,
On Tue, 14 Sep 2010 03:11:50 +0200 Reinhard Meyer u-boot@emk-elektronik.de wrote: ...
This patch consolidates bootcount_{store|load} for ARM by implementing a common version in arch/arm/lib/bootcount.c. This code is now used by all ARM variants that currently have these functions implemented.
Also supports two different bootcount versions:
a) Use 2 separate words (2 * 32bit) to store the bootcounter b) Use only 1 word (2 * 16bit) to store the bootcounter
The latter was already used by AT91.
More specific: only AT91SAM9260. There are many more AT91 SoCs that might want to use bootcount in the future!
Ok, I can fix this description.
The more I think about it, the less sense it makes to move bootcount access to arm/lib:
Handling bootcount is very SoC (maybe even board, if the SoC has no nonvolatile storage) and definitely not ARM specific.
SoC header or board config file can just define the nonvolatile storage address. We do not need to add the same load/store functions for each SoC.
Moving it from ARM-SoC specific files to a common ARM-lib file will cause many conditional compiles there.
The arch/powerpc/lib/bootcount.c is an example how the arm/lib/bootcount.c is going to look like in the long run.
We do not need to add storage address definition in common code. The SoC header or board config file could define the address. If the load/store functions need to be more complicated, we can provide weak default functions in the common ARM-lib code and let the board code define board specific functions.
...
PS: your bootcount.c misses the GPL header...
Ok.
Thanks, Anatolij
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
participants (2)
-
Anatolij Gustschin
-
Reinhard Meyer