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 <[email protected]> > >> --- > >> 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 <[email protected]> > >> +# > >> +# 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 [email protected] To unsubscribe, send a mail with subject "unsubscribe" to [email protected]
