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.
 */

> 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..

_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to