Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

2023-12-27 Thread Ilias Apalodimas
Hi Sumit,

Thanks this looks great


On Fri, 22 Dec 2023 at 08:12, Sumit Garg  wrote:
>
> Allow u-boot to build DTB from a different directory tree such that
> *-u-boot.dtsi files can be included from a common location. Currently
> that location is arch/$(ARCH)/dts/, so statically define that common
> location.
>
> This is needed for platform owners to start building DTB files from
> devicetree-rebasing directory but still being able to include
> *-u-boot.dtsi files.
>
> Signed-off-by: Sumit Garg 
> ---
>  scripts/Makefile.lib | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 27b9437027c..09330421856 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -159,18 +159,20 @@ cpp_flags  = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) 
> $(UBOOTINCLUDE) \
>  ld_flags   = $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F))
>
>  # Try these files in order to find the U-Boot-specific .dtsi include file
> -u_boot_dtsi_options = $(strip $(wildcard $(dir $<)$(basename $(notdir 
> $<))-u-boot.dtsi) \
> -   $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
> -   $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
> -   $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
> -   $(wildcard $(dir $<)u-boot.dtsi))
> +u_boot_dtsi_loc = $(srctree)/arch/$(ARCH)/dts/
> +
> +u_boot_dtsi_options = $(strip $(wildcard $(u_boot_dtsi_loc)$(basename 
> $(notdir $<))-u-boot.dtsi) \
> +   $(wildcard $(u_boot_dtsi_loc)$(subst 
> $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
> +   $(wildcard $(u_boot_dtsi_loc)$(subst 
> $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
> +   $(wildcard $(u_boot_dtsi_loc)$(subst 
> $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
> +   $(wildcard $(u_boot_dtsi_loc)u-boot.dtsi))
>
>  u_boot_dtsi_options_raw = $(warning Automatic .dtsi inclusion: options: \
> -   $(dir $<)$(basename $(notdir $<))-u-boot.dtsi \
> -   $(dir $<)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi \
> -   $(dir $<)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi \
> -   $(dir $<)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi \
> -   $(dir $<)u-boot.dtsi ... \
> +   $(u_boot_dtsi_loc)$(basename $(notdir $<))-u-boot.dtsi \
> +   $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi \
> +   $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi \
> +   $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi \
> +   $(u_boot_dtsi_loc)u-boot.dtsi ... \
> found: $(if $(u_boot_dtsi_options),"$(u_boot_dtsi_options)",nothing!))
>
>  # Uncomment for debugging
> @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES)
>  dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc\
>  $(UBOOTINCLUDE) \
>  -I$(dir $<) \
> +-I$(u_boot_dtsi_loc) \
>  -I$(srctree)/arch/$(ARCH)/dts/include   \
>  -I$(srctree)/include\
>  -D__ASSEMBLY__  \
> @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>   echo '$(pound)include "$(f)"' >> $(pre-tmp);) \
> $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) 
> $(pre-tmp) ; \
> $(DTC) -O dtb -o $@ -b 0 \
> -   -i $(dir $<) $(DTC_FLAGS) \
> +   -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \
> -d $(depfile).dtc.tmp $(dtc-tmp) || \
> (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \
> ; \
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 


Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

2023-12-26 Thread Simon Glass
Hi Sumit,

On Tue, Dec 26, 2023 at 10:09 AM Sumit Garg  wrote:
>
> On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> >
> > Hi Sumit,
> >
> > On Fri, Dec 22, 2023 at 6:12 AM Sumit Garg  wrote:
> > >
> > > Allow u-boot to build DTB from a different directory tree such that
> > > *-u-boot.dtsi files can be included from a common location. Currently
> > > that location is arch/$(ARCH)/dts/, so statically define that common
> > > location.
> > >
> > > This is needed for platform owners to start building DTB files from
> > > devicetree-rebasing directory but still being able to include
> > > *-u-boot.dtsi files.
> > >
> >
> > This is a v2 patch but I don't see a change log?
>
> I usually prefer to keep change logs in the cover letter [1].
>
> [1] 
> https://lore.kernel.org/all/20231222061208.3009970-1-sumit.g...@linaro.org/
>

As a reviewer I much prefer to see the changes with the patch. If you
use patman you can have both, I suppose.

> >
> > > Signed-off-by: Sumit Garg 
> > > ---
> > >  scripts/Makefile.lib | 25 ++---
> > >  1 file changed, 14 insertions(+), 11 deletions(-)
> >
> > Reviewed-by: Simon Glass 
> >

Regards,
Simon


Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

2023-12-26 Thread Sumit Garg
On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
>
> Hi Sumit,
>
> On Fri, Dec 22, 2023 at 6:12 AM Sumit Garg  wrote:
> >
> > Allow u-boot to build DTB from a different directory tree such that
> > *-u-boot.dtsi files can be included from a common location. Currently
> > that location is arch/$(ARCH)/dts/, so statically define that common
> > location.
> >
> > This is needed for platform owners to start building DTB files from
> > devicetree-rebasing directory but still being able to include
> > *-u-boot.dtsi files.
> >
>
> This is a v2 patch but I don't see a change log?

I usually prefer to keep change logs in the cover letter [1].

[1] https://lore.kernel.org/all/20231222061208.3009970-1-sumit.g...@linaro.org/

>
> > Signed-off-by: Sumit Garg 
> > ---
> >  scripts/Makefile.lib | 25 ++---
> >  1 file changed, 14 insertions(+), 11 deletions(-)
>
> Reviewed-by: Simon Glass 
>

Thanks.
-Sumit

> Regards,
> Simon


Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

2023-12-26 Thread Simon Glass
Hi Sumit,

On Fri, Dec 22, 2023 at 6:12 AM Sumit Garg  wrote:
>
> Allow u-boot to build DTB from a different directory tree such that
> *-u-boot.dtsi files can be included from a common location. Currently
> that location is arch/$(ARCH)/dts/, so statically define that common
> location.
>
> This is needed for platform owners to start building DTB files from
> devicetree-rebasing directory but still being able to include
> *-u-boot.dtsi files.
>

This is a v2 patch but I don't see a change log?

> Signed-off-by: Sumit Garg 
> ---
>  scripts/Makefile.lib | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass 

Regards,
Simon


Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

2023-12-22 Thread Tom Rini
On Fri, Dec 22, 2023 at 11:42:03AM +0530, Sumit Garg wrote:

> Allow u-boot to build DTB from a different directory tree such that
> *-u-boot.dtsi files can be included from a common location. Currently
> that location is arch/$(ARCH)/dts/, so statically define that common
> location.
> 
> This is needed for platform owners to start building DTB files from
> devicetree-rebasing directory but still being able to include
> *-u-boot.dtsi files.
> 
> Signed-off-by: Sumit Garg 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature