[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[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 <sr@denx.de>
Cc: Michael Zaidman <michael.zaidman@gmail.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Kim Phillips <kim.phillips@freescale.com>
Cc: Anatolij Gustschin <agust@denx.de>
---
 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

diff --git a/arch/powerpc/cpu/mpc5xxx/cpu.c b/arch/powerpc/cpu/mpc5xxx/cpu.c
index b20234d..44b8a7a 100644
--- a/arch/powerpc/cpu/mpc5xxx/cpu.c
+++ b/arch/powerpc/cpu/mpc5xxx/cpu.c
@@ -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:
diff --git a/arch/powerpc/cpu/mpc8260/commproc.c b/arch/powerpc/cpu/mpc8260/commproc.c
index 111a67c..c522bc5 100644
--- a/arch/powerpc/cpu/mpc8260/commproc.c
+++ b/arch/powerpc/cpu/mpc8260/commproc.c
@@ -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 */
diff --git a/arch/powerpc/cpu/mpc83xx/cpu.c b/arch/powerpc/cpu/mpc83xx/cpu.c
index 51180d6..fb32f01 100644
--- a/arch/powerpc/cpu/mpc83xx/cpu.c
+++ b/arch/powerpc/cpu/mpc83xx/cpu.c
@@ -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 */
diff --git a/arch/powerpc/cpu/mpc8xx/commproc.c b/arch/powerpc/cpu/mpc8xx/commproc.c
index a87a0dc..2c85377 100644
--- a/arch/powerpc/cpu/mpc8xx/commproc.c
+++ b/arch/powerpc/cpu/mpc8xx/commproc.c
@@ -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 */
diff --git a/arch/powerpc/cpu/ppc4xx/commproc.c b/arch/powerpc/cpu/ppc4xx/commproc.c
index a1696d3..6bf95e6 100644
--- a/arch/powerpc/cpu/ppc4xx/commproc.c
+++ b/arch/powerpc/cpu/ppc4xx/commproc.c
@@ -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 */
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 334e457..5f85502 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -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
diff --git a/arch/powerpc/lib/bootcount.c b/arch/powerpc/lib/bootcount.c
new file mode 100644
index 0000000..b7f7d92
--- /dev/null
+++ b/arch/powerpc/lib/bootcount.c
@@ -0,0 +1,90 @@
+/*
+ * (C) Copyright 2010
+ * Stefan Roese, DENX Software Engineering, sr at denx.de.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#if defined(CONFIG_MPC5xxx)
+#define CONFIG_BOOTCOUNT_ADDR	(MPC5XXX_CDM_BRDCRMB)
+#define CONFIG_SYS_BOOTCOUNT_USE_32BIT
+#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)
+#define CONFIG_BOOTCOUNT_ADDR	(((immap_t *)CONFIG_SYS_IMMR)->im_cpm.cp_dpmem + \
+				 CPM_BOOTCOUNT_ADDR)
+#endif /* defined(CONFIG_MPC8xx) */
+
+#if defined(CONFIG_MPC8260)
+#include <asm/cpm_8260.h>
+
+#define CONFIG_BOOTCOUNT_ADDR	(CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR)
+#endif /* defined(CONFIG_MPC8260) */
+
+#if defined(CONFIG_MPC8360)
+#include <asm/immap_qe.h>
+
+#define CONFIG_BOOTCOUNT_ADDR	(CONFIG_SYS_IMMR + 0x110000 + \
+				 QE_MURAM_SIZE - 2 * sizeof(unsigned long))
+#endif /* defined(CONFIG_MPC8360) */
+
+#if defined(CONFIG_4xx)
+#define CONFIG_BOOTCOUNT_ADDR	(CONFIG_SYS_OCM_DATA_ADDR + \
+				 CONFIG_SYS_BOOTCOUNT_ADDR)
+#endif /* defined(CONFIG_4xx) */
+
+#if !defined(CONFIG_BOOTCOUNT_ADDR)
+#error "CONFIG_BOOTCOUNT_LIMIT currently not implemented for this platform!"
+#endif
+
+void bootcount_store(ulong a)
+{
+	void *reg = (void *)CONFIG_BOOTCOUNT_ADDR;
+
+#if defined(CONFIG_SYS_BOOTCOUNT_USE_32BIT)
+	out_be32(reg, (BOOTCOUNT_MAGIC & 0xffff0000) | a);
+#else
+	out_be32(reg, a);
+	out_be32(reg + 4, BOOTCOUNT_MAGIC);
+#endif
+}
+
+ulong bootcount_load(void)
+{
+	void *reg = (void *)CONFIG_BOOTCOUNT_ADDR;
+
+#if defined(CONFIG_SYS_BOOTCOUNT_USE_32BIT)
+	if (in_be16(reg + 2) != (BOOTCOUNT_MAGIC & 0xffff))
+		return 0;
+	else
+		return in_be16(reg);
+#else
+	if (in_be32(reg + 4) != BOOTCOUNT_MAGIC)
+		return 0;
+	else
+		return in_be32(reg);
+#endif
+}
-- 
1.7.1

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[Detlev Zundel]

Hi Stefan,

> 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 <sr@denx.de>
> Cc: Michael Zaidman <michael.zaidman@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Kim Phillips <kim.phillips@freescale.com>
> Cc: Anatolij Gustschin <agust@denx.de>

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 <dzu@denx.de>

Cheers
  Detlev

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

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[Stefan Roese]

Hi Detlev,

On Tuesday 27 April 2010 16:01:11 Detlev Zundel wrote:
> > 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 <sr@denx.de>
> > Cc: Michael Zaidman <michael.zaidman@gmail.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Kim Phillips <kim.phillips@freescale.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> 
> 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.
 
> Apart from that:
> 
> Acked-by: Detlev Zundel <dzu@denx.de>

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

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[Michael Zaidman]

Hi Stefan,

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

On Mon, Apr 26, 2010 at 5:28 PM, Stefan Roese <sr@denx.de> wrote:
> 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 <sr@denx.de>
> Cc: Michael Zaidman <michael.zaidman@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Kim Phillips <kim.phillips@freescale.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> ---
> ?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
>

[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

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

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

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[Stefan Roese]

Hi Michael,

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

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

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

[U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC

[Michael Zaidman]

On Wed, Apr 28, 2010 at 10:14 AM, Stefan Roese <sr@denx.de> wrote:
> Hi Michael,
>
> 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
>>
>> > +#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.

 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

>
>> > +#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.
>
>> 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).

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

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