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

2010-04-28 Thread Michael Zaidman
On Wed, Apr 28, 2010 at 10:14 AM, Stefan Roese s...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2010-04-27 Thread 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 s...@denx.de
 Cc: Michael Zaidman michael.zaid...@gmail.com
 Cc: Wolfgang Denk w...@denx.de
 Cc: Kim Phillips kim.phill...@freescale.com
 Cc: Anatolij Gustschin ag...@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 d...@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: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2010-04-27 Thread 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 s...@denx.de
  Cc: Michael Zaidman michael.zaid...@gmail.com
  Cc: Wolfgang Denk w...@denx.de
  Cc: Kim Phillips kim.phill...@freescale.com
  Cc: Anatolij Gustschin ag...@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 d...@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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2010-04-27 Thread 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 s...@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 s...@denx.de
 Cc: Michael Zaidman michael.zaid...@gmail.com
 Cc: Wolfgang Denk w...@denx.de
 Cc: Kim Phillips kim.phill...@freescale.com
 Cc: Anatolij Gustschin ag...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2010-04-26 Thread 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 s...@denx.de
Cc: Michael Zaidman michael.zaid...@gmail.com
Cc: Wolfgang Denk w...@denx.de
Cc: Kim Phillips kim.phill...@freescale.com
Cc: Anatolij Gustschin ag...@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  0x) | a;
-}
-
-ulong bootcount_load (void)
-{
-   volatile ulong *save_addr = (volatile ulong *) (MPC5XXX_CDM_BRDCRMB);
-
-   if ((*save_addr  0x) != (BOOTCOUNT_MAGIC  0x))
-   return 0;
-   else
-   return (*save_addr  0x);
-}
-#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  (0x11 + 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 */