[PATCH v2 09/87] Rename ARCH_NPCM7xx

Rasmus Villemoes rasmus.villemoes at prevas.dk
Wed Feb 1 13:19:51 CET 2023


On 31/01/2023 15.16, Simon Glass wrote:
> Hi Rasmus,
> 
> On Mon, 30 Jan 2023 at 14:12, Rasmus Villemoes
> <rasmus.villemoes at prevas.dk> wrote:
>>
>> On 30/01/2023 16.54, Tom Rini wrote:
>>> On Sun, Jan 29, 2023 at 10:57:28PM +0100, Rasmus Villemoes wrote:
>>>> On 29/01/2023 01.57, Simon Glass wrote:
>>>>> CONFIG options must not use lower-case letter.
>>>>
>>>> Why?
>>>
>>> So, kconfiglib complains about these.
>>
>> Which IMO would be a bug in kconfiglib. Can you point me at where that
>> warning is in kconfiglib.py and how it looks and when one would
>> encounter it?
>>
>>> However, I can't find a formal
>>> language definition and the kernel documentation doesn't specify, merely
>>> imply that it should always be all uppercase.
>>
>> Well, yes, mostly, but since the de facto specification (namely, the
>> kernel's implementation) doesn't complain and the kernel's Kconfig files
>> do contain several examples of config symbols with lowercase characters,
>> why deviate? In particular, since we share a lot of code, if some piece
>> of kernel code has an IS_ENABLED(CONFIG_FOO876xx), why make it harder to
>> import and keep that in sync?
>>
>> Perhaps we can get Masahiro to tell us whether lowercase characters are
>> allowed in kconfig symbols or not.
>>
>> For reference, another kconfig-using project decided to fix their own
>> infrastructure around kconfig instead of enforcing uppercase symbols:
>>
>> https://github.com/zephyrproject-rtos/zephyr/issues/40420
> 
> That's all good context, thank you.
> 
> When we use #define it is normally with an upper-case string. The is
> the convention in U-Boot and Linux (and many other projects), I
> believe.

I don't disagree with the "normally", but that very word is precisely my
point: It's not universal, and there can be good reasons to deviate.
Both projects frown upon camelcase, so you don't see a lot of mixed-case
macros, but there's definitely a lot of all-lowercase ones (e.g.
iterators), and to pick just one example of a justified mixed-case one,
there's a "#define RANGE_mA" in some iio device.

 Also, having lower and upper case strings does become
> confusing, and inconsistent.
> 
> I was unaware that lower-case was allowed in Linux. It seems there are
> 35 cases of this in Linux.

Dunno how you reach that number, I can easily find 174. Among them stuff
like "config FONT_6x11" which is definitely more readable than with an
uppercase X. But yes, either way it's a very small fraction of all
config symbols, but OTOH I highly doubt linux would accept patches to
start renaming those without a very strong reason - it would break
out-of-tree defconfig files.

> But perhaps we should not allow it in U-Boot?

Discourage, definitely, perhaps even add something to checkpatch. But
renaming existing seems to be pointless churn, and definitely needs a
better rationale than "is not allowed" when that is manifestly not true.

Maybe prepend a patch updating codingstyle.rst and amend this to say "is
not allowed by the U-Boot coding standard". Or otherwise please provide
some reference answering my "why".

Rasmus



More information about the U-Boot mailing list