Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist

[Michael Tretter <m.tretter@pengutronix.de>]

On Wed, 02 Jun 2021 14:19:10 +0200, Christian Melki wrote:
> The only real difference here is that the symbol handling
> must always be present. Hence "-@" for default extra arg.
> Make paths variable but default to boot for the old dtb behavior.
> There is a lot of duplication going on here, but the main purpose
> is to separate dtb from dtbo handling with separate names and functions.

I am not convinced that it is a good idea to mix the handling of dtb and dtbo
in one rule file. Maybe it helps, if there is a separate rule for overlays,
but I didn't think this through.

> 
> Signed-off-by: Christian Melki <christian.melki@t2data.com>
> 
> diff --git a/platforms/dtc.in b/platforms/dtc.in
> index 5e8b35291..101d99836 100644
> --- a/platforms/dtc.in
> +++ b/platforms/dtc.in
> @@ -13,10 +13,18 @@ menuconfig DTC
>  if DTC
>  
>  config DTC_INSTALL_OFTREE
> -	bool "install oftree to /boot"
> +	bool "install oftrees to target path"
>  	help
> -	  Creates a package to install the 'oftree' file to /boot
> -	  of your target system.
> +	  Creates a package to install the 'oftree' files
> +	  to your target system.
> +
> +config DTC_INSTALL_OFTREE_OVERLAY
> +	bool "install oftrees overlays to target path"
> +	help
> +	  Creates a package to install the 'oftree' overlay
> +	  files to your target system.
> +
> +comment "device tree paths   ---"
>  
>  config DTC_OFTREE_DTS_PATH
>  	string "path to source dts file"
> @@ -25,6 +33,15 @@ config DTC_OFTREE_DTS_PATH
>  	  Define path to the dts source file. Multiple directories can be
>  	  specified separated by ':'.
>  
> +config DTC_OFTREE_DTO_PATH
> +	string "path to source dto (overlay) files"
> +	default "${PTXDIST_PLATFORMCONFIG_SUBDIR}/dts:${KERNEL_DIR}/arch/${GENERIC_KERNEL_ARCH}/boot/dts"
> +	help
> +	  Define path to the dto source files. Multiple directories can be
> +	  specified separated by ':'.
> +
> +comment "device tree sources ---"
> +
>  config DTC_OFTREE_DTS
>  	string "source dts file"
>  	default "<yourboard>.dts"
> @@ -34,7 +51,43 @@ config DTC_OFTREE_DTS
>  	  is used as a search path for the device tree files specified
>  	  here. Multiple dts files can be specified, separated by spaces.
>  
> +config DTC_OFTREE_DTO
> +	string "source dto file"
> +	default "<yourboard>.dto"
> +	help
> +	  Select the dts/dto files to use for the device tree binary overlay
> +	  blob generation. For relative file names DTC_OFTREE_DTO_PATH
> +	  is used as a search path for the device tree overlay files specified
> +	  here. Multiple dts overlay files can be specified, separated by spaces.
> +
> +if DTC_INSTALL_OFTREE
> +
> +comment "device tree binary install path  ---"
> +
> +config DTC_INSTALL_OFTREE_PATH
> +        string "oftree installation path"
> +	default "/boot"
> +	help
> +	  oftree target installation path
> +
> +endif
> +
> +if DTC_INSTALL_OFTREE_OVERLAY
> +
> +comment "device tree overlay install path ---"
> +
> +config DTC_INSTALL_OFTREE_OVERLAY_PATH
> +        string "oftree overlay installation path"
> +	default "/lib/firmware"
> +	help
> +	  oftree overlay target installation path
> +
> +endif
> +
>  config DTC_EXTRA_ARGS
>  	string "extra options passed to dtc"
> +	default "-@"

Building with symbols can increase the size of the device tree quite a lot. It
would be better to default to building with symbols only if overlays are
enabled.

Michael

> +	help
> +	  Defaults to -@ for compiling dtb/dtbo for resolving symbols.
>  
>  endif
> diff --git a/rules/dtc.make b/rules/dtc.make
> index 7c281e8f0..45543d3d4 100644
> --- a/rules/dtc.make
> +++ b/rules/dtc.make
> @@ -26,11 +26,18 @@ $(call ptx/error, PTXCONF_KERNEL_ARCH_STRING is no longer defined.)
>  $(call ptx/error, Use GENERIC_KERNEL_ARCH instead)
>  endif
>  
> +ifneq ($(subst PTXCONF_KERNEL_ARCH_STRING,,$(value PTXCONF_DTC_OFTREE_DTO_PATH)),$(value PTXCONF_DTC_OFTREE_DTO_PATH))
> +$(call ptx/error, invalid value for PTXCONF_DTC_OFTREE_DTO_PATH:)
> +$(call ptx/error, PTXCONF_KERNEL_ARCH_STRING is no longer defined.)
> +$(call ptx/error, Use GENERIC_KERNEL_ARCH instead)
> +endif
> +o
>  # ----------------------------------------------------------------------------
>  # Target-Install
>  # ----------------------------------------------------------------------------
>  
>  ptx/dtb = $(notdir $(basename $(strip $(1)))).dtb
> +ptx/dtbo = $(notdir $(basename $(strip $(1)))).dtbo
>  
>  dts/env = \
>  	$(call ptx/env) \
> @@ -40,17 +47,31 @@ dts/env = \
>  	dts_kernel_dir="$(KERNEL_DIR)" \
>  	dts_kernel_arch="$(GENERIC_KERNEL_ARCH)"
>  
> +dto/env = \
> +	$(call ptx/env) \
> +	dts_path=$(PTXCONF_DTC_OFTREE_DTO_PATH) \
> +	dts_dtb="$(strip $(1))" \
> +	dts_dts="$(strip $(2))" \
> +	dts_kernel_dir="$(KERNEL_DIR)" \
> +	dts_kernel_arch="$(GENERIC_KERNEL_ARCH)"
> +
>  %.dtb: $(STATEDIR)/dtc.install
>  	@$(call targetinfo)
>  	@$(call dts/env, $@, $(DTB_DTS)) ptxd_make_dts_dtb
>  	@$(call finish)
>  
> +%.dtbo: $(STATEDIR)/dtc.install
> +	@$(call targetinfo)
> +	@$(call dto/env, $@, $(DTBO_DTO)) ptxd_make_dts_dtb
> +	@$(call finish)
> +
>  DTC_DTB = $(foreach dts, $(call remove_quotes,$(PTXCONF_DTC_OFTREE_DTS)), $(IMAGEDIR)/$(call ptx/dtb, $(dts)))
> +DTC_DTBO = $(foreach dto, $(call remove_quotes,$(PTXCONF_DTC_OFTREE_DTO)), $(IMAGEDIR)/$(call ptx/dtbo, $(dto)))
>  
>  # make sure "ptxdist targetinstall kernel" generates a new device trees
> -$(STATEDIR)/kernel.targetinstall.post: $(DTC_DTB)
> +$(STATEDIR)/kernel.targetinstall.post: $(DTC_DTB) $(DTC_DTBO)
>  
> -$(STATEDIR)/dtc.targetinstall: $(DTC_DTB)
> +$(STATEDIR)/dtc.targetinstall: $(DTC_DTB) $(DTC_DTBO)
>  	@$(call targetinfo)
>  
>  ifdef PTXCONF_DTC_INSTALL_OFTREE
> @@ -61,10 +82,16 @@ ifdef PTXCONF_DTC_INSTALL_OFTREE
>  	@$(call install_fixup, dtc, DESCRIPTION, "oftree description for machine $(PTXCONF_PLATFORM)")
>  
>  	@$(call install_copy, dtc, 0, 0, 0755, /boot);
> +	@$(call install_copy, dtc, 0, 0, 0755, "$(PTXCONF_DTC_INSTALL_OFTREE_PATH)");
>  	@$(foreach dtb, $(DTC_DTB), \
>  		$(call install_copy, dtc, 0, 0, 0644, \
> -		"$(dtb)", "/boot/$(notdir $(dtb))")$(ptx/nl))
> -
> +		"$(dtb)", "$(PTXCONF_DTC_INSTALL_OFTREE_PATH)/$(notdir $(dtb))")$(ptx/nl))
> +ifdef PTXCONF_DTC_INSTALL_OFTREE_OVERLAY
> +	@$(call install_copy, dtc, 0, 0, 0755, "$(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH)");
> +	@$(foreach dtbo, $(DTC_DTBO), \
> +		$(call install_copy, dtc, 0, 0, 0644, \
> +		"$(dtbo)", "$(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH)/$(notdir $(dtbo))")$(ptx/nl))
> +endif
>  	@$(call install_finish, dtc)
>  endif
>  	@$(call touch)
> diff --git a/rules/post/dts.make b/rules/post/dts.make
> index ffa8bf2fc..4bb1856ff 100644
> --- a/rules/post/dts.make
> +++ b/rules/post/dts.make
> @@ -8,8 +8,12 @@
>  
>  #
>  # defined in post/ to make sure PTXCONF_DTC_OFTREE_DTS is fully defined
> +# defined in post/ to make sure PTXCONF_DTC_OFTREE_DTO is fully defined
>  #
>  $(foreach dts, $(call remove_quotes,$(PTXCONF_DTC_OFTREE_DTS)), \
>  	$(eval $(IMAGEDIR)/$(call ptx/dtb, $(dts)): DTB_DTS=$(dts)))
>  
> +$(foreach dto, $(call remove_quotes,$(PTXCONF_DTC_OFTREE_DTO)), \
> +	$(eval $(IMAGEDIR)/$(call ptx/dtbo, $(dto)): DTBO_DTO=$(dto)))
> +
>  # vim: syntax=make
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> 

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de