[U-Boot] [PATCH v2 5/9] Fix up inclusion of common.h

[Simon Glass]

Hi Masahiro,

On 16 May 2017 at 04:32, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
> Hi Simon,
>
>
> 2017-05-13 10:11 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> Hi Masahiro,
>>
>> On 10 May 2017 at 20:21, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
>>> Hi Simon
>>>
>>> 2017-05-11 6:43 GMT+09:00 Tom Rini <trini at konsulko.com>:
>>>> On Mon, May 01, 2017 at 09:18:48AM -0600, Simon Glass wrote:
>>>>
>>>>> It is good practice to include common.h as the first header. This ensures
>>>>> that required features like the DECLARE_GLOBAL_DATA_PTR macro,
>>>>> configuration options and common types are available.
>>>>>
>>>>> Fix up some files which currently don't do this. This is necessary because
>>>>> driver model will soon start using global data and configuration in the
>>>>> dm/ofnode.h header file, included via dm.h.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>
>>>> Reviewed-by: Tom Rini <trini at konsulko.com>
>>>
>>>
>>> NACK.
>>>
>>> include/common.h is really bad idea
>>> and this is a step backward.
>>>
>>> If you need something in your include/dm/ofnode.h
>>> you should include needed header(s) from it.
>>>
>>> Why do you need to touch lots of C files?
>>
>> All of these files fail to build when they cannot see global_data.
>
> Because you added DECLARE_GLOBAL_DATA_PTR in 8/9.

Yes that's right.

>
> If you always make sure each header is self-contained,
> you do not have to touch so many C files.

It depends what the objective is. If you are not careful you can end
up including lots of header files all the time.

It we are trying to reduce this (and I agree it is a good goal) we
should measure it, e.g. by having a buildman feature to count the
number of header files / lines including each each build.

>
>
>
>> Also we need access to CONFIG options in dm.h.
>
> DM related options are new enough to exist in Kconfig.
> No explicit header inclusion is required to reference options from Kconfig.

OK that makes sense and I certainly want to point to the future.

>
> To reference legacy options that have not converted to Kconfig,
> <config.h> should be included.
> <common.h> also works, but it is sometimes over-inclusion.

OK

>
>
>> So I think we have to
>> have common.h - it is (I think) a rule that all files should have
>> common.h and have it first, because any other header.
>
> I disagree.
> The concept of <common.h> is wrong.
> <common.h> always comes first in the inclusion list.  This is wrong too.
>
> For example in Linux, includes are often sorted alphabetically.
> This makes sense because headers can theoretically be included in any order.
>
>
> I always make efforts to reduce the dependency on <common.h>.
> <common.h> is missing from the code I wrote, and it is intentional.

I think I can include asm/global_data in the header instead, so I can
drop this patch.

Regards,
Simon