[U-Boot] [PATCH 08/08] include/configs: Better utilize CONFIG_SYS_NO_FLASH

[Wolfgang Denk <wd@denx.de>]

Dear Tom Rini,

In message <1334355606-16491-9-git-send-email-trini@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