powerpc: Consolidate bootcount_{store|load} for PowerPC

PATCH New Stefan Roese 3 participants 6 messages Apr 26, 2010 – Apr 28, 2010 PowerPC

Share & tools

Stefan Roese

author

2010-04-26 14:28 UTC #0 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

This patch consolidates bootcount_{store|load} for PowerPC by implementing a common version in arch/powerpc/lib/bootcount.c. This code is now used by all PowerPC variants that currently have these functions implemented.

The functions now use the proper IO-accessor functions to read/write the values.

This code also supports two different bootcount versions:

a) Use 2 seperate words (2 * 32bit) to store the bootcounter
b) Use only 1 word (2* 16bit) to store the bootcounter

Version b) was already used by MPC5xxx.

Signed-off-by Stefan Roese <[email protected]>

Cc Michael Zaidman <[email protected]>

Cc Wolfgang Denk <[email protected]>

Cc Kim Phillips <[email protected]>

Cc Anatolij Gustschin <[email protected]>

---
 arch/powerpc/cpu/mpc5xxx/cpu.c      |   20 --------
 arch/powerpc/cpu/mpc8260/commproc.c |   24 ---------
 arch/powerpc/cpu/mpc83xx/cpu.c      |   30 ------------
 arch/powerpc/cpu/mpc8xx/commproc.c  |   26 ----------
 arch/powerpc/cpu/ppc4xx/commproc.c  |   24 ---------
 arch/powerpc/lib/Makefile           |    1 +
 arch/powerpc/lib/bootcount.c        |   90 +++++++++++++++++++++++++++++++++++
 7 files changed, 91 insertions(+), 124 deletions(-)
 create mode 100644 arch/powerpc/lib/bootcount.c

arch/powerpc/cpu/mpc5xxx/cpu.c +0 ~~-20~~

index b20234d..44b8a7a 100644
@@ -154,26 +154,6 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 }
 #endif
 
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-
-void bootcount_store (ulong a)
-{
-	volatile ulong *save_addr = (volatile ulong *) (MPC5XXX_CDM_BRDCRMB);
-
-	*save_addr = (BOOTCOUNT_MAGIC & 0xffff0000) | a;
-}
-
-ulong bootcount_load (void)
-{
-	volatile ulong *save_addr = (volatile ulong *) (MPC5XXX_CDM_BRDCRMB);
-
-	if ((*save_addr & 0xffff0000) != (BOOTCOUNT_MAGIC & 0xffff0000))
-		return 0;
-	else
-		return (*save_addr & 0x0000ffff);
-}
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
-
 #ifdef CONFIG_MPC5xxx_FEC
 /* Default initializations for FEC controllers.  To override,
  * create a board-specific function called:

arch/powerpc/cpu/mpc8260/commproc.c +0 ~~-24~~

index 111a67c..c522bc5 100644
@@ -195,27 +195,3 @@ ulong post_word_load (void)
 }
 
 #endif	/* CONFIG_POST || CONFIG_LOGBUFFER*/
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-
-void bootcount_store (ulong a)
-{
-	volatile ulong *save_addr =
-		(volatile ulong *)(CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR);
-
-	save_addr[0] = a;
-	save_addr[1] = BOOTCOUNT_MAGIC;
-}
-
-ulong bootcount_load (void)
-{
-	volatile ulong *save_addr =
-		(volatile ulong *)(CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR);
-
-	if (save_addr[1] != BOOTCOUNT_MAGIC)
-		return 0;
-	else
-		return save_addr[0];
-}
-
-#endif /* CONFIG_BOOTCOUNT_LIMIT */

arch/powerpc/cpu/mpc83xx/cpu.c +0 ~~-30~~

index 51180d6..fb32f01 100644
@@ -302,33 +302,3 @@ int cpu_mmc_init(bd_t *bis)
 	return 0;
 #endif
 }
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-
-#if !defined(CONFIG_MPC8360)
-#error "CONFIG_BOOTCOUNT_LIMIT only for MPC8360 implemented"
-#endif
-
-#if !defined(CONFIG_BOOTCOUNT_ADDR)
-#define CONFIG_BOOTCOUNT_ADDR	(0x110000 + QE_MURAM_SIZE - 2 * sizeof(unsigned long))
-#endif
-
-#include <asm/io.h>
-
-void bootcount_store (ulong a)
-{
-	void *reg = (void *)(CONFIG_SYS_IMMR + CONFIG_BOOTCOUNT_ADDR);
-	out_be32 (reg, a);
-	out_be32 (reg + 4, BOOTCOUNT_MAGIC);
-}
-
-ulong bootcount_load (void)
-{
-	void *reg = (void *)(CONFIG_SYS_IMMR + CONFIG_BOOTCOUNT_ADDR);
-
-	if (in_be32 (reg + 4) != BOOTCOUNT_MAGIC)
-		return 0;
-	else
-		return in_be32 (reg);
-}
-#endif /* CONFIG_BOOTCOUNT_LIMIT */

arch/powerpc/cpu/mpc8xx/commproc.c +0 ~~-26~~

index a87a0dc..2c85377 100644
@@ -103,29 +103,3 @@ ulong post_word_load (void)
 }
 
 #endif	/* CONFIG_POST || CONFIG_LOGBUFFER*/
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-
-void bootcount_store (ulong a)
-{
-	volatile ulong *save_addr =
-		(volatile ulong *)( ((immap_t *) CONFIG_SYS_IMMR)->im_cpm.cp_dpmem +
-		                    CPM_BOOTCOUNT_ADDR );
-
-	save_addr[0] = a;
-	save_addr[1] = BOOTCOUNT_MAGIC;
-}
-
-ulong bootcount_load (void)
-{
-	volatile ulong *save_addr =
-		(volatile ulong *)( ((immap_t *) CONFIG_SYS_IMMR)->im_cpm.cp_dpmem +
-		                    CPM_BOOTCOUNT_ADDR );
-
-	if (save_addr[1] != BOOTCOUNT_MAGIC)
-		return 0;
-	else
-		return save_addr[0];
-}
-
-#endif /* CONFIG_BOOTCOUNT_LIMIT */

arch/powerpc/cpu/ppc4xx/commproc.c +0 ~~-24~~

index a1696d3..6bf95e6 100644
@@ -51,27 +51,3 @@ ulong post_word_load (void)
 }
 
 #endif	/* CONFIG_POST || CONFIG_LOGBUFFER*/
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
-
-void bootcount_store (ulong a)
-{
-	volatile ulong *save_addr =
-		(volatile ulong *)(CONFIG_SYS_OCM_DATA_ADDR + 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_OCM_DATA_ADDR + CONFIG_SYS_BOOTCOUNT_ADDR);
-
-	if (save_addr[1] != BOOTCOUNT_MAGIC)
-		return 0;
-	else
-		return save_addr[0];
-}
-
-#endif /* CONFIG_BOOTCOUNT_LIMIT */

arch/powerpc/lib/Makefile +1 -0

index 334e457..5f85502 100644
@@ -33,6 +33,7 @@ SOBJS-y	+= reloc.o
 COBJS-y	+= bat_rw.o
 COBJS-y	+= board.o
 COBJS-y	+= bootm.o
+COBJS-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount.o
 COBJS-y	+= cache.o
 COBJS-y	+= extable.o
 COBJS-y	+= interrupts.o

arch/powerpc/lib/bootcount.c new file +90 -0

90 lines not on this page · raw message

Signature · 1 line

1.7.1

Detlev Zundel

2010-04-27 14:01 UTC #1 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

Hi Stefan,

Quoted 13 lines

This patch consolidates bootcount_{store|load} for PowerPC by implementing a common version in arch/powerpc/lib/bootcount.c. This code is now used by all PowerPC variants that currently have these functions implemented.
The functions now use the proper IO-accessor functions to read/write the values.
This code also supports two different bootcount versions:
a) Use 2 seperate words (2 * 32bit) to store the bootcounter
b) Use only 1 word (2* 16bit) to store the bootcounter

Quoted 7 lines

Version b) was already used by MPC5xxx.
Signed-off-by Stefan Roese <[email protected]>
Cc Michael Zaidman <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Anatolij Gustschin <[email protected]>

Cool, thanks!

I have only a cosmetic suggestion - can we rename the option CONFIG_SYS_BOOTCOUNT_USE_32BIT to CONFIG_SYS_BOOTCOUNT_SINGLEWORD? According to your documentation this would describe the situation somewhat closer.

Apart from that:

Acked-by Detlev Zundel <[email protected]>

Cheers
Detlev

Signature · 7 lines

"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Stefan Roese

author

2010-04-27 14:11 UTC #2 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

Hi Detlev,

Quoted 21 lines · On Tuesday 27 April 2010 16:01:11 Detlev Zundel wrote:

Quoted 13 lines
This patch consolidates bootcount_{store|load} for PowerPC by implementing a common version in arch/powerpc/lib/bootcount.c. This code is now used by all PowerPC variants that currently have these functions implemented.
The functions now use the proper IO-accessor functions to read/write the values.
This code also supports two different bootcount versions:
a) Use 2 seperate words (2 * 32bit) to store the bootcounter
b) Use only 1 word (2* 16bit) to store the bootcounter
Quoted 7 lines
Version b) was already used by MPC5xxx.
Signed-off-by Stefan Roese <[email protected]>
Cc Michael Zaidman <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Anatolij Gustschin <[email protected]>

Quoted 6 lines

Cool, thanks!
I have only a cosmetic suggestion - can we rename the option CONFIG_SYS_BOOTCOUNT_USE_32BIT to CONFIG_SYS_BOOTCOUNT_SINGLEWORD? According to your documentation this would describe the situation somewhat closer.

Right. This name is better. I'll send an updated patch soon.

Quoted 3 lines

Apart from that:
Acked-by Detlev Zundel <[email protected]>

Thanks.

Cheers,
Stefan

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

Michael Zaidman

2010-04-27 15:56 UTC #3 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

In reply to Stefan Roese

Hi Stefan,

Good work, you obviously dived dipper than my patch did. Please see my comments below.

Quoted 28 lines · Stefan Roese

This patch consolidates bootcount_{store|load} for PowerPC by implementing a common version in arch/powerpc/lib/bootcount.c. This code is now used by all PowerPC variants that currently have these functions implemented.
The functions now use the proper IO-accessor functions to read/write the values.
This code also supports two different bootcount versions:
a) Use 2 seperate words (2 * 32bit) to store the bootcounter
b) Use only 1 word (2* 16bit) to store the bootcounter
Version b) was already used by MPC5xxx.
Signed-off-by Stefan Roese <[email protected]>
Cc Michael Zaidman <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Anatolij Gustschin <[email protected]>
---
?arch/powerpc/cpu/mpc5xxx/cpu.c ? ? ?| ? 20 --------
?arch/powerpc/cpu/mpc8260/commproc.c | ? 24 ---------
?arch/powerpc/cpu/mpc83xx/cpu.c ? ? ?| ? 30 ------------
?arch/powerpc/cpu/mpc8xx/commproc.c ?| ? 26 ----------
?arch/powerpc/cpu/ppc4xx/commproc.c ?| ? 24 ---------
?arch/powerpc/lib/Makefile ? ? ? ? ? | ? ?1 +
?arch/powerpc/lib/bootcount.c ? ? ? ?| ? 90 +++++++++++++++++++++++++++++++++++

Quoted 3 lines

?7 files changed, 91 insertions(+), 124 deletions(-) ?create mode 100644 arch/powerpc/lib/bootcount.c

[snip]

It makes sense to keep some measure of flexibility by giving to the user possibility to override the CONFIG_SYS_BOOTCOUNT_ADDR definition. It can be easily achieved by adding the following lines:

#ifdef CONFIG_SYS_BOOTCOUNT_ADDR
#define _BOOTCOUNT_ADDR CONFIG_SYS_BOOTCOUNT_ADDR
#else

arch/powerpc/lib/bootcount.c line 29

Quoted 3 lines

arch/powerpc/lib/bootcount.c new file +3 -0

+#if defined(CONFIG_MPC5xxx)
+#define CONFIG_BOOTCOUNT_ADDR  (MPC5XXX_CDM_BRDCRMB)
+#define CONFIG_SYS_BOOTCOUNT_USE_32BIT

Also CONFIG_ as stated in u-boot's README should specify selectable by user options. I suggest omitting of all the locally defined CONFIG_ in your code.

Review

Quoted 7 lines

(quoted diff) +7 -0

+#endif /* defined(CONFIG_MPC5xxx) */
+
+#if defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
+ ? ?defined(CONFIG_MPC850) || defined(CONFIG_MPC852T) || \
+ ? ?defined(CONFIG_MPC855) || \
+ ? ?defined(CONFIG_MPC860) || defined(CONFIG_MPC866) || \
+ ? ?defined(CONFIG_MPC885)

This fragment does not cover all mpc8xx permutations. We can cover them all by the following code:

#if	defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
	defined(CONFIG_MPC855) || defined(CONFIG_MPC855T)|| \
	defined(CONFIG_MPC850) || defined(CONFIG_MPC86x)

I just used the CONFIG_MPC86x which has been already constructed by common.h and accomplished the list by CPUs that for unknown to me reason are not mentioned in common.h Probably, the better way is to add them into the common.h...

Best regards,
Michael

Stefan Roese

author

2010-04-28 07:14 UTC #4 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

Hi Michael,

Quoted 8 lines · On Tuesday 27 April 2010 17:56:27 Michael Zaidman wrote:

It makes sense to keep some measure of flexibility by giving to the user possibility to override the CONFIG_SYS_BOOTCOUNT_ADDR definition. It can be easily achieved by adding the following lines:
#ifdef CONFIG_SYS_BOOTCOUNT_ADDR
#define _BOOTCOUNT_ADDR CONFIG_SYS_BOOTCOUNT_ADDR
#else

Quoted 7 lines

arch/powerpc/lib/bootcount.c line 29
Quoted 3 lines
arch/powerpc/lib/bootcount.c new file +3 -0
+#if defined(CONFIG_MPC5xxx)
+#define CONFIG_BOOTCOUNT_ADDR  (MPC5XXX_CDM_BRDCRMB)
+#define CONFIG_SYS_BOOTCOUNT_USE_32BIT
Also CONFIG_ as stated in u-boot's README should specify selectable by user options. I suggest omitting of all the locally defined CONFIG_ in your code.

I'll change CONFIG_BOOTCOUNT_ADDR to CONFIG_SYS_BOOTCOUNT_ADDR. And only overwrite/define this value, when not already defined.

Quoted 9 lines

arch/powerpc/lib/bootcount.c line 36
Quoted 7 lines
arch/powerpc/lib/bootcount.c new file +7 -0
+#endif /* defined(CONFIG_MPC5xxx) */
+
+#if defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
+    defined(CONFIG_MPC850) || defined(CONFIG_MPC852T) || \
+    defined(CONFIG_MPC855) || \
+    defined(CONFIG_MPC860) || defined(CONFIG_MPC866) || \
+    defined(CONFIG_MPC885)
This fragment does not cover all mpc8xx permutations.

I did run MAKEALL 8xx and nothing broke. But you may be right. I'll check this again.

Quoted 6 lines

We can cover
them all by the following code:

#if	defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
	defined(CONFIG_MPC855) || defined(CONFIG_MPC855T)|| \
	defined(CONFIG_MPC850) || defined(CONFIG_MPC86x)

Some 8xx variants are missing here as well (e.g. CONFIG_MPC852T, CONFIG_MPC885).

Quoted 4 lines

I just used the CONFIG_MPC86x which has been already constructed by common.h and accomplished the list by CPUs that for unknown to me reason are not mentioned in common.h Probably, the better way is to add them into the common.h...

CONFIG_8xx seems the way to go. I just noticed it and will use it in the next patch version.

Thanks.

Cheers,
Stefan

Michael Zaidman

2010-04-28 08:06 UTC #5 lore raw

PATCH 1/1 powerpc: Consolidate bootcount_{store|load} for PowerPC

Quoted 19 lines · Stefan Roese

Hi Michael,
Quoted 8 lines · On Tuesday 27 April 2010 17:56:27 Michael Zaidman wrote:
It makes sense to keep some measure of flexibility by giving to the user possibility to override the CONFIG_SYS_BOOTCOUNT_ADDR definition. It can be easily achieved by adding the following lines:
#ifdef CONFIG_SYS_BOOTCOUNT_ADDR
#define _BOOTCOUNT_ADDR CONFIG_SYS_BOOTCOUNT_ADDR
#else
Quoted 7 lines
arch/powerpc/lib/bootcount.c line 29
Quoted 3 lines
arch/powerpc/lib/bootcount.c new file +3 -0
+#if defined(CONFIG_MPC5xxx)
+#define CONFIG_BOOTCOUNT_ADDR ?(MPC5XXX_CDM_BRDCRMB)
+#define CONFIG_SYS_BOOTCOUNT_USE_32BIT
Also CONFIG_ as stated in u-boot's README should specify selectable by user options. I suggest omitting of all the locally defined CONFIG_ in your code.

Quoted 2 lines

I'll change CONFIG_BOOTCOUNT_ADDR to CONFIG_SYS_BOOTCOUNT_ADDR. And only overwrite/define this value, when not already defined.

Ok, I did it a slightly different way for the post_word cleanup in
http://lists.denx.de/pipermail/u-boot/2010-April/070729.html

Quoted 17 lines

Quoted 10 lines
Review
Quoted 8 lines
(quoted diff) +7 -0
+#endif /* defined(CONFIG_MPC5xxx) */
+
+#if defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
+ ? ?defined(CONFIG_MPC850) || defined(CONFIG_MPC852T) || \
+ ? ?defined(CONFIG_MPC855) || \
+ ? ?defined(CONFIG_MPC860) || defined(CONFIG_MPC866) || \
+ ? ?defined(CONFIG_MPC885)
This fragment does not cover all mpc8xx permutations.
I did run MAKEALL 8xx and nothing broke. But you may be right. I'll check this again.
Quoted 3 lines
We can cover
them all by the following code:

Quoted 6 lines

Quoted 3 lines
#if ? defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
? ? ? defined(CONFIG_MPC855) || defined(CONFIG_MPC855T)|| \
? ? ? defined(CONFIG_MPC850) || defined(CONFIG_MPC86x)
Some 8xx variants are missing here as well (e.g. CONFIG_MPC852T, CONFIG_MPC885).

No, they have already been taken in account by CONFIG_MPC86x in common.h

Quoted 8 lines

Quoted 4 lines
I just used the CONFIG_MPC86x which has been already constructed by common.h and accomplished the list by CPUs that for unknown to me reason are not mentioned in common.h Probably, the better way is to add them into the common.h...
CONFIG_8xx seems the way to go. I just noticed it and will use it in the next patch version.

Agree.

Regards,
Michael