From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Mon, 8 Jul 2013 09:24:36 -0400 Subject: [U-Boot] [PATCH v2 1/5] bootm: Handle errors consistently In-Reply-To: References: <1372969032-20009-1-git-send-email-sjg@chromium.org> <20130705125909.GJ16630@bill-the-cat> <20130705201549.GK16630@bill-the-cat> <20130705202927.GL16630@bill-the-cat> Message-ID: <20130708132436.GM16630@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Jul 05, 2013 at 01:48:30PM -0700, Simon Glass wrote: > Hi Tom, > > On Fri, Jul 5, 2013 at 1:29 PM, Tom Rini wrote: > > > On Fri, Jul 05, 2013 at 01:21:09PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, Jul 5, 2013 at 1:15 PM, Tom Rini wrote: > > > > > > > On Fri, Jul 05, 2013 at 12:52:03PM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Fri, Jul 5, 2013 at 5:59 AM, Tom Rini wrote: > > > > > > > > > > > On Thu, Jul 04, 2013 at 01:17:07PM -0700, Simon Glass wrote: > > > > > > > > > > > > > A recent bootm fix left the error path incomplete. Reinstate > > this so > > > > that > > > > > > > failures in bootm stages are handled properly. > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > --- > > > > > > > Changes in v2: > > > > > > > - Correct checking in the no-error case > > > > > > > > > > > > > > > > > > OK, this conflicts with the change I posted (and pushed later than > > I > > > > > > thought I had). Can you confirm the code is good in mainline now? > > > > > > Thanks! > > > > > > > > > > > > > > > > It's close, but I think it still needs this near the end > > > > > of do_bootm_states(), something like: > > > > > > > > > > else if (ret == BOOTM_ERR_RESET) do_reset(cmdtp, flag, argc, argv); > > + > > > > else > > > > > if (ret) + puts("subcommand not supported\n"); return ret; > > > > > > > > > > If you agree, I can prepare a patch as part of the bootz update. > > > > > > > > How do we get there in the code? When we do any subcalls is where > > we've > > > > got that puts already. Failures from that point on are either the OS > > > > bootm part failed (and return is > 0) or one of the BOOTM_ERR codes. > > Or > > > > did I miss a case still? > > > > > > > > > > I think this is when the boot_os function returns an error. At least the > > > old code had quite a lot of printf()s for that case. > > > > We had a printf per subcommand before, and just a single one now. We > > didn't say the 'go' subcommand failed however, just what the > > function happened to print out. > > Yes that looks right to me. But I believe that the GO command is not > supposed to return, so it might be harmless to put the message there in all > cases. If not, we can add a special case for GO. OK, I've been re-reading the before and after code, and at least wrt error messages and such, we're OK now. The 'GO' state is allowed to return 1 and usually does both for detectable errors (for example, FIT image for VxWorks, but a bad image) and "wth? we've been returned to from the OS". And I don't want to add a won't be reached string to the build given how badly we see compilers dealing with optimizing strings, along with the code bloat reason. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: