[U-Boot] [PATCH v2 1/9] dm: Use dm.h header when driver mode is used

[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:12, 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:44AM -0600, Simon Glass wrote:
>>>
>>>> This header includes things that are needed to make driver build. Adjust
>>>> existing users to include that always, even if other dm/ includes are
>>>> present
>>>>
>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>
>>> Reviewed-by: Tom Rini <trini at konsulko.com>
>>>
>>
>> I'd say this is a bad idea.
>> I believe .c files should include headers that are really necessary.
>>
>> Mostly, drivers need only dm/device.h, but this commit
>> requires additional parse of dm/uclass.h and dm/platdata.h.
>>
>> Rather, it is better to deprecate dm.h.
>>
>> Its concept is DM common header that you force drivers to include
>> where some in them may not be necessary.
>
> I did consider this right at the start but I think it is too painful
> for users. There are only a few files that we pull in so the overhead
> is not great. It avoids having to add new headers because some other
> function is used.
>
> One option might be to define all the structs in one header, since
> those are the things that are really painful to figure out. We could
> then make the function name prefixes fully consistent with the header
> file name (mostly they are, but see lists.h and root.h). That would
> make it easier.

Still I do not get your point.

include/dm.h includes 3 headers and they are used for different purposes.

Most of drivers need only <dm/device.h>.
<dm/platdata.h> is mostly used in board files.
<dm/uclass.h> is used for core frameworks,
(and when getting access to a different uclass).


Each file just includes only needed headers, and
nothing confusing.


I do not see any benefit in this patch.



>>
>> It is a similar idea to include/common.h,
>> which is one of the biggest design mistakes in U-Boot.
>
> We have been slowing pulling things out of common.h - see for example
> mapmem.h and vsprint.h. We also have a lot of files in include/ which
> really should be arch-specific.
>
> But in any case I think common.h is useful just to include the
> configuration and some common declarations (like global_data). The
> problem is that it has too much in it.

The concept of common.h itself is wrong.

If global_data is needed, <asm-generic/global_data.h> should be included.
If printf() is needed, it should be declared in include/stdio.h or somewhere.



-- 
Best Regards
Masahiro Yamada