[U-Boot] [PATCH v2 5/5] RFC: tegra: Convert to using environment files

[Simon Glass]

Hi Stephen,

On Wed, Jun 26, 2013 at 2:16 PM, Stephen Warren <swarren at 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