On 04/13/2012 11:22 PM, Wolfgang Denk wrote:
Dear Tom Rini,
In message<1334355606-16491-9-git-send-email-tr...@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.
So, at the end of the day it's an artifact of using cpp to make our
configs rather than something like Kconfig.
--- 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.
I agree. Some boards are laid out like this already, and in my
re-factoring of some of the TI parts config files,
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>
And perhaps we should drop IMLS/FLASH from config_cmd_default these days?
Sorry, but I don't think this patch is a good idea.
OK, I'll drop it from the general clean-ups and re-factor the configs
I'm focusing on to have cmds done near the end. Thanks!
--
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
