-
Notifications
You must be signed in to change notification settings - Fork 153
Rb3gen2: Add support for Open Boot firmware (TF-A, OP-TEE and U-Boot) build #1172
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?
Changes from 1 commit
074bc4f
297f464
d25d38f
aa97c88
beff201
5b04d7e
2376dc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,14 @@ KERNEL_DEVICETREE ?= " \ | |
| qcom/qcs6490-rb3gen2-vision-mezzanine.dtb \ | ||
| " | ||
|
|
||
| EXTRA_IMAGEDEPENDS += "${@bb.utils.contains('MACHINE_FEATURES', 'tfa', 'trusted-firmware-a', '', d)}" | ||
| PREFERRED_PROVIDER_virtual/bootloader = "${@bb.utils.contains('MACHINE_FEATURES','u-boot','u-boot','',d)}" | ||
| TFA_UBOOT:pn-trusted-firmware-a = "${@bb.utils.contains('MACHINE_FEATURES','u-boot','1','0',d)}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels strange at a first glance that machine configuration (which should also define MACHINE_FEATURES) also depends on them. Please add a comment here, how is it expected to be enabled. |
||
|
|
||
| MACHINE_ESSENTIAL_EXTRA_RRECOMMENDS += " \ | ||
| packagegroup-rb3gen2-firmware \ | ||
| packagegroup-rb3gen2-hexagon-dsp-binaries \ | ||
| packagegroup-optee \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably don't want when optee is not enabled/available. no?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added checks for "optee" as a machine feature within the "packagegroup-optee" since yocto-check-layers complained about it. Adding a machine feature check here felt redundant but I can add it if you want.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see. I think it should be the other way around. packagegroup-optee should include them always, and we should include the packagegroup here only when optee is enabled.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did that initially, but yocto-check-layers complained about "packagegroup-optee" as an independent entity since a machine compatible is required to build
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to figure out if this is the right thing to do..
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, let me add machine check here as well for consistency.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please keep the list sorted |
||
| " | ||
|
|
||
| QCOM_CDT_FILE = "cdt_core_kit" | ||
|
|
||
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.
is tfa a new MACHINE_FEATURE?
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.
Yeah, it is a new machine feature since you asked for it in previous review.
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 should document them, somehow, somewhere..
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.
Okay, I can document them here
conf/machine/include/qcom-common.inc.