[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Igor Grinberg]

On 07/04/11 23:32, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
>> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
>>> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
>>>> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
>>>>> +	   Hopefully there will never be this many machines. 
>>>>> +	   Can't use 0 since 0 is already used as a mach-type. */
>>>>> +	gd->bd->bi_arch_number = 0xffffffff; 
>>>>>  
>>>>>  	gd->bd->bi_baudrate = gd->baudrate;
>>>>>  	/* Ram ist board specific, so move it to board code ... */
>>>>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>>>>> index 802e833..70b3b76 100644
>>>>> --- a/arch/arm/lib/bootm.c
>>>>> +++ b/arch/arm/lib/bootm.c
>>>>> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>>>>>  		printf ("Using machid 0x%x from environment\n", machid);
>>>>>  	}
>>>>>  
>>>>> +#ifdef DEBUG
>>>>> +	if(machid==0xffffffff) {
>>>>> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
>>>> s/finish/start/ ;-)
>>>>
>>> I'll have to disagree here.  Linux will decompress and some functions
>>> will run but it will eventually stop, hence will not finish.
>> On further investigation, you're right, it doesn't finish
>> starting/booting.  Sorry for the noise.

To remove all doubts, why not make it:
Warning: machid not set! Linux will not be able to boot properly!

>>>> Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
>>>>
>>> The compiler can't know what machid will be at runtime. Maybe a "would
>>> you like to continue?" prompt could work.
>> Since the kernel throws a nice fat error message when the MACH_TYPE
>> doesn't match what it was compiled for, I don't see the point to adding
>> another message at the same point in the development process.
> I didn't see that message. Do you know what lines of code in the
> kernel print it? Or maybe just the message itself? 
> If the kernel can check the value why would it need to be passed 
> in the first place?
>
>> Perhaps use the constant CONFIG_MACH_TYPE, set to 0xffffffff.  Each
>> board config file sets it to MACH_TYPE_WHATEVER and then you could
>> do:
>>
>> #if CONFIG_MACH_TYPE == 0xffffffff
>> #warning "Machine type not set!  Linux will not finish booting!"
>> #endif
>>
>> You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
>> might be a good place.
>>
>> If the maintainers choose to move to a menuconfig style configuration
>> system, this logic could be handled in there (invalid config file).
> Right now CONFIG_MACH_TYPE is only used in a few boards and isn't used
> in core u-boot code, so I ignored it. I would agree that perhaps
> adding a CONFIG_MACH_TYPE to u-boot would be a more elegant solution
> than checking to make sure that it is a valid value before boot, but
> that would be another patch.

There is a patch ("arm: add CONFIG_MACH_TYPE option and documentation") pending
that adds CONFIG_MACH_TYPE (in case Jason missed it) as the formal config option.
But, again, it can be _runtime_ configurable, so you _can't_ fail the compilation.


-- 
Regards,
Igor.