Openembedded Core Discussions
 help / color / mirror / Atom feed
From: adrian.freihofer@gmail.com
To: Randy MacLeod <randy.macleod@windriver.com>,
	kavinaya@qti.qualcomm.com,
	 openembedded-core@lists.openembedded.org
Cc: reatmon@ti.com, benjamin.missey@non.se.com
Subject: Re: [OE-core] [PATCH v2] fitimage: Add support for UBOOT_MKIMAGE_EXTRA_OPTS
Date: Wed, 17 Dec 2025 22:31:30 +0100	[thread overview]
Message-ID: <7a13d7376a83bb7deb6374124bc42669d86803fc.camel@gmail.com> (raw)
In-Reply-To: <57dd38bf-653d-4b74-8de4-d05f4d2b9797@windriver.com>

Hi Kavinaya, Randy

On Mon, 2025-12-15 at 13:17 -0500, Randy MacLeod wrote: 
-D dtcoption" Provide special options to the device tree compiler that is used to create the image.> Kavinaya, Adrian,   and others who have touched:
>     meta/classes-recipe/kernel-fit-image.bbclass
>  in the last  ~ year.
>  
> This needs more review than we had time for in the YP patch review
> meeting today.
>  
> I think the concern was that there had been some independent
> improvements to fitimage
>  and that perhaps no one was looking at the overall design.
>   
> This may be a red herring but for example, is there overlap with:
>     meta/classes/kernel-fitimage.bbclass
>     # Options for the device tree compiler passed to mkimage '-D'
> feature:
>     UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb -p 2000"
>  
That's almost the same. mkimage calls the dtc compiler.
The UBOOT_MKIMAGE_DTCOPTS variable contains arguments which are passed
to dtc.

There is also UBOOT_MKIMAGE_SIGN_ARGS which is passed to mkimage when
it gets called for signing an image. Also a slightly different topic.

The new variable UBOOT_MKIMAGE_EXTRA_OPTS is different. It provides
other options which are handled by mkimage itself.
Adding a new variable seams to be needed.

> If you think this is a sensible, maintainable change, do say so and
> explain your point of view.
>   
> Please don't shoot the messenger, as I don't work with u-boot or
> fitImage myself !
Thank you for adding me.

>  
> Thanks,
> ../Randy
>  
>  
> On 2025-12-10 6:04 a.m., Kavinaya S via lists.openembedded.org wrote:
>  
> > Currently, mkimage options in U-Boot recipes are fixed, which
> > limits
> > flexibility for platforms that require additional mkimage
> > arguments.
> > Introduce UBOOT_MKIMAGE_EXTRA_OPTS to allow passing extra options
> > to
> > mkimage during image generation.
> > 
> > This is a generic need because different SoCs and boot
> > configurations
> > often require mkimage flags beyond the defaults. For example:
> > - `-E` enables external data in FIT images, which is essential for
> >   modular boot setups, secure boot workflows, and reducing image
> > size.
> > - `-B 8` enforces 8-byte alignment, ensuring compatibility with
> > boot
> >   ROM requirements, improving memory access efficiency, and
> > supporting
> >   predictable offsets for multi-component FIT images.
> > 
> > By exposing this variable, we provide a clean and extensible
> > mechanism
> > for developers to meet hardware-specific and security requirements
> > without hardcoding options in recipes.
> > 
> > Example:
> > 
> > UBOOT_MKIMAGE_EXTRA_OPTS = "-B 8 -E"
> > will result in the mkimage command being invoked as:
> > `mkimage -B 8 -E -f fitImage.its fitImage.itb`
> > 
> > Signed-off-by: Kavinaya S <kavinaya@qti.qualcomm.com>
> > ---
> >  meta/classes-recipe/kernel-fit-image.bbclass | 1 +
> >  meta/conf/image-fitimage.conf | 3 +++
> >  meta/lib/oe/fitimage.py | 3 +++
> >  3 files changed, 7 insertions(+)
> > 
> > diff --git a/meta/classes-recipe/kernel-fit-image.bbclass
> > b/meta/classes-recipe/kernel-fit-image.bbclass
> > index fd0d21ceee..5456311f00 100644
> > --- a/meta/classes-recipe/kernel-fit-image.bbclass
> > +++ b/meta/classes-recipe/kernel-fit-image.bbclass
> > @@ -56,6 +56,7 @@ python do_compile() {
> >          d.getVar('HOST_PREFIX'), d.getVar('UBOOT_ARCH'),
> > d.getVar("FIT_CONF_PREFIX"),
> >          oe.types.boolean(d.getVar('FIT_KERNEL_SIGN_ENABLE')),
> > d.getVar("FIT_KERNEL_SIGN_KEYDIR"),
> >          d.getVar("UBOOT_MKIMAGE"),
> > d.getVar("UBOOT_MKIMAGE_DTCOPTS"),
> > + d.getVar('UBOOT_MKIMAGE_EXTRA_OPTS'),
> >          d.getVar("UBOOT_MKIMAGE_SIGN"),
> > d.getVar("UBOOT_MKIMAGE_SIGN_ARGS"),
> >          d.getVar('FIT_HASH_ALG'), d.getVar('FIT_SIGN_ALG'),
> > d.getVar('FIT_PAD_ALG'),
> >          d.getVar('FIT_KERNEL_SIGN_KEYNAME'),
> > diff --git a/meta/conf/image-fitimage.conf b/meta/conf/image-
> > fitimage.conf
> > index 090ee148f4..e470adad5c 100644
> > --- a/meta/conf/image-fitimage.conf
> > +++ b/meta/conf/image-fitimage.conf
> > @@ -47,6 +47,9 @@ FIT_LINUX_BIN ?= "linux.bin"
> >  # Allow user to select the default DTB for FIT image when multiple
> > dtb's exists.
> >  FIT_CONF_DEFAULT_DTB ?= ""
> >  
> > +# Additional mkimage options for FIT image creation
> > +UBOOT_MKIMAGE_EXTRA_OPTS ?= ""

Defining an UBOOT_ name-spaced variable in image-fitimage.conf looks
strange. But I see why you ended up with that:
 * All the mkimage related variables start with UBOOT_
 * This variable is used by the kernel-fit-image.bbclass only. Defining
   it in uboot-config.bbclass looks strange as well.

But, all UBOOT_ variables are defined in uboot-config.bbclass. Would it
be more consistent if the variable would be defined in uboot-
config.bbclass or if the variable would be renamed to
FIT_MKIMAGE_EXTRA_OPTS?

> > +
> >  # length of address in number of <u32> cells
> >  # ex: 1 32bits address, 2 64bits address
> >  FIT_ADDRESS_CELLS ?= "1"
> > diff --git a/meta/lib/oe/fitimage.py b/meta/lib/oe/fitimage.py
> > index f303799155..15a36310e0 100644
> > --- a/meta/lib/oe/fitimage.py
> > +++ b/meta/lib/oe/fitimage.py
> > @@ -156,6 +156,7 @@ class ItsNodeRootKernel(ItsNode):
> >      def __init__(self, description, address_cells, host_prefix,
> > arch, conf_prefix,
> >                   sign_enable=False, sign_keydir=None,
> >                   mkimage=None, mkimage_dtcopts=None,
> > + mkimage_extra_opts=None,
> >                   mkimage_sign=None, mkimage_sign_args=None,
> >                   hash_algo=None, sign_algo=None, pad_algo=None,
> >                   sign_keyname_conf=None,
> > @@ -177,6 +178,7 @@ class ItsNodeRootKernel(ItsNode):
> >          self._sign_keydir = sign_keydir
> >          self._mkimage = mkimage
> >          self._mkimage_dtcopts = mkimage_dtcopts
> > + self._mkimage_extra_opts = shlex.split(mkimage_extra_opts)

The class should work with the default parameters. But this just
explodes.

>>> import shlex
>>> shlex.split(None)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
shlex.split(None)
~~~~~~~~~~~^^^^^^
File "/usr/lib64/python3.13/shlex.py", line 308, in split
raise ValueError("s argument must not be None")
ValueError: s argument must not be None


This would work:

self._mkimage_extra_opts = shlex.split(mkimage_extra_opts) if
mkimage_extra_opts else []


> >          self._mkimage_sign = mkimage_sign
> >          self._mkimage_sign_args = mkimage_sign_args
> >          self._hash_algo = hash_algo
> > @@ -483,6 +485,7 @@ class ItsNodeRootKernel(ItsNode):
> >      def run_mkimage_assemble(self, itsfile, fitfile):
> >          cmd = [
> >              self._mkimage,
> > + *self._mkimage_extra_opts,

Also this one explodes with None. But if the initialization above gets
fixed, this will be fine as well.

> >              '-f', itsfile,
> >              fitfile
> >          ]
> >  


Thank you.
Regards,
Adrian

> >   
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#227471):
> > https://lists.openembedded.org/g/openembedded-core/message/227471
> > Mute This Topic:
> > https://lists.openembedded.org/mt/116710036/3616765
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-
> > core/unsub [randy.macleod@windriver.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> > 
> >  
>  
> 
>  
>  


  reply	other threads:[~2025-12-17 21:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-10 11:04 [PATCH v2] fitimage: Add support for UBOOT_MKIMAGE_EXTRA_OPTS Kavinaya S
2025-12-15 18:17 ` [OE-core] " Randy MacLeod
2025-12-17 21:31   ` adrian.freihofer [this message]
2025-12-18 11:34     ` Kavinaya S
2025-12-18 11:35     ` Kavinaya S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a13d7376a83bb7deb6374124bc42669d86803fc.camel@gmail.com \
    --to=adrian.freihofer@gmail.com \
    --cc=benjamin.missey@non.se.com \
    --cc=kavinaya@qti.qualcomm.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=randy.macleod@windriver.com \
    --cc=reatmon@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox