Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Maxime Ripard 
On Fri, Nov 17, 2017 at 03:40:51PM +0100, Lukasz Majewski wrote:
> Hi Maxime,
> 
> > Hi Lukasz, Tom,
> > 
> > On Fri, Nov 17, 2017 at 03:00:15PM +0100, Lukasz Majewski wrote:
> > > On Fri, 17 Nov 2017 08:41:05 -0500
> > > Tom Rini  wrote:
> > >   
> > > > On Fri, Nov 17, 2017 at 10:24:37AM +0100, Lukasz Majewski wrote:  
> > > > > Hi Maxime,
> > > > > 
> > > > > > Now that we have everything in place to support multiple
> > > > > > environment, let's make sure the current code can use it.
> > > > > > 
> > > > > > The priority used between the various environment is the same
> > > > > > one that was used in the code previously.
> > > > > > 
> > > > > > At read / init times, the highest priority environment is
> > > > > > going to be detected, and we'll use the same one without
> > > > > > lookup during writes. This should implement the same
> > > > > > behaviour than we currently have.
> > > > > > 
> > > > > > Signed-off-by: Maxime Ripard
> > > > > >  ---
> > > > > >  env/env.c | 75
> > > > > > ++-
> > > > > > 1 file changed, 50 insertions(+), 25 deletions(-)
> > > > > > 
> > > > > > diff --git a/env/env.c b/env/env.c
> > > > > > index 1d13220aa79b..6af9f897b0ae 100644
> > > > > > --- a/env/env.c
> > > > > > +++ b/env/env.c
> > > > > > @@ -26,33 +26,58 @@ static struct env_driver
> > > > > > *_env_driver_lookup(enum env_location loc) return NULL;
> > > > > >  }
> > > > > >  
> > > > > > +static enum env_location env_locations[] = {
> > > > > > +#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
> > > > > > +   ENVL_UNKNOWN,
> > > > > > +};
> > > > > > +
> > > > > > +static enum env_location env_load_location;
> > > > > > +
> > > > > >  static enum env_location env_get_location(enum env_operation
> > > > > > op, int prio) {
> > > > > > -   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 ENVO_GET_CHAR:
> > > > > > +   case ENVO_INIT:
> > > > > > +   case ENVO_LOAD:
> > > > > > +   if (prio >= ARRAY_SIZE(env_locations))
> > > > > > +   return -ENODEV;
> > > > > > +
> > > > > > +   return env_load_location =
> > > > > > env_locations[prio]; +
> > > > > > +   case ENVO_SAVE:
> > > > > > +   return env_load_location;
> > > > > > +   }
> > > > > > +
> > > > > > +   return ENVL_UNKNOWN;
> > > > > >  }
> > > > > >  
> > > > > >  static struct env_driver *env_driver_lookup(enum
> > > > > > env_operation op, int prio)
> > > > > 
> > > > > I'm just wondering...
> > > > > 
> > > > > Do you think that it may be helpful to have a way to force
> > > > > particular environment to be used (despite of the priority)?
> > > > 
> > > > That's one of the hard parts of this, yes.  I think I had
> > > > suggested before that we handle that in a follow up series?
> > > >   
> > > 
> > > I don't want to say that this must be done now. I just can imagine
> > > that we may need such feature in some point.  
>

Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Lukasz Majewski 
Hi Maxime,

> Hi Lukasz, Tom,
> 
> On Fri, Nov 17, 2017 at 03:00:15PM +0100, Lukasz Majewski wrote:
> > On Fri, 17 Nov 2017 08:41:05 -0500
> > Tom Rini  wrote:
> >   
> > > On Fri, Nov 17, 2017 at 10:24:37AM +0100, Lukasz Majewski wrote:  
> > > > Hi Maxime,
> > > > 
> > > > > Now that we have everything in place to support multiple
> > > > > environment, let's make sure the current code can use it.
> > > > > 
> > > > > The priority used between the various environment is the same
> > > > > one that was used in the code previously.
> > > > > 
> > > > > At read / init times, the highest priority environment is
> > > > > going to be detected, and we'll use the same one without
> > > > > lookup during writes. This should implement the same
> > > > > behaviour than we currently have.
> > > > > 
> > > > > Signed-off-by: Maxime Ripard
> > > > >  ---
> > > > >  env/env.c | 75
> > > > > ++-
> > > > > 1 file changed, 50 insertions(+), 25 deletions(-)
> > > > > 
> > > > > diff --git a/env/env.c b/env/env.c
> > > > > index 1d13220aa79b..6af9f897b0ae 100644
> > > > > --- a/env/env.c
> > > > > +++ b/env/env.c
> > > > > @@ -26,33 +26,58 @@ static struct env_driver
> > > > > *_env_driver_lookup(enum env_location loc) return NULL;
> > > > >  }
> > > > >  
> > > > > +static enum env_location env_locations[] = {
> > > > > +#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
> > > > > + ENVL_UNKNOWN,
> > > > > +};
> > > > > +
> > > > > +static enum env_location env_load_location;
> > > > > +
> > > > >  static enum env_location env_get_location(enum env_operation
> > > > > op, int prio) {
> > > > > - 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 ENVO_GET_CHAR:
> > > > > + case ENVO_INIT:
> > > > > + case ENVO_LOAD:
> > > > > + if (prio >= ARRAY_SIZE(env_locations))
> > > > > + return -ENODEV;
> > > > > +
> > > > > + return env_load_location =
> > > > > env_locations[prio]; +
> > > > > + case ENVO_SAVE:
> > > > > + return env_load_location;
> > > > > + }
> > > > > +
> > > > > + return ENVL_UNKNOWN;
> > > > >  }
> > > > >  
> > > > >  static struct env_driver *env_driver_lookup(enum
> > > > > env_operation op, int prio)
> > > > 
> > > > I'm just wondering...
> > > > 
> > > > Do you think that it may be helpful to have a way to force
> > > > particular environment to be used (despite of the priority)?
> > > 
> > > That's one of the hard parts of this, yes.  I think I had
> > > suggested before that we handle that in a follow up series?
> > >   
> > 
> > I don't want to say that this must be done now. I just can imagine
> > that we may need such feature in some point.  
> 
> So I guess this question is also part of the comment Lukasz had on the
> other patch.
> 
> The current implementation is effectively hardcoding the environment
> by doing two things:
>   - Rejecting any priority other than 0,

Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Maxime Ripard 
Hi Lukasz, Tom,

On Fri, Nov 17, 2017 at 03:00:15PM +0100, Lukasz Majewski wrote:
> On Fri, 17 Nov 2017 08:41:05 -0500
> Tom Rini  wrote:
> 
> > On Fri, Nov 17, 2017 at 10:24:37AM +0100, Lukasz Majewski wrote:
> > > Hi Maxime,
> > >   
> > > > Now that we have everything in place to support multiple
> > > > environment, let's make sure the current code can use it.
> > > > 
> > > > The priority used between the various environment is the same one
> > > > that was used in the code previously.
> > > > 
> > > > At read / init times, the highest priority environment is going
> > > > to be detected, and we'll use the same one without lookup during
> > > > writes. This should implement the same behaviour than we
> > > > currently have.
> > > > 
> > > > Signed-off-by: Maxime Ripard 
> > > > ---
> > > >  env/env.c | 75
> > > > ++- 1
> > > > file changed, 50 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/env/env.c b/env/env.c
> > > > index 1d13220aa79b..6af9f897b0ae 100644
> > > > --- a/env/env.c
> > > > +++ b/env/env.c
> > > > @@ -26,33 +26,58 @@ static struct env_driver
> > > > *_env_driver_lookup(enum env_location loc) return NULL;
> > > >  }
> > > >  
> > > > +static enum env_location env_locations[] = {
> > > > +#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
> > > > +   ENVL_UNKNOWN,
> > > > +};
> > > > +
> > > > +static enum env_location env_load_location;
> > > > +
> > > >  static enum env_location env_get_location(enum env_operation op,
> > > > int prio) {
> > > > -   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 ENVO_GET_CHAR:
> > > > +   case ENVO_INIT:
> > > > +   case ENVO_LOAD:
> > > > +   if (prio >= ARRAY_SIZE(env_locations))
> > > > +   return -ENODEV;
> > > > +
> > > > +   return env_load_location = env_locations[prio];
> > > > +
> > > > +   case ENVO_SAVE:
> > > > +   return env_load_location;
> > > > +   }
> > > > +
> > > > +   return ENVL_UNKNOWN;
> > > >  }
> > > >  
> > > >  static struct env_driver *env_driver_lookup(enum env_operation
> > > > op, int prio)  
> > > 
> > > I'm just wondering...
> > > 
> > > Do you think that it may be helpful to have a way to force
> > > particular environment to be used (despite of the priority)?  
> > 
> > That's one of the hard parts of this, yes.  I think I had suggested
> > before that we handle that in a follow up series?
> > 
> 
> I don't want to say that this must be done now. I just can imagine that
> we may need such feature in some point.

So I guess this question is also part of the comment Lukasz had on the
other patch.

The current implementation is effectively hardcoding the environment
by doing two things:
  - Rejecting any priority other than 0, which is the highest. It
basically says to the code that there is no further alternative
and that its loop should stop there.
  - Returning the first environment in the array.

If one wanted to

Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Lukasz Majewski 
On Fri, 17 Nov 2017 08:41:05 -0500
Tom Rini  wrote:

> On Fri, Nov 17, 2017 at 10:24:37AM +0100, Lukasz Majewski wrote:
> > Hi Maxime,
> >   
> > > Now that we have everything in place to support multiple
> > > environment, let's make sure the current code can use it.
> > > 
> > > The priority used between the various environment is the same one
> > > that was used in the code previously.
> > > 
> > > At read / init times, the highest priority environment is going
> > > to be detected, and we'll use the same one without lookup during
> > > writes. This should implement the same behaviour than we
> > > currently have.
> > > 
> > > Signed-off-by: Maxime Ripard 
> > > ---
> > >  env/env.c | 75
> > > ++- 1
> > > file changed, 50 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/env/env.c b/env/env.c
> > > index 1d13220aa79b..6af9f897b0ae 100644
> > > --- a/env/env.c
> > > +++ b/env/env.c
> > > @@ -26,33 +26,58 @@ static struct env_driver
> > > *_env_driver_lookup(enum env_location loc) return NULL;
> > >  }
> > >  
> > > +static enum env_location env_locations[] = {
> > > +#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
> > > + ENVL_UNKNOWN,
> > > +};
> > > +
> > > +static enum env_location env_load_location;
> > > +
> > >  static enum env_location env_get_location(enum env_operation op,
> > > int prio) {
> > > - 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 ENVO_GET_CHAR:
> > > + case ENVO_INIT:
> > > + case ENVO_LOAD:
> > > + if (prio >= ARRAY_SIZE(env_locations))
> > > + return -ENODEV;
> > > +
> > > + return env_load_location = env_locations[prio];
> > > +
> > > + case ENVO_SAVE:
> > > + return env_load_location;
> > > + }
> > > +
> > > + return ENVL_UNKNOWN;
> > >  }
> > >  
> > >  static struct env_driver *env_driver_lookup(enum env_operation
> > > op, int prio)  
> > 
> > I'm just wondering...
> > 
> > Do you think that it may be helpful to have a way to force
> > particular environment to be used (despite of the priority)?  
> 
> That's one of the hard parts of this, yes.  I think I had suggested
> before that we handle that in a follow up series?
> 

I don't want to say that this must be done now. I just can imagine that
we may need such feature in some point.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpQ7drJqEd48.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Tom Rini 
On Fri, Nov 17, 2017 at 10:24:37AM +0100, Lukasz Majewski wrote:
> Hi Maxime,
> 
> > Now that we have everything in place to support multiple environment,
> > let's make sure the current code can use it.
> > 
> > The priority used between the various environment is the same one
> > that was used in the code previously.
> > 
> > At read / init times, the highest priority environment is going to be
> > detected, and we'll use the same one without lookup during writes.
> > This should implement the same behaviour than we currently have.
> > 
> > Signed-off-by: Maxime Ripard 
> > ---
> >  env/env.c | 75
> > ++- 1
> > file changed, 50 insertions(+), 25 deletions(-)
> > 
> > diff --git a/env/env.c b/env/env.c
> > index 1d13220aa79b..6af9f897b0ae 100644
> > --- a/env/env.c
> > +++ b/env/env.c
> > @@ -26,33 +26,58 @@ static struct env_driver *_env_driver_lookup(enum
> > env_location loc) return NULL;
> >  }
> >  
> > +static enum env_location env_locations[] = {
> > +#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
> > +   ENVL_UNKNOWN,
> > +};
> > +
> > +static enum env_location env_load_location;
> > +
> >  static enum env_location env_get_location(enum env_operation op, int
> > prio) {
> > -   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 ENVO_GET_CHAR:
> > +   case ENVO_INIT:
> > +   case ENVO_LOAD:
> > +   if (prio >= ARRAY_SIZE(env_locations))
> > +   return -ENODEV;
> > +
> > +   return env_load_location = env_locations[prio];
> > +
> > +   case ENVO_SAVE:
> > +   return env_load_location;
> > +   }
> > +
> > +   return ENVL_UNKNOWN;
> >  }
> >  
> >  static struct env_driver *env_driver_lookup(enum env_operation op,
> > int prio)
> 
> I'm just wondering...
> 
> Do you think that it may be helpful to have a way to force particular
> environment to be used (despite of the priority)?

That's one of the hard parts of this, yes.  I think I had suggested
before that we handle that in a follow up series?

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 06/10] env: Support multiple environments

2017-11-17 Thread Lukasz Majewski 
Hi Maxime,

> Now that we have everything in place to support multiple environment,
> let's make sure the current code can use it.
> 
> The priority used between the various environment is the same one
> that was used in the code previously.
> 
> At read / init times, the highest priority environment is going to be
> detected, and we'll use the same one without lookup during writes.
> This should implement the same behaviour than we currently have.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  env/env.c | 75
> ++- 1
> file changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/env/env.c b/env/env.c
> index 1d13220aa79b..6af9f897b0ae 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -26,33 +26,58 @@ static struct env_driver *_env_driver_lookup(enum
> env_location loc) return NULL;
>  }
>  
> +static enum env_location env_locations[] = {
> +#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
> + ENVL_UNKNOWN,
> +};
> +
> +static enum env_location env_load_location;
> +
>  static enum env_location env_get_location(enum env_operation op, int
> prio) {
> - 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 ENVO_GET_CHAR:
> + case ENVO_INIT:
> + case ENVO_LOAD:
> + if (prio >= ARRAY_SIZE(env_locations))
> + return -ENODEV;
> +
> + return env_load_location = env_locations[prio];
> +
> + case ENVO_SAVE:
> + return env_load_location;
> + }
> +
> + return ENVL_UNKNOWN;
>  }
>  
>  static struct env_driver *env_driver_lookup(enum env_operation op,
> int prio)

I'm just wondering...

Do you think that it may be helpful to have a way to force particular
environment to be used (despite of the priority)?

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpN3rrIAK5Di.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot