[U-Boot] [PATCH V4 02/13] spl: mmc: add break statements in spl_mmc_load_image()

Nikita Kiryanov nikita at compulab.co.il
Sun Nov 8 16:11:43 CET 2015


The original intention of the mmc load_image() function was to try multiple
boot modes before failing. This is evident by the lack of break statements
in the switch, and the following line in the default case:
puts("spl: mmc: no boot mode left to try\n");

This implementation is problematic because:
- The availability of alternative boot modes is very arbitrary since it
depends on the specific order of the switch cases. If your boot mode happens to
be the first case, then you'll have a bunch of other boot modes as alternatives.
If it happens to be the last case, then you have none.
- Opting in/out is tied to config options, so the only way for you to prevent an
alternative boot mode from being attempted is to give up on the feature completely.
- This implementation makes the code more complicated and difficult to
understand.

Address these issues by inserting a break statements between the cases to make the
function try only one boot mode.

Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
Cc: Igor Grinberg <grinberg at compulab.co.il>
Cc: Paul Kocialkowski <contact at paulk.fr>
Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
Cc: Tom Rini <trini at konsulko.com>
Cc: Simon Glass <sjg at chromium.org>
Reviewed-by: Simon Glass <sjg at chromium.org>
Reviewed-by: Tom Rini <trini at konsulko.com>
---
Changes in V4:
	- No changes.

Changes in V3:
	- No changes.

Changes in V2:
	- Removed hang() in default cases, moved default case into ifdef
	  LIBCOMMON.

 common/spl/spl_mmc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index ce58c58..6011f77 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -164,6 +164,7 @@ void spl_mmc_load_image(void)
 		if (!err)
 			return;
 #endif
+		break;
 	case MMCSD_MODE_FS:
 		debug("spl: mmc boot mode: fs\n");
 
@@ -203,6 +204,7 @@ void spl_mmc_load_image(void)
 #endif
 #endif
 #endif
+		break;
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	case MMCSD_MODE_EMMCBOOT:
 		/*
@@ -240,15 +242,14 @@ void spl_mmc_load_image(void)
 		if (!err)
 			return;
 #endif
+		break;
 #endif
 	case MMCSD_MODE_UNDEFINED:
-	default:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-		if (err)
-			puts("spl: mmc: no boot mode left to try\n");
-		else
-			puts("spl: mmc: wrong boot mode\n");
+	default:
+		puts("spl: mmc: wrong boot mode\n");
 #endif
-		hang();
 	}
+
+	hang();
 }
-- 
1.9.1



More information about the U-Boot mailing list