All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trevor Woerner <twoerner@gmail.com>
To: yocto-patches@lists.yoctoproject.org
Subject: Re: [yocto-patches] [meta-rockchip][PATCH 2/2] fitimage rework
Date: Fri, 13 Jun 2025 10:42:25 -0400	[thread overview]
Message-ID: <20250613144224.GA6216@localhost> (raw)
In-Reply-To: <10a6a94d-2433-4517-9af5-5e78febfda79@cherry.de>

On Fri 2025-06-13 @ 02:32:01 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
> 
> On 6/13/25 12:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> > oe-core has completely re-written the fitImage support starting roughly at
> > commit [1], update meta-rockchip to match.
> > 
> 
> Thanks for looking into that!
> 
> > Most of the MACHINEs in meta-rockchip use a fitImage, but some don't. Create
> > a boolean variable (RK_FITIMAGE), enabled by default, to keep track of which
> > ones do and which ones don't. Use this variable to decide how to configure
> > various image-related fields.
> > 
> > Run tested on the following with RAUC configured:
> > 	- nanopi-m4b
> > 	- nanopi-r2s
> > 	- radxa-zero-3e
> > 	- rock-pi-4b
> > 	- rock-pi-e
> > 	- rock-pi-s
> > 
> > Run tested on the following without RAUC:
> > 	- radxa-zero-3e
> > 	- rock-pi-e
> > 	- rock-pi-s
> > 
> > [1] oe-core: 3442d9297dca ("oe-selftest: fitimage: test external dtb")
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> >   conf/machine/include/px30.inc              | 3 +--
> >   conf/machine/include/rk3066.inc            | 1 +
> >   conf/machine/include/rk3188.inc            | 1 +
> >   conf/machine/include/rk3288.inc            | 1 +
> >   conf/machine/include/rk3308.inc            | 3 +--
> >   conf/machine/include/rk3328.inc            | 3 +--
> >   conf/machine/include/rk3399.inc            | 3 +--
> >   conf/machine/include/rk3566.inc            | 3 +--
> >   conf/machine/include/rk3568.inc            | 3 +--
> >   conf/machine/include/rk3588s.inc           | 3 +--
> >   conf/machine/include/rockchip-defaults.inc | 2 ++
> >   conf/machine/include/rockchip-extlinux.inc | 8 ++++----
> >   conf/machine/include/rockchip-fitimage.inc | 4 ++++
> 
> Missing info in the top README about this new variable I believe?
> 
> >   13 files changed, 20 insertions(+), 18 deletions(-)
> >   create mode 100644 conf/machine/include/rockchip-fitimage.inc
> > 
> > diff --git a/conf/machine/include/px30.inc b/conf/machine/include/px30.inc
> > index 8173cb19be2c..825c1fae76d7 100644
> > --- a/conf/machine/include/px30.inc
> > +++ b/conf/machine/include/px30.inc
> > @@ -11,8 +11,7 @@ require conf/machine/include/arm/armv8a/tune-cortexa35.inc
> >   require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> 
> question: should this be only included when RK_FITIMAGE = "True"?
>
> some sort of inherit_defer but for require? Or maybe we should set the
> variables in rockchip-fitimage only if RK_FITIMAGE = "True" and leave them
> as is if False?
> 
> It seems like SoCs with RK_FITIMAGE = "False" should not inherit
> rockchip-fitimage.inc and those with True should? What happens if you have a
> board outside of meta-rockchip with px30 which you don't want to support
> fitImage by default (that's my case, so I guess I'll need to build this to
> figure it out :) ).

Yes, exactly. I would have preferred to have it setup such that
conf/machine/include/rockchip-fitimage.inc is either:
	1) required conditionally
	2) would include some sort of if/else logic to only apply fit-related
	   options when RK_FITIMAGE is true
but I couldn't find some mechanism to do that.

I could think about it some more, I haven't looked into inherit_defer.

> >   TFA_PLATFORM = "px30"
> >   TFA_BUILD_TARGET = "bl31"
> > diff --git a/conf/machine/include/rk3066.inc b/conf/machine/include/rk3066.inc
> > index a898309bbf88..dc0c7734812c 100644
> > --- a/conf/machine/include/rk3066.inc
> > +++ b/conf/machine/include/rk3066.inc
> > @@ -9,6 +9,7 @@ require conf/machine/include/arm/armv7a/tune-cortexa9.inc
> >   SERIAL_CONSOLES = "115200;ttyS2"
> > +RK_FITIMAGE = "False"
> >   KBUILD_DEFCONFIG = "multi_v7_defconfig"
> >   KERNEL_FEATURES:append:rk3066 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
> >   KERNEL_IMAGETYPE ?= "zImage"
> > diff --git a/conf/machine/include/rk3188.inc b/conf/machine/include/rk3188.inc
> > index 554d4f1c904d..3e1fc7e70822 100644
> > --- a/conf/machine/include/rk3188.inc
> > +++ b/conf/machine/include/rk3188.inc
> > @@ -9,6 +9,7 @@ require conf/machine/include/arm/armv7a/tune-cortexa9.inc
> >   SERIAL_CONSOLES = "115200;ttyFIQ0"
> > +RK_FITIMAGE = "False"
> >   KBUILD_DEFCONFIG = "multi_v7_defconfig"
> >   KERNEL_FEATURES:append:rk3188 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
> >   KERNEL_IMAGETYPE ?= "zImage"
> > diff --git a/conf/machine/include/rk3288.inc b/conf/machine/include/rk3288.inc
> > index 06fda69a3eb7..5cc117e633fc 100644
> > --- a/conf/machine/include/rk3288.inc
> > +++ b/conf/machine/include/rk3288.inc
> > @@ -9,6 +9,7 @@ require conf/machine/include/arm/armv7a/tune-cortexa17.inc
> >   SERIAL_CONSOLES = "115200;ttyS2"
> > +RK_FITIMAGE = "False"
> >   KBUILD_DEFCONFIG ?= "multi_v7_defconfig"
> >   KERNEL_FEATURES:append:rk3288 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
> >   KERNEL_IMAGETYPE ?= "zImage"
> > diff --git a/conf/machine/include/rk3308.inc b/conf/machine/include/rk3308.inc
> > index d30901f15b4f..ce1cebee1aac 100644
> > --- a/conf/machine/include/rk3308.inc
> > +++ b/conf/machine/include/rk3308.inc
> > @@ -13,8 +13,7 @@ SERIAL_CONSOLES = "1500000;ttyS0"
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3308 = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   UBOOT_SUFFIX ?= "itb"
> >   UBOOT_ENTRYPOINT ?= "0x06000000"
> > diff --git a/conf/machine/include/rk3328.inc b/conf/machine/include/rk3328.inc
> > index e6f810dcd2ca..4924b06f030a 100644
> > --- a/conf/machine/include/rk3328.inc
> > +++ b/conf/machine/include/rk3328.inc
> > @@ -12,8 +12,7 @@ require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3328 = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   TFA_PLATFORM = "rk3328"
> >   TFA_BUILD_TARGET = "bl31"
> > diff --git a/conf/machine/include/rk3399.inc b/conf/machine/include/rk3399.inc
> > index cd1be49064ed..eb23f49370fc 100644
> > --- a/conf/machine/include/rk3399.inc
> > +++ b/conf/machine/include/rk3399.inc
> > @@ -12,8 +12,7 @@ require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3399 = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   TFA_PLATFORM = "rk3399"
> >   TFA_BUILD_TARGET = "bl31"
> > diff --git a/conf/machine/include/rk3566.inc b/conf/machine/include/rk3566.inc
> > index 6386ec7eb51c..c9647a66aa42 100644
> > --- a/conf/machine/include/rk3566.inc
> > +++ b/conf/machine/include/rk3566.inc
> > @@ -11,8 +11,7 @@ require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3566 = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   PREFERRED_PROVIDER_trusted-firmware-a = "rockchip-rkbin-tf-a"
> >   PREFERRED_PROVIDER_optee-os = "rockchip-rkbin-optee-os"
> > diff --git a/conf/machine/include/rk3568.inc b/conf/machine/include/rk3568.inc
> > index bcf9dd8b0f44..e89aa388951a 100644
> > --- a/conf/machine/include/rk3568.inc
> > +++ b/conf/machine/include/rk3568.inc
> > @@ -11,8 +11,7 @@ require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3568 = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   PREFERRED_PROVIDER_trusted-firmware-a = "rockchip-rkbin-tf-a"
> >   PREFERRED_PROVIDER_optee-os = "rockchip-rkbin-optee-os"
> > diff --git a/conf/machine/include/rk3588s.inc b/conf/machine/include/rk3588s.inc
> > index 6ec344abae5b..265244753b5a 100644
> > --- a/conf/machine/include/rk3588s.inc
> > +++ b/conf/machine/include/rk3588s.inc
> > @@ -10,8 +10,7 @@ require conf/machine/include/rockchip-wic.inc
> >   KBUILD_DEFCONFIG ?= "defconfig"
> >   KERNEL_FEATURES:append:rk3588s = " bsp/rockchip/remove-non-rockchip-arch-arm64.scc"
> > -KERNEL_CLASSES = "kernel-fitimage"
> > -KERNEL_IMAGETYPE ?= "fitImage"
> > +require conf/machine/include/rockchip-fitimage.inc
> >   PREFERRED_PROVIDER_trusted-firmware-a = "rockchip-rkbin-tf-a"
> >   PREFERRED_PROVIDER_optee-os = "rockchip-rkbin-optee-os"
> > diff --git a/conf/machine/include/rockchip-defaults.inc b/conf/machine/include/rockchip-defaults.inc
> > index 85ec7b944d1d..3835408ed8d2 100644
> > --- a/conf/machine/include/rockchip-defaults.inc
> > +++ b/conf/machine/include/rockchip-defaults.inc
> > @@ -1,7 +1,9 @@
> >   # meta-rockchip default settings
> >   MACHINEOVERRIDES =. "${@bb.utils.contains('ROCKCHIP_CLOSED_TPL', '1', 'closed-tpl:', '', d)}"
> >   MACHINEOVERRIDES =. "rockchip:"
> > +
> >   # kernel
> > +RK_FITIMAGE ?= "True"
> 
> What about using a more typical 0/1 like we do for other variables?
> ENABLE_STATELESS_VPU_GST or RKBIN_DDR_RECONFIGURE for example?

lol, the last time i used a 0/1 you said: a boolean would be more appropriate
;-)

> 
> >   PREFERRED_PROVIDER_virtual/kernel ?= "linux-yocto"
> >   KCONFIG_MODE ?= "alldefconfig"
> > diff --git a/conf/machine/include/rockchip-extlinux.inc b/conf/machine/include/rockchip-extlinux.inc
> > index fddab735bbf7..95a38d4dad0c 100644
> > --- a/conf/machine/include/rockchip-extlinux.inc
> > +++ b/conf/machine/include/rockchip-extlinux.inc
> > @@ -13,13 +13,13 @@ NONFITDT ?= "${@d.getVar('KERNEL_DEVICETREE').split()[0].split('/')[1]}"
> >   UBOOT_EXTLINUX ?= "1"
> >   UBOOT_EXTLINUX_ROOT ?= "root=PARTLABEL=rootfsA"
> > -UBOOT_EXTLINUX_FDTDIR ?= "${@bb.utils.contains('KERNEL_IMAGETYPE', 'fitImage', '', '/boot', d)}"
> > -UBOOT_EXTLINUX_FDT ?= "${@bb.utils.contains('KERNEL_IMAGETYPE', 'fitImage', '', '/boot/${NONFITDT}', d)}"
> > +UBOOT_EXTLINUX_FDTDIR ?= "${@ '' if bb.utils.to_boolean(d.getVar('RK_FITIMAGE'),False) else '/boot'}"
> > +UBOOT_EXTLINUX_FDT ?= "${@ '' if bb.utils.to_boolean(d.getVar('RK_FITIMAGE'),False) else '/boot/${NONFITDT}'}"
> 
> I guess you can avoid the ,False part as the default is None, which should
> fail your if condition. Just a matter of taste though.

If that works, great! I dislike the silly ",False" part.

> >   UBOOT_EXTLINUX_CONSOLE ?= "earlycon console=tty1 console=${RK_CONSOLE_DEVICE},${RK_CONSOLE_BAUD}n8"
> >   UBOOT_EXTLINUX_KERNEL_ARGS ?= "rootwait rw rootfstype=ext4"
> > -UBOOT_EXTLINUX_KERNEL_IMAGE ?= "/boot/${KERNEL_IMAGETYPE}"
> > +UBOOT_EXTLINUX_KERNEL_IMAGE ?= "/boot/${@ 'fitImage' if bb.utils.to_boolean(d.getVar('RK_FITIMAGE'),False) else '${KERNEL_IMAGETYPE}'}"
> 
> Ditto.
> 
> >   UBOOT_EXTLINUX_LABELS ?= "default"
> >   UBOOT_EXTLINUX_MENU_DESCRIPTION:default ?= "${MACHINE}"
> >   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "u-boot-extlinux"
> > -MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "kernel-image ${@bb.utils.contains('KERNEL_IMAGETYPE', 'fitImage', '', 'kernel-devicetree', d)}"
> > +MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "${@ 'linux-yocto-fitimage' if bb.utils.to_boolean(d.getVar('RK_FITIMAGE'),False) else 'kernel-devicetree'}"
> > diff --git a/conf/machine/include/rockchip-fitimage.inc b/conf/machine/include/rockchip-fitimage.inc
> > new file mode 100644
> > index 000000000000..e275faf70349
> > --- /dev/null
> > +++ b/conf/machine/include/rockchip-fitimage.inc
> > @@ -0,0 +1,4 @@
> > +KERNEL_CLASSES += "kernel-fit-extra-artifacts"
> > +KERNEL_IMAGETYPE = "Image"
> > +RRECOMMENDS:${KERNEL_PACKAGE_NAME}-base = ""
> > +KERNEL_DEPLOY_DEPEND = "linux-yocto-fitimage:do_deploy"
> 
> Should this be a ?= so that we can easily override/change it?

Sounds good, probably the KERNEL_IMAGETYPE too?


> Looks ok to me otherwise, I have px30, rk3399 and rk3588 boards which do not
> use fit images so I'll try to check this next week. Considering
> meta-rockchip doesn't build at all anymore, I guess you may want to fix this
> ASAP your way and I can figure out a way to fix it up "further" for
> non-fitimages scenario?).

I agree. But i think there are some v2 things i could do now.

> 
> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1640): https://lists.yoctoproject.org/g/yocto-patches/message/1640
> Mute This Topic: https://lists.yoctoproject.org/mt/113621966/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 


  reply	other threads:[~2025-06-13 14:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-13 10:23 [meta-rockchip][PATCH 1/2] trusted-firmware-a: remove no-longer-required patches Trevor Woerner
2025-06-13 10:23 ` [meta-rockchip][PATCH 2/2] fitimage rework Trevor Woerner
2025-06-13 12:32   ` [yocto-patches] " Quentin Schulz
2025-06-13 14:42     ` Trevor Woerner [this message]
2025-06-13 12:11 ` [yocto-patches] [meta-rockchip][PATCH 1/2] trusted-firmware-a: remove no-longer-required patches Quentin Schulz
2025-06-13 15:05   ` Trevor Woerner
2025-06-13 15:06 ` Trevor Woerner

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=20250613144224.GA6216@localhost \
    --to=twoerner@gmail.com \
    --cc=yocto-patches@lists.yoctoproject.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.