[U-Boot] [PATCH v3 09/15] env: Support multiple environments

[Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>]

On 06.02.2018 09:20, Joakim Tjernlund wrote:
> On Thu, 1970-01-01 at 00:00 +0000, Simon Goldschmidt wrote:
>
> .....
>>> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>>    env/env.c | 80 +++++++++++++++++++++++++++++++++++---------------------
>>>    1 file changed, 50 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/env/env.c b/env/env.c
>>> index 906f28ee50a1..1182fdb545db 100644
>>> --- a/env/env.c
>>> +++ b/env/env.c
>>> @@ -26,6 +26,41 @@ static struct env_driver *_env_driver_lookup(enum env_location loc)
>>>        return NULL;
>>>    }
>>>
>>> +static enum env_location env_locations[] = {
> Don't use static/global variables. They cause a lot of relocation work/size
> and is less flexible.

In this specific case, I think this array should be const anyway, would 
that prevent the relocation problems you see?

> There is no way to #define ENVL_EEPROM to a function
> when a variable.

ENVL_EEPROM is an enum value, why would you define it to a function?

Simon


>
>   Jocke
>
>>> +#ifdef CONFIG_ENV_IS_IN_EEPROM
>>> +     ENVL_EEPROM,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_FAT
>>> +     ENVL_FAT,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_FLASH
>>> +     ENVL_FLASH,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_MMC
>>> +     ENVL_MMC,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_NAND
>>> +     ENVL_NAND,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_NVRAM
>>> +     ENVL_NVRAM,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_REMOTE
>>> +     ENVL_REMOTE,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
>>> +     ENVL_SPI_FLASH,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_IN_UBI
>>> +     ENVL_UBI,
>>> +#endif
>>> +#ifdef CONFIG_ENV_IS_NOWHERE
>>> +     ENVL_NOWHERE,
>>> +#endif
>>> +};
>>> +
>>> +static enum env_location env_load_location = ENVL_UNKNOWN;
>>> +
>>>    /**
>>>     * env_get_location() - Returns the best env location for a board
>>>     * @op: operations performed on the environment
>>> @@ -45,36 +80,21 @@ static struct env_driver *_env_driver_lookup(enum env_location loc)
>>>     */
>>>    static enum env_location env_get_location(enum env_operation op, int prio)
>>>    {
>>> -     /*
>>> -      * We support a single environment, so any environment asked
>>> -      * with a priority that is not zero is out of our supported
>>> -      * bounds.
>>> -      */
>>> -     if (prio >= 1)
>>> -             return ENVL_UNKNOWN;
>>> -
>>> -     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;
>>> +     switch (op) {
>>> +     case ENVOP_GET_CHAR:
>>> +     case ENVOP_INIT:
>>> +     case ENVOP_LOAD:
>>> +             if (prio >= ARRAY_SIZE(env_locations))
>>> +                     return ENVL_UNKNOWN;
>>> +
>>> +             env_load_location = env_locations[prio];
>>> +             return env_load_location;
>>> +
>>> +     case ENVOP_SAVE:
>>> +             return env_load_location;
>>> +     }
>>> +
>>> +     return ENVL_UNKNOWN;
>>>    }
>>>
>>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot