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

[Masahiro Yamada]

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.

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



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

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.


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


-- 
Best Regards
Masahiro Yamada