Skip to content
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

Fix compile error for Mbed TF-M V8M target #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ccli8
Copy link

@ccli8 ccli8 commented Jul 8, 2021

[x] I confirm this contribution is my own and I agree to license it with Apache 2.0.
[x] I confirm the moderators may change the PR before merging it in.
[x] I understand the release model prohibits detailed Git history and my contribution will be recorded to the list at the bottom of CONTRIBUTING.md.

Summary of changes

This PR tries to fix compile error for Mbed TF-M V8M target, usually without FLASHIAP:

  1. Use DEVICE_FLASH to exclude FLASHIAP code
  2. Fix MBED_MAJOR_VERSION isn't available, which is needed to exclude mbedtls_psa_crypto_free().

Fix compile error for Mbed TF-M V8M target, usually without FLASHIAP
1. Use DEVICE_FLASH to exclude FLASHIAP code
2. Fix MBED_MAJOR_VERSION doesn't get in to exclude mbedtls_psa_crypto_free().
@ccli8
Copy link
Author

ccli8 commented Jul 22, 2021

Have a look?

@marcuschangarm
Copy link

Thank you, I'll take a look.

@@ -14,6 +14,7 @@
// limitations under the License.
// ----------------------------------------------------------------------------
#ifdef MBED_CONF_MBED_CLOUD_CLIENT_PSA_SUPPORT
#include "platform/mbed_version.h"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccli8
I don't see any of the #define in this file being used: https://github.com/ARMmbed/mbed-os/blob/master/platform/include/platform/mbed_version.h
What compile errors are you seeing without this include file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcuschangarm I met link error with undefined mbedtls_psa_crypto_free symbol:

Compile [100.0%]: psa_driver_crypto.c
[Warning] psa_driver_crypto.c@547,5: implicit declaration of function 'mbedtls_psa_crypto_free' is invalid in C99 [-Wimplicit-function-declaration]
Link: mbed-cloud-client-example
[Warning] @0,0: L3912W: Option 'legacyalign' is deprecated.
[Error] @0,0: L6218E: Undefined symbol mbedtls_psa_crypto_free (referred from BUILD/NU_M2354/ARM/mbed-cloud-client/factory-configurator-client/psa-driver/source/psa_driver_crypto.o).

Checking the lines below, MBED_MAJOR_VERSION is not available, which is defined in mbed_version.h:

SA_PV_LOG_TRACE_FUNC_ENTER_NO_ARGS();
#if !(defined(TARGET_TFM) && (MBED_MAJOR_VERSION > 5))
mbedtls_psa_crypto_free();
#endif
SA_PV_LOG_TRACE_FUNC_EXIT_NO_ARGS();

@teetak01 teetak01 added this to the 4.11.0 milestone Aug 17, 2021
@teetak01 teetak01 added the Approved PR has been approved and will be integrated to internal development repository label Aug 17, 2021
@teetak01
Copy link
Contributor

Thanks @ccli8 @marcuschangarm

I'll mark this for next public release as the changes have already been integrated internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved PR has been approved and will be integrated to internal development repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants