[PATCH v3 08/14] cmd: env: add env select command

[Patrick DELAUNAY]

Hi Tom

> From: Tom Rini <trini at konsulko.com>
> Sent: vendredi 26 juin 2020 22:55
> 
> On Thu, Jun 25, 2020 at 09:59:52AM +0200, Patrick Delaunay wrote:
> 
> > Add the new command 'env select' to force the persistent storage of
> > environment, saved in gd->env_load_prio.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> [snip]
> > +	/* search priority by driver */
> > +	for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
> > +		if (entry->location == env_get_location(ENVOP_LOAD, prio)) {
> > +			/* when priority change, reset the ENV flags */
> > +			if (gd->env_load_prio != prio) {
> > +				gd->env_load_prio = prio;
> > +				gd->env_valid = ENV_INVALID;
> > +				gd->flags &= ~GD_FLG_ENV_DEFAULT;
> > +			}
> > +			printf("OK\n");
> > +			return 0;
> > +		}
> > +	}
> 
> So, after we do this, is some follow up env command required to initialize the
> environment to now exist somewhere else?  Or will we have initialized all
> configured locations during boot, and don't have to?
> But what will happen if we select say "nand" but it's not present so didn't init.  Will
> things fail gracefully (not panic) ?  Thanks!

I was not sure if a automatic load was needed in this command, as I add a sparate load command
in this patch I don't add an  automatic load => and default env is used

My expected sequence for the env commands was:

	env  select  <target> 
	env load
	...
	env save

when user try to select a not compiled backend the command return "driver not found"
(tested on sandbox, I will add unitary test for this point)

or when backend is not accessible by any priority, the select command return pritority not found
	manual test on sandbox, with env_locations[] = { ENVL_NOWHERE };	

	=> env select EXT4
	Select Environment on EXT4: priority not found

I don't think that abort can occur for other commands because
- the env backend for all priotity are initialized at boot (env_init use the same loop than env select)

- in all env function (env_load / env_reload / env_save it is tested again
   (to check if the backend was correctly initilializaed, if .init() retur 0= 
		if (!env_has_inited(drv->location))
			return -ENODEV;

- all direct acces (in env_get_char for example) are intercepted because gd->env_valid == ENV_INVALID
     => by default (without explicite reload) the default environent is used, exactly when the load failed.
     => "gd->env_addr" is never used


but user can also execute the sequence 

	env  select  <target>
	env save

and here the command behavior  is strange....
because the default is forced on each select

	env select EXT4
	env load

	env select MMC    (reset on default  environment)
	env save

=> I expect at the end of the sequence the EXT4 env was copied in MMC....
     But in finally only the default environment is saved in MMC

I don't sure of the expected behavior by user for these commands....

what it is better solution ?
add a option to select command ? 
force the load after the select ?

Patrick