Hi Masahiro, On Fri, 8 May 2020 at 01:38, Masahiro Yamada <masahi...@kernel.org> wrote: > > On Fri, May 8, 2020 at 11:41 AM Simon Glass <s...@chromium.org> wrote: > > > > Hi Masahiro, > > > > On Thu, 7 May 2020 at 20:31, Masahiro Yamada <masahi...@kernel.org> wrote: > > > > > > Hi Simon, > > > > > > > > > On Fri, May 8, 2020 at 10:37 AM Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Masahiro, > > > > > > > > On Thu, 7 May 2020 at 07:10, Masahiro Yamada > > > > <yamada.masah...@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.masah...@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