Re: [OE-core] [PATCH v2 1/4] oe.fitimage: Support multiple kernel nodes and configuration selection

["adrian.freihofer@gmail.com" <adrian.freihofer@gmail.com>]

Hi Vahid,

Thanks for working on this, being able to put several kernels into one
FIT image is a useful feature. I had a closer look at the series and
have a few comments.


On Tue, 2026-10-20 at 15:35 +0200, Vahid Gharaee via
lists.openembedded.org wrote:
> From: Vahid Gharaee <v.gharaee@reinhausen.com>
> 
> Signed-off-by: Vahid Gharaee <vgharaee@gmail.com>

The author address does not match the Signed-off-by. Please use the
same identity for both.

The commit message has no body. Please explain why the change is
needed. Nothing in this patch uses the new kernel parameter yet, so it
would also help to say that this prepares for the following patches.

For the shortlog, changes to meta/lib/oe/fitimage.py usually use the
"kernel-fit-image:" prefix, e.g.:
  kernel-fit-image: support multiple kernel nodes

> ---
>  meta/lib/oe/fitimage.py | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/lib/oe/fitimage.py b/meta/lib/oe/fitimage.py
> index 484e5c5ee0..91ccc31d46 100644
> --- a/meta/lib/oe/fitimage.py
> +++ b/meta/lib/oe/fitimage.py
> @@ -193,6 +193,7 @@ class ItsNodeRootKernel(ItsNode):
>          self._dtbs = []
>          self._dtb_alias = []
>          self._kernel = None
> +        self._kernels = {}

Do we need both _kernel and _kernels? Dicts keep insertion order, so
the default kernel is always the first entry of _kernels. I'd drop
_kernel and derive the default, e.g.:

    @property
    def _default_kernel(self):
        return next(iter(self._kernels.values()), None)

That avoids keeping two attributes in sync.

>          self._ramdisk = None
>          self._bootscr = None
>          self._setup = None
> @@ -268,8 +269,8 @@ class ItsNodeRootKernel(ItsNode):
>      def fitimage_emit_section_kernel(self, kernel_id, kernel_path,
> compression,
>          load, entrypoint, mkimage_kernel_type, entrysymbol=None):
>          """Emit the fitImage ITS kernel section"""
> -        if self._kernel:
> -            bb.fatal("Kernel section already exists in the ITS
> file.")
> +        if kernel_id in self._kernels:
> +            bb.fatal("Kernel section '%s' already exists in the ITS
> file." % kernel_id)
>          if entrysymbol:
>              result = subprocess.run([self._host_prefix + "nm",
> "vmlinux"], capture_output=True, text=True)
>              for line in result.stdout.splitlines():
> @@ -294,7 +295,10 @@ class ItsNodeRootKernel(ItsNode):
>              compression,
>              opt_props,
>          )
> -        self._kernel = kernel_node
> +        self._kernels[kernel_id] = kernel_node
> +
> +        if self._kernel is None:
> +            self._kernel = kernel_node
>  
>      def fitimage_emit_section_dtb(self, dtb_id, dtb_path,
> dtb_loadaddress=None,
>                                    dtbo_loadaddress=None,
> add_compatible=False):
> @@ -425,7 +429,7 @@ class ItsNodeRootKernel(ItsNode):
>  
>          self._loadables.append(loadable_node)
>  
> -    def _fitimage_emit_one_section_config(self, conf_node_name,
> dtb=None):
> +    def _fitimage_emit_one_section_config(self, conf_node_name,
> dtb=None, kernel=None):
>          """Emit the fitImage ITS configuration section"""
>          opt_props = {}
>          conf_desc = []
> @@ -436,9 +440,11 @@ class ItsNodeRootKernel(ItsNode):
>          if dtb and os.path.splitext(dtb.name)[1] == ".dtbo":
>              include_kernel = 0
>  
> -        if self._kernel and include_kernel:
> +        selected_kernel = kernel if kernel is not None else
> self._kernel
> +
> +        if selected_kernel and include_kernel:

For a .dtbo configuration, a kernel passed in explicitly is silently
dropped. That's fine for the generated per-DTB configurations. But with
4/4, FIT_CONFIG_KERNEL[x] is ignored without notice if
FIT_CONFIG_DTB[x]
is an overlay. I'd suggest failing in that case, either here or in
fitimage_emit_section_config_custom().

Regards,
Adrian

>              conf_desc.append("Linux kernel")
> -            opt_props["kernel"] = self._kernel.name
> +            opt_props["kernel"] = selected_kernel.name
>              if self._sign_enable:
>                  sign_entries.append("kernel")
>  
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#248288):
> https://lists.openembedded.org/g/openembedded-core/message/248288
> Mute This Topic: https://lists.openembedded.org/mt/121849065/4454582
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> adrian.freihofer@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-