Hi Stephen,
On Wed, Jun 26, 2013 at 2:16 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 06/24/2013 02:46 PM, Simon Glass wrote:
This seems more intuitive that the current #define way of doing things. The resulting code is shorter, avoids the quoting and line continuation pain, and also improves the clumsy way that stdio variables are created:
#ifdef CONFIG_VIDEO_TEGRA #define STDOUT_LCD ",lcd" #else #define STDOUT_LCD "" #endif
... #define TEGRA_DEVICE_SETTINGS \ "stdout=serial" STDOUT_LCD "\0" \ ...
The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since it depends on the SOC type and we probably don't want to add .emv files for each board at this stage.
Presumably e.g. seaboard.env could #include "tegra20.env" in order to allow *.env to define MEM_LAYOUT_ENV_SETTINGS?
Hmmm maybe.
BTW, what's the #include -I path for these files? Perhaps it's worth setting it up as board/$vendor/$board/env board/$vendor/env arch/$arch/env to match the dtc+cpp patches I posted?
We could do that. I can't think of a down-side, but we need to decide on where these files should go and what each directory should contain. I am thinking that perhaps arch/arm/env might make sense and arch/arm/cpu/armv7/env also.
On the other hand, I'm really not keen to take this all to far on a first pass, and gain experience with it before building in lots of bells and whistles.
diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
+bootcmd_mmc0=setenv devnum 0; run mmc_boot +bootcmd_mmc1=setenv devnum 1; run mmc_booxt +boot_targets+= mmc1 mmc0
The first two lines there have no space after = and the last has space after +=. Does that get stripped? Should it?
We do need that space. There is no stripping done here as it reduces flexibility.
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
-#ifdef CONFIG_BOOTCOMMAND
-#define BOOTCMDS_COMMON ""
-#else
...
Overall this change seems reasonable, but as I alluded to earlier, removing this from tegra-common-post.h will break Tegra boards for vendors other than NVIDIA. I guess that's part of why this patch is RFC.
Yes, that patch would need a bit of testing plus coming up with a solution to the problem you identified (see my comments above).
Regards, Simon