[PATCH 02/88] treewide: Correct invalid Kconfig syntax and warnings

[Tom Rini]

On Fri, Feb 10, 2023 at 12:24:24PM +0000, Alexey Brodkin wrote:
> Hi Tom, Daniel, all,
> 
> > On 1/27/23 14:45, Tom Rini wrote:
> > > On Mon, Jan 23, 2023 at 02:59:05PM -0700, Simon Glass wrote:
> > >
> > >> In several places a 'select' is used to select a choice, which is not
> > >> supported by Kconfig. In other places, the filename for the 'source'
> > >> command is not in quites.
> > >>
> > >> Fix these two problems throughout the tree, so that kconfiglib does not
> > >> show any more warnings.
> > >>
> > >> Signed-off-by: Simon Glass <sjg at chromium.org>
> > >
> > > OK, to summarize what I just said in another email and clarify future
> > > work. Please first split this patch in to its own series that corrects
> > > each type of problem, per commit. The missing quotes for example, and
> > > then the extra whitespace ones. Next, commenting out a select is wrong,
> > > and each case needs to be better understood / fixed. I'm honestly not
> > > sure if asking endianness for MIPS is right and if we should select that
> > > from boards too, like ARC, but probably. The ARC_MMU one also should
> > > just not be asked, I suspect, but as a separate patch where you cc
> > > Alexey, we'll find out :)  And so on, for each.  Thanks.
> > >
> 
> Sorry for not chiming-in sooner, but I'm here now ;)
> 
> > For MIPS the endianess (and also architecture/ISA level) needs to be able to be set
> > by the user via menuconfig as most MIPS cores or SoCs can support multiple variants.
> > The idea is that the specific SoC or machine just sets the supported options to
> > restrict the options the user can choose. The board's defconfig should set the
> > required default value for each option but must not *select* it.
> > 
> > See the Boston board for example:
> > 
> > config TARGET_BOSTON
> >         bool "Support Boston"
> >         ...
> >         select SUPPORTS_BIG_ENDIAN
> >         select SUPPORTS_CPU_MIPS32_R1
> >         select SUPPORTS_CPU_MIPS32_R2
> >         select SUPPORTS_CPU_MIPS32_R6
> >         select SUPPORTS_CPU_MIPS64_R1
> >         select SUPPORTS_CPU_MIPS64_R2
> >         select SUPPORTS_CPU_MIPS64_R6
> >         select SUPPORTS_LITTLE_ENDIAN
> > 
> > 
> > 
> > A possible fix for ARC could be:
> 
> To be honest I don't quite understand what's a problem which we try to fix here
> and so it's not clear what to advise.

The way we (rather than the Linux kernel) do Kconfig, all architectures
worth of Kconfigs are loaded at once. And using "select" on a symbol
that's defined in a choice isn't strictly legal (but it doesn't fail to
work, today).

> ARC cores as opposed to MIPS are configured in IP design process and so
> if a particular chip is designed as little-endian and already produced in
> silicon, it cannot be turned into big-endian.
> 
> Still ARCompact (AKA ARCv1) & ARCv2 processors could be configured as either
> little- or big-endian, thus we used to support both variants in U-Boot.
> And it's still there, so hopefully we may keep it.

OK.

> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -53,8 +53,6 @@ config ARC
> >          select SUPPORT_OF_CONTROL
> >          select SYS_CACHE_SHIFT_7
> >          select TIMER
> > -       select SYS_BIG_ENDIAN if CPU_BIG_ENDIAN
> > -       select SYS_LITTLE_ENDIAN if !CPU_BIG_ENDIAN
> >  
> >   config ARM
> >          bool "ARM architecture"
> > @@ -490,7 +488,7 @@ endif
> >  
> >   source "board/keymile/Kconfig"
> >  
> > -if MIPS || MICROBLAZE
> > +if MIPS || MICROBLAZE || ARC
> >  
> >   choice
> >          prompt "Endianness selection"
> > @@ -502,11 +500,11 @@ choice
> >  
> >   config SYS_BIG_ENDIAN
> >          bool "Big endian"
> > -       depends on (SUPPORTS_BIG_ENDIAN && MIPS) || MICROBLAZE
> > +       depends on (SUPPORTS_BIG_ENDIAN && MIPS) || MICROBLAZE || (CPU_BIG_ENDIAN && ARC)
> >  
> >   config SYS_LITTLE_ENDIAN
> >          bool "Little endian"
> > -       depends on (SUPPORTS_LITTLE_ENDIAN && MIPS) || MICROBLAZE
> > +       depends on (SUPPORTS_LITTLE_ENDIAN && MIPS) || MICROBLAZE || (CPU_LITTLE_ENDIAN && ARC)
> >  
> >   endchoice
> > 
> > 
> > 
> > A *make savedefconfig* should than automatically add *CONFIG_SYS_LITTLE_ENDIAN=y* or
> > *CONFIG_SYS_BIG_ENDIAN=y* to the ARC board defconfig's.
> 
> That proposal looks reasonable and allows to keep existing code as it is,
> but can we just have only 1 config option which is used for endianess selection?
> I.e. why having both "SYS_BIG_ENDIAN" & "CPU_BIG_ENDIAN"?

Today, ARC is the one that uses both SYS_x_ENDIAN and CPU_BIG_ENDIAN
(set or lack of being set), so if you can update the ARC side?

-- 
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/20230210/579f22b3/attachment.sig>