On Thu, Feb 18, 2016 at 05:42:29PM +0100, Guillaume Gardet wrote: > > > Le 18/02/2016 17:38, Nikita Kiryanov a écrit : > >On Thu, Feb 18, 2016 at 05:11:46PM +0100, Guillaume Gardet wrote: > >> > >>Le 18/02/2016 17:07, Nikita Kiryanov a écrit : > >>>On Thu, Feb 18, 2016 at 09:36:01AM -0500, Tom Rini wrote: > >>>>On Thu, Feb 18, 2016 at 04:25:29PM +0200, Nikita Kiryanov wrote: > >>>>>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 <[email protected]> > >>>>>>>>>>>Cc: Tom Rini <[email protected]> > >>>>>>>>>>>Cc: Nikita Kiryanov <[email protected]> > >>>>>>>>>>>Cc: Igor Grinberg <[email protected]> > >>>>>>>>>>>Cc: Paul Kocialkowski <[email protected]> > >>>>>>>>>>>Cc: Pantelis Antoniou <[email protected]> > >>>>>>>>>>>Cc: Simon Glass <[email protected]> > >>>>>>>>>>>Cc: Matwey V. Kornilov <[email protected]> > >>>>>>>>>>> > >>>>>>>>>>>--- > >>>>>>>>>>> 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. > >>>>> */ > >>>>But it's not board specific, it's use-case specific. > >>>The comment I proposed does not suggest it's board specific, just that > >>>this specific use case is used by someone. > >>> > >>>>instead of shoving both SPL and U-Boot into the correct magic raw > >>>>location, just shove SPL there and let U-Boot itself be in the /boot > >>>>partition. This is just as applicable on say imx6 as it is on TI parts. > >>>I don't think that's clear enough that this is the purpose of the > >>>missing break statement. It's a little too implicit. What I'm suggesting > >>>is that we make it a bit more explicit, barring a rewrite. > >>So, maybe just: > >> /* If raw mode fails, try fs mode. */ > >>? > >That'll work too. > > If Tom is ok, I will send a V2.
OK with me, thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
