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

M2354: Support Nuvoton M2354 #124

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Aug 13, 2021

This PR tries to upstream M2354 port to mainstream. It includes:

  1. Change trusted-firmware-m download path to Nuvoton forked repository (nuvoton_mbed_m2354_tfm-1.3) for M2354
  2. Add post-build copy paths specific to M2354 (tfm_ns_import.yaml)
  3. Detect python/python3 command name across platforms
  4. Enable Mbed CLI 2 (CMake)
  5. Enable Greentea test path
    1. Disable PSA compliance test, which M2354 hasn't supported yet
    2. Add compare log (REGRESSION.log only)

NOTE1: For upgrade to TF-M 1.4 for M2354, change branch to nuvoton_mbed_m2354_tfm-1.4.
NOTE2: About GCC toolchain, 9 2020-q2-update (and earlier)/10-2020-q4-major have issues on -Os/-mcmse respectively. This PR is verified with 10 2021.07.
NOTE3: If you run TEST_S tests, you will meet TFM_SP_PS_TEST test broken failure. It is caused by TFM_SP_PS_TEST stack overflow.

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

@ccli8 Thanks for the PR, it looks perfect to me!

Could you please add NU_M2354 to CI:
.travis.yml:

    # NU_M2354

    - <<: *compile-tests
      name: "Compile Regression tests - NU_M2354"
      env: TARGET_NAME=NU_M2354 CACHE_NAME=NU_M2354

.circleci/config.yml:

      - compile:
          target: "NU_M2354"

then we'll be able to see build results with Mbed CLI 1 & 2 in the PR.

@LDong-Arm LDong-Arm requested a review from a team August 13, 2021 09:46
@LDong-Arm LDong-Arm requested a review from a team August 13, 2021 09:49
@LDong-Arm
Copy link
Contributor

NOTE2: About GCC toolchain, 9 2020-q2-update (and earlier)/10-2020-q4-major have issues on -Os/-mcmse respectively. This PR is verified with 10 2021.07.

@ccli8 Thanks for letting us know. We're also planning to update GCC to 10 2021.07, so let me raise a PR to update GCC used in CI (earlier comment) first.

1.  Change trusted-firmware-m download path to Nuvoton forked repository (nuvoton_mbed_m2354_tfm-1.3)
2.  Add post-build copy paths specific to M2354 (tfm_ns_import.yaml)
3.  Detect python/python3 command name across platforms
4.  Enable Mbed CLI 2 (CMake)
5.  Enable Greentea test path
    (1) Disable PSA compliance test, which M2354 hasn't supported yet
    (2) Add compare log (REGRESSION.log only)
6.  Update CI configuration
7.  Update readme

NOTE1: Dummy the script for erasing flash storage for Greentea test because drag-n-drop flash invokes Mass Erase which will erase the whole flash.
NOTE2: As ITS/PS/NV counter regions are changed (flash_layout.h), the script for erasing flash storage needs update accordingly. But this can skip with above.
NOTE3: To cover root-RSA-3072.pem missing in TF-M install (https://developer.trustedfirmware.org/T957), change its source path for copy:
       From: install/image_signing/keys/root-RSA-3072.pem
       To: ../bl2/ext/mcuboot/root-RSA-3072.pem
NOTE4: For PSA Firmware Update test, in CMakeLists (for Mbed CLI 2), test lib tfm_test_suite_fwu_ns is needed.
NOTE5: REGRESSION.log needs to contain PSA Firmware Update test.
@ccli8 ccli8 force-pushed the nuvoton_mbed_m2354_tfm-1.3_wip branch from 036d47e to 2d3cd7e Compare August 13, 2021 10:21
@ccli8
Copy link
Contributor Author

ccli8 commented Aug 13, 2021

@LDong-Arm Updated .travis.yml and .circleci/config.yml

@LDong-Arm
Copy link
Contributor

@LDong-Arm Updated .travis.yml and .circleci/config.yml

Thanks. Now waiting for builds to finish.

The CI is build-only (no running), and older versions of GCC can still build it, just the image is invalid. So I think we can get this PR in without waiting for my toolchain upgrade PR.

@LDong-Arm LDong-Arm self-requested a review August 13, 2021 10:35
Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

LGTM. Ready for merge unless others have further comments?

@LDong-Arm LDong-Arm requested review from a team and Patater August 13, 2021 10:55
@LDong-Arm
Copy link
Contributor

@Patater It would be good to get this in before migrating to GitHub Actions for CI?

@Patater Patater merged commit 843e2d8 into ARMmbed:master Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants