Re: [RFC][POWERPC] bootwrapper: Add a firmware-independent

[Milton Miller <miltonm@bga.com>]

On Friday, Feb 8, 2008 David Gibson wrote:
> On Fri, Feb 01, 2008 at 11:55:42PM -0700, Grant Likely wrote:
> From: Grant Likely <grant.likely at secretlab.ca>

[snip]
>> +void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
>> +   unsigned long r6, unsigned long r7)
>> +{
>> +const u32 *na, *ns, *reg, *timebase;
>> +u64 memsize64;
>> +int node, size, i;
>> +
>> +/* Allocate initial heap for probing the tree */
>> +simple_alloc_init(initial_heap, sizeof(initial_heap), 32, 64);
>> +
>> +/* Make sure FDT blob is sane */
>> +if (fdt_check_header(_dtb_start) != 0)
>> +fatal("Invalid device tree blob\n");
> 
> I think most of these fatal()s are pretty pointless.  This is
> platform_init(), so the console won't even have been initialized to
> actually print any of the messages.  Precisely because this is
> simpleboot, in which every bit of information the wrapper has comes
> from teh device tree, if the provided blob is so bad as to fail these
> basic tests, we're totally stuffed anyway.  It'll take a hardware
> debugger to track down, and I don't think the fatal()s will actually
> help much at that point.


My experience is the opposite: fatal is very useful even with a hardware debugger.  Since the code knows what is wrong, one only has to get the printf assembly buffer out of the map and dump it out.  Also, one can find which function called fatal from the nia and/or lr.  Much much easier than figuring out what happend when the prcessor jumps into the middle of code because it took an exception.

Also, one can patch in a debug output routine, possibly even in the __zlib_init.  Having the assertions is good.

That said, we should be conservative in calling something an error.

milton