[U-Boot] [PATCH v2 1/5] bootm: Handle errors consistently

[Tom Rini <trini@ti.com>]

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 <trini@ti.com> 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 <trini@ti.com> 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 <trini@ti.com> 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 <sjg@chromium.org>
> > > > > > > ---
> > > > > > > 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: <http://lists.denx.de/pipermail/u-boot/attachments/20130708/5efee49e/attachment.pgp>