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


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

2017-11-16 Thread Maxime Ripard
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)
-- 
2.14.3

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