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

[Christian Melki <christian.melki@t2data.com>]

On 6/3/21 9:21 AM, Michael Tretter wrote:
> 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.
> 

Splitting them works for me. I have no opinion here.
I thought that this was an easy way to start a seed for refinement.

>>
>> 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
> 

You're right. Also I think older dtcs might have a problem with 
defaulting to symbol generation.

>> +	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
> 

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