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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > > > > > --- > > > > > > > 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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
