* [ptxdist] [RFC] Add DTB overlay handling to ptxdist @ 2021-06-02 12:19 Christian Melki 2021-06-03 7:21 ` Michael Tretter 0 siblings, 1 reply; 6+ messages in thread From: Christian Melki @ 2021-06-02 12:19 UTC (permalink / raw) To: ptxdist 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. 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 "-@" + 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist 2021-06-02 12:19 [ptxdist] [RFC] Add DTB overlay handling to ptxdist Christian Melki @ 2021-06-03 7:21 ` Michael Tretter 2021-06-03 10:43 ` Christian Melki 0 siblings, 1 reply; 6+ messages in thread From: Michael Tretter @ 2021-06-03 7:21 UTC (permalink / raw) To: ptxdist 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist 2021-06-03 7:21 ` Michael Tretter @ 2021-06-03 10:43 ` Christian Melki 2021-08-04 7:10 ` Michael Olbrich 0 siblings, 1 reply; 6+ messages in thread From: Christian Melki @ 2021-06-03 10:43 UTC (permalink / raw) To: ptxdist, Michael Tretter 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist 2021-06-03 10:43 ` Christian Melki @ 2021-08-04 7:10 ` Michael Olbrich 2021-08-06 11:42 ` Christian Melki 0 siblings, 1 reply; 6+ messages in thread From: Michael Olbrich @ 2021-08-04 7:10 UTC (permalink / raw) To: Christian Melki; +Cc: ptxdist, Michael Tretter On Thu, Jun 03, 2021 at 12:43:10PM +0200, Christian Melki wrote: > 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. I actually disagree with Michael here. In fact, I basically merged the dtc package into the kernel package. And I think the overlays should be added there as well. The reality is, that devicetrees, cannot be built without the kernel source tree. There are just too many includes. And once you start having multiple kernel packages, a separate dtc package gets really messy. And the dependencies got really complex. So I refactored the helper functions and they are now used by the kernel package. I think that can be extended to allow building overlays as well. Maybe just a different output dir? I'm not sure. Some comments that will still be valid for a new version: > > > 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 ':'. Hmm, the default is the same. Why not share the search path? > > > + > > > +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" 'yourboard' does not make sense to me for overlays. Maybe 'youroverlay'? > > > + 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 What's the use-case for this? The kernel is still in /boot so putting the devicetree elsewhere does not make sense to me. > > > + > > > +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" I'm guessing, this is the path that you need, right? Do we really need an option for this? Michael, I what do you think? > > > + 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. ack. > > > + 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 I don't think this is necessary here. For the dts path, this was just introduced to help migrating from old versions where PTXCONF_KERNEL_ARCH_STRING was used. > > > # ---------------------------------------------------------------------------- > > > # 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)"); Not needed. The directory is implicitly created below. > > > + @$(foreach dtbo, $(DTC_DTBO), \ > > > + $(call install_copy, dtc, 0, 0, 0644, \ > > > + "$(dtbo)", "$(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH)/$(notdir $(dtbo))")$(ptx/nl)) $(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH) includes quotes, so this would result in: ""/lib/firmware"/something.dtbo" Michael > > > +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 > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ ptxdist mailing list ptxdist@pengutronix.de To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist 2021-08-04 7:10 ` Michael Olbrich @ 2021-08-06 11:42 ` Christian Melki 2021-08-09 13:20 ` Michael Tretter 0 siblings, 1 reply; 6+ messages in thread From: Christian Melki @ 2021-08-06 11:42 UTC (permalink / raw) To: ptxdist, Michael Tretter On 8/4/21 9:10 AM, Michael Olbrich wrote: > On Thu, Jun 03, 2021 at 12:43:10PM +0200, Christian Melki wrote: >> 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. > > I actually disagree with Michael here. In fact, I basically merged the dtc > package into the kernel package. And I think the overlays should be added > there as well. > The reality is, that devicetrees, cannot be built without the kernel source > tree. There are just too many includes. And once you start having multiple > kernel packages, a separate dtc package gets really messy. And the > dependencies got really complex. > > So I refactored the helper functions and they are now used by the kernel > package. I think that can be extended to allow building overlays as well. > Maybe just a different output dir? I'm not sure. > > Some comments that will still be valid for a new version: > >>>> 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 ':'. > > Hmm, the default is the same. Why not share the search path? > I was thinking that DTBO:s do not have to come from the kernel tree or the exact default dts directory. For example. Firmware functions can provide overlays placed per FPGA function. You add a function and an overlay. So it's just added flexibility as I regard overlays as very dynamic. >>>> + >>>> +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" > > 'yourboard' does not make sense to me for overlays. Maybe 'youroverlay'? > Absolutely. >>>> + 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 > > What's the use-case for this? The kernel is still in /boot so putting the > devicetree elsewhere does not make sense to me. > Just some added flexibility I guess. Overlays often end up in /lib/firmware for example, but someone might choose another dtb placement together with the overlays somewhere else. >>>> + >>>> +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" > > I'm guessing, this is the path that you need, right? Do we really need an > option for this? > Michael, I what do you think? > Overlays typically end up there yes. Xilinx and a lot of other providers use /lib/firmware. For example, you can autoload a FPGA bitstream with the overlay insertion, so to reset/reload the FPGA depending on the overlays inserted. Anyway. I was just adding flexibility. If a project wants to put FPGA bitstreams and their overlays somewhere else. >>>> + 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. > > ack. > >>>> + 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 > > I don't think this is necessary here. For the dts path, this was just > introduced to help migrating from old versions where > PTXCONF_KERNEL_ARCH_STRING was used. > Ok. >>>> # ---------------------------------------------------------------------------- >>>> # 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)"); > > Not needed. The directory is implicitly created below. > >>>> + @$(foreach dtbo, $(DTC_DTBO), \ >>>> + $(call install_copy, dtc, 0, 0, 0644, \ >>>> + "$(dtbo)", "$(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH)/$(notdir $(dtbo))")$(ptx/nl)) > > $(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH) includes quotes, so this would > result in: > > ""/lib/firmware"/something.dtbo" > Ack. > Michael > >>>> +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 >> > _______________________________________________ ptxdist mailing list ptxdist@pengutronix.de To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptxdist] [RFC] Add DTB overlay handling to ptxdist 2021-08-06 11:42 ` Christian Melki @ 2021-08-09 13:20 ` Michael Tretter 0 siblings, 0 replies; 6+ messages in thread From: Michael Tretter @ 2021-08-09 13:20 UTC (permalink / raw) To: ptxdist, christian.melki On Fri, 06 Aug 2021 13:42:33 +0200, Christian Melki wrote: > On 8/4/21 9:10 AM, Michael Olbrich wrote: > > On Thu, Jun 03, 2021 at 12:43:10PM +0200, Christian Melki wrote: > >> 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. > > > > I actually disagree with Michael here. In fact, I basically merged the dtc > > package into the kernel package. And I think the overlays should be added > > there as well. > > The reality is, that devicetrees, cannot be built without the kernel source > > tree. There are just too many includes. And once you start having multiple > > kernel packages, a separate dtc package gets really messy. And the > > dependencies got really complex. For board device tree that usually include some dtsi from the kernel tree, this is very valid, and building the board device trees with the kernel is correct. However, for overlays the situation is different. All overlays that I have seen so far are pretty self-contained. If they are including dtsi files, these are located in the same directory (or at least in the same repository). Some others use the definitions from the include/dt-bindings directory of the kernel, but that's it. Therefore, while the overlays still need some headers from the kernel source tree, they are much more closely coupled with the thing that the overlay is describing. > > > > So I refactored the helper functions and they are now used by the kernel > > package. I think that can be extended to allow building overlays as well. > > Maybe just a different output dir? I'm not sure. > > > > Some comments that will still be valid for a new version: > > > >>>> 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 ':'. > > > > Hmm, the default is the same. Why not share the search path? > > > > I was thinking that DTBO:s do not have to come from the kernel tree or > the exact default dts directory. For example. Firmware functions can > provide overlays placed per FPGA function. You add a function and an > overlay. Yes. However, if your overlay describes the function, you probably want to bundle the function and overlay together. Thus, you would create a single ptxdist rule file, that downloads and installs both of them. That ties back to what I described above. Maybe we need an actual separate packages for device tree overlays? The package would for example provide the FPGA bitstream and device tree overlay, and the rule file would call some helper function to compile the dts to a dtbo. The package could also specify, against which kernel source tree the overlay should be compiled. Maybe, this is similar to how external kernel modules are build? Maybe, we needs something like 'ptxdist newpackage dt-overlay'? > > So it's just added flexibility as I regard overlays as very dynamic. > > >>>> + > >>>> +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" > > > > 'yourboard' does not make sense to me for overlays. Maybe 'youroverlay'? > > > > Absolutely. > > >>>> + 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 > > > > What's the use-case for this? The kernel is still in /boot so putting the > > devicetree elsewhere does not make sense to me. > > > > Just some added flexibility I guess. Overlays often end up in > /lib/firmware for example, but someone might choose another dtb > placement together with the overlays somewhere else. > > >>>> + > >>>> +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" > > > > I'm guessing, this is the path that you need, right? Do we really need an > > option for this? > > Michael, I what do you think? > > > > Overlays typically end up there yes. Xilinx and a lot of other providers > use /lib/firmware. For example, you can autoload a FPGA bitstream with > the overlay insertion, so to reset/reload the FPGA depending on the > overlays inserted. > Anyway. I was just adding flexibility. If a project wants to put FPGA > bitstreams and their overlays somewhere else. There are various locations where overlays could be installed and are expected to be found. /lib/firmware is one of them, but /boot/overlays (next to the board device tree, see Raspberry Pi) is another option. The actual location depends on the application that loads/applies the overlay. This needs to be configurable, and this probably needs to be configured per dt-overlay package. Michael > > >>>> + 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. > > > > ack. > > > >>>> + 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 > > > > I don't think this is necessary here. For the dts path, this was just > > introduced to help migrating from old versions where > > PTXCONF_KERNEL_ARCH_STRING was used. > > > > Ok. > > >>>> # ---------------------------------------------------------------------------- > >>>> # 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)"); > > > > Not needed. The directory is implicitly created below. > > > >>>> + @$(foreach dtbo, $(DTC_DTBO), \ > >>>> + $(call install_copy, dtc, 0, 0, 0644, \ > >>>> + "$(dtbo)", "$(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH)/$(notdir $(dtbo))")$(ptx/nl)) > > > > $(PTXCONF_DTC_INSTALL_OFTREE_OVERLAY_PATH) includes quotes, so this would > > result in: > > > > ""/lib/firmware"/something.dtbo" > > > > Ack. > > > Michael > > > >>>> +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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-09 13:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-02 12:19 [ptxdist] [RFC] Add DTB overlay handling to ptxdist Christian Melki 2021-06-03 7:21 ` Michael Tretter 2021-06-03 10:43 ` Christian Melki 2021-08-04 7:10 ` Michael Olbrich 2021-08-06 11:42 ` Christian Melki 2021-08-09 13:20 ` Michael Tretter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox