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

[Mike Frysinger]

On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
> +__attribute__((__noreturn__))
> +void emergency_restart(void)
> +{
> +	__board_emergency_restart();
> +	__arch_emergency_restart();
> +
> +	/* Fallback to the old do_reset() until everything is converted. */
> +	do_reset(NULL, 0, 0, NULL);
> +
> +	printf("EMERGENCY RESTART: All attempts to reboot failed!");

missing a trailing newline, and i'd just call puts() to bypass useless format 
string parsing

> +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");

no space before the "(", and i'd use puts() here

> +	udelay(50000);

this doesnt sit well with me.  i dont see why this matters ... we dont have 
any delays today, and i dont think we should introduce any.

> +	printf("*** SYSTEM RESTART FAILED ***\n");

puts()

> +__attribute__((__weak__))
> +int __board_restart(void)
> +{
> +	/* Fallthrough to architecture-specific code */
> +	return 0;
> +}
> +
> +__attribute__((__weak__))
> +int __arch_restart(void)
> +{
> +	/* Fallthrough to legacy do_reset() code */
> +	return 0;
> +}

what use is there in having a return value ?  the do_reset() funcs today dont 
return, which means they have no meaningful reset value.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110307/c82df44f/attachment.pgp