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

Support Nuvoton's new target NUMAKER_IOT_M263A #11122

Merged
merged 14 commits into from
Aug 26, 2019

Conversation

cyliangtw
Copy link
Contributor

Description

This PR adds support for Nuvoton's new target NUMAKER_IOT_M263A, which is Cortex-M23 without Trustzone based.

Pull request type

[ ] Fix
[ ] Refactor
[x] New Target
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Greentea & CI test log

Pass Greentea test in GCC, ARMCC & IAR tool-chain.
Snippet of Greentea test log as below:

mbedgt: test case report:
| target                  | platform_name     | test suite                                                                           | test case                                                                                         | passed | failed | result | elapsed_time (sec) |
|-------------------------|-------------------|--------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------|--------|--------|--------|--------------------|
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-features-device_key-tests-device_key-functionality                           | Device Key - derived key consistency 16 byte key                                                  | 1      | 0      | OK     | 0.16               |
.............................................
stack memory                                                         | 1      | 0      | OK     | 0.0                |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-mbedtls-multi                                                          | Crypto: sha256_multi                                                                              | 1      | 0      | OK     | 0.08               |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-mbedtls-multi                                                          | Crypto: sha256_split                                                                              | 1      | 0      | OK     | 0.02               |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-mbedtls-selftest                                                       | mbedtls_entropy_self_test                                                                         | 1      | 0      | OK     | 0.03               |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-mbedtls-selftest                                                       | mbedtls_sha256_self_test                                                                          | 1      | 0      | OK     | 3.19               |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-mbedtls-selftest                                                       | mbedtls_sha512_self_test                                                                          | 1      | 0      | OK     | 4.69               |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-network-l3ip                                                           | L3IP_START                                                                                        | 1      | 0      | OK     | 0.0                |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | mbed-os-tests-network-l3ip                                                           | L3IP_STOP                                                                                         | 1      | 0      | OK     | 0.0                |
mbedgt: test case results: 670 OK
mbedgt: completed in 2297.04 sec
[mbed] Working path "C:\ARM_mbed\examples\mbed-test" (program)

Pass CI test in GCC, ARMCC & IAR tool-chain.

mbedgt: test suite report:
| target                  | platform_name     | test suite                  | result | elapsed_time (sec) | copy_method |
|-------------------------|-------------------|-----------------------------|--------|--------------------|-------------|
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-analogin          | OK     | 19.97              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-businout          | OK     | 35.41              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-digitalio         | OK     | 20.68              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-i2c               | OK     | 21.54              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-interruptin       | OK     | 20.41              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-pwm_fall          | OK     | 31.68              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-pwm_rise          | OK     | 31.67              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-pwm_rise_fall     | OK     | 183.88             | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-api-spi               | OK     | 20.82              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-analogin  | OK     | 22.84              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-digitalio | OK     | 21.32              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-i2c       | OK     | 19.0               | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-pwm       | OK     | 19.42              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-pwmout    | OK     | 19.05              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-assumptions-spi       | OK     | 19.61              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-concurrent-comms      | OK     | 20.85              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-concurrent-gpio       | OK     | 22.54              | default     |
| NUMAKER_IOT_M263A-ARMC6 | NUMAKER_IOT_M263A | tests-concurrent-mixed      | OK     | 21.26              | default     |
mbedgt: test suite results: 18 OK

For details, please open attached log files:
m263_greentea_gcc_0723.log
m263_greentea_iar_0726.log
m263_greentea_arm_0730.log
m263_ci_gcc_0723.log
m263_ci_iar_0723.log
m263_ci_arm_0730.log

@ciarmcom ciarmcom requested review from Ronny-Liu and a team July 30, 2019 09:00
@ciarmcom
Copy link
Member

@cyliangtw, thank you for your changes.
@Ronny-Liu @ARMmbed/mbed-os-maintainers please review.

@@ -0,0 +1,344 @@
/* mbed Microcontroller Library
Copy link
Member

Choose a reason for hiding this comment

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

@Patater could you review the crypto related parts?

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

Thanks for that, it looks good. Could you re-license all the files to be Apache? https://os.mbed.com/docs/mbed-os/v5.13/contributing/index.html#licensing

"FLASH_CMSIS_ALGO"
],
"macros": [
"MBED_FAULT_HANDLER_DISABLED",
Copy link
Member

Choose a reason for hiding this comment

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

It's disabled by default. But what's the reason behind this flag here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bulislaw , it's mbed_fault_handler.c not support Cortex-M23 in early June. However, last mbed_fault_handler.c seems consider TARGET_M23. Let me do some verification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bulislaw , After verification, it could support Cortex-M23 and not disabled it by default. Please see the commit 70c8a3b

"release_versions": ["5"],
"device_name": "M263KIAAE",
"bootloader_supported": true,
"tickless-from-us-ticker": true,
Copy link
Member

Choose a reason for hiding this comment

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

That will disable deep sleep, is that necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bulislaw , not support deep sleep in this stage.

@cyliangtw
Copy link
Contributor Author

@bulislaw , about re-license to be Apache, I updated all hal files to fulfill SPDX-License-Identifier: Apache-2.0 in commit 606eb68

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

LGTM @ARMmbed/mbed-os-maintainers please have a second look

Copy link
Contributor

@Ronny-Liu Ronny-Liu left a comment

Choose a reason for hiding this comment

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

LGTM

* <b>Copyright Notice</b>
*
* Copyright (C) 2019 Nuvoton Technology Corp. All rights reserved.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this license matches what we expect from contributions.
Please see: https://os.mbed.com/docs/mbed-os/v5.13/contributing/license.html

Do you have any other license available for this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SeppoTakalo , so far, seems many targets device folder still not follow your expectation of license, for ex: mbed-os\targets\TARGET_STM\TARGET_STM32F4\TARGET_MTS_MDOT_F411RE\device .
Anyway, I did re-license files of M261 device folder to fulfill SPDX-License-Identifier: Apache-2.0 in commit d863637

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SeppoTakalo, do you have any concern on this commit d863637 for re-license ?

* @brief Timer PWM Controller(Timer PWM) driver source file
*
* @copyright (C) 2019 Nuvoton Technology Corp. All rights reserved.
*****************************************************************************/
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like StdDriver is missing the acceptable license. Please add it (requested previously by @SeppoTakalo )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 , @SeppoTakalo ,
StdDriver come from SDK, so we try to keep the same originally for easy maintenance & upgrade.
After refer to other targets, could you accept the license as below:

  @copyright (C) 2019 Nuvoton Technology Corp. All rights reserved.
  
   Redistribution and use in source and binary forms, with or without modification,
   are permitted provided that the following conditions are met:
     1. Redistributions of source code must retain the above copyright notice,
        this list of conditions and the following disclaimer.
     2. Redistributions in binary form must reproduce the above copyright notice,
        this list of conditions and the following disclaimer in the documentation
        and/or other materials provided with the distribution.
     3. Neither the name of Nuvoton Technology Corp. nor the names of its contributors
        may be used to endorse or promote products derived from this software
        without specific prior written permission.
  
   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
   AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
   IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
   DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
   FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
   DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
   SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
   CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
   OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks fine to me (BSD-3 clause license as I understand it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 , @SeppoTakalo ,
Added acceptable license into std-drivers & register header files in commit a1e8122.
So, all the target relative files should fulfill your request.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2019

@cyliangtw Please review travis failures (device_name related). I aborted internal CI for now

@cyliangtw
Copy link
Contributor Author

@0xc0170 , the log show the device_name issue related to index.json of tools\arm_pack_manager, it's resolved in PR Add Nuvoton M261 sub-family #11171

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 13, 2019

@0xc0170 , the log show the device_name issue related to index.json of tools\arm_pack_manager, it's resolved in PR Add Nuvoton M261 sub-family #11171

+1 , the PR was merged today. I'll restart travis now

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 13, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Aug 13, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 13, 2019

GCC fails to build, please review

[DEBUG] Return: 1
[DEBUG] Output: /tmp/ccaIqElp.s: Assembler messages:
[DEBUG] Output: /tmp/ccaIqElp.s:989: Error: instruction not supported in Thumb16 mode -- `subs r0,r1'
[DEBUG] Output: /tmp/ccaIqElp.s:1055: Error: instruction not supported in Thumb16 mode -- `adds r2,#1'

@kjbracey
Copy link
Contributor

That error is #11102, fixed by #11208.

@cyliangtw
Copy link
Contributor Author

@0xc0170 , all travis-ci are success after rebase. Please check.

@mbed-ci
Copy link

mbed-ci commented Aug 22, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@cyliangtw
Copy link
Contributor Author

@0xc0170 , I added one more commit to avoid GCC tool chain issue mentioned in #10898.
Please start CI again.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 26, 2019

Ci restarted

@mbed-ci
Copy link

mbed-ci commented Aug 26, 2019

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 26, 2019

There was an example issue we fixed, restarting the CI now

@mbed-ci
Copy link

mbed-ci commented Aug 26, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 6
Build artifacts

@0xc0170 0xc0170 merged commit 421ad37 into ARMmbed:master Aug 26, 2019
@ccli8 ccli8 deleted the nuvoton_m263 branch October 23, 2019 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants