u-boot: remove driver lookup loop from env_save()

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 behaviour causes env_save() to fall into an infinite loop when the low-level drv->save() call fails. The env_save() function should not loop through the environment location list but it should save the environment into the location stored in gd->env_load_location by the last env_load() call. Signed-off-by: Nicholas Faustini <nicholas.faustini@azcomtech.com> Reviewed-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>

u-boot: remove driver lookup loop from env_save()
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 behaviour causes env_save() to fall into an infinite loop when the low-level drv->save() call fails. The env_save() function should not loop through the environment location list but it should save the environment into the location stored in gd->env_load_location by the last env_load() call. Signed-off-by: Nicholas Faustini <nicholas.faustini@azcomtech.com> Reviewed-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
Nicholas Faustini · Tom Rini · Eric Lee · Eric Lee · Eric Lee · Eric Lee
1 parent b8448051df
Showing 2 changed files with 17 additions and 19 deletions Side-by-side Diff
env/env.c
include/asm-generic/global_data.h
@@ -119,21 +119,12 @@
  */
 __weak enum env_location env_get_location(enum env_operation op, int prio)
 {
-	switch (op) {
-	case ENVOP_GET_CHAR:
-	case ENVOP_INIT:
-	case ENVOP_LOAD:
-		if (prio >= ARRAY_SIZE(env_locations))
-			return ENVL_UNKNOWN;
+	if (prio >= ARRAY_SIZE(env_locations))
+		return ENVL_UNKNOWN;
  
-		gd->env_load_location = env_locations[prio];
-		return gd->env_load_location;
+	gd->env_load_prio = prio;
  
-	case ENVOP_SAVE:
-		return gd->env_load_location;
-	}
-
-	return ENVL_UNKNOWN;
+	return env_locations[prio];
 }
  
  
  
  
  
  
@@ -205,22 +196,29 @@
 			return 0;
 	}
  
+	/*
+	 * In case of invalid environment, we set the 'default' env location
+	 * to the highest priority. In this way, next calls to env_save()
+	 * will restore the environment at the right place.
+	 */
+	env_get_location(ENVOP_LOAD, 0);
+
 	return -ENODEV;
 }
  
 int env_save(void)
 {
 	struct env_driver *drv;
-	int prio;
  
-	for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
+	drv = env_driver_lookup(ENVOP_SAVE, gd->env_load_prio);
+	if (drv) {
 		int ret;
  
 		if (!drv->save)
-			continue;
+			return -ENODEV;
  
 		if (!env_has_inited(drv->location))
-			continue;
+			return -ENODEV;
  
 		printf("Saving Environment to %s... ", drv->name);
 		ret = drv->save();
@@ -50,7 +50,7 @@
 	unsigned long env_addr;		/* Address  of Environment struct */
 	unsigned long env_valid;	/* Environment valid? enum env_valid */
 	unsigned long env_has_init;	/* Bitmask of boolean of struct env_location offsets */
-	int env_load_location;
+	int env_load_prio;		/* Priority of the loaded environment */
  
 	unsigned long ram_base;		/* Base address of RAM used by U-Boot */
 	unsigned long ram_top;		/* Top address of RAM used by U-Boot */
...	...	@@ -119,21 +119,12 @@
119	119	*/
120	120	__weak enum env_location env_get_location(enum env_operation op, int prio)
121	121	{
122		- switch (op) {
123		- case ENVOP_GET_CHAR:
124		- case ENVOP_INIT:
125		- case ENVOP_LOAD:
126		- if (prio >= ARRAY_SIZE(env_locations))
127		- return ENVL_UNKNOWN;
	122	+ if (prio >= ARRAY_SIZE(env_locations))
	123	+ return ENVL_UNKNOWN;
128	124
129		- gd->env_load_location = env_locations[prio];
130		- return gd->env_load_location;
	125	+ gd->env_load_prio = prio;
131	126
132		- case ENVOP_SAVE:
133		- return gd->env_load_location;
134		- }
135		-
136		- return ENVL_UNKNOWN;
	127	+ return env_locations[prio];
137	128	}
138	129
139	130
140	131
141	132
142	133
143	134
...	...	@@ -205,22 +196,29 @@
205	196	return 0;
206	197	}
207	198
	199	+ /*
	200	+ * In case of invalid environment, we set the 'default' env location
	201	+ * to the highest priority. In this way, next calls to env_save()
	202	+ * will restore the environment at the right place.
	203	+ */
	204	+ env_get_location(ENVOP_LOAD, 0);
	205	+
208	206	return -ENODEV;
209	207	}
210	208
211	209	int env_save(void)
212	210	{
213	211	struct env_driver *drv;
214		- int prio;
215	212
216		- for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
	213	+ drv = env_driver_lookup(ENVOP_SAVE, gd->env_load_prio);
	214	+ if (drv) {
217	215	int ret;
218	216
219	217	if (!drv->save)
220		- continue;
	218	+ return -ENODEV;
221	219
222	220	if (!env_has_inited(drv->location))
223		- continue;
	221	+ return -ENODEV;
224	222
225	223	printf("Saving Environment to %s... ", drv->name);
226	224	ret = drv->save();
...	...	@@ -50,7 +50,7 @@
50	50	unsigned long env_addr; /* Address of Environment struct */
51	51	unsigned long env_valid; /* Environment valid? enum env_valid */
52	52	unsigned long env_has_init; /* Bitmask of boolean of struct env_location offsets */
53		- int env_load_location;
	53	+ int env_load_prio; /* Priority of the loaded environment */
54	54
55	55	unsigned long ram_base; /* Base address of RAM used by U-Boot */
56	56	unsigned long ram_top; /* Top address of RAM used by U-Boot */