[U-Boot] [RFC PATCH] u-boot: remove driver lookup loop from env_save()

[Lukasz Majewski]

Hi Simon,

> Hi Nicholas,
> 
> On 10 July 2018 at 06:57, Nicholas Faustini
> <nicholas.faustini at azcomtech.com> wrote:
> > When called with ENVOP_SAVE, env_get_location() only returns the
> > gd->env_load_location variable without actually checking for
> > the environment location and priority. This allows saving the
> > environment into the same location that has been previously loaded.
> >
> > This behaviour causes env_save() to fall into an infinite loop when
> > the low-level drv->save() call fails.  
> 
> Why is this? Should it not stop eventually? 

I can confirm that there is a problem as described here - when you fail
to write to eMMC device (like e.g. 2, not 0), then we loop forever.

I do see such situation with Odroid XU3 when switching to BLK.

> Do we need a limit on
> prio?
> 
> >
> > The env_save() function should not loop through the environment
> > location list but it should use the previously discovered
> > environment driver once.  
> 
> What is that? It should be possible to write the env to multiple
> places. Also what is the previously discovered environment?
> 
> >
> > Signed-off-by: Nicholas Faustini <nicholas.faustini at azcomtech.com>
> > ---
> >
> >  env/env.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/env/env.c b/env/env.c
> > index 5c0842a..897d197 100644
> > --- a/env/env.c
> > +++ b/env/env.c
> > @@ -211,16 +211,16 @@ int env_load(void)
> >  int env_save(void)
> >  {
> >         struct env_driver *drv;
> > -       int prio;
> >
> > -       for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio));
> > prio++) {
> > +       drv = env_driver_lookup(ENVOP_SAVE, 0);
> > +       if (drv) {
> >                 int ret;
> >
> >                 if (!drv->save)
> > -                       continue;
> > +                       return -ENODEV;
> >
> >                 if (!env_has_inited(drv->location))
> > -                       continue;
> > +                       return -ENODEV;
> >
> >                 printf("Saving Environment to %s... ", drv->name);
> >                 ret = drv->save();
> > --
> > 2.7.4
> >  
> 
> Regards,
> Simon




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: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/028ac2f8/attachment.sig>