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

[Tom Rini <trini@konsulko.com>]

On Wed, Jul 26, 2017 at 12:02:55PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 26 July 2017 at 11:23, Tom Rini <trini@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@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@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?

Hmm, I thought at the end of your series it was at least linkable to
have more than one at a time.  But even so, I think we'll still need a
build-time "user selected X as default" when more than one are allowed.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170726/4c9b20c3/attachment.sig>