Makefile: Include vendor common library in include search path

Message ID 1446708504-5936-1-git-send-email-nm@ti.com
State Superseded
Headers show

Commit Message

Nishanth Menon Nov. 5, 2015, 7:28 a.m.
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(-)

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Nishanth Menon Nov. 5, 2015, 7:32 a.m. | #1
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/

> 



-- 
Regards,
Nishanth Menon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Nishanth Menon Nov. 6, 2015, 3:30 a.m. | #2
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?
-- 
---
Regards,
Nishanth Menon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Nishanth Menon Nov. 6, 2015, 5:10 p.m. | #3
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

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

Patch

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/