Support Nuvoton's NUMAKER_PFM_M2351 target #7302
Conversation
ARMC6/M2351 Greentea test report |
ARMC6/M2351 CI test report |
|
@ccli8 - Thanks for the PR and great work. I will test it locally and share the results soon. Only concern is related to multiple hex files, our plan was to merge secure/non-secure hex files and flash as single image. I understand we need some changes in our tool to do that, @theotherjimmy can comment more on that. @0xc0170 - Question will be to allow multiple binaries till we have support in tools? Please note Nuvoton firmware does support flashing of multiple hex files. |
That is a question for @theotherjimmy cc @bulislaw |
|
Target device entry in json file consist of Building test for
I suspect error is because platform_name is |
|
@ccli8 Yes please see the mbed-ls mocking documentation https://github.com/ARMmbed/mbed-ls#mocking-renaming-platforms |
|
I have verified green tea test for Version 1.0 board and it was successful. Testing with Version 1.1 / Version 1.2 is pending |
targets/targets.json
Outdated
| "inherits": ["NUMAKER_PFM_M2351"], | ||
| "device_has_remove": ["TRNG"] | ||
| }, | ||
| "NUMAKER_PFM_M2351_NS": { |
There was a problem hiding this comment.
Do we need NUMAKER_PFM_M2351_NS? Can we use NUMAKER_PFM_M2351 only for non-secure?
There was a problem hiding this comment.
Could we not have the _S variant either, and rely on the target.core override.
There was a problem hiding this comment.
_S variant is required, else in secure examples we will have to suggest code change in targets.json which is not a good idea.
_NS is actually the duplicate, hence I suggested to remove that.
There was a problem hiding this comment.
A few things:
target.coreis an allowed override inmbed_app.jsontarget.device_hasis also override-able inmbed_app.json- The difference between
_Sand_NSistarget.coreandtarget.device_has - There are core-specific scan rules available
This implies that the base target can be converted into the 2 variants entirely with mbed_app.json.
I recommend that we do not have variants, default to Non-Secure, and allow users to override or create a custom target for compiler secure partitions.
There was a problem hiding this comment.
NUMAKER_PFM_M2351 defaults to non-secure target.
NUMAKER_PFM_M2351_S means secure target.
NUMAKER_PFM_M2351_NS means non-secure target and is duplicate of NUMAKER_PFM_M2351.
The presence of NUMAKER_PFM_M2351_S/NUMAKER_PFM_M2351_NS is to avoid ambiguous with just NUMAKER_PFM_M2351. Besides, it may not so straightforward for user to create these variants in mbed_app.json.
There was a problem hiding this comment.
@ccli8 This creates confusion: As a user, I have to know what variant I'm using instead of "pick my board and go". We have the same complaint with the NRF51 based targets and their variants. I would find it especially confusing if one of the variants is a duplicate!
You imply that it's complicated or difficult to add the differences between the _S and base variant changes to mbed_app.json, yet the targets differ by core and a single entry in device_has. Have you tried it? What makes you say that it would be difficult?
There was a problem hiding this comment.
@ccli8 - Also we do not want user to think Mbed OS can be secure, _S variant is only to help in development of secure side, it should not be the final secure application for a target.
_S in Mbed OS code will give false impression to users. We expect target owners to place a secure binary / cmse library and any required header file in target specific folder for Nuvoton M2351 it should be TARGET_NUVOTON\TARGET_M2351\TARGET_NUMAKER_PFM_M2351
There was a problem hiding this comment.
@deepikabhavnani - For easy hands on, Nuvoton could provide one secure binary/cmse lib. However, secure code could encapsulate algorithm and partition HW resource/IPs dependent on each domain application. For example, some system regards SPI as secure, another system regards SPI as non-secure. In echo system, some company or maker must take over both secure + non-secure code, some relies on IDH, IDH will be major on secure code development and system house could develop or customize non-secure code based on IDH's secure binary/cmse lib.
If developer needs to use multiple SDK for secure/non-secure developing, it will be more confused & difficult than proprietary SDK/BSP.
So, in IoT platform viewpoint, it seems mandatory to fulfill both of secure code and non-secure code development, especially secure code dependent on each IDH's system architecture. Thus, strongly recommend to keep _S, else I can't image how to promote.
There was a problem hiding this comment.
I'm going to continue to say that the _S variant is not useful, as the following mbed_app.json does the same thing:
{
"target_overrides": {
"NUMAKER_PFM_M2351": {
"target.core": "Cortex-M23",
"target.device_has_remove": ["TRNG"]
}
}
}anyone that needs to switch often can add the target into custom_targets.json or create multiple application configurations.
There was a problem hiding this comment.
@deepikabhavnani @theotherjimmy I remove NUMAKER_PFM_M2351_S/NUMAKER_PFM_M2351_NS targets from targets.json in d82c914. The secure code/non-secure code are updated accordingly.
Greentea test on Version 1.2 was successful as well |
|
@deepikabhavnani So we have GT tests with version 1.1 remaining? |
|
@theotherjimmy - I don;t think there is version 1.1. Release is with hardware version 1.2 only. I did testing with version 1.0 with some changes as version 1.2 was not available to me at that time. Test success with version 1.2 is only required. |
|
@deepikabhavnani - Yes, test with board v1.2 is only required. |
|
Marked as dependent on #7029 |
|
Thanks for cleaning up |
|
@ccli8 Can you rebase? PR 7029 was merged |
|
@ccli8 - Build on linux system fails and was fixed with below change. Please add the fix. |
@0xc0170 I've done rebase. |
ccli8
force-pushed
the
nuvoton_m2351_v1.1
branch
from
5e33139 to
455f1fd
Compare
|
@deepikabhavnani I change default_toolchain from GCC_ARM to ARMC6 and merge your change with supported_toolchains into one commit 455f1fd. |
|
@deepikabhavnani @studavekar Change added that fixes CI. |
|
/morph build |
Build : SUCCESSBuild number : 2611 Triggering tests/morph test |
Test : SUCCESSBuild number : 2362 |
NUMAKER_PFM_M2351 not present in this test |
Exporter Build : FAILUREBuild number : 2253 |
It's not added we need to check the stability of devices as it's not running DAPLINK, if the device doesn't have stable interface firmware it will not be added. Since it will fail other PR. |
|
/morph build |
Build : SUCCESSBuild number : 2616 Triggering tests/morph test |
Exporter Build : FAILUREBuild number : 2257 |
Test : SUCCESSBuild number : 2365 |
|
/morph build |
|
Tested locally for ARMC6 mbedgt: test case results: 542 OK |
Build : SUCCESSBuild number : 2626 Triggering tests/morph test |
Test : SUCCESSBuild number : 2373 |
Exporter Build : SUCCESSBuild number : 2265 |
Support Nuvoton's NUMAKER_PFM_M2351 target
Description
This PR supports Nuvoton's new target NUMAKER_PFM_M2351, which is Cortex-M23 based.
Support toolchains
Pass Greentea/CI test
Need to fix toolchain issue with
cmse_nonsecure_callerandprintf/scanfDoesn't support yet because
cmsis_iccarm.hdoesn't support ARMv8M BL completely.Partition flash/SRAM/peripherals
This version doesn't support configurable for partitioning flash/SRAM/peripherals yet.
Build secure/non-secure code
Pull request type
@deepikabhavnani @cyliangtw