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 <guillaume.gar...@free.fr> > >>>>>>>>>Cc: Tom Rini <tr...@konsulko.com> > >>>>>>>>>Cc: Nikita Kiryanov <nik...@compulab.co.il> > >>>>>>>>>Cc: Igor Grinberg <grinb...@compulab.co.il> > >>>>>>>>>Cc: Paul Kocialkowski <cont...@paulk.fr> > >>>>>>>>>Cc: Pantelis Antoniou <pa...@antoniou-consulting.com> > >>>>>>>>>Cc: Simon Glass <s...@chromium.org> > >>>>>>>>>Cc: Matwey V. Kornilov <matwey.korni...@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. > >>> */ > >>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. > > > Guillaume > > > > >>-- > >>Tom > > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot