Re: [PATCH v3 11/13] Makefile: remove hardcoded device tree source directory

[Neha Malcom Francis <n-francis@ti.com>]

Hi Sumit

On 27/02/24 14:29, Sumit Garg wrote:
> Hi Neha,
> 
> On Tue, 27 Feb 2024 at 13:53, Neha Malcom Francis <n-francis@ti.com> wrote:
>>
>> Hi Bryan
>>
>> On 27/02/24 02:49, Bryan Brattlof wrote:
>>> Some boards that choose to utilize the OF_UPSTREAM directory for their
>>> device tree files will need to specify that directory instead of the
>>> traditional arch/$(ARCH)/dts/* path.
>>>
>>> Include the correct path to the board's dtbs depending on if OF_UPSTREAM
>>> is selected or not.
>>>
>>> Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
>>> Signed-off-by: Bryan Brattlof <bb@ti.com>
>>> ---
>>>    Makefile             | 18 ++++++++++++++----
>>>    scripts/Makefile.spl | 17 +++++++++++++----
>>>    2 files changed, 27 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 0f0c7f30d2717..51b57d26857f1 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1184,6 +1184,16 @@ dt_binding_check: scripts_dtc
>>>    quiet_cmd_copy = COPY    $@
>>>          cmd_copy = cp $< $@
>>>
>>> +ifeq ($(CONFIG_OF_UPSTREAM),y)
>>> +ifeq ($(CONFIG_ARM64),y)
>>
>> This would fail in case of R5 builds, is there any reason why you choose not to
>> use OF_UPSTREAM for R5 builds?
> 
> Although I will let Bryan elaborate more here, the essence here is
> that OF_UPSTREAM=y means that the target DTS files come from
> dts/upstream/src/<arch>/.
> 
>>
>> I understand that R5 builds will require U-Boot DTS (from U-Boot code base) that
>> (ideally) should inherit the kernel core DTS (from upstream), is this support
>> not there at present? Let me know if I understand incorrectly.
>>
> 
> However, with OF_UPSTREAM=n you can have U-Boot DTS placed in
> arch/$(ARCH)/dts/ like the R5 case but you can include whatever common
> pieces required from dts/upstream/src/<arch>/.
> 

Ah understood. That makes sense, thanks!

> -Sumit
> 
>>> +dt_dir := dts/upstream/src/arm64
>>> +else
>>> +dt_dir := dts/upstream/src/$(ARCH)
>>> +endif
>>> +else
>>> +dt_dir := arch/$(ARCH)/dts
>>> +endif
>>> +
>>>    ifeq ($(CONFIG_MULTI_DTB_FIT),y)
>>>
>>>    ifeq ($(CONFIG_MULTI_DTB_FIT_LZO),y)
>>> @@ -1209,7 +1219,7 @@ endif
>>>
>>>    MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
>>>        -a 0 -e 0 -E \
>>> -     $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null
>>> +     $(patsubst %,-b $(dt_dir)/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null
>>>
>>>    MKIMAGEFLAGS_fit-dtb.blob += -B 0x8
>>>
>>> @@ -1407,9 +1417,9 @@ MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
>>>        -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
>>>        -p $(CONFIG_FIT_EXTERNAL_OFFSET) \
>>>        -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
>>> -     $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(DEVICE_TREE))) \
>>> -     $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) \
>>> -     $(patsubst %,-b arch/$(ARCH)/dts/%.dtbo,$(subst ",,$(CONFIG_OF_OVERLAY_LIST)))
>>> +     $(patsubst %,-b $(dt_dir)/%.dtb,$(subst ",,$(DEVICE_TREE))) \
>>> +     $(patsubst %,-b $(dt_dir)/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) \
>>> +     $(patsubst %,-b $(dt_dir)/%.dtbo,$(subst ",,$(CONFIG_OF_OVERLAY_LIST)))
>>>    else
>>>    MKIMAGEFLAGS_u-boot.img = -A $(ARCH) -T firmware -C none -O u-boot \
>>>        -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
>>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
>>> index 407fc52376a50..d074ba2350065 100644
>>> --- a/scripts/Makefile.spl
>>> +++ b/scripts/Makefile.spl
>>> @@ -559,9 +559,15 @@ FORCE:
>>>    $(obj)/dts/dt-$(SPL_NAME).dtb: dts/dt.dtb
>>>        $(Q)$(MAKE) $(build)=$(obj)/dts spl_dtbs
>>>
>>> -PHONY += dts_dir
>>> -dts_dir:
>>> -     $(shell [ -d $(obj)/dts ] || mkdir -p $(obj)/dts)
>>> +ifeq ($(CONFIG_OF_UPSTREAM),y)
>>> +ifeq ($(CONFIG_ARM64),y)
>>> +dt_dir := dts/upstream/src/arm64
>>> +else
>>> +dt_dir := dts/upstream/src/$(ARCH)
>>> +endif
>>> +else
>>> +dt_dir := arch/$(ARCH)/dts
>>> +endif
>>>
>>>    # Declare the contents of the .PHONY variable as phony.  We keep that
>>>    # information in a variable so we can use it in if_changed and friends.
>>> @@ -569,8 +575,11 @@ dts_dir:
>>>
>>>    SPL_OF_LIST_TARGETS = $(patsubst %,dts/%.dtb,$(subst ",,$(CONFIG_SPL_OF_LIST)))
>>>    SHRUNK_ARCH_DTB = $(addprefix $(obj)/,$(SPL_OF_LIST_TARGETS))
>>> +$(dir $(SHRUNK_ARCH_DTB)):
>>> +     $(shell [ -d $@ ] || mkdir -p $@)
>>> +
>>>    .SECONDEXPANSION:
>>> -$(SHRUNK_ARCH_DTB): $$(patsubst $(obj)/dts/%, arch/$(ARCH)/dts/%, $$@) dts_dir
>>> +$(SHRUNK_ARCH_DTB): $$(patsubst $(obj)/dts/%, $(dt_dir)/%, $$@) $(dir $(SHRUNK_ARCH_DTB))
>>>        $(call if_changed,fdtgrep)
>>>
>>>    targets += $(SPL_OF_LIST_TARGETS)
>>>
>>
>> --
>> Thanking You
>> Neha Malcom Francis

-- 
Thanking You
Neha Malcom Francis