From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guillaume Gardet Date: Thu, 18 Feb 2016 14:31:08 +0100 Subject: [U-Boot] [PATCH] spl: if MMCSD_MODE_RAW fails, try MMCSD_MODE_FS, if available In-Reply-To: <20160218130754.GA23954@skynet> References: <56C36A04.2020102@free.fr> <1455696567-11977-1-git-send-email-guillaume.gardet@free.fr> <20160217202722.GW23166@bill-the-cat> <20160218091925.GA15017@skynet> <56C597A8.9000408@free.fr> <20160218130754.GA23954@skynet> Message-ID: <56C5C79C.2080107@free.fr> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>> Cc: Tom Rini >>>>> Cc: Nikita Kiryanov >>>>> Cc: Igor Grinberg >>>>> Cc: Paul Kocialkowski >>>>> Cc: Pantelis Antoniou >>>>> Cc: Simon Glass >>>>> Cc: Matwey V. Kornilov >>>>> >>>>> --- >>>>> 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 think, "Fall through" code comment explains it (as you done with MMCSD_MODE_EMMCBOOT). I (and openSUSE ARM) do not pretend to be the only user(s) of this feature, so I won't add my name there. If you mean commit message, I think the current one is enough. Guillaume > >> 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 >>>