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

[Wolfgang Denk]

Dear Kyle Moffett,

In message <1299519462-25320-2-git-send-email-Kyle.D.Moffett at boeing.com> you wrote:
> In preparation for making system restart use a generic set of hooks for
> boards and architectures, we define some wrappers and weak stubs.
> 
> 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?

...
> +/*
> + * This MUST be called from normal interrupts-on context.  If it requires
> + * extended interaction with external hardware it SHOULD react to Ctrl-C.

??? Why such complicated restrictions for a simple command that is
just supposed to reset the board?

> + * 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.

> +int system_restart(void)
> +{
> +	int err;
> +
> +	/*
> +	 * Print a nice message and wait a bit to make sure it goes out the
> +	 * console properly.
> +	 */
> +	printf ("Restarting...\n");
> +	udelay(50000);
> +
> +	/* First let the board code try to reboot */
> +	err = __board_restart();
> +	if (err)
> +		goto failed;
> +
> +	/* Now call into the architecture-specific code */
> +	err = __arch_restart();
> +	if (err)
> +		goto failed;
> +
> +	/* Fallback to the old do_reset() until everything is converted. */
> +	err = do_reset(NULL, 0, 0, NULL);
> +
> +failed:
> +	printf("*** SYSTEM RESTART FAILED ***\n");
> +	return err;
> +}

You are making a simple thing pretty much complicated.  This adds lots
of code and I cannot see any benefits.

My initial feeling is a plain NAK, for this and the rest of the patch
series.  Why would we want all this?

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
They're usually so busy thinking about what  happens  next  that  the
only  time they ever find out what is happening now is when they come
to look back on it.                 - Terry Pratchett, _Wyrd Sisters_