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?
> Then, if someone (you, me or someone else) wants to improve this code, the
> way you suggested, it would be very nice.
> But it will need a lot more work, tests and reviews.
>
>
> Guillaume
>
> >
> >>--
> >>Tom
> >
> >
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
