Hi, On Wed, 1 Feb 2023 at 08:48, Tom Rini <tr...@konsulko.com> wrote: > > On Wed, Feb 01, 2023 at 01:19:51PM +0100, Rasmus Villemoes wrote: > > 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
Yes I was missing an underscore. git grep -E "CONFIG_([A-Za-z0-9_]*)\b" |sed -n "s/.*\bCONFIG_\([A-Za-z0-9_]*\)\b.*/\1/p" | sort |uniq | grep "[a-z]" |wc -l 187 But some are not real. > > 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. FONT_6X11 isn't that bad :-) > > > > > 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". OK would you like to have a crack at these? > > I suspect the answer here is that we should: > - Patch (and submit upstream) kconfiglib.py to accept lowercase [a-z] as > valid symbol names, as they are. That project is dead, unfortunately. I've had a PR outstanding for over a year. It looks like someone here might take it on, once we can figure out naming, etc. Rasmus, would you like to take a look at the one in U-Boot? Here is what Zephyr did: https://github.com/zephyrproject-rtos/zephyr/issues/40420 > - Add a sentence or two to codingstyle.rst about discouraging lowercase, > as suggested by Rasmus > - Drop the rename parts of this series. If people still want lower case that's OK with me. We have so few I felt it was just confusing, plus the fact that the widely use kconfiglib doesn't support it... Regards, Simon
