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. > > -- > Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot