Hi Simon,
2017-05-13 10:11 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 10 May 2017 at 20:12, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2017-05-11 6:43 GMT+09:00 Tom Rini trini@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@chromium.org
Reviewed-by: Tom Rini trini@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.