Dear Patrick Delaunay, In message <20200124133332.3.I42c79507524e5ad68e85fd60bbd686c4c59523ae@changeid> you wrote: > Check the current ENV location, dynamically provided by the weak > function env_get_location to be sure that the environment can be > persistent. > > The compilation flag ENV_IS_IN_DEVICE is not enough when the board > dynamically select the available storage location (according boot > device for example). > > This patch solves issue for stm32mp1 platform, when the boot device > is USB. > > Signed-off-by: Patrick Delaunay <patrick.delau...@st.com> > --- > > cmd/nvedit.c | 13 ++++++++++--- > env/env.c | 18 ------------------ > include/env_internal.h | 20 ++++++++++++++++++++ > 3 files changed, 30 insertions(+), 21 deletions(-) > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > index 3d1054e763..a37b7c094a 100644 > --- a/cmd/nvedit.c > +++ b/cmd/nvedit.c > @@ -1269,9 +1269,16 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag, > /* evaluate whether environment can be persisted */ > if (eval_flags & ENV_INFO_IS_PERSISTED) { > #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) > - if (!quiet) > - printf("Environment can be persisted\n"); > - eval_results |= ENV_INFO_IS_PERSISTED; > + enum env_location loc = env_get_location(ENVOP_SAVE,
Please do not declare variables right in the middle of the code! > +++ b/env/env.c > @@ -105,24 +105,6 @@ static void env_set_inited(enum env_location location) > gd->env_has_init |= BIT(location); > } > > -/** > - * env_get_location() - Returns the best env location for a board > - * @op: operations performed on the environment > - * @prio: priority between the multiple environments, 0 being the > - * highest priority > - * > - * This will return the preferred environment for the given priority. > - * This is overridable by boards if they need to. > - * > - * All implementations are free to use the operation, the priority and > - * any other data relevant to their choice, but must take into account > - * the fact that the lowest prority (0) is the most important location > - * in the system. The following locations should be returned by order > - * of descending priorities, from the highest to the lowest priority. > - * > - * Returns: > - * an enum env_location value on success, a negative error code otherwise > - */ > __weak enum env_location env_get_location(enum env_operation op, int prio) I think it is a really bad idea to remove the comment from the implementation. Please keep it here. > --- a/include/env_internal.h > +++ b/include/env_internal.h > @@ -209,6 +209,26 @@ struct env_driver { > > extern struct hsearch_data env_htab; > > +/** > + * env_get_location() - Returns the best env location for a board > + * @op: operations performed on the environment > + * @prio: priority between the multiple environments, 0 being the > + * highest priority > + * > + * This will return the preferred environment for the given priority. > + * This is overridable by boards if they need to. > + * > + * All implementations are free to use the operation, the priority and > + * any other data relevant to their choice, but must take into account > + * the fact that the lowest prority (0) is the most important location > + * in the system. The following locations should be returned by order > + * of descending priorities, from the highest to the lowest priority. > + * > + * Returns: > + * an enum env_location value on success, a negative error code otherwise > + */ > +enum env_location env_get_location(enum env_operation op, int prio); If absolutely necessary, copuy only what is needed for API documentation. Best regards, Wolfgang Denk -- 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: w...@denx.de "This is a test of the Emergency Broadcast System. If this had been an actual emergency, do you really think we'd stick around to tell you?"