[U-Boot] [PATCH V3 3/4] ARM: Warn when the machine ID isn't set.

Albert ARIBAUD
2011-07-17T09:00:47Z

Hi all,

Le 17/07/2011 10:26, stefano babic a ?crit :
> Am 17/07/2011 08:53, schrieb Igor Grinberg:
>>>> Also, in the printf line, you are mixing tabs with spaces
>>>> (sorry for not noticing this in previous versions...).
>>> ...and if you want to print something only for debug purposes, the best
>>> way is to substitute printf() with debug() and get rid of #ifdef.
>>>
>>> + if (machid == BI_ARCH_NUMBER_INVALID)
>>> + debug("Warning: machid not set.\n");
>>
>> That is understood completely and that is not what I'm asking...
>> I think that this warning should be printed not just for debug purposes...
>> So, I'd prefer:
>>
>> + if (machid == BI_ARCH_NUMBER_INVALID)
>> + printf("Warning: machid not set.\n");
>>
>> with no #ifdefs.
>
> Agree. And because the goal of thi patch is to warn when the mach-id is
> not set, I am expecting to see this warning on the console without
> recompiling the code.
>
>> So, I'm asking is it essential to make it only for debug purposes?
>
> IMHO, I think no.
>
>> Are there any cases when this code will harm if no #define DEBUG is specified?
>
> Agree with you, I do not see any reason to output the warning only if
> DEBUG is set

Agreed as well, only more so: I see reason for this warning *only* if it
is emitted in non-DEBUG builds. Such a warning is emitted to warn
against possible future complications; if it is only emitted in DEBUG
builds, then when the complication actually happens, that is, in a
production build, the developer is deprived of an important clue
regarding the cause.

> Best regards,
> Stefano Babic

Amicalement,
--
Albert.