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

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

[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.

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
+}
-- 
1.7.0.4

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

[Reinhard Meyer]

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

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

[Reinhard Meyer]

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...

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

[Anatolij Gustschin]

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 at denx.de