Hi, On Wed, Jul 11, 2018 at 10:33:28AM +0200, Nicholas wrote: > Hi Simon, > > thanks for your comments and clarifications. I realize that I was not > super clear when describing the problem. > > On mer, 2018-07-11 at 07:09 +0200, Simon Goldschmidt wrote: > > Hi, > > > > sorry for the disclaimer in the last mail. Still don't know why this > > is > > the default here :-( > > > > Resending without the disclaimer to make possible follow-ups cleaner: > > > > On 10.07.2018 22:49, Simon Glass wrote: > > > > > > Hi Nicholas, > > > > > > On 10 July 2018 at 06:57, Nicholas Faustini > > > <nicholas.faust...@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? Do we need a limit on > > > prio? > > See my description in this mail: > > > > https://lists.denx.de/pipermail/u-boot/2018-April/324728.html > > > > Unfortunately, I got diverted at that time and could not follow up > > on > > this. Essentially, the question is raised what 'env save' is supposed > > to > > do with multiple environments. > > > > Currently, env_save() effectively stores only to the location where > > the > > env has been loaded from, which is questionable. But storing to all > > locations or the default location might not be correct, either. > > I have the same feeling about env_save(). Loading in multiple > environments is straightforward. Saving is not. > > I personally think that it makes more sense that env_save() saves into > the same location from which the enviroment has been previously loaded > (that is, current implementation). Otherwise, the logic becomes error- > prone since an user may env_load() from one location and env_save() > into another, resulting in misalignments which requires a lot of extra > logic in order to avoid such misalignments.
Or worse, you might end up erasing something that shouldn't be erased on your board. > > Maybe the 'env save' command might need a parameter the tells it > > where > > to save? > > Having a parameter to env_save() might be a solution but in my opinion > it would break the priority logic: an user follows the priority logic > when loading but she can work around that when saving. Moreover, having > the location embedded into env_*() functions is a great nice to have > imho. I agree. > Maybe a solution could be to have an env_save() function which acts in > a similar way as proposed in my patch and an env_save_prio() function, > which acts like the env_load() i.e. looking for the best working > location instead of relying on what has been stored into gd- > >env_load_location. I don't really see a use-case for overriding wherever the environment at the user-level actually. At the board level, for redundancy or transition, yes, definitely, but we can already do that. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot