Re: [PATCH] linux-custom: compile only those specified dtbs

["'Jan Kiszka' via isar-users" <isar-users@googlegroups.com>]

On 02.08.24 13:41, 'Nicusor Huhulea' via isar-users wrote:
> The current implementation makes a full kernel build and that means it will
> compile all dtbs specified by the bsp makefile e.g arch/arm64/boot/dts/ti/Makefile
> Currently the required dtbs are given by the DTB_FILES.
>
> These changes are checking if the DTB_FILES is specified, compile only those
> and avoids compiling all the dtbs from the bsp directory. Otherwise build all dtbs
> specified by the kernel source.
> Preventing the automatic dtb builds is made by using target specific build
> commands e.g KERNEL_IMAGETYPE
> The compilation of the dtbs accepts the following formats e.g:
> a) full path from the architecture directory:
>    arch/${ARCH}/boot/dts/bsp_dir/test.dts
>    arch/${ARCH}/boot/dts/bsp_dir/test.dtb
> b) relative path from the dts directory: bsp_dir/test.dts
> c) device tree blob file: bsp_dir/test.dtb
> Compilation of the dtbs is somewhat similar to KERNEL_DEVICETREE from OE when
> it comes to declaring the path to the dts, but for now in the DTB_FILES variable
> only Device Tree Blobs(.dtb) needs to be declared due to the fact that is being used
> in do_copy_boot_files task.

Can you provide some numbers on how much can be saved this way? Time or
(I rather assume) space?

>
> Signed-off-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
> ---
>  meta-isar/conf/machine/imx6-sabrelite.conf    |  4 +-
>  meta-isar/conf/machine/phyboard-mira.conf     |  4 +-
>  .../linux/files/debian/isar/build.tmpl        | 43 ++++++++++++++++---
>  .../linux/files/debian/isar/install.tmpl      | 31 ++++++++++++-
>  meta/recipes-kernel/linux/linux-custom.inc    |  6 +++
>  5 files changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/meta-isar/conf/machine/imx6-sabrelite.conf b/meta-isar/conf/machine/imx6-sabrelite.conf
> index e48823d6..6ca0a907 100644
> --- a/meta-isar/conf/machine/imx6-sabrelite.conf
> +++ b/meta-isar/conf/machine/imx6-sabrelite.conf
> @@ -15,4 +15,6 @@ MKUBIFS_ARGS := "-m 0x1000 -e 0x3e000 -c 1500"
>  UBINIZE_ARGS = "-vv -m 0x1000 -p 0x40000"
>  IMAGE_FSTYPES ?= "ubi-ubifs"
>
> -DTB_FILES = "imx6q-sabrelite.dtb"
> +KERNEL_IMAGETYPE = "zImage"
> +
> +DTB_FILES = "nxp/imx/imx6q-sabrelite.dtb"
> diff --git a/meta-isar/conf/machine/phyboard-mira.conf b/meta-isar/conf/machine/phyboard-mira.conf
> index feec4e54..d64b4624 100644
> --- a/meta-isar/conf/machine/phyboard-mira.conf
> +++ b/meta-isar/conf/machine/phyboard-mira.conf
> @@ -16,10 +16,12 @@ MKUBIFS_ARGS := "-m 0x800 -e 0x1f000 -c 8012"
>  UBINIZE_ARGS = "-vv -m 0x800 -p 0x20000"
>  IMAGE_FSTYPES ?= "ubi ubifs"
>
> -DTB_FILES = "imx6q-phytec-mira-rdk-nand.dtb"
> +DTB_FILES = "nxp/imx/imx6q-phytec-mira-rdk-nand.dtb"

This change is only correct when also the kernel is 6.5 or newer (i.e.
when the dts folder of arm was split into vendors) - which it was in
fact. So, this is a separate fix that should be factored out.

>
>  IMAGE_INSTALL += "barebox"
>
> +KERNEL_IMAGETYPE = "zImage"
> +
>  BAREBOX_CONFIG = "imx_v7_defconfig"
>  BAREBOX_ENV = "phytec-mira-env"
>  BAREBOX_BASE_BIN = "barebox-phytec-phycore-imx6q-som-nand-1gib"
> diff --git a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> index bafc5ca4..b18c8f20 100644
> --- a/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> +++ b/meta/recipes-kernel/linux/files/debian/isar/build.tmpl
> @@ -21,22 +21,55 @@ do_build() {
>      KR=$(${MAKE} O=${KERNEL_BUILD_DIR} -s --no-print-directory kernelrelease)
>      sed -i "s/@KR@/${KR}/g" ${S}/debian/control ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.*
>
> -    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then # Build kernel scripts and tools
> -        ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} ${KERNEL_EXTRA_BUILDARGS} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}"
> +    MAKE_CMD_PREFIX="${MAKE} O=\"${KERNEL_BUILD_DIR}\" ${PARALLEL_MAKE} KCFLAGS=\"${KCFLAGS}\" KAFLAGS=\"${KAFLAGS}\""
> +    if echo "${DEB_BUILD_PROFILES}" | grep -q "kernel"; then
> +        if [ -n "${KERNEL_IMAGETYPE}" ]; then # Override the default compilation
> +            eval "${MAKE_CMD_PREFIX} ${KERNEL_EXTRA_BUILDARGS} ${KERNEL_IMAGETYPE} modules"

Please avoid eval whenever possible, it can give surprising results when
the provided variables change.

Why do we need it at all? Why not something like

${MAKE} ${MAKE_COMMON_ARGS} ...

?

> +            compile_dtbs
> +        else # Full build
> +            eval "${MAKE_CMD_PREFIX} ${KERNEL_EXTRA_BUILDARGS}"
> +        fi
>      elif echo "${DEB_BUILD_PROFILES}" | grep -q "kbuild"; then # Build kernel scripts and tools
> -        ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} ${KERNEL_EXTRA_BUILDARGS} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" scripts
> +        eval "${MAKE_CMD_PREFIX} ${KERNEL_EXTRA_BUILDARGS} scripts"
>          if grep -q -E "CONFIG_STACK_VALIDATION=y|CONFIG_HAVE_OBJTOOL=y" ${KERNEL_BUILD_DIR}/.config && [ -d "tools/objtool" ]; then
> -            ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" tools/objtool || true
> +            eval "${MAKE_CMD_PREFIX} tools/objtool" || true
>          fi
>          if grep -q "CONFIG_MODULES=y" ${KERNEL_BUILD_DIR}/.config; then
> -            ${MAKE} O=${KERNEL_BUILD_DIR} ${PARALLEL_MAKE} KCFLAGS="${KCFLAGS}" KAFLAGS="${KAFLAGS}" modules_prepare
> +            eval "${MAKE_CMDPREFIX} modules_prepare"
>          fi
>      fi
>
> +

Stray newline.

>      # Stop tracing
>      set +x
>  }
>
> +compile_dtbs() {
> +    prefix="arch/${ARCH}/boot/dts/"
> +    for dtb in ${DTB_FILES}; do
> +        # Check if the path is relative (starts with arch/${ARCH}/)
> +        # and if it does then extract the relative path from the architecture specific path
> +        if [[ "${dtb}" == "${prefix}"* ]]; then
> +            relative_dtb_path="${dtb#${prefix}}"
> +        else
> +            # directly use the given DTB if not prefixed with arch/${ARCH}/"
> +            relative_dtb_path="${dtb}"
> +        fi
> +
> +        # Check if it's a .dts file
> +        if [[ "${relative_dtb_path}" == *.dts ]]; then
> +            # Change .dts to .dtb
> +            relative_dtb_path="${relative_dtb_path%.dts}.dtb"
> +        fi
> +
> +        eval "${MAKE_CMD_PREFIX} ${relative_dtb_path}"
> +        if [ $? -ne 0 ]; then
> +            echo "${dtb} failed to compile" >&2
> +            exit 1
> +        fi
> +    done
> +}
> +
>  print_settings() {
>      cat <<EOF
>  # Build settings:
> diff --git a/meta/recipes-kernel/linux/files/debian/isar/install.tmpl b/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> index 00011517..fd2d3b5b 100644
> --- a/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> +++ b/meta/recipes-kernel/linux/files/debian/isar/install.tmpl
> @@ -124,7 +124,36 @@ install_hooks() {
>
>  install_dtbs() {
>      [ -n "${CONFIG_OF}" ] || return 0
> -    ${MAKE} O=${O} INSTALL_DTBS_PATH=${deb_img_dir}/usr/lib/linux-image-${krel} dtbs_install
> +    prefix="arch/${ARCH}/boot/dts/"
> +    dtb_bsp_dir=""
> +    src_dir=""
> +    dst_dir=""
> +
> +    # the assumption is that there's only one BSP in the DTB_FILES
> +    for dtb in ${DTB_FILES}; do
> +        dtb_bsp_dir=$(dirname "${dtb}")
> +        if [ "${dtb_bsp_dir}" == "${prefix}" ]; then
> +            continue
> +        elif [[ "${dtb_bsp_dir}" == "${prefix}"* ]]; then
> +            # get the relative BSP directory
> +            dtb_bsp_dir="${dtb_bsp_dir#"${prefix}"}"
> +        fi
> +
> +        if [ -n "${dtb_bsp_dir}" ]; then
> +            break
> +        fi
> +    done
> +
> +    src_dir="${O}/${prefix}${dtb_bsp_dir}"
> +    dst_dir="${deb_img_dir}/usr/lib/linux-image-${krel}/${dtb_bsp_dir}"
> +    mkdir -p "${dst_dir}"
> +
> +    if find "${src_dir}" -maxdepth 1 -name '*.dtb' | grep -q .; then
> +        cp "${src_dir}"/*.dtb "${dst_dir}/"
> +        echo "All dtbs files have been copied to ${src_dir}."
> +    else
> +        echo "No dtb files found!"
> +    fi
>  }
>
>  install_kmods() {
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
> index 6aa70fd3..cac3c91a 100644
> --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -76,6 +76,7 @@ TEMPLATE_VARS += "                \
>      KERNEL_DEBIAN_DEPENDS         \
>      KERNEL_BUILD_DIR              \
>      KERNEL_FILE                   \
> +    KERNEL_IMAGETYPE              \
>      KERNEL_HEADERS_DEBIAN_DEPENDS \
>      LINUX_VERSION_EXTENSION       \
>      KERNEL_NAME_PROVIDED          \
> @@ -84,12 +85,17 @@ TEMPLATE_VARS += "                \
>      KAFLAGS                       \
>      DISTRIBUTOR                   \
>      KERNEL_EXTRA_BUILDARGS        \
> +    DTB_FILES                     \
>  "
>
>  inherit dpkg
>  inherit template
>  inherit kbuildtarget
>
> +DTB_FILES ?=""

Missing space after ?=.

> +
> +KERNEL_IMAGETYPE ?= ""
> +
>  # Add custom cflags to the kernel build
>  KCFLAGS ?= "-fdebug-prefix-map=${CURDIR}=."
>  KAFLAGS ?= "-fdebug-prefix-map=${CURDIR}=."

Jan

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/43250db6-2540-48ee-8ba6-afe3fd9b56f9%40web.de.