On 31/01/2023 15.16, Simon Glass wrote: > Hi Rasmus, > > On Mon, 30 Jan 2023 at 14:12, Rasmus Villemoes > <rasmus.villem...@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
