[PATCH] ARM: uniphier: delete or replace <common.h> includes

[Masahiro Yamada]

On Fri, May 8, 2020 at 11:41 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Masahiro,
>
> On Thu, 7 May 2020 at 20:31, Masahiro Yamada <masahiroy at kernel.org> wrote:
> >
> > Hi Simon,
> >
> >
> > On Fri, May 8, 2020 at 10:37 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Masahiro,
> > >
> > > On Thu, 7 May 2020 at 07:10, Masahiro Yamada
> > > <yamada.masahiro at socionext.com> wrote:
> > > >
> > > > <common.h> pulls in a lot of bloat. <common.h> is unneeded in most of
> > > > places.
> > > >
> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> > > > ---
> > > >
> > > >
> > >
> > > I'm wary of this. I think that every file should include common.h
> >
> >
> > I disagree.
> >
> > "Please include <common.h> at the beginning of every file"
> > is a fragile rule.
> > You have no way to check it.
>
> We can add it to checkpatch.


checkpatch is weak since not all people run it.



> >
> > Our goal is to get rid of the
> > special treatment of <common.h>
>
> As we get closer though I've been thinking about the goal. Do we want
> people to include config.h specifically if common.h has nothing in it?

Yes and no.

In theory, if we succeed in converting all CONFIG options
to Kconfig, config.h will go away. But we are still
much far from the goal.

Maybe, we should give '-include include/config.h' from the
command line as we do '-include include/linux/kconfig.h'.


> I feel it is safer to keep common.h, perhaps just with config.h
> included, until we fully understand what we need.
>
> >
> >
> >
> > > and
> > > the solution is to remove the bloat. I have been plugging away at
> > > that. There is a pending series that reduces it down further, to 14
> > > includes. Please help review!
> > >
> > > http://patchwork.ozlabs.org/project/uboot/list/?series=169491
> >
> > I saw it.
> >
> > Humans cannot check it.
> > If buildman does not raise a flag, it is fine.
>
> OK.
>
> >
> >
> >
> > >
> > > The problem is that when someone uses #ifdef CONFIG options the
> > > config.h has to be included. So your patch is a bit brittle. As soon
> > > as someone uses CONFIG it may break.
> >
> >
> > For the legacy CONFIG options, yes.
> > The options in Kconfig are all safe.
>
> How come? If config.h is included, the options are not defined.


The top Makefile forces every file to include
include/linux/kconfig.h, which includes
include/generated/autoconf.h

So, CONFIG options from Kconfig are automatically included.

This does not happen for legacy CONFIG options
in include/configs/*.h


> >
> > Common options were mostly moved to Kconfig.
> >
> > We still have lots in scripts/config_whitelist.txt
> > but most of them are platform-specific craps.
>
> Yes...perhaps we should try to have two whitelists, so we know which
> ones matter more.


I do not think so.
The criteria between the two whitelists
is ambiguous.
We should not do what we cannot do perfectly.



--
Best Regards
Masahiro Yamada