Message ID | 1446995514-26357-3-git-send-email-nikita@compulab.co.il |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
On Sun, Nov 08, 2015 at 05:11:43PM +0200, Nikita Kiryanov wrote: > 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@compulab.co.il> > Cc: Igor Grinberg <grinberg@compulab.co.il> > Cc: Paul Kocialkowski <contact@paulk.fr> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: Simon Glass <sjg@chromium.org> > Reviewed-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Tom Rini <trini@konsulko.com> Applied to u-boot/master, thanks!
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(); }