mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
* [ptxdist] [PATCH 0/2] Add support for device tree overlays
@ 2021-12-07 15:39 Michael Riesch
  2021-12-07 15:39 ` [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add " Michael Riesch
  2021-12-07 15:39 ` [ptxdist] [PATCH 2/2] kernel: activate " Michael Riesch
  0 siblings, 2 replies; 12+ messages in thread
From: Michael Riesch @ 2021-12-07 15:39 UTC (permalink / raw)
  To: ptxdist; +Cc: christian.melki, m.tretter, Michael Riesch

Hi all,

This series introduces support for device tree overlays in ptxdist.
The first patch makes the dtb compilation routine more general and
adds suitable helpers for the overlay generation, which can be used
by any package.
The second patch activates the support for overlays for the kernel
package.

This series was inspired by Christian Melki's RFC in June 2021 [0]
but implements a slightly more general approach as *every* package
can generate device tree overlays here. This is envisaged to come in
handy for e.g., firmware or FPGA bitstream packages that alter the
hardware description. I had some discussions with Michael Tretter
about this flexible approach that I gratefully acknowledge at this
point.

Looking forward to your comments!

Best regards,
Michael

[0] https://lore.ptxdist.org/ptxdist/20210602121910.2527-1-christian.melki@t2data.com/

Michael Riesch (2):
  ptxd_make_world_{dtb,dtbo}: add support for device tree overlays
  kernel: activate support for device tree overlays

 platforms/kernel.in                  | 28 +++++++++-
 rules/kernel.make                    |  9 +++
 rules/post/ptxd_make_world_dtbo.make | 21 +++++++
 scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
 4 files changed, 116 insertions(+), 24 deletions(-)
 create mode 100644 rules/post/ptxd_make_world_dtbo.make

-- 
2.30.2


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-07 15:39 [ptxdist] [PATCH 0/2] Add support for device tree overlays Michael Riesch
@ 2021-12-07 15:39 ` Michael Riesch
  2021-12-07 17:30   ` Christian Melki
  2021-12-07 15:39 ` [ptxdist] [PATCH 2/2] kernel: activate " Michael Riesch
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Riesch @ 2021-12-07 15:39 UTC (permalink / raw)
  To: ptxdist; +Cc: christian.melki, m.tretter, Michael Riesch

Make ptxd_make_dtb more general and provide suitable wrappers to
support the generation of device tree overlays.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 rules/post/ptxd_make_world_dtbo.make | 21 +++++++
 scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
 2 files changed, 80 insertions(+), 23 deletions(-)
 create mode 100644 rules/post/ptxd_make_world_dtbo.make

diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
new file mode 100644
index 000000000..61babc653
--- /dev/null
+++ b/rules/post/ptxd_make_world_dtbo.make
@@ -0,0 +1,21 @@
+# -*-makefile-*-
+#
+# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
+#
+# For further information about the PTXdist project and license conditions
+# see the README file.
+#
+
+world/dtbo/env = \
+	$(call world/env, $(1)) \
+	pkg_dtso_path="$($(1)_DTSO_PATH)" \
+	pkg_dtso="$($(1)_DTSO)" \
+	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
+	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
+	pkg_arch="$(GENERIC_KERNEL_ARCH)"
+
+world/dtbo = \
+	$(call world/dtbo/env,$(strip $(1))) \
+	ptxd_make_world_dtbo
+
+# vim: syntax=make
diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
index f5e796b9d..c1ded5ffb 100644
--- a/scripts/lib/ptxd_make_world_dtb.sh
+++ b/scripts/lib/ptxd_make_world_dtb.sh
@@ -9,18 +9,10 @@
 ptxd_make_dtb() {
     local dtc dts tmp_dts dtb deps tmp_deps
 
-    if [[ "${dts_dts}" =~ ^/.* ]]; then
-	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
-	    "Use <PKG>_DTS_PATH to specify the search path."
-    fi
-
-    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
-	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
-    fi
-    dts="${ptxd_reply}"
-    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
+    dts="${dtb_source}"
+    dtb="${dtb_target}"
 
-    dtc="${pkg_build_dir}/scripts/dtc/dtc"
+    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
     if [ ! -x "${dtc}" ]; then
 	dtc=dtc
     fi
@@ -39,16 +31,16 @@ ptxd_make_dtb() {
 	-Wp,-MT,${dtb_deps_target} \
 	-nostdinc \
 	-I"$(dirname "${dts}")" \
-	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
-	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
-	-I${pkg_dir}/scripts/dtc/include-prefixes \
-	-I${pkg_dir}/drivers/of/testcase-data \
-	-I${pkg_dir}/include \
+	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
+	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
+	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
+	-I${dtb_kernel_dir}/drivers/of/testcase-data \
+	-I${dtb_kernel_dir}/include \
 	-undef -D__DTS__ -x assembler-with-cpp \
 	-o ${tmp_dts} \
 	${dts} &&
 
-    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
+    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
 	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
     # empty line in case all dependencies were removed
     echo >> "${deps}" &&
@@ -57,9 +49,9 @@ ptxd_make_dtb() {
     echo "DTC $(ptxd_print_path "${dtb}")" &&
     ptxd_eval \
 	"${dtc}" \
-	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
+	"${dtb_extra_args}" \
 	-i "$(dirname "${dts}")" \
-	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
+	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
 	-d "${tmp_deps}" \
 	-I dts -O dtb -b 0 \
 	-o "${dtb}" "${tmp_dts}" &&
@@ -81,17 +73,61 @@ ptxd_make_dtb() {
 export -f ptxd_make_dtb
 
 
+ptxd_make_world_dtbo() {
+    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
+
+    ptxd_make_world_init || break
+
+    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
+    dtb_extra_args="-@"
+    dtb_kernel_dir="${pkg_kernel_src}"
+    if [ -z $dtb_kernel_dir ]; then
+	dtb_kernel_dir="${pkg_dir}"
+    fi
+
+    echo -e "\nBuilding device tree overlays..."
+
+    for overlay in ${pkg_dtso}; do
+	if [[ "${overlay}" =~ ^/.* ]]; then
+	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
+			 "Use <PKG>_DTSO_PATH to specify the search path."
+	fi
+
+	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
+	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
+	fi
+	dtb_source="${ptxd_reply}"
+	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
+
+	ptxd_make_dtb || break
+    done
+}
+export -f ptxd_make_world_dtbo
+
+
 ptxd_make_world_dtb() {
-    local dtb_deps_target dtb_dir
+    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
 
     ptxd_make_world_init || break
 
     dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
-    dtb_dir="${pkg_pkg_dir}/boot"
+    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
+    dtb_kernel_dir="${pkg_dir}"
+
+    echo -e "\nBuilding device trees..."
+
+    for tree in ${pkg_dts}; do
+	if [[ "${tree}" =~ ^/.* ]]; then
+	    ptxd_bailout "'${tree}' must not be an absolute path!" \
+			 "Use <PKG>_DTS_PATH to specify the search path."
+	fi
 
-    echo -e "\nBuilding Device trees..."
+	if ! ptxd_in_path pkg_dts_path "${tree}"; then
+	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
+	fi
+	dtb_source="${ptxd_reply}"
+	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
 
-    for dts_dts in ${pkg_dts}; do
 	ptxd_make_dtb || break
     done
 }
-- 
2.30.2


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ptxdist] [PATCH 2/2] kernel: activate support for device tree overlays
  2021-12-07 15:39 [ptxdist] [PATCH 0/2] Add support for device tree overlays Michael Riesch
  2021-12-07 15:39 ` [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add " Michael Riesch
@ 2021-12-07 15:39 ` Michael Riesch
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Riesch @ 2021-12-07 15:39 UTC (permalink / raw)
  To: ptxdist; +Cc: christian.melki, m.tretter, Michael Riesch

Activate the recently introduced support for device tree overlays
for the kernel package.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 platforms/kernel.in | 28 +++++++++++++++++++++++++++-
 rules/kernel.make   |  9 +++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/platforms/kernel.in b/platforms/kernel.in
index 9976436ce..2c332e2cb 100644
--- a/platforms/kernel.in
+++ b/platforms/kernel.in
@@ -186,7 +186,7 @@ config KERNEL_DTS_PATH
 	help
 	  Define path to the dts source file. Multiple directories can be
 	  specified separated by ':'. A relative path will be expanded relative
-	  to the workspace and all other layers. Only on of the specified paths
+	  to the workspace and all other layers. Only one of the specified paths
 	  can be a relative path.
 
 config KERNEL_DTS
@@ -200,6 +200,32 @@ config KERNEL_DTS
 
 endif
 
+menuconfig KERNEL_DTBO
+	bool
+	prompt "Build device tree overlays    "
+
+if KERNEL_DTBO
+
+config KERNEL_DTSO_PATH
+	string "path to overlay file(s)"
+	default "${PTXDIST_PLATFORMCONFIG_SUBDIR}/dts/overlays:${KERNEL_DIR}/arch/${GENERIC_KERNEL_ARCH}/boot/dts"
+	help
+	  Define path to the dts(o) source file(s). Multiple directories can be
+	  specified separated by ':'. A relative path will be expanded relative
+	  to the workspace and all other layers. Only one of the specified paths
+	  can be a relative path.
+
+config KERNEL_DTSO
+	string "overlay file(s)"
+	default "<youroverlay>.dtso"
+	help
+	  Select the dts(o) file(s) to be compiled. For relative file
+	  names KERNEL_DTSO_PATH is used as a search path for the device
+	  tree files specified here.
+	  Multiple dts(o) files can be specified, separated by spaces.
+
+endif
+
 config KERNEL_CODE_SIGNING
 	prompt "use code signing infrastructure"
 	select KERNEL_OPENSSL
diff --git a/rules/kernel.make b/rules/kernel.make
index 0ecf5f4e9..1b70faf21 100644
--- a/rules/kernel.make
+++ b/rules/kernel.make
@@ -29,7 +29,11 @@ KERNEL_BUILD_DIR	:= $(KERNEL_DIR)-build
 KERNEL_CONFIG		:= $(call ptx/in-platformconfigdir, $(call remove_quotes, $(PTXCONF_KERNEL_CONFIG)))
 KERNEL_DTS_PATH		:= $(call remove_quotes,$(PTXCONF_KERNEL_DTS_PATH))
 KERNEL_DTS		:= $(call remove_quotes,$(PTXCONF_KERNEL_DTS))
+KERNEL_DTSO_PATH	:= $(call remove_quotes,$(PTXCONF_KERNEL_DTSO_PATH))
+KERNEL_DTSO		:= $(call remove_quotes,$(PTXCONF_KERNEL_DTSO))
 KERNEL_DTB_FILES	:= $(addsuffix .dtb,$(basename $(KERNEL_DTS)))
+KERNEL_DTBO_FILES	:= $(addsuffix .dtbo,$(basename $(KERNEL_DTSO)))
+KERNEL_DTBO_DIR		:= /boot/overlays
 KERNEL_LICENSE		:= GPL-2.0-only
 KERNEL_SOURCE		:= $(SRCDIR)/$(KERNEL).$(KERNEL_SUFFIX)
 KERNEL_DEVPKG		:= NO
@@ -265,6 +269,7 @@ ifdef PTXCONF_KERNEL_MODULES_INSTALL
 	@$(call world/install, KERNEL)
 endif
 	@$(call world/dtb, KERNEL)
+	@$(call world/dtbo, KERNEL)
 	@$(call touch)
 
 # ----------------------------------------------------------------------------
@@ -294,6 +299,10 @@ ifdef PTXCONF_KERNEL_INSTALL
 	@$(foreach dtb, $(KERNEL_DTB_FILES), \
 		$(call install_copy, kernel, 0, 0, 0644, -, \
 			/boot/$(dtb), n)$(ptx/nl))
+
+	@$(foreach dtbo, $(KERNEL_DTBO_FILES), \
+		$(call install_copy, kernel, 0, 0, 0644, -, \
+			$(KERNEL_DTBO_DIR)/$(dtbo), n)$(ptx/nl))
 endif
 
 # install the ELF kernel image for debugging purpose
-- 
2.30.2


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-07 15:39 ` [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add " Michael Riesch
@ 2021-12-07 17:30   ` Christian Melki
  2021-12-08  8:31     ` Michael Olbrich
  2021-12-09  6:26     ` Michael Riesch
  0 siblings, 2 replies; 12+ messages in thread
From: Christian Melki @ 2021-12-07 17:30 UTC (permalink / raw)
  To: Michael Riesch, ptxdist; +Cc: m.tretter

Hi!

I like this take on dtbos!

On 12/7/21 16:39, Michael Riesch wrote:
> Make ptxd_make_dtb more general and provide suitable wrappers to
> support the generation of device tree overlays.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
>  2 files changed, 80 insertions(+), 23 deletions(-)
>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
> 
> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
> new file mode 100644
> index 000000000..61babc653
> --- /dev/null
> +++ b/rules/post/ptxd_make_world_dtbo.make
> @@ -0,0 +1,21 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +world/dtbo/env = \
> +	$(call world/env, $(1)) \
> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
> +	pkg_dtso="$($(1)_DTSO)" \
> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
> +
> +world/dtbo = \
> +	$(call world/dtbo/env,$(strip $(1))) \
> +	ptxd_make_world_dtbo
> +
> +# vim: syntax=make
> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
> index f5e796b9d..c1ded5ffb 100644
> --- a/scripts/lib/ptxd_make_world_dtb.sh
> +++ b/scripts/lib/ptxd_make_world_dtb.sh
> @@ -9,18 +9,10 @@
>  ptxd_make_dtb() {
>      local dtc dts tmp_dts dtb deps tmp_deps
>  
> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
> -	    "Use <PKG>_DTS_PATH to specify the search path."
> -    fi
> -
> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
> -    fi
> -    dts="${ptxd_reply}"
> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
> +    dts="${dtb_source}"
> +    dtb="${dtb_target}"
>  
> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
>      if [ ! -x "${dtc}" ]; then
>  	dtc=dtc
>      fi
> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
>  	-Wp,-MT,${dtb_deps_target} \
>  	-nostdinc \
>  	-I"$(dirname "${dts}")" \
> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
> -	-I${pkg_dir}/drivers/of/testcase-data \
> -	-I${pkg_dir}/include \
> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
> +	-I${dtb_kernel_dir}/include \
>  	-undef -D__DTS__ -x assembler-with-cpp \
>  	-o ${tmp_dts} \
>  	${dts} &&
>  
> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
>      # empty line in case all dependencies were removed
>      echo >> "${deps}" &&
> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
>      echo "DTC $(ptxd_print_path "${dtb}")" &&
>      ptxd_eval \
>  	"${dtc}" \
> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
> +	"${dtb_extra_args}" \
>  	-i "$(dirname "${dts}")" \
> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
>  	-d "${tmp_deps}" \
>  	-I dts -O dtb -b 0 \
>  	-o "${dtb}" "${tmp_dts}" &&
> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
>  export -f ptxd_make_dtb
>  
>  
> +ptxd_make_world_dtbo() {
> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> +
> +    ptxd_make_world_init || break
> +
> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> +    dtb_extra_args="-@"

Looks ok. Do we need to do a dtc version check?
1.4.4+ I think. Maybe a bit excessive?

> +    dtb_kernel_dir="${pkg_kernel_src}"
> +    if [ -z $dtb_kernel_dir ]; then
> +	dtb_kernel_dir="${pkg_dir}"
> +    fi
> +
> +    echo -e "\nBuilding device tree overlays..."
> +
> +    for overlay in ${pkg_dtso}; do
> +	if [[ "${overlay}" =~ ^/.* ]]; then
> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
> +			 "Use <PKG>_DTSO_PATH to specify the search path."
> +	fi
> +
> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
> +	fi
> +	dtb_source="${ptxd_reply}"
> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
> +
> +	ptxd_make_dtb || break
> +    done
> +}
> +export -f ptxd_make_world_dtbo
> +
> +
>  ptxd_make_world_dtb() {
> -    local dtb_deps_target dtb_dir
> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>  
>      ptxd_make_world_init || break
>  
>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> -    dtb_dir="${pkg_pkg_dir}/boot"
> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"

Not sure I follow here. Must not the dtb itself be built with -@ for
symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?

> +    dtb_kernel_dir="${pkg_dir}"
> +
> +    echo -e "\nBuilding device trees..."
> +
> +    for tree in ${pkg_dts}; do
> +	if [[ "${tree}" =~ ^/.* ]]; then
> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
> +			 "Use <PKG>_DTS_PATH to specify the search path."
> +	fi
>  
> -    echo -e "\nBuilding Device trees..."
> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
> +	fi
> +	dtb_source="${ptxd_reply}"
> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"

Static output dir path?

>  
> -    for dts_dts in ${pkg_dts}; do
>  	ptxd_make_dtb || break
>      done
>  }
> 


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-07 17:30   ` Christian Melki
@ 2021-12-08  8:31     ` Michael Olbrich
  2021-12-09  7:20       ` Michael Riesch
  2021-12-09  6:26     ` Michael Riesch
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Olbrich @ 2021-12-08  8:31 UTC (permalink / raw)
  To: Christian Melki; +Cc: ptxdist, Michael Riesch, m.tretter

On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
> Hi!
> 
> I like this take on dtbos!
> 
> On 12/7/21 16:39, Michael Riesch wrote:
> > Make ptxd_make_dtb more general and provide suitable wrappers to
> > support the generation of device tree overlays.
> > 
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> > ---
> >  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
> >  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
> >  2 files changed, 80 insertions(+), 23 deletions(-)
> >  create mode 100644 rules/post/ptxd_make_world_dtbo.make
> > 
> > diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
> > new file mode 100644
> > index 000000000..61babc653
> > --- /dev/null
> > +++ b/rules/post/ptxd_make_world_dtbo.make
> > @@ -0,0 +1,21 @@
> > +# -*-makefile-*-
> > +#
> > +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
> > +#
> > +# For further information about the PTXdist project and license conditions
> > +# see the README file.
> > +#
> > +
> > +world/dtbo/env = \
> > +	$(call world/env, $(1)) \
> > +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
> > +	pkg_dtso="$($(1)_DTSO)" \
> > +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
> > +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \

Right, reaching into the source tree of another package is always ugly.
Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
set for the generic kernel package (out-of-tree modules need it as well)
but not for the kernel package template.

> > +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
> > +
> > +world/dtbo = \
> > +	$(call world/dtbo/env,$(strip $(1))) \
> > +	ptxd_make_world_dtbo
> > +
> > +# vim: syntax=make
> > diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
> > index f5e796b9d..c1ded5ffb 100644
> > --- a/scripts/lib/ptxd_make_world_dtb.sh
> > +++ b/scripts/lib/ptxd_make_world_dtb.sh
> > @@ -9,18 +9,10 @@
> >  ptxd_make_dtb() {
> >      local dtc dts tmp_dts dtb deps tmp_deps
> >  
> > -    if [[ "${dts_dts}" =~ ^/.* ]]; then
> > -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
> > -	    "Use <PKG>_DTS_PATH to specify the search path."
> > -    fi
> > -
> > -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
> > -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
> > -    fi
> > -    dts="${ptxd_reply}"
> > -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
> > +    dts="${dtb_source}"
> > +    dtb="${dtb_target}"
> >  
> > -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
> > +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
> >      if [ ! -x "${dtc}" ]; then
> >  	dtc=dtc
> >      fi
> > @@ -39,16 +31,16 @@ ptxd_make_dtb() {
> >  	-Wp,-MT,${dtb_deps_target} \
> >  	-nostdinc \
> >  	-I"$(dirname "${dts}")" \
> > -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
> > -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
> > -	-I${pkg_dir}/scripts/dtc/include-prefixes \
> > -	-I${pkg_dir}/drivers/of/testcase-data \
> > -	-I${pkg_dir}/include \
> > +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
> > +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
> > +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
> > +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
> > +	-I${dtb_kernel_dir}/include \
> >  	-undef -D__DTS__ -x assembler-with-cpp \
> >  	-o ${tmp_dts} \
> >  	${dts} &&
> >  
> > -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
> > +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
> >  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
> >      # empty line in case all dependencies were removed
> >      echo >> "${deps}" &&
> > @@ -57,9 +49,9 @@ ptxd_make_dtb() {
> >      echo "DTC $(ptxd_print_path "${dtb}")" &&
> >      ptxd_eval \
> >  	"${dtc}" \
> > -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
> > +	"${dtb_extra_args}" \
> >  	-i "$(dirname "${dts}")" \
> > -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
> > +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
> >  	-d "${tmp_deps}" \
> >  	-I dts -O dtb -b 0 \
> >  	-o "${dtb}" "${tmp_dts}" &&
> > @@ -81,17 +73,61 @@ ptxd_make_dtb() {
> >  export -f ptxd_make_dtb
> >  
> >  
> > +ptxd_make_world_dtbo() {
> > +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> > +
> > +    ptxd_make_world_init || break
> > +
> > +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> > +    dtb_extra_args="-@"
> 
> Looks ok. Do we need to do a dtc version check?
> 1.4.4+ I think. Maybe a bit excessive?

We cannot build dtbos if that's not supported, so a check won't really
help.

> > +    dtb_kernel_dir="${pkg_kernel_src}"
> > +    if [ -z $dtb_kernel_dir ]; then
> > +	dtb_kernel_dir="${pkg_dir}"
> > +    fi

    dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"

should work. And I think this can be moved to the shared code above. It
works just fine because pkg_kernel_src is never set for the dts case (but
we might want to allow it in the future).

> > +
> > +    echo -e "\nBuilding device tree overlays..."
> > +
> > +    for overlay in ${pkg_dtso}; do
> > +	if [[ "${overlay}" =~ ^/.* ]]; then
> > +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
> > +			 "Use <PKG>_DTSO_PATH to specify the search path."
> > +	fi
> > +
> > +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
> > +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
> > +	fi
> > +	dtb_source="${ptxd_reply}"
> > +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
> > +
> > +	ptxd_make_dtb || break
> > +    done
> > +}
> > +export -f ptxd_make_world_dtbo
> > +
> > +
> >  ptxd_make_world_dtb() {
> > -    local dtb_deps_target dtb_dir
> > +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >  
> >      ptxd_make_world_init || break
> >  
> >      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> > -    dtb_dir="${pkg_pkg_dir}/boot"
> > +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
> 
> Not sure I follow here. Must not the dtb itself be built with -@ for
> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?

I don't know much about dtbos. If -@ is required for the dtb to make dtbos
work, then this should be propagated automatically. Add something to
world/dtb/env, maybe with a noprompt kernel options
KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
needed.

> > +    dtb_kernel_dir="${pkg_dir}"
> > +
> > +    echo -e "\nBuilding device trees..."
> > +
> > +    for tree in ${pkg_dts}; do

Please keep the dts_dts variable name.

Michael

> > +	if [[ "${tree}" =~ ^/.* ]]; then
> > +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
> > +			 "Use <PKG>_DTS_PATH to specify the search path."
> > +	fi
> >  
> > -    echo -e "\nBuilding Device trees..."
> > +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
> > +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
> > +	fi
> > +	dtb_source="${ptxd_reply}"
> > +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
> 
> Static output dir path?
> 
> >  
> > -    for dts_dts in ${pkg_dts}; do
> >  	ptxd_make_dtb || break
> >      done
> >  }
> > 
> 
> 
> _______________________________________________
> 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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-07 17:30   ` Christian Melki
  2021-12-08  8:31     ` Michael Olbrich
@ 2021-12-09  6:26     ` Michael Riesch
  2021-12-09  7:08       ` Michael Olbrich
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Riesch @ 2021-12-09  6:26 UTC (permalink / raw)
  To: christian.melki, ptxdist; +Cc: m.tretter

Hi Christian,

On 12/7/21 6:30 PM, Christian Melki wrote:
> Hi!
> 
> I like this take on dtbos!

Thanks for your review!

> On 12/7/21 16:39, Michael Riesch wrote:
>> Make ptxd_make_dtb more general and provide suitable wrappers to
>> support the generation of device tree overlays.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>> ---
>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
>>  2 files changed, 80 insertions(+), 23 deletions(-)
>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
>>
>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
>> new file mode 100644
>> index 000000000..61babc653
>> --- /dev/null
>> +++ b/rules/post/ptxd_make_world_dtbo.make
>> @@ -0,0 +1,21 @@
>> +# -*-makefile-*-
>> +#
>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
>> +#
>> +# For further information about the PTXdist project and license conditions
>> +# see the README file.
>> +#
>> +
>> +world/dtbo/env = \
>> +	$(call world/env, $(1)) \
>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
>> +	pkg_dtso="$($(1)_DTSO)" \
>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
>> +
>> +world/dtbo = \
>> +	$(call world/dtbo/env,$(strip $(1))) \
>> +	ptxd_make_world_dtbo
>> +
>> +# vim: syntax=make
>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
>> index f5e796b9d..c1ded5ffb 100644
>> --- a/scripts/lib/ptxd_make_world_dtb.sh
>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
>> @@ -9,18 +9,10 @@
>>  ptxd_make_dtb() {
>>      local dtc dts tmp_dts dtb deps tmp_deps
>>  
>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
>> -	    "Use <PKG>_DTS_PATH to specify the search path."
>> -    fi
>> -
>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
>> -    fi
>> -    dts="${ptxd_reply}"
>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
>> +    dts="${dtb_source}"
>> +    dtb="${dtb_target}"
>>  
>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
>>      if [ ! -x "${dtc}" ]; then
>>  	dtc=dtc
>>      fi
>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
>>  	-Wp,-MT,${dtb_deps_target} \
>>  	-nostdinc \
>>  	-I"$(dirname "${dts}")" \
>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
>> -	-I${pkg_dir}/drivers/of/testcase-data \
>> -	-I${pkg_dir}/include \
>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
>> +	-I${dtb_kernel_dir}/include \
>>  	-undef -D__DTS__ -x assembler-with-cpp \
>>  	-o ${tmp_dts} \
>>  	${dts} &&
>>  
>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
>>      # empty line in case all dependencies were removed
>>      echo >> "${deps}" &&
>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
>>      ptxd_eval \
>>  	"${dtc}" \
>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
>> +	"${dtb_extra_args}" \
>>  	-i "$(dirname "${dts}")" \
>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
>>  	-d "${tmp_deps}" \
>>  	-I dts -O dtb -b 0 \
>>  	-o "${dtb}" "${tmp_dts}" &&
>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
>>  export -f ptxd_make_dtb
>>  
>>  
>> +ptxd_make_world_dtbo() {
>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>> +
>> +    ptxd_make_world_init || break
>> +
>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>> +    dtb_extra_args="-@"
> 
> Looks ok. Do we need to do a dtc version check?
> 1.4.4+ I think. Maybe a bit excessive?

As pointed out by Michael Olbrich this won't help.

> 
>> +    dtb_kernel_dir="${pkg_kernel_src}"
>> +    if [ -z $dtb_kernel_dir ]; then
>> +	dtb_kernel_dir="${pkg_dir}"
>> +    fi
>> +
>> +    echo -e "\nBuilding device tree overlays..."
>> +
>> +    for overlay in ${pkg_dtso}; do
>> +	if [[ "${overlay}" =~ ^/.* ]]; then
>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
>> +	fi
>> +
>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
>> +	fi
>> +	dtb_source="${ptxd_reply}"
>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
>> +
>> +	ptxd_make_dtb || break
>> +    done
>> +}
>> +export -f ptxd_make_world_dtbo
>> +
>> +
>>  ptxd_make_world_dtb() {
>> -    local dtb_deps_target dtb_dir
>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>  
>>      ptxd_make_world_init || break
>>  
>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>> -    dtb_dir="${pkg_pkg_dir}/boot"
>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
> 
> Not sure I follow here. Must not the dtb itself be built with -@ for
> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?

You are right, thanks for pointing that out. Will fix this in v2.

> 
>> +    dtb_kernel_dir="${pkg_dir}"
>> +
>> +    echo -e "\nBuilding device trees..."
>> +
>> +    for tree in ${pkg_dts}; do
>> +	if [[ "${tree}" =~ ^/.* ]]; then
>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
>> +			 "Use <PKG>_DTS_PATH to specify the search path."
>> +	fi
>>  
>> -    echo -e "\nBuilding Device trees..."
>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
>> +	fi
>> +	dtb_source="${ptxd_reply}"
>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
> 
> Static output dir path?

Not sure whether I understand your comment correctly, but the path has
been copied from the existing script. It can be discussed whether it
makes sense to install the compiled device trees to an alternative
location, but I think this is a different topic and beyond the scope of
this series.

Best regards,
Michael

> 
>>  
>> -    for dts_dts in ${pkg_dts}; do
>>  	ptxd_make_dtb || break
>>      done
>>  }
>>
> 

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-09  6:26     ` Michael Riesch
@ 2021-12-09  7:08       ` Michael Olbrich
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Olbrich @ 2021-12-09  7:08 UTC (permalink / raw)
  To: Michael Riesch; +Cc: christian.melki, ptxdist, m.tretter

On Thu, Dec 09, 2021 at 07:26:53AM +0100, Michael Riesch wrote:
> Hi Christian,
> 
> On 12/7/21 6:30 PM, Christian Melki wrote:
> > Hi!
> > 
> > I like this take on dtbos!
> 
> Thanks for your review!
> 
> > On 12/7/21 16:39, Michael Riesch wrote:
> >> Make ptxd_make_dtb more general and provide suitable wrappers to
> >> support the generation of device tree overlays.
> >>
> >> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> >> ---
> >>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
> >>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
> >>  2 files changed, 80 insertions(+), 23 deletions(-)
> >>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
> >>
> >> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
> >> new file mode 100644
> >> index 000000000..61babc653
> >> --- /dev/null
> >> +++ b/rules/post/ptxd_make_world_dtbo.make
> >> @@ -0,0 +1,21 @@
> >> +# -*-makefile-*-
> >> +#
> >> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
> >> +#
> >> +# For further information about the PTXdist project and license conditions
> >> +# see the README file.
> >> +#
> >> +
> >> +world/dtbo/env = \
> >> +	$(call world/env, $(1)) \
> >> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
> >> +	pkg_dtso="$($(1)_DTSO)" \
> >> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
> >> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
> >> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
> >> +
> >> +world/dtbo = \
> >> +	$(call world/dtbo/env,$(strip $(1))) \
> >> +	ptxd_make_world_dtbo
> >> +
> >> +# vim: syntax=make
> >> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
> >> index f5e796b9d..c1ded5ffb 100644
> >> --- a/scripts/lib/ptxd_make_world_dtb.sh
> >> +++ b/scripts/lib/ptxd_make_world_dtb.sh
> >> @@ -9,18 +9,10 @@
> >>  ptxd_make_dtb() {
> >>      local dtc dts tmp_dts dtb deps tmp_deps
> >>  
> >> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
> >> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
> >> -	    "Use <PKG>_DTS_PATH to specify the search path."
> >> -    fi
> >> -
> >> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
> >> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
> >> -    fi
> >> -    dts="${ptxd_reply}"
> >> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
> >> +    dts="${dtb_source}"
> >> +    dtb="${dtb_target}"
> >>  
> >> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
> >> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
> >>      if [ ! -x "${dtc}" ]; then
> >>  	dtc=dtc
> >>      fi
> >> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
> >>  	-Wp,-MT,${dtb_deps_target} \
> >>  	-nostdinc \
> >>  	-I"$(dirname "${dts}")" \
> >> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
> >> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
> >> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
> >> -	-I${pkg_dir}/drivers/of/testcase-data \
> >> -	-I${pkg_dir}/include \
> >> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
> >> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
> >> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
> >> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
> >> +	-I${dtb_kernel_dir}/include \
> >>  	-undef -D__DTS__ -x assembler-with-cpp \
> >>  	-o ${tmp_dts} \
> >>  	${dts} &&
> >>  
> >> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
> >> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
> >>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
> >>      # empty line in case all dependencies were removed
> >>      echo >> "${deps}" &&
> >> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
> >>      echo "DTC $(ptxd_print_path "${dtb}")" &&
> >>      ptxd_eval \
> >>  	"${dtc}" \
> >> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
> >> +	"${dtb_extra_args}" \
> >>  	-i "$(dirname "${dts}")" \
> >> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
> >> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
> >>  	-d "${tmp_deps}" \
> >>  	-I dts -O dtb -b 0 \
> >>  	-o "${dtb}" "${tmp_dts}" &&
> >> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
> >>  export -f ptxd_make_dtb
> >>  
> >>  
> >> +ptxd_make_world_dtbo() {
> >> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >> +
> >> +    ptxd_make_world_init || break
> >> +
> >> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >> +    dtb_extra_args="-@"
> > 
> > Looks ok. Do we need to do a dtc version check?
> > 1.4.4+ I think. Maybe a bit excessive?
> 
> As pointed out by Michael Olbrich this won't help.
> 
> > 
> >> +    dtb_kernel_dir="${pkg_kernel_src}"
> >> +    if [ -z $dtb_kernel_dir ]; then
> >> +	dtb_kernel_dir="${pkg_dir}"
> >> +    fi
> >> +
> >> +    echo -e "\nBuilding device tree overlays..."
> >> +
> >> +    for overlay in ${pkg_dtso}; do
> >> +	if [[ "${overlay}" =~ ^/.* ]]; then
> >> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
> >> +			 "Use <PKG>_DTSO_PATH to specify the search path."
> >> +	fi
> >> +
> >> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
> >> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
> >> +	fi
> >> +	dtb_source="${ptxd_reply}"
> >> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
> >> +
> >> +	ptxd_make_dtb || break
> >> +    done
> >> +}
> >> +export -f ptxd_make_world_dtbo
> >> +
> >> +
> >>  ptxd_make_world_dtb() {
> >> -    local dtb_deps_target dtb_dir
> >> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >>  
> >>      ptxd_make_world_init || break
> >>  
> >>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >> -    dtb_dir="${pkg_pkg_dir}/boot"
> >> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
> > 
> > Not sure I follow here. Must not the dtb itself be built with -@ for
> > symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
> 
> You are right, thanks for pointing that out. Will fix this in v2.
> 
> > 
> >> +    dtb_kernel_dir="${pkg_dir}"
> >> +
> >> +    echo -e "\nBuilding device trees..."
> >> +
> >> +    for tree in ${pkg_dts}; do
> >> +	if [[ "${tree}" =~ ^/.* ]]; then
> >> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
> >> +			 "Use <PKG>_DTS_PATH to specify the search path."
> >> +	fi
> >>  
> >> -    echo -e "\nBuilding Device trees..."
> >> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
> >> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
> >> +	fi
> >> +	dtb_source="${ptxd_reply}"
> >> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
> > 
> > Static output dir path?
> 
> Not sure whether I understand your comment correctly, but the path has
> been copied from the existing script. It can be discussed whether it
> makes sense to install the compiled device trees to an alternative
> location, but I think this is a different topic and beyond the scope of
> this series.

Yes, please leave it as it is. I don't see a use-case where the device-tree
is not in the same directory as the Kernel itself. And we've installed both
to /boot for a long time now and nobody complained.

The situation is differently for overlays. It depends on how they are
loaded. A bootloader might look somewhere next to the kernel, but if the
overlays are loaded at runtime, then /lib/firmware is also possible.

So fix location for the device-tree itself and a configurable for the
overlays makes sense to me.

Michael

-- 
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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-08  8:31     ` Michael Olbrich
@ 2021-12-09  7:20       ` Michael Riesch
  2021-12-09  8:53         ` Michael Olbrich
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Riesch @ 2021-12-09  7:20 UTC (permalink / raw)
  To: Christian Melki, ptxdist, m.tretter

Hello Michael,

On 12/8/21 9:31 AM, Michael Olbrich wrote:
> On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
>> Hi!
>>
>> I like this take on dtbos!
>>
>> On 12/7/21 16:39, Michael Riesch wrote:
>>> Make ptxd_make_dtb more general and provide suitable wrappers to
>>> support the generation of device tree overlays.
>>>
>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>>> ---
>>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
>>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
>>>  2 files changed, 80 insertions(+), 23 deletions(-)
>>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
>>>
>>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
>>> new file mode 100644
>>> index 000000000..61babc653
>>> --- /dev/null
>>> +++ b/rules/post/ptxd_make_world_dtbo.make
>>> @@ -0,0 +1,21 @@
>>> +# -*-makefile-*-
>>> +#
>>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
>>> +#
>>> +# For further information about the PTXdist project and license conditions
>>> +# see the README file.
>>> +#
>>> +
>>> +world/dtbo/env = \
>>> +	$(call world/env, $(1)) \
>>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
>>> +	pkg_dtso="$($(1)_DTSO)" \
>>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
>>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
> 
> Right, reaching into the source tree of another package is always ugly.
> Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
> set for the generic kernel package (out-of-tree modules need it as well)
> but not for the kernel package template.

Could you elaborate a bit on why this is required?

What action should be carried out if this variable is not set to 'NO' or
must this not happen in the first place?

> 
>>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
>>> +
>>> +world/dtbo = \
>>> +	$(call world/dtbo/env,$(strip $(1))) \
>>> +	ptxd_make_world_dtbo
>>> +
>>> +# vim: syntax=make
>>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
>>> index f5e796b9d..c1ded5ffb 100644
>>> --- a/scripts/lib/ptxd_make_world_dtb.sh
>>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
>>> @@ -9,18 +9,10 @@
>>>  ptxd_make_dtb() {
>>>      local dtc dts tmp_dts dtb deps tmp_deps
>>>  
>>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
>>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
>>> -	    "Use <PKG>_DTS_PATH to specify the search path."
>>> -    fi
>>> -
>>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
>>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
>>> -    fi
>>> -    dts="${ptxd_reply}"
>>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
>>> +    dts="${dtb_source}"
>>> +    dtb="${dtb_target}"
>>>  
>>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
>>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
>>>      if [ ! -x "${dtc}" ]; then
>>>  	dtc=dtc
>>>      fi
>>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
>>>  	-Wp,-MT,${dtb_deps_target} \
>>>  	-nostdinc \
>>>  	-I"$(dirname "${dts}")" \
>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
>>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
>>> -	-I${pkg_dir}/drivers/of/testcase-data \
>>> -	-I${pkg_dir}/include \
>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
>>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
>>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
>>> +	-I${dtb_kernel_dir}/include \
>>>  	-undef -D__DTS__ -x assembler-with-cpp \
>>>  	-o ${tmp_dts} \
>>>  	${dts} &&
>>>  
>>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
>>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
>>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
>>>      # empty line in case all dependencies were removed
>>>      echo >> "${deps}" &&
>>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
>>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
>>>      ptxd_eval \
>>>  	"${dtc}" \
>>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
>>> +	"${dtb_extra_args}" \
>>>  	-i "$(dirname "${dts}")" \
>>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
>>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
>>>  	-d "${tmp_deps}" \
>>>  	-I dts -O dtb -b 0 \
>>>  	-o "${dtb}" "${tmp_dts}" &&
>>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
>>>  export -f ptxd_make_dtb
>>>  
>>>  
>>> +ptxd_make_world_dtbo() {
>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>> +
>>> +    ptxd_make_world_init || break
>>> +
>>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>> +    dtb_extra_args="-@"
>>
>> Looks ok. Do we need to do a dtc version check?
>> 1.4.4+ I think. Maybe a bit excessive?
> 
> We cannot build dtbos if that's not supported, so a check won't really
> help.

Ack.

> 
>>> +    dtb_kernel_dir="${pkg_kernel_src}"
>>> +    if [ -z $dtb_kernel_dir ]; then
>>> +	dtb_kernel_dir="${pkg_dir}"
>>> +    fi
> 
>     dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"
> 
> should work. And I think this can be moved to the shared code above. It
> works just fine because pkg_kernel_src is never set for the dts case (but
> we might want to allow it in the future).

Ack.

> 
>>> +
>>> +    echo -e "\nBuilding device tree overlays..."
>>> +
>>> +    for overlay in ${pkg_dtso}; do
>>> +	if [[ "${overlay}" =~ ^/.* ]]; then
>>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
>>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
>>> +	fi
>>> +
>>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
>>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
>>> +	fi
>>> +	dtb_source="${ptxd_reply}"
>>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
>>> +
>>> +	ptxd_make_dtb || break
>>> +    done
>>> +}
>>> +export -f ptxd_make_world_dtbo
>>> +
>>> +
>>>  ptxd_make_world_dtb() {
>>> -    local dtb_deps_target dtb_dir
>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>>  
>>>      ptxd_make_world_init || break
>>>  
>>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>> -    dtb_dir="${pkg_pkg_dir}/boot"
>>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
>>
>> Not sure I follow here. Must not the dtb itself be built with -@ for
>> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
> 
> I don't know much about dtbos. If -@ is required for the dtb to make dtbos
> work, then this should be propagated automatically. Add something to
> world/dtb/env, maybe with a noprompt kernel options
> KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
> needed.

Will change this in v2.

Thanks and regards,
Michael

> 
>>> +    dtb_kernel_dir="${pkg_dir}"
>>> +
>>> +    echo -e "\nBuilding device trees..."
>>> +
>>> +    for tree in ${pkg_dts}; do
> 
> Please keep the dts_dts variable name.
> 
> Michael
> 
>>> +	if [[ "${tree}" =~ ^/.* ]]; then
>>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
>>> +			 "Use <PKG>_DTS_PATH to specify the search path."
>>> +	fi
>>>  
>>> -    echo -e "\nBuilding Device trees..."
>>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
>>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
>>> +	fi
>>> +	dtb_source="${ptxd_reply}"
>>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
>>
>> Static output dir path?
>>
>>>  
>>> -    for dts_dts in ${pkg_dts}; do
>>>  	ptxd_make_dtb || break
>>>      done
>>>  }
>>>
>>
>>
>> _______________________________________________
>> 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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-09  7:20       ` Michael Riesch
@ 2021-12-09  8:53         ` Michael Olbrich
  2021-12-09  9:58           ` Michael Riesch
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Olbrich @ 2021-12-09  8:53 UTC (permalink / raw)
  To: Michael Riesch; +Cc: Christian Melki, ptxdist, m.tretter

On Thu, Dec 09, 2021 at 08:20:05AM +0100, Michael Riesch wrote:
> On 12/8/21 9:31 AM, Michael Olbrich wrote:
> > On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
> >> I like this take on dtbos!
> >>
> >> On 12/7/21 16:39, Michael Riesch wrote:
> >>> Make ptxd_make_dtb more general and provide suitable wrappers to
> >>> support the generation of device tree overlays.
> >>>
> >>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> >>> ---
> >>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
> >>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
> >>>  2 files changed, 80 insertions(+), 23 deletions(-)
> >>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
> >>>
> >>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
> >>> new file mode 100644
> >>> index 000000000..61babc653
> >>> --- /dev/null
> >>> +++ b/rules/post/ptxd_make_world_dtbo.make
> >>> @@ -0,0 +1,21 @@
> >>> +# -*-makefile-*-
> >>> +#
> >>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
> >>> +#
> >>> +# For further information about the PTXdist project and license conditions
> >>> +# see the README file.
> >>> +#
> >>> +
> >>> +world/dtbo/env = \
> >>> +	$(call world/env, $(1)) \
> >>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
> >>> +	pkg_dtso="$($(1)_DTSO)" \
> >>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
> >>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
> > 
> > Right, reaching into the source tree of another package is always ugly.
> > Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
> > set for the generic kernel package (out-of-tree modules need it as well)
> > but not for the kernel package template.
> 
> Could you elaborate a bit on why this is required?
> 
> What action should be carried out if this variable is not set to 'NO' or
> must this not happen in the first place?

If pre-built archives[1] are used are used then the source tree may not be
available. So if the source/build tree of a package is used by any other
package then <PKG>_DEVPKG must be set to ensure that the package is always
built.

It's a somewhat obscure feature of PTXdist.

Michael

[1] https://www.ptxdist.org/doc/daily_work_section.html#using-pre-built-archives

> >>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
> >>> +
> >>> +world/dtbo = \
> >>> +	$(call world/dtbo/env,$(strip $(1))) \
> >>> +	ptxd_make_world_dtbo
> >>> +
> >>> +# vim: syntax=make
> >>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
> >>> index f5e796b9d..c1ded5ffb 100644
> >>> --- a/scripts/lib/ptxd_make_world_dtb.sh
> >>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
> >>> @@ -9,18 +9,10 @@
> >>>  ptxd_make_dtb() {
> >>>      local dtc dts tmp_dts dtb deps tmp_deps
> >>>  
> >>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
> >>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
> >>> -	    "Use <PKG>_DTS_PATH to specify the search path."
> >>> -    fi
> >>> -
> >>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
> >>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
> >>> -    fi
> >>> -    dts="${ptxd_reply}"
> >>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
> >>> +    dts="${dtb_source}"
> >>> +    dtb="${dtb_target}"
> >>>  
> >>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
> >>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
> >>>      if [ ! -x "${dtc}" ]; then
> >>>  	dtc=dtc
> >>>      fi
> >>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
> >>>  	-Wp,-MT,${dtb_deps_target} \
> >>>  	-nostdinc \
> >>>  	-I"$(dirname "${dts}")" \
> >>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
> >>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
> >>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
> >>> -	-I${pkg_dir}/drivers/of/testcase-data \
> >>> -	-I${pkg_dir}/include \
> >>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
> >>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
> >>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
> >>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
> >>> +	-I${dtb_kernel_dir}/include \
> >>>  	-undef -D__DTS__ -x assembler-with-cpp \
> >>>  	-o ${tmp_dts} \
> >>>  	${dts} &&
> >>>  
> >>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
> >>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
> >>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
> >>>      # empty line in case all dependencies were removed
> >>>      echo >> "${deps}" &&
> >>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
> >>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
> >>>      ptxd_eval \
> >>>  	"${dtc}" \
> >>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
> >>> +	"${dtb_extra_args}" \
> >>>  	-i "$(dirname "${dts}")" \
> >>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
> >>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
> >>>  	-d "${tmp_deps}" \
> >>>  	-I dts -O dtb -b 0 \
> >>>  	-o "${dtb}" "${tmp_dts}" &&
> >>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
> >>>  export -f ptxd_make_dtb
> >>>  
> >>>  
> >>> +ptxd_make_world_dtbo() {
> >>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >>> +
> >>> +    ptxd_make_world_init || break
> >>> +
> >>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >>> +    dtb_extra_args="-@"
> >>
> >> Looks ok. Do we need to do a dtc version check?
> >> 1.4.4+ I think. Maybe a bit excessive?
> > 
> > We cannot build dtbos if that's not supported, so a check won't really
> > help.
> 
> Ack.
> 
> > 
> >>> +    dtb_kernel_dir="${pkg_kernel_src}"
> >>> +    if [ -z $dtb_kernel_dir ]; then
> >>> +	dtb_kernel_dir="${pkg_dir}"
> >>> +    fi
> > 
> >     dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"
> > 
> > should work. And I think this can be moved to the shared code above. It
> > works just fine because pkg_kernel_src is never set for the dts case (but
> > we might want to allow it in the future).
> 
> Ack.
> 
> > 
> >>> +
> >>> +    echo -e "\nBuilding device tree overlays..."
> >>> +
> >>> +    for overlay in ${pkg_dtso}; do
> >>> +	if [[ "${overlay}" =~ ^/.* ]]; then
> >>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
> >>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
> >>> +	fi
> >>> +
> >>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
> >>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
> >>> +	fi
> >>> +	dtb_source="${ptxd_reply}"
> >>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
> >>> +
> >>> +	ptxd_make_dtb || break
> >>> +    done
> >>> +}
> >>> +export -f ptxd_make_world_dtbo
> >>> +
> >>> +
> >>>  ptxd_make_world_dtb() {
> >>> -    local dtb_deps_target dtb_dir
> >>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >>>  
> >>>      ptxd_make_world_init || break
> >>>  
> >>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >>> -    dtb_dir="${pkg_pkg_dir}/boot"
> >>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
> >>
> >> Not sure I follow here. Must not the dtb itself be built with -@ for
> >> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
> > 
> > I don't know much about dtbos. If -@ is required for the dtb to make dtbos
> > work, then this should be propagated automatically. Add something to
> > world/dtb/env, maybe with a noprompt kernel options
> > KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
> > needed.
> 
> Will change this in v2.
> 
> Thanks and regards,
> Michael
> 
> > 
> >>> +    dtb_kernel_dir="${pkg_dir}"
> >>> +
> >>> +    echo -e "\nBuilding device trees..."
> >>> +
> >>> +    for tree in ${pkg_dts}; do
> > 
> > Please keep the dts_dts variable name.
> > 
> > Michael
> > 
> >>> +	if [[ "${tree}" =~ ^/.* ]]; then
> >>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
> >>> +			 "Use <PKG>_DTS_PATH to specify the search path."
> >>> +	fi
> >>>  
> >>> -    echo -e "\nBuilding Device trees..."
> >>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
> >>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
> >>> +	fi
> >>> +	dtb_source="${ptxd_reply}"
> >>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
> >>
> >> Static output dir path?
> >>
> >>>  
> >>> -    for dts_dts in ${pkg_dts}; do
> >>>  	ptxd_make_dtb || break
> >>>      done
> >>>  }
> >>>
> >>
> >>
> >> _______________________________________________
> >> 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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-09  8:53         ` Michael Olbrich
@ 2021-12-09  9:58           ` Michael Riesch
  2021-12-09 10:46             ` Michael Olbrich
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Riesch @ 2021-12-09  9:58 UTC (permalink / raw)
  To: Christian Melki, ptxdist, m.tretter

Hello Michael,

On 12/9/21 9:53 AM, Michael Olbrich wrote:
> On Thu, Dec 09, 2021 at 08:20:05AM +0100, Michael Riesch wrote:
>> On 12/8/21 9:31 AM, Michael Olbrich wrote:
>>> On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
>>>> I like this take on dtbos!
>>>>
>>>> On 12/7/21 16:39, Michael Riesch wrote:
>>>>> Make ptxd_make_dtb more general and provide suitable wrappers to
>>>>> support the generation of device tree overlays.
>>>>>
>>>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>>>>> ---
>>>>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
>>>>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
>>>>>  2 files changed, 80 insertions(+), 23 deletions(-)
>>>>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
>>>>>
>>>>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
>>>>> new file mode 100644
>>>>> index 000000000..61babc653
>>>>> --- /dev/null
>>>>> +++ b/rules/post/ptxd_make_world_dtbo.make
>>>>> @@ -0,0 +1,21 @@
>>>>> +# -*-makefile-*-
>>>>> +#
>>>>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
>>>>> +#
>>>>> +# For further information about the PTXdist project and license conditions
>>>>> +# see the README file.
>>>>> +#
>>>>> +
>>>>> +world/dtbo/env = \
>>>>> +	$(call world/env, $(1)) \
>>>>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
>>>>> +	pkg_dtso="$($(1)_DTSO)" \
>>>>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
>>>>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
>>>
>>> Right, reaching into the source tree of another package is always ugly.
>>> Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
>>> set for the generic kernel package (out-of-tree modules need it as well)
>>> but not for the kernel package template.
>>
>> Could you elaborate a bit on why this is required?
>>
>> What action should be carried out if this variable is not set to 'NO' or
>> must this not happen in the first place?
> 
> If pre-built archives[1] are used are used then the source tree may not be
> available. So if the source/build tree of a package is used by any other
> package then <PKG>_DEVPKG must be set to ensure that the package is always
> built.
> 
> It's a somewhat obscure feature of PTXdist.
> 
> Michael
> 
> [1] https://www.ptxdist.org/doc/daily_work_section.html#using-pre-built-archives
> 

Thanks for the explanation, now I understand why this is necessary.
However, I am still not sure what I can do about it in this patch series.

Can I really change the <PKG>_DEVPKG (e.g., KERNEL_CUSTOM_DEVPKG where
KERNEL_CUSTOM is derived from the kernel template) at the time
world/dtbo is called (from MY_OVERLAY package)? Would it have any effect?

I guess I can output a warning, but as far as I understand the damage is
already done at this point and if there are no kernel sources in
MY_OVERLAY_KERNEL_DIR the dtc command might fail miserably.

How would I determine the kernel package name from the kernel directory
to begin with?

Best regards,
Michael

>>>>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
>>>>> +
>>>>> +world/dtbo = \
>>>>> +	$(call world/dtbo/env,$(strip $(1))) \
>>>>> +	ptxd_make_world_dtbo
>>>>> +
>>>>> +# vim: syntax=make
>>>>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
>>>>> index f5e796b9d..c1ded5ffb 100644
>>>>> --- a/scripts/lib/ptxd_make_world_dtb.sh
>>>>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
>>>>> @@ -9,18 +9,10 @@
>>>>>  ptxd_make_dtb() {
>>>>>      local dtc dts tmp_dts dtb deps tmp_deps
>>>>>  
>>>>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
>>>>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
>>>>> -	    "Use <PKG>_DTS_PATH to specify the search path."
>>>>> -    fi
>>>>> -
>>>>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
>>>>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
>>>>> -    fi
>>>>> -    dts="${ptxd_reply}"
>>>>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
>>>>> +    dts="${dtb_source}"
>>>>> +    dtb="${dtb_target}"
>>>>>  
>>>>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
>>>>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
>>>>>      if [ ! -x "${dtc}" ]; then
>>>>>  	dtc=dtc
>>>>>      fi
>>>>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
>>>>>  	-Wp,-MT,${dtb_deps_target} \
>>>>>  	-nostdinc \
>>>>>  	-I"$(dirname "${dts}")" \
>>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
>>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
>>>>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
>>>>> -	-I${pkg_dir}/drivers/of/testcase-data \
>>>>> -	-I${pkg_dir}/include \
>>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
>>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
>>>>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
>>>>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
>>>>> +	-I${dtb_kernel_dir}/include \
>>>>>  	-undef -D__DTS__ -x assembler-with-cpp \
>>>>>  	-o ${tmp_dts} \
>>>>>  	${dts} &&
>>>>>  
>>>>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
>>>>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
>>>>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
>>>>>      # empty line in case all dependencies were removed
>>>>>      echo >> "${deps}" &&
>>>>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
>>>>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
>>>>>      ptxd_eval \
>>>>>  	"${dtc}" \
>>>>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
>>>>> +	"${dtb_extra_args}" \
>>>>>  	-i "$(dirname "${dts}")" \
>>>>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
>>>>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
>>>>>  	-d "${tmp_deps}" \
>>>>>  	-I dts -O dtb -b 0 \
>>>>>  	-o "${dtb}" "${tmp_dts}" &&
>>>>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
>>>>>  export -f ptxd_make_dtb
>>>>>  
>>>>>  
>>>>> +ptxd_make_world_dtbo() {
>>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>>>> +
>>>>> +    ptxd_make_world_init || break
>>>>> +
>>>>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>>>> +    dtb_extra_args="-@"
>>>>
>>>> Looks ok. Do we need to do a dtc version check?
>>>> 1.4.4+ I think. Maybe a bit excessive?
>>>
>>> We cannot build dtbos if that's not supported, so a check won't really
>>> help.
>>
>> Ack.
>>
>>>
>>>>> +    dtb_kernel_dir="${pkg_kernel_src}"
>>>>> +    if [ -z $dtb_kernel_dir ]; then
>>>>> +	dtb_kernel_dir="${pkg_dir}"
>>>>> +    fi
>>>
>>>     dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"
>>>
>>> should work. And I think this can be moved to the shared code above. It
>>> works just fine because pkg_kernel_src is never set for the dts case (but
>>> we might want to allow it in the future).
>>
>> Ack.
>>
>>>
>>>>> +
>>>>> +    echo -e "\nBuilding device tree overlays..."
>>>>> +
>>>>> +    for overlay in ${pkg_dtso}; do
>>>>> +	if [[ "${overlay}" =~ ^/.* ]]; then
>>>>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
>>>>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
>>>>> +	fi
>>>>> +
>>>>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
>>>>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
>>>>> +	fi
>>>>> +	dtb_source="${ptxd_reply}"
>>>>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
>>>>> +
>>>>> +	ptxd_make_dtb || break
>>>>> +    done
>>>>> +}
>>>>> +export -f ptxd_make_world_dtbo
>>>>> +
>>>>> +
>>>>>  ptxd_make_world_dtb() {
>>>>> -    local dtb_deps_target dtb_dir
>>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>>>>  
>>>>>      ptxd_make_world_init || break
>>>>>  
>>>>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>>>> -    dtb_dir="${pkg_pkg_dir}/boot"
>>>>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
>>>>
>>>> Not sure I follow here. Must not the dtb itself be built with -@ for
>>>> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
>>>
>>> I don't know much about dtbos. If -@ is required for the dtb to make dtbos
>>> work, then this should be propagated automatically. Add something to
>>> world/dtb/env, maybe with a noprompt kernel options
>>> KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
>>> needed.
>>
>> Will change this in v2.
>>
>> Thanks and regards,
>> Michael
>>
>>>
>>>>> +    dtb_kernel_dir="${pkg_dir}"
>>>>> +
>>>>> +    echo -e "\nBuilding device trees..."
>>>>> +
>>>>> +    for tree in ${pkg_dts}; do
>>>
>>> Please keep the dts_dts variable name.
>>>
>>> Michael
>>>
>>>>> +	if [[ "${tree}" =~ ^/.* ]]; then
>>>>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
>>>>> +			 "Use <PKG>_DTS_PATH to specify the search path."
>>>>> +	fi
>>>>>  
>>>>> -    echo -e "\nBuilding Device trees..."
>>>>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
>>>>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
>>>>> +	fi
>>>>> +	dtb_source="${ptxd_reply}"
>>>>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
>>>>
>>>> Static output dir path?
>>>>
>>>>>  
>>>>> -    for dts_dts in ${pkg_dts}; do
>>>>>  	ptxd_make_dtb || break
>>>>>      done
>>>>>  }
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-09  9:58           ` Michael Riesch
@ 2021-12-09 10:46             ` Michael Olbrich
  2021-12-09 10:52               ` Michael Riesch
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Olbrich @ 2021-12-09 10:46 UTC (permalink / raw)
  To: Michael Riesch; +Cc: Christian Melki, ptxdist, m.tretter

On Thu, Dec 09, 2021 at 10:58:03AM +0100, Michael Riesch wrote:
> On 12/9/21 9:53 AM, Michael Olbrich wrote:
> > On Thu, Dec 09, 2021 at 08:20:05AM +0100, Michael Riesch wrote:
> >> On 12/8/21 9:31 AM, Michael Olbrich wrote:
> >>> On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
> >>>> I like this take on dtbos!
> >>>>
> >>>> On 12/7/21 16:39, Michael Riesch wrote:
> >>>>> Make ptxd_make_dtb more general and provide suitable wrappers to
> >>>>> support the generation of device tree overlays.
> >>>>>
> >>>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> >>>>> ---
> >>>>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
> >>>>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
> >>>>>  2 files changed, 80 insertions(+), 23 deletions(-)
> >>>>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
> >>>>>
> >>>>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
> >>>>> new file mode 100644
> >>>>> index 000000000..61babc653
> >>>>> --- /dev/null
> >>>>> +++ b/rules/post/ptxd_make_world_dtbo.make
> >>>>> @@ -0,0 +1,21 @@
> >>>>> +# -*-makefile-*-
> >>>>> +#
> >>>>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
> >>>>> +#
> >>>>> +# For further information about the PTXdist project and license conditions
> >>>>> +# see the README file.
> >>>>> +#
> >>>>> +
> >>>>> +world/dtbo/env = \
> >>>>> +	$(call world/env, $(1)) \
> >>>>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
> >>>>> +	pkg_dtso="$($(1)_DTSO)" \
> >>>>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
> >>>>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
> >>>
> >>> Right, reaching into the source tree of another package is always ugly.
> >>> Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
> >>> set for the generic kernel package (out-of-tree modules need it as well)
> >>> but not for the kernel package template.
> >>
> >> Could you elaborate a bit on why this is required?
> >>
> >> What action should be carried out if this variable is not set to 'NO' or
> >> must this not happen in the first place?
> > 
> > If pre-built archives[1] are used are used then the source tree may not be
> > available. So if the source/build tree of a package is used by any other
> > package then <PKG>_DEVPKG must be set to ensure that the package is always
> > built.
> > 
> > It's a somewhat obscure feature of PTXdist.
> > 
> > Michael
> > 
> > [1] https://www.ptxdist.org/doc/daily_work_section.html#using-pre-built-archives
> > 
> 
> Thanks for the explanation, now I understand why this is necessary.
> However, I am still not sure what I can do about it in this patch series.
> 
> Can I really change the <PKG>_DEVPKG (e.g., KERNEL_CUSTOM_DEVPKG where
> KERNEL_CUSTOM is derived from the kernel template) at the time
> world/dtbo is called (from MY_OVERLAY package)? Would it have any effect?
> 
> I guess I can output a warning, but as far as I understand the damage is
> already done at this point and if there are no kernel sources in
> MY_OVERLAY_KERNEL_DIR the dtc command might fail miserably.
> 
> How would I determine the kernel package name from the kernel directory
> to begin with?

Ah, sorry. I didn't mean that there is anything for you to change. Just
something to keep in mind when you use it.

Michael

> >>>>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
> >>>>> +
> >>>>> +world/dtbo = \
> >>>>> +	$(call world/dtbo/env,$(strip $(1))) \
> >>>>> +	ptxd_make_world_dtbo
> >>>>> +
> >>>>> +# vim: syntax=make
> >>>>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
> >>>>> index f5e796b9d..c1ded5ffb 100644
> >>>>> --- a/scripts/lib/ptxd_make_world_dtb.sh
> >>>>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
> >>>>> @@ -9,18 +9,10 @@
> >>>>>  ptxd_make_dtb() {
> >>>>>      local dtc dts tmp_dts dtb deps tmp_deps
> >>>>>  
> >>>>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
> >>>>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
> >>>>> -	    "Use <PKG>_DTS_PATH to specify the search path."
> >>>>> -    fi
> >>>>> -
> >>>>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
> >>>>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
> >>>>> -    fi
> >>>>> -    dts="${ptxd_reply}"
> >>>>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
> >>>>> +    dts="${dtb_source}"
> >>>>> +    dtb="${dtb_target}"
> >>>>>  
> >>>>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
> >>>>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
> >>>>>      if [ ! -x "${dtc}" ]; then
> >>>>>  	dtc=dtc
> >>>>>      fi
> >>>>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
> >>>>>  	-Wp,-MT,${dtb_deps_target} \
> >>>>>  	-nostdinc \
> >>>>>  	-I"$(dirname "${dts}")" \
> >>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
> >>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
> >>>>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
> >>>>> -	-I${pkg_dir}/drivers/of/testcase-data \
> >>>>> -	-I${pkg_dir}/include \
> >>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
> >>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
> >>>>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
> >>>>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
> >>>>> +	-I${dtb_kernel_dir}/include \
> >>>>>  	-undef -D__DTS__ -x assembler-with-cpp \
> >>>>>  	-o ${tmp_dts} \
> >>>>>  	${dts} &&
> >>>>>  
> >>>>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
> >>>>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
> >>>>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
> >>>>>      # empty line in case all dependencies were removed
> >>>>>      echo >> "${deps}" &&
> >>>>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
> >>>>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
> >>>>>      ptxd_eval \
> >>>>>  	"${dtc}" \
> >>>>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
> >>>>> +	"${dtb_extra_args}" \
> >>>>>  	-i "$(dirname "${dts}")" \
> >>>>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
> >>>>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
> >>>>>  	-d "${tmp_deps}" \
> >>>>>  	-I dts -O dtb -b 0 \
> >>>>>  	-o "${dtb}" "${tmp_dts}" &&
> >>>>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
> >>>>>  export -f ptxd_make_dtb
> >>>>>  
> >>>>>  
> >>>>> +ptxd_make_world_dtbo() {
> >>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >>>>> +
> >>>>> +    ptxd_make_world_init || break
> >>>>> +
> >>>>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >>>>> +    dtb_extra_args="-@"
> >>>>
> >>>> Looks ok. Do we need to do a dtc version check?
> >>>> 1.4.4+ I think. Maybe a bit excessive?
> >>>
> >>> We cannot build dtbos if that's not supported, so a check won't really
> >>> help.
> >>
> >> Ack.
> >>
> >>>
> >>>>> +    dtb_kernel_dir="${pkg_kernel_src}"
> >>>>> +    if [ -z $dtb_kernel_dir ]; then
> >>>>> +	dtb_kernel_dir="${pkg_dir}"
> >>>>> +    fi
> >>>
> >>>     dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"
> >>>
> >>> should work. And I think this can be moved to the shared code above. It
> >>> works just fine because pkg_kernel_src is never set for the dts case (but
> >>> we might want to allow it in the future).
> >>
> >> Ack.
> >>
> >>>
> >>>>> +
> >>>>> +    echo -e "\nBuilding device tree overlays..."
> >>>>> +
> >>>>> +    for overlay in ${pkg_dtso}; do
> >>>>> +	if [[ "${overlay}" =~ ^/.* ]]; then
> >>>>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
> >>>>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
> >>>>> +	fi
> >>>>> +
> >>>>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
> >>>>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
> >>>>> +	fi
> >>>>> +	dtb_source="${ptxd_reply}"
> >>>>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
> >>>>> +
> >>>>> +	ptxd_make_dtb || break
> >>>>> +    done
> >>>>> +}
> >>>>> +export -f ptxd_make_world_dtbo
> >>>>> +
> >>>>> +
> >>>>>  ptxd_make_world_dtb() {
> >>>>> -    local dtb_deps_target dtb_dir
> >>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
> >>>>>  
> >>>>>      ptxd_make_world_init || break
> >>>>>  
> >>>>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
> >>>>> -    dtb_dir="${pkg_pkg_dir}/boot"
> >>>>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
> >>>>
> >>>> Not sure I follow here. Must not the dtb itself be built with -@ for
> >>>> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
> >>>
> >>> I don't know much about dtbos. If -@ is required for the dtb to make dtbos
> >>> work, then this should be propagated automatically. Add something to
> >>> world/dtb/env, maybe with a noprompt kernel options
> >>> KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
> >>> needed.
> >>
> >> Will change this in v2.
> >>
> >> Thanks and regards,
> >> Michael
> >>
> >>>
> >>>>> +    dtb_kernel_dir="${pkg_dir}"
> >>>>> +
> >>>>> +    echo -e "\nBuilding device trees..."
> >>>>> +
> >>>>> +    for tree in ${pkg_dts}; do
> >>>
> >>> Please keep the dts_dts variable name.
> >>>
> >>> Michael
> >>>
> >>>>> +	if [[ "${tree}" =~ ^/.* ]]; then
> >>>>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
> >>>>> +			 "Use <PKG>_DTS_PATH to specify the search path."
> >>>>> +	fi
> >>>>>  
> >>>>> -    echo -e "\nBuilding Device trees..."
> >>>>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
> >>>>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
> >>>>> +	fi
> >>>>> +	dtb_source="${ptxd_reply}"
> >>>>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
> >>>>
> >>>> Static output dir path?
> >>>>
> >>>>>  
> >>>>> -    for dts_dts in ${pkg_dts}; do
> >>>>>  	ptxd_make_dtb || break
> >>>>>      done
> >>>>>  }
> >>>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> 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] 12+ messages in thread

* Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays
  2021-12-09 10:46             ` Michael Olbrich
@ 2021-12-09 10:52               ` Michael Riesch
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Riesch @ 2021-12-09 10:52 UTC (permalink / raw)
  To: Christian Melki, ptxdist, m.tretter

Hello again,

On 12/9/21 11:46 AM, Michael Olbrich wrote:
> On Thu, Dec 09, 2021 at 10:58:03AM +0100, Michael Riesch wrote:
>> On 12/9/21 9:53 AM, Michael Olbrich wrote:
>>> On Thu, Dec 09, 2021 at 08:20:05AM +0100, Michael Riesch wrote:
>>>> On 12/8/21 9:31 AM, Michael Olbrich wrote:
>>>>> On Tue, Dec 07, 2021 at 06:30:54PM +0100, Christian Melki wrote:
>>>>>> I like this take on dtbos!
>>>>>>
>>>>>> On 12/7/21 16:39, Michael Riesch wrote:
>>>>>>> Make ptxd_make_dtb more general and provide suitable wrappers to
>>>>>>> support the generation of device tree overlays.
>>>>>>>
>>>>>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>>>>>>> ---
>>>>>>>  rules/post/ptxd_make_world_dtbo.make | 21 +++++++
>>>>>>>  scripts/lib/ptxd_make_world_dtb.sh   | 82 ++++++++++++++++++++--------
>>>>>>>  2 files changed, 80 insertions(+), 23 deletions(-)
>>>>>>>  create mode 100644 rules/post/ptxd_make_world_dtbo.make
>>>>>>>
>>>>>>> diff --git a/rules/post/ptxd_make_world_dtbo.make b/rules/post/ptxd_make_world_dtbo.make
>>>>>>> new file mode 100644
>>>>>>> index 000000000..61babc653
>>>>>>> --- /dev/null
>>>>>>> +++ b/rules/post/ptxd_make_world_dtbo.make
>>>>>>> @@ -0,0 +1,21 @@
>>>>>>> +# -*-makefile-*-
>>>>>>> +#
>>>>>>> +# Copyright (C) 2020 by Michael Tretter <m.tretter@pengutronix.de>
>>>>>>> +#
>>>>>>> +# For further information about the PTXdist project and license conditions
>>>>>>> +# see the README file.
>>>>>>> +#
>>>>>>> +
>>>>>>> +world/dtbo/env = \
>>>>>>> +	$(call world/env, $(1)) \
>>>>>>> +	pkg_dtso_path="$($(1)_DTSO_PATH)" \
>>>>>>> +	pkg_dtso="$($(1)_DTSO)" \
>>>>>>> +	pkg_dtbo_dir="$($(1)_DTBO_DIR)" \
>>>>>>> +	pkg_kernel_src="$($(1)_KERNEL_DIR)" \
>>>>>
>>>>> Right, reaching into the source tree of another package is always ugly.
>>>>> Make sure that <PKG>_DEVPKG is set to 'NO' for the package. It's already
>>>>> set for the generic kernel package (out-of-tree modules need it as well)
>>>>> but not for the kernel package template.
>>>>
>>>> Could you elaborate a bit on why this is required?
>>>>
>>>> What action should be carried out if this variable is not set to 'NO' or
>>>> must this not happen in the first place?
>>>
>>> If pre-built archives[1] are used are used then the source tree may not be
>>> available. So if the source/build tree of a package is used by any other
>>> package then <PKG>_DEVPKG must be set to ensure that the package is always
>>> built.
>>>
>>> It's a somewhat obscure feature of PTXdist.
>>>
>>> Michael
>>>
>>> [1] https://www.ptxdist.org/doc/daily_work_section.html#using-pre-built-archives
>>>
>>
>> Thanks for the explanation, now I understand why this is necessary.
>> However, I am still not sure what I can do about it in this patch series.
>>
>> Can I really change the <PKG>_DEVPKG (e.g., KERNEL_CUSTOM_DEVPKG where
>> KERNEL_CUSTOM is derived from the kernel template) at the time
>> world/dtbo is called (from MY_OVERLAY package)? Would it have any effect?
>>
>> I guess I can output a warning, but as far as I understand the damage is
>> already done at this point and if there are no kernel sources in
>> MY_OVERLAY_KERNEL_DIR the dtc command might fail miserably.
>>
>> How would I determine the kernel package name from the kernel directory
>> to begin with?
> 
> Ah, sorry. I didn't mean that there is anything for you to change. Just
> something to keep in mind when you use it.

OK, that's alright then :-) Thanks for the valuable pointer!

v2 will follow soon.

Best regards,
Michael

> 
> Michael
> 
>>>>>>> +	pkg_arch="$(GENERIC_KERNEL_ARCH)"
>>>>>>> +
>>>>>>> +world/dtbo = \
>>>>>>> +	$(call world/dtbo/env,$(strip $(1))) \
>>>>>>> +	ptxd_make_world_dtbo
>>>>>>> +
>>>>>>> +# vim: syntax=make
>>>>>>> diff --git a/scripts/lib/ptxd_make_world_dtb.sh b/scripts/lib/ptxd_make_world_dtb.sh
>>>>>>> index f5e796b9d..c1ded5ffb 100644
>>>>>>> --- a/scripts/lib/ptxd_make_world_dtb.sh
>>>>>>> +++ b/scripts/lib/ptxd_make_world_dtb.sh
>>>>>>> @@ -9,18 +9,10 @@
>>>>>>>  ptxd_make_dtb() {
>>>>>>>      local dtc dts tmp_dts dtb deps tmp_deps
>>>>>>>  
>>>>>>> -    if [[ "${dts_dts}" =~ ^/.* ]]; then
>>>>>>> -	ptxd_bailout "'${dts_dts}' must not be an absolute path!" \
>>>>>>> -	    "Use <PKG>_DTS_PATH to specify the search path."
>>>>>>> -    fi
>>>>>>> -
>>>>>>> -    if ! ptxd_in_path pkg_dts_path "${dts_dts}"; then
>>>>>>> -	ptxd_bailout "Device-tree '${dts_dts}' not found in '${pkg_dts_path}'."
>>>>>>> -    fi
>>>>>>> -    dts="${ptxd_reply}"
>>>>>>> -    dtb="${dtb_dir}/$(basename ${dts/%.dts/.dtb})"
>>>>>>> +    dts="${dtb_source}"
>>>>>>> +    dtb="${dtb_target}"
>>>>>>>  
>>>>>>> -    dtc="${pkg_build_dir}/scripts/dtc/dtc"
>>>>>>> +    dtc="${dtb_kernel_dir}/scripts/dtc/dtc"
>>>>>>>      if [ ! -x "${dtc}" ]; then
>>>>>>>  	dtc=dtc
>>>>>>>      fi
>>>>>>> @@ -39,16 +31,16 @@ ptxd_make_dtb() {
>>>>>>>  	-Wp,-MT,${dtb_deps_target} \
>>>>>>>  	-nostdinc \
>>>>>>>  	-I"$(dirname "${dts}")" \
>>>>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts \
>>>>>>> -	-I${pkg_dir}/arch/${pkg_arch}/boot/dts/include \
>>>>>>> -	-I${pkg_dir}/scripts/dtc/include-prefixes \
>>>>>>> -	-I${pkg_dir}/drivers/of/testcase-data \
>>>>>>> -	-I${pkg_dir}/include \
>>>>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts \
>>>>>>> +	-I${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts/include \
>>>>>>> +	-I${dtb_kernel_dir}/scripts/dtc/include-prefixes \
>>>>>>> +	-I${dtb_kernel_dir}/drivers/of/testcase-data \
>>>>>>> +	-I${dtb_kernel_dir}/include \
>>>>>>>  	-undef -D__DTS__ -x assembler-with-cpp \
>>>>>>>  	-o ${tmp_dts} \
>>>>>>>  	${dts} &&
>>>>>>>  
>>>>>>> -    sed -e "\;^ ${pkg_dir}[^ ]*;d" \
>>>>>>> +    sed -e "\;^ ${dtb_kernel_dir}[^ ]*;d" \
>>>>>>>  	-e 's;^ \([^ \]*\); $(wildcard \1);' "${tmp_deps}" > "${deps}" &&
>>>>>>>      # empty line in case all dependencies were removed
>>>>>>>      echo >> "${deps}" &&
>>>>>>> @@ -57,9 +49,9 @@ ptxd_make_dtb() {
>>>>>>>      echo "DTC $(ptxd_print_path "${dtb}")" &&
>>>>>>>      ptxd_eval \
>>>>>>>  	"${dtc}" \
>>>>>>> -	$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS) \
>>>>>>> +	"${dtb_extra_args}" \
>>>>>>>  	-i "$(dirname "${dts}")" \
>>>>>>> -	-i "${pkg_dir}/arch/${pkg_arch}/boot/dts" \
>>>>>>> +	-i "${dtb_kernel_dir}/arch/${pkg_arch}/boot/dts" \
>>>>>>>  	-d "${tmp_deps}" \
>>>>>>>  	-I dts -O dtb -b 0 \
>>>>>>>  	-o "${dtb}" "${tmp_dts}" &&
>>>>>>> @@ -81,17 +73,61 @@ ptxd_make_dtb() {
>>>>>>>  export -f ptxd_make_dtb
>>>>>>>  
>>>>>>>  
>>>>>>> +ptxd_make_world_dtbo() {
>>>>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>>>>>> +
>>>>>>> +    ptxd_make_world_init || break
>>>>>>> +
>>>>>>> +    dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>>>>>> +    dtb_extra_args="-@"
>>>>>>
>>>>>> Looks ok. Do we need to do a dtc version check?
>>>>>> 1.4.4+ I think. Maybe a bit excessive?
>>>>>
>>>>> We cannot build dtbos if that's not supported, so a check won't really
>>>>> help.
>>>>
>>>> Ack.
>>>>
>>>>>
>>>>>>> +    dtb_kernel_dir="${pkg_kernel_src}"
>>>>>>> +    if [ -z $dtb_kernel_dir ]; then
>>>>>>> +	dtb_kernel_dir="${pkg_dir}"
>>>>>>> +    fi
>>>>>
>>>>>     dtb_kernel_dir="${pkg_kernel_src:-${pkg_dir}}"
>>>>>
>>>>> should work. And I think this can be moved to the shared code above. It
>>>>> works just fine because pkg_kernel_src is never set for the dts case (but
>>>>> we might want to allow it in the future).
>>>>
>>>> Ack.
>>>>
>>>>>
>>>>>>> +
>>>>>>> +    echo -e "\nBuilding device tree overlays..."
>>>>>>> +
>>>>>>> +    for overlay in ${pkg_dtso}; do
>>>>>>> +	if [[ "${overlay}" =~ ^/.* ]]; then
>>>>>>> +	    ptxd_bailout "'${overlay}' must not be an absolute path!" \
>>>>>>> +			 "Use <PKG>_DTSO_PATH to specify the search path."
>>>>>>> +	fi
>>>>>>> +
>>>>>>> +	if ! ptxd_in_path pkg_dtso_path "${overlay}"; then
>>>>>>> +	    ptxd_bailout "Overlay '${overlay}' not found in '${pkg_dtso_path}'."
>>>>>>> +	fi
>>>>>>> +	dtb_source="${ptxd_reply}"
>>>>>>> +	dtb_target="${pkg_pkg_dir}/${pkg_dtbo_dir}/$(basename ${overlay/%.dts*/.dtbo})"
>>>>>>> +
>>>>>>> +	ptxd_make_dtb || break
>>>>>>> +    done
>>>>>>> +}
>>>>>>> +export -f ptxd_make_world_dtbo
>>>>>>> +
>>>>>>> +
>>>>>>>  ptxd_make_world_dtb() {
>>>>>>> -    local dtb_deps_target dtb_dir
>>>>>>> +    local dtb_deps_target dtb_extra_args dtb_kernel_dir dtb_source dtb_target
>>>>>>>  
>>>>>>>      ptxd_make_world_init || break
>>>>>>>  
>>>>>>>      dtb_deps_target="${ptx_state_dir}/${pkg_stamp}"
>>>>>>> -    dtb_dir="${pkg_pkg_dir}/boot"
>>>>>>> +    dtb_extra_args="$(ptxd_get_ptxconf PTXCONF_DTC_EXTRA_ARGS)"
>>>>>>
>>>>>> Not sure I follow here. Must not the dtb itself be built with -@ for
>>>>>> symbols if dtbos are used? Does EXTRA_ARGS default to -@ or something?
>>>>>
>>>>> I don't know much about dtbos. If -@ is required for the dtb to make dtbos
>>>>> work, then this should be propagated automatically. Add something to
>>>>> world/dtb/env, maybe with a noprompt kernel options
>>>>> KERNEL_DTS_SUPPORT_OVERLAYS or something like that. And select it as
>>>>> needed.
>>>>
>>>> Will change this in v2.
>>>>
>>>> Thanks and regards,
>>>> Michael
>>>>
>>>>>
>>>>>>> +    dtb_kernel_dir="${pkg_dir}"
>>>>>>> +
>>>>>>> +    echo -e "\nBuilding device trees..."
>>>>>>> +
>>>>>>> +    for tree in ${pkg_dts}; do
>>>>>
>>>>> Please keep the dts_dts variable name.
>>>>>
>>>>> Michael
>>>>>
>>>>>>> +	if [[ "${tree}" =~ ^/.* ]]; then
>>>>>>> +	    ptxd_bailout "'${tree}' must not be an absolute path!" \
>>>>>>> +			 "Use <PKG>_DTS_PATH to specify the search path."
>>>>>>> +	fi
>>>>>>>  
>>>>>>> -    echo -e "\nBuilding Device trees..."
>>>>>>> +	if ! ptxd_in_path pkg_dts_path "${tree}"; then
>>>>>>> +	    ptxd_bailout "Overlay '${tree}' not found in '${pkg_dts_path}'."
>>>>>>> +	fi
>>>>>>> +	dtb_source="${ptxd_reply}"
>>>>>>> +	dtb_target="${pkg_pkg_dir}/boot/$(basename ${tree/%.dts/.dtb})"
>>>>>>
>>>>>> Static output dir path?
>>>>>>
>>>>>>>  
>>>>>>> -    for dts_dts in ${pkg_dts}; do
>>>>>>>  	ptxd_make_dtb || break
>>>>>>>      done
>>>>>>>  }
>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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] 12+ messages in thread

end of thread, other threads:[~2021-12-09 10:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07 15:39 [ptxdist] [PATCH 0/2] Add support for device tree overlays Michael Riesch
2021-12-07 15:39 ` [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add " Michael Riesch
2021-12-07 17:30   ` Christian Melki
2021-12-08  8:31     ` Michael Olbrich
2021-12-09  7:20       ` Michael Riesch
2021-12-09  8:53         ` Michael Olbrich
2021-12-09  9:58           ` Michael Riesch
2021-12-09 10:46             ` Michael Olbrich
2021-12-09 10:52               ` Michael Riesch
2021-12-09  6:26     ` Michael Riesch
2021-12-09  7:08       ` Michael Olbrich
2021-12-07 15:39 ` [ptxdist] [PATCH 2/2] kernel: activate " Michael Riesch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox