[U-Boot] [PATCH v2 22/34] env: Add a new implementation of environment access

[Simon Glass]

Hi Tom,

On 26 July 2017 at 11:23, Tom Rini <trini at konsulko.com> wrote:
> On Wed, Jul 26, 2017 at 10:32:51AM -0600, Simon Glass wrote:
>> Hi Tom,
>>
>> On 26 July 2017 at 10:04, Tom Rini <trini at konsulko.com> wrote:
>> > On Sun, Jul 23, 2017 at 09:19:57PM -0600, Simon Glass wrote:
>> >
>> >> We plan to move to a environment access via drivers for each location
>> >> where the environment can be stored. Add an implementation for this. So
>> >> far it is not used, but will be pressed into service in a future patch.
>> >>
>> >> Signed-off-by: Simon Glass <sjg at chromium.org>
>> > [snip]
>> >> +static enum env_location env_get_default_location(void)
>> >> +{
>> >> +     if IS_ENABLED(CONFIG_ENV_IS_IN_DATAFLASH)
>> >> +             return ENVL_DATAFLASH;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_EEPROM)
>> >> +             return ENVL_EEPROM;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_FAT)
>> >> +             return ENVL_FAT;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_FLASH)
>> >> +             return ENVL_FLASH;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
>> >> +             return ENVL_MMC;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_NAND)
>> >> +             return ENVL_NAND;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_NVRAM)
>> >> +             return ENVL_NVRAM;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_REMOTE)
>> >> +             return ENVL_REMOTE;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)
>> >> +             return ENVL_SPI_FLASH;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_IN_UBI)
>> >> +             return ENVL_UBI;
>> >> +     else if IS_ENABLED(CONFIG_ENV_IS_NOWHERE)
>> >> +             return ENVL_NOWHERE;
>> >> +     else
>> >> +             return ENVL_UNKNOWN;
>> >> +}
>> >
>> > I think the kernel has a few examples of how the Kconfig side of things
>> > looks so that you can offer N options and then pick a default one (io
>> > scheduler, etc).  We should do that rather than alphabetical IS_ENABLED
>> > tests.
>>
>> Yes it is possible to have a choice in Kconfig, but for this first
>> series I have left it as a option. My plan is to allow the environment
>> to be in multiple places in the future, e.g. try using mmc and fall
>> back to SPI flash. I think that the environment location should be
>> able to be a run-time decision.
>>
>> What do you think?
>
> I think run-time decision will be helpful, yeah, but we'll still need a
> way to say "this is the default place to start from" at build time and
> be explicit about it, rather than rely on link order or "first probed"
> or whatever.

At present you can really only enable one environment location and the
above function enforces that. This function is intended to provide the
'default place to start from'. It's just that it's not enforced by
Kconfig. What do you think we should do in the interim? I suspect I
could add a patch to change it to a choice (in the interim) now that
the Kconfig version is done?

Regards,
Simon