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

Nuvoton: Support export IAR8 project #10282

Merged
merged 1 commit into from Apr 4, 2019

Conversation

Projects
None yet
8 participants
@ccli8
Copy link
Contributor

commented Apr 1, 2019

Description

This PR adds support for export IAR8 project on Nuvoton targets:

  • NUMAKER_PFM_NANO130
  • NUMAKER_PFM_NUC472
  • NUMAKER_PFM_M453
  • NUMAKER_PFM_M487/NUMAKER_IOT_M487

With IAR project successfully exported, to successfully develop and debug on Nuvoton targets, it needs extra steps. See doc here.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Apr 1, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

@ccli8, thank you for your changes.
@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@0xc0170

0xc0170 approved these changes Apr 1, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

With IAR project successfully exported, to successfully develop and debug on Nuvoton targets, it needs extra steps. See doc here.

Will this be doc anywhere else , at least referenced ? I read the doc, don't understand the last section - "Check configurations in IAR EWARM IDE" - if I do export now, would I need to do any of these steps or would it work out of the box?

@bridadan
Copy link
Contributor

left a comment

You're changes look fine to me, but I also have the same questions as @0xc0170. If these are manual changes, perhaps these could be integrated into the exporter template so users don't have to make these changes.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 2, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 1
Build artifacts

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

This looks ready to be brought in, but @ccli8 would you mind answering the reviewers' thoughts/concerns before we do?

[Nuvoton] Support export IAR8 project
1. Override IlinkOverrideProgramEntryLabel and IlinkProgramEntryLabel to specify
   entry point for debuger.
2. Refer to doc at the link below for post-export steps. Usually, 'export' is nearly
   out of the box and just install 'Nu-Link Driver (IAR)' to update Nuvoton device
   database in IAR.
   https://github.com/OpenNuvoton/NuMaker-mbed-docs/blob/master/IAR/DEBUG_IAR.md

@ccli8 ccli8 force-pushed the OpenNuvoton:nuvoton_export-iar8 branch from 816dd4d to 50f6870 Apr 2, 2019

@ccli8

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

@0xc0170 @bridadan Update to make the exported IAR project out of the box. Now, the only extra step is to install Nu-Link Driver (IAR). The Debug IAR doc is also updated to reflect the change. I will ask addition of its link in board pages.

@cmonr cmonr requested review from bridadan and 0xc0170 Apr 2, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

CI restarted whilst waiting on re-reviews since CI load is light atm.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

@ccli8 Why do these target need different entry point ? All targets should use the same boot sequence.
It should be __iar_program_start , defined in mbed_boot_iar.c

Changes looks fine, not certain if this is needed - do we need to change the default boot sequence - why?

Checking one of the targets here. There are couple of reset handlers (3), and iar_program_start is called in Reset_Handler_2. Do we miss anything in the boot sequence?

@ccli8

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

@0xc0170 Most targets reboots from Reset_Handler, calls SystemInit and then __iar_program_start. Nuvoton targets also follow this boot sequence. In debugger, PC will directly set to entry point on reset (per my test). If entry point is set to __iar_program_start, Reset_Handler code is not run. This will cause problem for e.g. NUMAKER_PFM_M487 target, in which some extra RAM initializatioin is done in SystemInit.

@bridadan
Copy link
Contributor

left a comment

Tools changes look fine, thanks! (Though the conversation that @0xc0170 should probably be addressed before merging this)

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

@ARMmbed/mbed-os-core
@ccli8 made some good observations about IAR8's debugger reset behavior. Should this change be looked at for all exports?

Oh, and I don't think that this target-specific change be blocked on said question since this change appears to be properly target-specific.

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 2, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 2
Build artifacts

@cmonr cmonr added ready for merge and removed needs: review labels Apr 4, 2019

@0xc0170 0xc0170 merged commit 8b5157b into ARMmbed:master Apr 4, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+32 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9149 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@0xc0170 0xc0170 removed the ready for merge label Apr 4, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

We should look at this if we should not make the change to have this by default set to Reset_Handler (not iar program start function).

@ccli8 ccli8 deleted the OpenNuvoton:nuvoton_export-iar8 branch Apr 8, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

@ccli8 Is there anything needed for this PR? I run nightly on 5.12.1 and some of these targets fail IAR8 exporter with the error: examples\mbed-os-example-blinky\mbed-os\cmsis\TARGET_CORTEX_M\cmsis_iccarm.h(94) : Fatal Error[Pe035]: #error directive: "Unknown target." Error while running C/C++ Compiler or similar.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

You should be able to reproduce using blinky: Exporting mbed-os-example-blinky NUMAKER_PFM_NANO130 iar - it fails

@ccli8

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

It seems that Nu-Link_IAR_Driver is not installed in your test environment.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Created internal ticket MBEDOSTEST-589

@OPpuolitaival

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

@timurh01 can you install missing package?

@timurh01

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

@0xc0170 @OPpuolitaival IAR driver is now installed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.