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

[Simon Glass]

Hi Masahiro,

On Fri, 8 May 2020 at 01:38, Masahiro Yamada <masahiroy at kernel.org> wrote:
>
> 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.

They should! In fact I think we should ask people to run patman since
it does a better job with maintainers for each patch.

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

Given that we are getting rid of the old CONFIG we should probably not do that.

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

OK I see. If I knew that once I forgot it.

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

Yes it is hard. I was thinking of splitting them by if they are
referenced in common files (i.e. not just board/ and arch/).

Regards,
Simon