On Thu, Jul 12, 2018 at 09:02:26AM +0200, Simon Goldschmidt wrote: > > > On 11.07.2018 15:50, Maxime Ripard wrote: > > On Wed, Jul 11, 2018 at 12:44:23PM +0200, Nicholas wrote: > > > > > > 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. > > > > > > > > Well, the use case I saw was that I wanted to test redundant > > > > environment storage. I admit this is not an end user use case but > > > > a developer use case, so I guess you're right. > > > > > > > > So after fixing this endless loops I see two questions: - to which > > > > environment do we store if the one in 'env_load_location' fails > > > > to store? > > > > > > Good question. My opinion is that it strongly depends on what we want > > > to achieve with the implementation: do we want 1) to keep the > > > consistency between load and save, or we want 2) to guarantee to be > > > able to load/save from at least one location? > > > > > > If 1) then a failure on env_save() simply fails and doesn't store > > > anything. In other words we are multi-env when loading but single-env > > > when storing. We are actually binding env_save() to the last > > > env_load()'s result. > > > > > > If 2) then a failure on env_save() will try all the available locations > > > but we are open to misalignments. Here we are full multi-env since > > > env_save() and env_load() are not bound together. > > > > In that case, we don't want to store to a lower priority, but to a > > higher one. Otherwise, the environment will be saved just fine, but > > will not be read next time, which will be very confusing to the user. > > > > And with a higher priority, you might end up overwriting something you > > weren't expecting to overwrite, so I'd vote 1. > > I agree that 1 would be best. But from reading the code, unless I'm totally > wrong, it seems that the patch sent by Nicholas does not suffice: > > If no location contained a valid environment (e.g. no environment written > yet), the lowest priority will be written as 'gd->env_load_location' is set > to the lowest priority from iterating locations in 'env_load()'. > > So we might have to reset 'gd->env_load_location' to highest prio if loading > the environment fails.
That would make sense yes. 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
