[U-Boot] [PATCH 08/08] include/configs: Better utilize CONFIG_SYS_NO_FLASH
Wolfgang Denk
wd at denx.de
Sat Apr 14 08:22:23 CEST 2012
Dear Tom Rini,
In message <1334355606-16491-9-git-send-email-trini at ti.com> you wrote:
> In config files which it is clear when CONFIG_SYS_NO_FLASH will be set
s/which it is clear when/where it is clear that/ ?
> (either unconditionally or based on logic that can happen early in the
> config file), ensure that we set that _before_ we include
> config_cmd_default.h so that the logic in that file will not enable
> CONFIG_CMD_(FLASH|IMLS). This saves us from having to undef it in the
> board files.
I'm not sure if I like that. It makes the code not easier to
understand, but actually less obvious.
> --- a/include/configs/PN62.h
> +++ b/include/configs/PN62.h
> @@ -56,13 +56,12 @@
> /*
> * Command line configuration.
> */
> +#define CONFIG_SYS_NO_FLASH /* There is no FLASH memory */
> #include <config_cmd_default.h>
...
> -#define CONFIG_SYS_NO_FLASH 1 /* There is no FLASH memory */
> -
> #define CONFIG_ENV_IS_NOWHERE 1 /* Store ENV in memory only */
> #define CONFIG_ENV_OFFSET 0x00004000 /* Offset of Environment Sector */
> #define CONFIG_ENV_SIZE 0x00002000 /* Total Size of Environment Sector */
Logically, it makes more sense to me to look for the NO_FLASH setting
close to where we are dfinging such things - the relation to the
"Command line configuration" is pretty obscure.
> diff --git a/include/configs/SIMPC8313.h b/include/configs/SIMPC8313.h
> index 0976077..8b51f6c 100644
> --- a/include/configs/SIMPC8313.h
> +++ b/include/configs/SIMPC8313.h
> @@ -105,8 +105,6 @@
> /*
> * FLASH on the Local Bus
> */
> -#define CONFIG_SYS_NO_FLASH
> -
> #if !defined(CONFIG_NAND_SPL)
> #define CONFIG_SYS_RAMBOOT
> #endif
> @@ -347,9 +345,8 @@
> /*
> * Command line configuration.
> */
> +#define CONFIG_SYS_NO_FLASH
> #include <config_cmd_default.h>
> -#undef CONFIG_CMD_IMLS
> -#undef CONFIG_CMD_FLASH
That's even worse here.
I actually tend to believe it is a bad idea to have the #ifdef for
CONFIG_SYS_NO_FLASH in <config_cmd_default.h>
Sorry, but I don't think this patch is a good idea.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Wish not to seem, but to be, the best." - Aeschylus
More information about the U-Boot
mailing list