From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Thu, 09 Dec 2021 11:46:54 +0100 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by lore.white.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mvGwv-0010LE-VM for lore@lore.pengutronix.de; Thu, 09 Dec 2021 11:46:54 +0100 Received: from localhost ([127.0.0.1] helo=metis.ext.pengutronix.de) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1mvGwv-0007bF-6W; Thu, 09 Dec 2021 11:46:53 +0100 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mvGwa-0007aK-JD; Thu, 09 Dec 2021 11:46:32 +0100 Received: from mol by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1mvGwa-0000Fk-1P; Thu, 09 Dec 2021 11:46:32 +0100 Date: Thu, 9 Dec 2021 11:46:32 +0100 From: Michael Olbrich To: Michael Riesch Message-ID: <20211209104632.GP15196@pengutronix.de> Mail-Followup-To: Michael Riesch , Christian Melki , ptxdist@pengutronix.de, m.tretter@pengutronix.de References: <20211207153905.2386379-1-michael.riesch@wolfvision.net> <20211207153905.2386379-2-michael.riesch@wolfvision.net> <5d9dbc5e-8330-34cd-3a4b-859ebfae277e@t2data.com> <20211208083149.GF15196@pengutronix.de> <0ee4879b-13d2-44dc-e963-09b880a6f310@wolfvision.net> <20211209085356.GM15196@pengutronix.de> <132c595a-f8e4-a7c6-c830-d0911eb0f920@wolfvision.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <132c595a-f8e4-a7c6-c830-d0911eb0f920@wolfvision.net> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 11:44:36 up 294 days, 14:08, 158 users, load average: 0.44, 0.27, 0.21 User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [ptxdist] [PATCH 1/2] ptxd_make_world_{dtb, dtbo}: add support for device tree overlays X-BeenThere: ptxdist@pengutronix.de X-Mailman-Version: 2.1.29 Precedence: list List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: ptxdist@pengutronix.de Cc: Christian Melki , ptxdist@pengutronix.de, m.tretter@pengutronix.de Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ptxdist" X-SA-Exim-Connect-IP: 127.0.0.1 X-SA-Exim-Mail-From: ptxdist-bounces@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false 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 > >>>>> --- > >>>>> 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 > >>>>> +# > >>>>> +# 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 _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 _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 _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 _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 _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 _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