-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fixes for SCP-firmware support #6789
Conversation
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.
- For "drivers: include stdbool.h from stpmic1_regulator.h":
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
- For the other commits, with CI fixed...
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Thanks for the review. I fixed the bug that made CI to fail. I'll apply the review tag if CI tests are happy. |
core/lib/scmi-server/sub.mk
Outdated
@@ -167,6 +167,7 @@ endif | |||
endif | |||
|
|||
cflags-lib-$(CFG_SCPFW_MOD_$4) += -DBUILD_HAS_MOD_$4 | |||
scpfw-cmake-flags-y += -DCFG_SCPFW_MOD_$4=$(CFG_SCPFW_MOD_$4) |
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.
@vingu-linaro: considering your P-R #6769, the change above would deserve to factorize declaration of CMake directives in SCP-firmware as part of your M-R #948:
patch for SCP-firmware/arch/none/optee/CMakeLists.txt
(or maybe to apply in SCP-firmware/product/optee/common/CMakeList.txt):
0001-product-optee-declare-CFG_SCPFW_MOD_xxx-CMake-direct.patch.txt
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.
@etienne-lms: Yes, I have added it into my SCP-firmware MR. I will push this new MR soon and will put your 2 other patches on top of this 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.
Thanks.
23ec2f1
to
a0d1aff
Compare
I have removed the change that creates CMake |
core/lib/scmi-server/sub.mk
Outdated
# CMake does not need to check the cross compilation toolchain since we do not | ||
# compile any source file with CMake, we only generate some SCP-firmware | ||
# files. | ||
scpfw-cmake-flags-y = -DCMAKE_C_COMPILER_WORKS=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.
should be
scpfw-cmake-flags-y += -DCMAKE_C_COMPILER_WORKS=1
to not overwrite other cmake config
made the change in #6769
@jforissier, I've applied your review tag on the commit you acked but I changed the other commit: @vingu-linaro noticed to stddef.h was missing in the stpmic1_regulator.h. |
For "drivers: include standard header files from stpmic1_regulator.h":
|
Add missing inclusion of stdbool.h and stddef.h in stpmic1_regulator.h. The issue was revealed when upgrading to latest SCP-firmware source tree. Fixes: 9cb0d51 ("drivers: stpmic1: export regulators API in a specific header file") Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Enable CMake directive CMAKE_C_COMPILER_WORKS to prevent SCP-firmware CMake configuration sequence to check the cross compilation toolchain since it is not needed here: OP-TEE only uses CMake to configure SCP-firmware, not to build source files. This change is required when building OP-TEE with CFG_SCMI_SCPFW=y and using a CMake >= 3.27.0. Suggested-by: Thomas Bourgoin <thomas.bourgoin@foss.st.com> Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com> Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
thanks. tag applied. |
These changes apply to the SCP-firmware OP-TEE currently points to (v2.13.0). They'll need to be considered also when we'll upgrade to a more recent version of SCP-firmware, as proposed in P-R #6769.