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

[Masahiro Yamada]

Hi Simon

2017-05-17 19:09 GMT+09:00 Simon Glass <sjg at chromium.org>:
> 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.

When we end up including lots of unnecessary things,
it is a sign that we need cleanups.
This would happen if we put unrelated misc things into a single header.

So, cleaning of <common.h> as you did
is really nice.



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

Agree.
I think #include <asm/global.h> is the right thing to do.





-- 
Best Regards
Masahiro Yamada