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.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to