diff mbox

Makefile: Include vendor common library in include search path

Message ID 20151106182348.GA14217@ogun.home
State New
Headers show

Commit Message

Nishanth Menon Nov. 6, 2015, 6:23 p.m. UTC 
On 11:10-20151106, Nishanth Menon wrote:
> On 11/05/2015 10:50 PM, Masahiro Yamada wrote:

> > 2015-11-06 12:30 GMT+09:00 Nishanth Menon <nm@ti.com>:

> >> On Thu, Nov 5, 2015 at 6:15 PM, Simon Glass <sjg@chromium.org> wrote:

> >>> Hi,

> >>>

> >>> On 5 November 2015 at 00:32, Nishanth Menon <nm@ti.com> wrote:

> >>>> On 11/05/2015 01:28 AM, Nishanth Menon wrote:

> >>>>> When the vendor common libraries exists, then board should be able to

> >>>>> reference headers located there, rather than having to do weird logic

> >>>>> such as '#include "../common/xyz.h"'.

> >>>>>

> >>>>> Signed-off-by: Nishanth Menon <nm@ti.com>

> >>>>> ---

> >>>>>

> >>>>>  Makefile                | 1 +

> >>>>>  board/ti/am57xx/board.c | 2 +-

> >>>>>  2 files changed, 2 insertions(+), 1 deletion(-)

> >>>>

> >>>> Arrgh.. Apologies on the diffstat messup, but anyways, the patch does

> >>>> apply, and will wait to repost in case of further comments.

> >>>>

> >>>>>

> >>>>> diff --git a/Makefile b/Makefile

> >>>>> index 3c21f8ddf9e9..75d5ea802dfd 100644

> >>>>> --- a/Makefile

> >>>>> +++ b/Makefile

> >>>>> @@ -620,6 +620,7 @@ c_flags := $(KBUILD_CFLAGS) $(cpp_flags)

> >>>>>  # U-Boot objects....order is important (i.e. start must be first)

> >>>>>

> >>>>>  HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makefile),y,n)

> >>>>> +UBOOTINCLUDE += $(if $(HAVE_VENDOR_COMMON_LIB:y=1), -I$(srctree)/board/$(VENDOR)/common)

> >>>>>

> >>>>>  libs-y += lib/

> >>>>>  libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/

> >>>>>

> >>>>

> >>>

> >>> I suppose this is OK. But it might be hard to figure out where a

> >>> header file is coming from. I wonder if we could make it support:

> >>>

> >>> #include <board-common/...>

> >>>

> >>> and it would find the file?

> >>>

> >>

> >> Hmmm... Are folks ok if I create a softlink?

> > 

> > 

> > Or,  you can maybe add the following into board/ti/am57xx/Makefile.

> > 

> > subdir-ccflags-y := -I$(src)/../common

> > 

> > then, you can use #include <xyz.h> from your board/ti/am57xx/board.c

> 

> I could, OR i could even do ../common/xyz.h (which is what current

> board files do)

> 

> > 

> > 

> > 

> > But, I personally wouldn't do this in this case

> > for the reason Simon mentioned.

> > 

> 

> I agree as well. but the files are built without explicit board level

> Makefile rules, but the root Makefile(which makes this happen) does

> not ensure the headers are made available - that sounds contradictory.

> 

> > 

> > How many vender-common do we have

> > and is it worth supporting  #include <board-common/...>?

> 

> Surprisingly larger set that what I expected:

> $ git branch -v|grep "^*"

> * master c3c016cf7536 sf: Add SPI NOR protection mechanism

> $ find board/ -iname "common"|wc -l

> 19

> $ git grep "#include \"../common" board/|wc -l

> 141

> 

Illustration Purposes only: example of how a
#include <board-common/xyz.h" could be done - is this what folks want me
to do?
-- 
Regards,
Nishanth Menon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Nishanth Menon Nov. 9, 2015, 5:37 p.m. UTC | #1 
On 11/09/2015 10:03 AM, Masahiro Yamada wrote:
> 2015-11-09 1:38 GMT+09:00 Tom Rini <trini@konsulko.com>:

>> On Sun, Nov 08, 2015 at 09:56:18AM -0600, menon.nishanth@gmail.com wrote:

>>> On Sat, Nov 7, 2015 at 8:18 PM, Tom Rini <trini@konsulko.com> wrote:

>>>

>>>> Conceptually fine with me.  Do we really need a config option for it

>>>> tho?

>>>

>>> I was following the lines of CONFIG_CREATE_ARCH_SYMLINK here. Is there

>>> some reason why you'd not want it so?

>>

>> Masahiro?

>>

> 

> Is this an improvement?

> 

> 

> I am unhappy because I was hoping

> we could stop creating symbolic links during building

> in a long run.


OK. what do we do?
option 1:
#include "..common/xyz.h"
141 something usage in board files do this for now..

option 2:
-I...common (auto generated - original patch)
#include "xyz.h"
we dont like that since it does not tell us that xyz.h is a
board-common header.

option 3:
#include <board-common/xyz.h>

this needs a soft link which we dont like either.

are there any other ways to do this?

-- 
Regards,
Nishanth Menon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

diff --git a/Kconfig b/Kconfig
index 821b464db207..6e825be0545e 100644
--- a/Kconfig
+++ b/Kconfig
@@ -224,6 +224,8 @@  endmenu		# Boot images
 
 source "common/Kconfig"
 
+source "board/Kconfig"
+
 source "dts/Kconfig"
 
 source "net/Kconfig"
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d756cbc55a2..62c9b90eac79 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -356,6 +356,7 @@  config TARGET_AM335X_EVM
 	select DM
 	select DM_SERIAL
 	select DM_GPIO
+	select CREATE_BOARD_SYMLINK
 
 config TARGET_AM335X_SL50
 	bool "Support am335x_sl50"
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
index d668982cc1dd..3f9b969c2271 100644
--- a/scripts/Makefile.autoconf
+++ b/scripts/Makefile.autoconf
@@ -125,6 +125,23 @@  else
 	ln -fsn $$dest arch/$(ARCH)/include/asm/arch
 endif
 endif
+ifdef CONFIG_CREATE_BOARD_SYMLINK
+ifneq ($(KBUILD_SRC),)
+	if [ -d $(KBUILD_SRC)/board/$(VENDOR)/common/include ]; then \
+		dest=board/$(VENDOR)/common/include;			\
+	else								\
+		dest=board/$(VENDOR)/common;				\
+	fi;								\
+	ln -fsn $(KBUILD_SRC)/$$dest include/board-common
+else
+	if [ -d board/$(VENDOR)/common/include ]; then \
+		dest=board/$(VENDOR)/common/include;			\
+	else								\
+		dest=board/$(VENDOR)/common;				\
+	fi;								\
+	ln -fsn ../$$dest include/board-common
+endif
+endif
 
 PHONY += FORCE
 FORCE: