From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masahiro Yamada Date: Tue, 16 May 2017 18:56:04 +0900 Subject: [U-Boot] [PATCH v2 1/9] dm: Use dm.h header when driver mode is used In-Reply-To: References: <20170501151852.26670-1-sjg@chromium.org> <20170501151852.26670-2-sjg@chromium.org> <20170510214300.GR12511@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, 2017-05-13 10:11 GMT+09:00 Simon Glass : > Hi Masahiro, > > On 10 May 2017 at 20:12, Masahiro Yamada wrote: >> Hi Simon, >> >> >> 2017-05-11 6:43 GMT+09:00 Tom Rini : >>> 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 >>> >>> Reviewed-by: Tom Rini >>> >> >> 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 . is mostly used in board files. 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, should be included. If printf() is needed, it should be declared in include/stdio.h or somewhere. -- Best Regards Masahiro Yamada