Re: [U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC
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
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
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
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
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 */