Hi,
On Wed, 1 Feb 2023 at 08:48, Tom Rini trini@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.villemoes@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:
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