On Fri, Nov 06, 2015 at 03:58:17PM -0800, Simon Glass wrote:
Hi,
On 6 November 2015 at 10:23, Nishanth Menon nm@ti.com wrote:
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? 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_SYMLINKconfig 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: --
It looks reasonable to me. Tom and Masahiro may have opinions on this too.
Conceptually fine with me. Do we really need a config option for it tho?