[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear "Moffett, Kyle D",

In message <A60AEA13-1206-4699-9302-0DF9C0F9DE28 at boeing.com> you wrote:
> 
> >> The new wrapper functions are:
> >>  system_restart()     -  Normal system reboot (IE: user request)
> >>  emergency_restart()  -  Critical error response (IE: panic(), etc)
> > 
> > What is the difference between these two - and why do we need
> > different functions at all?
> > 
> > A reset is a reset is a reset, isn't it?
>
> That might be true *IF* all boards could actually perform a real hardware reset.
>
> Some can't, and instead they just jump to their reset vector (Nios-II) or to flash (some ppc 74xx/7xx systems).

So this is the "reset" on these boards, then.

> If the board just panic()ed or got an unhandled trap or exception, then you
> don't want to do a soft-reset that assumes everything is OK.  A startup in
> a bad environment like that could corrupt FLASH or worse.  Right now there
> is no way to tell the difference, but the lower-level arch-specific code
> really should care.

I don't understand your chain of arguments.

If there really is no better way to implement the reset on such
boards, then what else can we do?

And if there are more things that could be done to provide a "better"
reset, then why should we not always do these?

> So system_restart() is what you use when the system is in a good normal
> operating condition.  The emergency_restart() is what gets called from panic()
> or in other places where a crash has happened.

Why?  What's the difference?

> >> + * If this function fails to guarantee a clean reboot or receives a Ctrl-C
> >> + * keystroke it SHOULD return with an error (-1).
> > 
> > A "reset" is supposed to take place immediately, and unconditionally.
> > If you need delays and ^C handling and other bells and whistles,
> > please add these to your own code, but not here.
>
> There's no Ctrl-C handling anywhere in this code, it will all be in my own
> __board_restart() hook.  As above, this documentation is just describing the

There is no ^C handling supposed to be in any reset hook.

You are changing user interfaces to very low-level and intentinally
simple commands in a complicated way, and I don;t see any advantage of
either this complexity nor your changes.

> guarantees provided to underlying __board_restart() and __arch_restart()
> hooks; if they check for Ctrl-C while polling external hardware and return
> an error then that's fine.

No, it is not, because it is not supposed to be done.

You could as well implement a "reset" cpmmand that actually turns on a
fan and the LCD backlight - that would be similarly useful.

> > My initial feeling is a plain NAK, for this and the rest of the patch
> > series.  Why would we want all this?
>
> While I was going through the hooks I noticed that several of them were
> explicitly NOT safe if the board was in the middle of a panic() for whatever

Can you please peovide some specific xamples?  I don't understand what
you are talking about.

> reason, so I split off the __*_emergency_restart() hooks separately to allow
> architectures to handle them cleanly.
>
> My own board needs both processor modules to synchronize resets to allow
> them to come back up at all, which means that a "reset" may block for an
> arbitrary amount of time waiting for the other module to cleanly shut down
> and restart (or waiting for somebody to type "reset" on the other U-Boot).
> If someone just types "reset" on the console, I want to allow them to hit
> Ctrl-C to interrupt the process.

This is not what the "reset" command is supposed to do.  The reset
command is supposed to be the software equivalent of someone pressing
the reset button on your board - to the extend possible to be
implemented in software.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If God had a beard, he'd be a UNIX programmer.