-
Notifications
You must be signed in to change notification settings - Fork 154
Multi-DTB Fitimage implementation #1274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
`qcom_fitimage.py` imports OE-Core provided FIT image generation script, `fitimage.py`, and extends it to support Qualcomm-specific requirements for DTB-only FIT images. Some enhancements include enabling compatibility string handling and custom '.its' generation for Qualcomm platforms. Signed-off-by: Kavinaya S <[email protected]>
Introduce qcom-dtb-metadata recipe to build `qcom-metadata.dtb`, which is required by fit-image.its for generating the FIT image (ITB). This recipe fetches sources from the qualcomm-linux/qcom-dtb-metadata repo and installs the generated `qcom-metadata.dtb` into the deploy directory. Signed-off-by: Kavinaya S <[email protected]>
This `dtb-fit-image.bbclass` creates fit-image.its and the corresponding FIT image (ITB). This class initializes the ITS structure, adds DTB sections to it including qcom-metadata.dtb, modifies ITS for Qualcomm specific requirements, and assembles the final image using mkimage. Signed-off-by: Kavinaya S <[email protected]>
Add U-Boot and FIT-related variables required to generate Qualcomm specific fitImage and ITS files. These include UBOOT_CONFIG, default DTB configuration, mkimage options and hash algorithm settings. Signed-off-by: Kavinaya S <[email protected]>
…ormat Update linux-qcom-dtbbin.bbclass to include dtb-fit-image.bbclass and generate multi-dtb.vfat containing qclinux_fit.img alongside combined-dtb.dtb. Signed-off-by: Kavinaya S <[email protected]>
Set compatible string as "qcom,qcs8275-iot" for monaco-evk.dtb to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible strings as "qcom,qcs9075-iot" and "qcom,qcs9075v2-iot" for lemans-evk.dtb to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible string as "qcom,qcm6490-idp" for qcm6490-idp.dtb to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible strings as "qcom,qcs615-adp" and "qcom,qcs615v1.1-adp" for qcs615-ride.dtb to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible string as "qcom,qcs8300-adp" for qcs8300-ride.dtb to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible strings for supported ride/ride-r3 dtbs to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
Set compatible strings for supported rb3gen2 dtbs to ensure proper configuration when generating FIT images. Signed-off-by: Kavinaya S <[email protected]>
| import oe.fitimage | ||
| from oe.fitimage import ItsNodeRootKernel, get_compatible_from_dtb | ||
|
|
||
| class QcomItsNodeRoot(ItsNodeRootKernel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to document the Qualcomm-specific requirements for DTB-only FIT images to be able to properly review this series of patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it seems to be documented here https://github.com/qualcomm-linux/qcom-dtb-metadata/blob/main/Documentation.md. it should have been clear in the PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment to this class explaining why it needs to extend ItsNodeRootKernel.
| # | ||
| # This file contains functions for Qualcomm specific DTB-only fitimage generation, | ||
| # which imports classes from OE-Core fitimage.py and enhances to meet Qualcomm FIT | ||
| # specifications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a link to https://github.com/qualcomm-linux/qcom-dtb-metadata/blob/main/Documentation.md here.
| @@ -0,0 +1,131 @@ | |||
| # Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather rename this file to be specific to DTB (e.g. qcom_dtb_fitimage or similar), as this will later confuse people since we do also expect u-boot users to leverage the traditional fit image from oe-core (making it just qcom_fit gives the impression that this is just a qcom implementation of the generic fit).
| @@ -0,0 +1,131 @@ | |||
| # Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. | |||
| # SPDX-License-Identifier: BSD-3-Clause-Clear | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are extending fitimage.py from OE, which is GPL-2.0-only, we should align the license here.
| def set_extra_opts(self, mkimage_extra_opts): | ||
| self._mkimage_extra_opts = shlex.split(mkimage_extra_opts) if mkimage_extra_opts else [] | ||
|
|
||
| # Override DTB section to allow compatible_override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a python-based function specific comment instead.
| self._fitimage_emit_one_section_config(conf_name, dtb_node) | ||
| else: | ||
| # Currently exactly one kernel is supported. | ||
| self._fitimage_emit_one_section_config(self._conf_prefix + "1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kernel?
| compatible = ret.stdout.strip().split() | ||
| except subprocess.CalledProcessError: | ||
| compatible = None | ||
| return compatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you just duplicated this function?
| UBOOT_CONFIG ??= "qcom" | ||
| UBOOT_CONFIG[qcom] ?= "qcom_defconfig" | ||
|
|
||
| # Pass additional options to mkimage command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should instead explain why the extra options are needed here.
| self.configurations.add_property('default', default_conf) | ||
|
|
||
| # Override mkimage assemble to inject extra opts | ||
| def run_mkimage_assemble(self, itsfile, fitfile): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change can probably go directly to oe-core upstream, you are just adding support for extra mkimage arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Waiting for https://patchwork.yoctoproject.org/project/oe-core/patch/[email protected]/ to be accepted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, replied asking for clarification for the reason why we need to customize the options here, it is not clear even in this PR.
|
|
||
| SRCREV = "6b9e4b9c093cba36a80035af103015f0a7d3f9fc" | ||
|
|
||
| PV = "0.1+git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This revision is exactly the v0.1 tag, no need for +git.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also rename the recipe to be specific to the version used, which is v0.1 (instead of _git).
|
|
||
| do_compile() { | ||
| oe_runmake | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need, already covered by base.bbclass.
| PV = "0.1+git" | ||
|
|
||
| inherit deploy | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do_configure[noexec] = "1"
|
|
||
| do_deploy() { | ||
| install -d ${DEPLOY_DIR_IMAGE} | ||
| install -m 0644 ${S}/qcom-metadata.dtb ${DEPLOY_DIR_IMAGE}/qcom-metadata.dtb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
${B} instead
| @@ -0,0 +1,111 @@ | |||
| inherit kernel-arch uboot-config | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uboot-config?
| python fitimage_init_rootnode() { | ||
| import sys, os | ||
| file_path = d.getVar('FILE') | ||
| customfit = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(file_path))), 'lib') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pathlib instead of this nested dirname calls.
| sys.path.insert(0, customfit) | ||
| bb.note("Added to sys.path (parse-time): %s" % customfit) | ||
| import oe.types | ||
| from qcom_fitimage import QcomItsNodeRoot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move import lines together to the initial ones, add an empty line after.
| d.getVar('UBOOT_SIGN_KEYNAME'), | ||
| oe.types.boolean(d.getVar('FIT_SIGN_INDIVIDUAL')), d.getVar('UBOOT_SIGN_IMG_KEYNAME') | ||
| ) | ||
| d.setVar("__fit_root_node", root_node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a more standard OE var, use caps here.
|
|
||
| python do_compile_qcom_fitimage() { | ||
| bb.build.exec_func('fitimage_generate_its', d) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to create all these functions here? It is fine to have it all under a single do_compile_qcom_fitimage function, similar to the way it is done at meta/classes-recipe/kernel-fit-image.bbclass.
|
|
||
| do_deploy_qcom_fitimage() { | ||
| install -m 0644 "${DEPLOYDIR}/fitImage" "${DEPLOY_DIR_IMAGE}/fitImage" | ||
| install -m 0644 "${DEPLOYDIR}/fit-image.its" "${DEPLOY_DIR_IMAGE}/fit-image.its" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how the normal kernel-fit is also deployed, pick a different name for both the binary and its.
|
|
||
| # Variables to generate ITS file | ||
| UBOOT_CONFIG ??= "qcom" | ||
| UBOOT_CONFIG[qcom] ?= "qcom_defconfig" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this related to fit at all?
The only thing we're using from u-boot is mkimage.
| UBOOT_CONFIG[qcom] ?= "qcom_defconfig" | ||
|
|
||
| # Pass additional options to mkimage command | ||
| UBOOT_MKIMAGE_EXTRA_OPTS = "-E -B 0x0001000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
| FIT_CONF_DEFAULT_DTB = "1" | ||
|
|
||
| # Set hash algorithm | ||
| FIT_HASH_ALG = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These options will also affect users using kernel-fit (e.g. u-boot users).
|
|
||
| DTBBIN_DEPLOYDIR = "${WORKDIR}/qcom_dtbbin_deploy-${PN}" | ||
| DTBBIN_SIZE ?= "4096" | ||
| DTBBIN_SIZE ?= "65536" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also impact the size of every dtb vfat image we generate.
|
We should find a way to also validate the ITS created here, as we are also relying on the base fit class from oe-core. Might be good to create a selftest for this class. |
Add support for DTB-only FIT image generation
Extend OE-Core FIT image script to handle Qualcomm-specific needs:
This enables packaging multiple DTBs in one image for different SoCs and boards, allowing UEFI firmware to
select the correct DTB dynamically during boot.