[U-Boot] [PATCH] spl: if MMCSD_MODE_RAW fails, try MMCSD_MODE_FS, if available

[Nikita Kiryanov]

Hi Tom, Guillaume,

On Wed, Feb 17, 2016 at 03:27:22PM -0500, Tom Rini wrote:
> On Wed, Feb 17, 2016 at 09:09:27AM +0100, Guillaume GARDET wrote:
> 
> > Since commit fd61d39970b9901217efc7536d9f3a61b4e1752a:
> >         spl: mmc: add break statements in spl_mmc_load_image() 
> > RAW and FS boot modes are now exclusive again. So, if MMCSD_MODE_RAW fails, the
> > board hangs. This patch allows to try MMCSD_MODE_FS then, if available.
> > 
> > It has been tested on a beaglebone black to boot on an EXT partition.
> > 
> > Signed-off-by: Guillaume GARDET <guillaume.gardet at free.fr>
> > Cc: Tom Rini <trini at konsulko.com>
> > Cc: 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: Simon Glass <sjg at chromium.org>
> > Cc: Matwey V. Kornilov <matwey.kornilov at gmail.com>
> > 
> > ---
> >  common/spl/spl_mmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> > index c3931c6..2eef0f2 100644
> > --- a/common/spl/spl_mmc.c
> > +++ b/common/spl/spl_mmc.c
> > @@ -284,7 +284,7 @@ int spl_mmc_load_image(u32 boot_device)
> >  		if (!err)
> >  			return err;
> >  #endif
> > -		break;
> > +		/* Fall through */
> >  	case MMCSD_MODE_FS:
> >  		debug("spl: mmc boot mode: fs\n");
> 
> This also essentially reverts fd61d399.  So Nikita, was there a specific
> use case that was broken before, or was the code just unclear in
> intentions here?  Thanks!

There was no broken use case that I'm aware of. The change was made as
part of a code improvement series and was meant to address what I
consider to be bad and problematic design. Instead of reverting it
though, how about implementing something similar to what I did in the
main common/spl/spl.c:board_init_r()? You would have a weak function
that will default to the original spl_boot_mode() if not overridden,
and allow the user to define a sequence of boot modes otherwise.

> 
> -- 
> Tom