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

[Nikita Kiryanov]

On Thu, Feb 18, 2016 at 02:31:08PM +0100, Guillaume Gardet wrote:
> 
> 
> Le 18/02/2016 14:07, Nikita Kiryanov a écrit :
> >On Thu, Feb 18, 2016 at 11:06:32AM +0100, Guillaume Gardet wrote:
> >>Hi Tom, Nikita ,
> >>
> >>Le 18/02/2016 10:19, Nikita Kiryanov a écrit :
> >>>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.
> >>The thing is you broke a wanted behavior currently in use. So, the priority is to come back to the previous behavior.
> >Could you add a comment indicating that this is wanted behavior that
> >has a user, and who the user is?
> 
> Not sure what you mean.

I mean something like:
/* If raw mode fails, try fs mode. Some boards, such as beaglebone black,
 * depend on this funcitonality.
 */

> I think, "Fall through" code comment explains it (as you done with MMCSD_MODE_EMMCBOOT).

That was meant to communicate a very different type of coupling between
the two cases though..