[PATCH v2 09/87] Rename ARCH_NPCM7xx

[Tom Rini]

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 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".

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.
- Add a sentence or two to codingstyle.rst about discouraging lowercase,
  as suggested by Rasmus
- Drop the rename parts of this series.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230201/c195903a/attachment.sig>