On 12.07.2018 09:20, Maxime Ripard wrote:
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.
Nicholas has sent v4 of this patch meanwhile, but it's not in reply to v1.
Any comments on that?
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
