Re: [PATCH 4/5] configure: Add dtc option

[BALATON Zoltan]

On Tue, 10 Jun 2025, Bernhard Beschow wrote:
> The next patch will make dtc mandatory for boards with a bundled DTB, causing
> these boards to be omitted when dtc is missing. Allow packagers to force the
> build of those boards by providing an option.

I don't like this. This might result in some machines missing from some 
distros if they don't notice this and forget to update their packages and 
these are machines I care about. Also what guarantees that we don't get 
bugs by compiling the device tree with different dtc versions which might 
generate different binaries? The binaries we have now are known to work 
and don't take much space so keeping them for convenience might be 
simpler. If it worked so far why change it now?

If this was using dtc from the submodule when not using system libfdt then 
at least it would not result in dropped machines and we had a known 
working dtc to compile with. Why isn't that used when we already have it 
for libfdt?

Regards,
BALATON Zoltan

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> meson.build                   | 12 ++++++++++++
> Kconfig.host                  |  3 +++
> meson_options.txt             |  3 +++
> pc-bios/dtb/meson.build       |  1 -
> scripts/meson-buildoptions.sh |  3 +++
> 5 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 34729c2a3d..8a03fbaed8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2119,6 +2119,15 @@ if fdt_opt == 'internal'
>   fdt = libfdt_proj.get_variable('libfdt_dep')
> endif
>
> +dtc = not_found
> +dtc_opt = get_option('dtc')
> +if dtc_opt == 'enabled' or (dtc_opt == 'auto' and have_system)
> +  dtc = find_program('dtc', required: dtc_opt == 'enabled')
> +  if not dtc.found() and dtc_opt == 'enabled'
> +    error('dtc not found')
> +  endif
> +endif
> +
> rdma = not_found
> if not get_option('rdma').auto() or have_system
>   rdma_libs = [cc.find_library('rdmacm', has_headers: ['rdma/rdma_cma.h'],
> @@ -2477,6 +2486,7 @@ config_host_data.set('CONFIG_FREEBSD', host_os == 
> 'freebsd')
> config_host_data.set('CONFIG_CAPSTONE', capstone.found())
> config_host_data.set('CONFIG_COCOA', cocoa.found())
> config_host_data.set('CONFIG_DARWIN', host_os == 'darwin')
> +config_host_data.set('CONFIG_DTC', dtc.found())
> config_host_data.set('CONFIG_FDT', fdt.found())
> config_host_data.set('CONFIG_FUZZ', get_option('fuzzing'))
> config_host_data.set('CONFIG_GCOV', get_option('b_coverage'))
> @@ -3314,6 +3324,7 @@ host_kconfig = \
>   (libcbor.found() ? ['CONFIG_LIBCBOR=y'] : []) + \
>   (gnutls.found() ? ['CONFIG_GNUTLS=y'] : []) + \
>   (x11.found() ? ['CONFIG_X11=y'] : []) + \
> +  (dtc.found() ? ['CONFIG_DTC=y'] : []) + \
>   (fdt.found() ? ['CONFIG_FDT=y'] : []) + \
>   (have_vhost_user ? ['CONFIG_VHOST_USER=y'] : []) + \
>   (have_vhost_vdpa ? ['CONFIG_VHOST_VDPA=y'] : []) + \
> @@ -4703,6 +4714,7 @@ summary_info += {'sphinx-build':      sphinx_build}
> if config_host.has_key('GDB')
>   summary_info += {'gdb':             config_host['GDB']}
> endif
> +summary_info += {'dtc':               dtc}
> summary_info += {'iasl':              iasl}
> summary_info += {'genisoimage':       config_host['GENISOIMAGE']}
> if host_os == 'windows' and have_ga
> diff --git a/Kconfig.host b/Kconfig.host
> index 933425c74b..82fc8848dd 100644
> --- a/Kconfig.host
> +++ b/Kconfig.host
> @@ -29,6 +29,9 @@ config IVSHMEM
> config TPM
>     bool
>
> +config DTC
> +    bool
> +
> config FDT
>     bool
>
> diff --git a/meson_options.txt b/meson_options.txt
> index a442be2995..d0e56564fb 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -316,6 +316,9 @@ option('vduse_blk_export', type: 'feature', value: 'auto',
>
> option('capstone', type: 'feature', value: 'auto',
>        description: 'Whether and how to find the capstone library')
> +option('dtc', type: 'combo', value: 'auto',
> +       choices: ['disabled', 'enabled', 'auto'],
> +       description: 'Whether to find the device tree compiler')
> option('fdt', type: 'combo', value: 'auto',
>        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
>        description: 'Whether and how to find the libfdt library')
> diff --git a/pc-bios/dtb/meson.build b/pc-bios/dtb/meson.build
> index 993032949f..9fcdbeff14 100644
> --- a/pc-bios/dtb/meson.build
> +++ b/pc-bios/dtb/meson.build
> @@ -5,7 +5,6 @@ dtbs = [
>   'petalogix-s3adsp1800.dtb',
> ]
>
> -dtc = find_program('dtc', required: false)
> if dtc.found()
>   foreach out : dtbs
>     f = fs.replace_suffix(out, '.dts')
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 73e0770f42..e8c452ffb0 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -116,6 +116,7 @@ meson_options_help() {
>   printf "%s\n" '  dmg             dmg image format support'
>   printf "%s\n" '  docs            Documentations build support'
>   printf "%s\n" '  dsound          DirectSound sound support'
> +  printf "%s\n" '  dtc             Whether to find the device tree compiler'
>   printf "%s\n" '  fuse            FUSE block device export'
>   printf "%s\n" '  fuse-lseek      SEEK_HOLE/SEEK_DATA support for FUSE exports'
>   printf "%s\n" '  gcrypt          libgcrypt cryptography support'
> @@ -310,6 +311,8 @@ _meson_option_parse() {
>     --disable-docs) printf "%s" -Ddocs=disabled ;;
>     --enable-dsound) printf "%s" -Ddsound=enabled ;;
>     --disable-dsound) printf "%s" -Ddsound=disabled ;;
> +    --enable-dtc) printf "%s" -Ddtc=enabled ;;
> +    --disable-dtc) printf "%s" -Ddtc=disabled ;;
>     --enable-fdt) printf "%s" -Dfdt=enabled ;;
>     --disable-fdt) printf "%s" -Dfdt=disabled ;;
>     --enable-fdt=*) quote_sh "-Dfdt=$2" ;;