From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Wed, 17 May 2017 04:09:41 -0600 Subject: [U-Boot] [PATCH v2 5/9] Fix up inclusion of common.h In-Reply-To: References: <20170501151852.26670-1-sjg@chromium.org> <20170501151852.26670-6-sjg@chromium.org> <20170510214318.GV12511@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 Masahiro, On 16 May 2017 at 04:32, Masahiro Yamada wrote: > Hi Simon, > > > 2017-05-13 10:11 GMT+09:00 Simon Glass : >> Hi Masahiro, >> >> On 10 May 2017 at 20:21, Masahiro Yamada wrote: >>> Hi Simon >>> >>> 2017-05-11 6:43 GMT+09:00 Tom Rini : >>>> 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 >>>> >>>> Reviewed-by: Tom Rini >>> >>> >>> 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. It we are trying to reduce this (and I agree it is a good goal) we should measure it, e.g. by having a buildman feature to count the number of header files / lines including each each build. > > > >> 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. OK that makes sense and I certainly want to point to the future. > > To reference legacy options that have not converted to Kconfig, > should be included. > also works, but it is sometimes over-inclusion. OK > > >> 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 is wrong. > 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 . > is missing from the code I wrote, and it is intentional. I think I can include asm/global_data in the header instead, so I can drop this patch. Regards, Simon