[U-Boot] [PATCH 1/2] Changes for auto detection of NAND or OneNAND flash on u-boot
Scott Wood
scottwood at freescale.com
Mon Mar 23 22:49:13 CET 2009
On Mon, Mar 23, 2009 at 04:44:03PM +0530, Manikandan Pillai wrote:
> The patch has been broken to 2 files. It has also been tested with NAND and
> OneNAND configuration on OMAP3 EVM board.
>
> The following changes are for the common files.
>
> Signed-off-by: Manikandan Pillai <mani.pillai at ti.com>
This could use a better changelog description -- nothing in here
indicates that the change has to do with the environment, much less what
the nature of the change is, what situations call for it, etc.
Text like "Changes for..." aren't particularly descriptive, especially in
the subject -- *all* changesets have changes in them. :-)
> diff --git a/common/Makefile b/common/Makefile
> index f13cd11..a6c55d2 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -58,6 +58,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
> COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
> COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
> COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
> +COBJS-$(CONFIG_ENV_IS_RUNTIME_SEL) += env_onenand.o env_nand.o
That's quite a generic name for something that refers specifically to a
choice between OneNAND and NAND, and thus implies that both are selected.
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 68c673e..628bcf4 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR;
> !defined(CONFIG_ENV_IS_IN_NAND) && \
> !defined(CONFIG_ENV_IS_IN_ONENAND) && \
> !defined(CONFIG_ENV_IS_IN_SPI_FLASH) && \
> + !defined(CONFIG_ENV_IS_RUNTIME_SEL) && \
> !defined(CONFIG_ENV_IS_NOWHERE)
> # error Define one of CONFIG_ENV_IS_IN_{NVRAM|EEPROM|FLASH|DATAFLASH|ONENAND|SPI_FLASH|NOWHERE}
> #endif
> @@ -66,6 +67,10 @@ DECLARE_GLOBAL_DATA_PTR;
> #define XMK_STR(x) #x
> #define MK_STR(x) XMK_STR(x)
>
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> +extern saveenv_p saveenv;
> +#endif
externs belong in header files.
> /************************************************************************
> ************************************************************************/
>
> diff --git a/common/env_common.c b/common/env_common.c
> index 6be3bb0..b692900 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -46,8 +46,13 @@ DECLARE_GLOBAL_DATA_PTR;
>
> extern env_t *env_ptr;
>
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> +extern env_get_char_spec_p env_get_char_spec;
> +extern env_relocate_spec_p env_relocate_spec;
> +#else
> extern void env_relocate_spec (void);
> extern uchar env_get_char_spec(int);
> +#endif
No #ifdef here. The caller interface to the functions should not change
just because the implementation is changing (or if it must change, change
it for *all* configurations).
In general, this patch has too many #ifdefs. Please try to refactor
things into a common way rather than separating things into "that way"
and "this way".
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> +char *nand_env_name_spec = "NAND";
> +#else
> char * env_name_spec = "NAND";
> -
> +#endif
This does not need an ifdef. Either namespace-protect the name and
provide a weak alias, or (better IMO) just have the NAND env init code
store "NAND" into a globally defined env_name_spec (or env->name or
similar).
> diff --git a/include/common.h b/include/common.h
> index b75ea60..fd3da2c 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -243,12 +243,20 @@ extern ulong load_addr; /* Default Load Address */
> void doc_probe(unsigned long physadr);
>
> /* common/cmd_nvedit.c */
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> +typedef uchar (*env_get_char_spec_p)(int index);
> +typedef int (*env_init_p)(void);
> +typedef int (*saveenv_p)(void);
> +typedef void (*env_relocate_spec_p)(void);
> +#else
> int env_init (void);
> +int saveenv(void);
> +#endif
> void env_relocate (void);
> int envmatch (uchar *, int);
> char *getenv (char *);
> int getenv_r (char *name, char *buf, unsigned len);
> -int saveenv (void);
Drop the _p. We don't do hungarian notation.
Might want to group all the things that change based on environment type
into one struct.
> #ifdef CONFIG_PPC /* ARM version to be fixed! */
> int inline setenv (char *, char *);
> #else
> diff --git a/lib_arm/board.c b/lib_arm/board.c
> index 09eaaf2..f52ac00 100644
> --- a/lib_arm/board.c
> +++ b/lib_arm/board.c
Is there anything in these changes that would require other architectures
to change to keep from breaking?
> @@ -60,6 +60,11 @@ DECLARE_GLOBAL_DATA_PTR;
>
> ulong monitor_flash_len;
>
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> +extern void gpmc_init_late(void);
> +extern void print_board_info(void);
> +#endif
> +
> #ifdef CONFIG_HAS_DATAFLASH
> extern int AT91F_DataflashInit(void);
> extern void dataflash_print_info(void);
> @@ -259,12 +264,19 @@ static int arm_pci_init(void)
> typedef int (init_fnc_t) (void);
>
> int print_cpuinfo (void); /* test-only */
> +#ifdef CONFIG_ENV_IS_RUNTIME_SEL
> +extern env_init_p env_init;
> +#endif
>
> init_fnc_t *init_sequence[] = {
> cpu_init, /* basic cpu dependent setup */
> board_init, /* basic board dependent setup */
> interrupt_init, /* set up exceptions */
> +#ifdef CONFIG_ENV_IS_RUNTIME_SEL
> + NULL, /* initialize environment */
> +#else
> env_init, /* initialize environment */
> +#endif
> init_baudrate, /* initialze baudrate settings */
> serial_init, /* serial communications setup */
> console_init_f, /* stage 1 init of console */
> @@ -350,14 +362,26 @@ void start_armboot (void)
> /* armboot_start is defined in the board-specific linker script */
> mem_malloc_init (_armboot_start - CONFIG_SYS_MALLOC_LEN);
>
> +#if defined(CONFIG_ENV_IS_RUNTIME_SEL)
> + gpmc_init_late(); /* in SRAM or SDRAM, finish GPMC */
> + env_init();
> + init_baudrate(); /* initialze baudrate settings */
> + serial_init(); /* serial communications setup */
> + console_init_f(); /* stage 1 init of console */
> + display_banner();
> + print_board_info();
> +#else
You appear to be calling init_baudrate(), etc. twice.
-Scott
More information about the U-Boot
mailing list