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

Add GD32_E103VB as new target #9203

Merged
merged 4 commits into from
Jan 14, 2019
Merged

Add GD32_E103VB as new target #9203

merged 4 commits into from
Jan 14, 2019

Conversation

ChazJin
Copy link
Contributor

@ChazJin ChazJin commented Dec 27, 2018

Description

Pull request type

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

Reviewers

CC @Ronny-Liu @0xc0170 @ashok-rao please review when you're available, many thanks!

@ciarmcom
Copy link
Member

@ChazJin, thank you for your changes.
@ashok-rao @0xc0170 @Ronny-Liu@ARMmbed/mbed-os-maintainers please review.

@ChazJin
Copy link
Contributor Author

ChazJin commented Dec 27, 2018

The test results based on MBED OS 5.11 are as follows(WIN10, run "mbed test -t GCC_ARM/ARM/IAR -m GD32_E103VB" directly).

GD32E103VB_ARM_5_11.txt
GD32E103VB_IAR_5_11.txt
GD32E103VB_GCC_ARM_5_11.txt

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 2, 2019

Merge branch 'master' into gd32e10x

Is this merging latest master changes to your branch? Can this be eliminated by doing rebase?

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Waiting for master merge commit (otherwise code looks fine to me)

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 2, 2019

Merge branch 'master' into gd32e10x

Is this merging latest master changes to your branch? Can this be eliminated by doing rebase?

I resolve the conflict directly on the web, and I think this should be the latest master change. If you don't think it's appropriate, I'll try ‘git reset’ this commit and rebase it. Is that right? Thanks! @0xc0170

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 2, 2019

Please do, the web interface is not ideal in this case and rather resolve this locally

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 3, 2019

Please do, the web interface is not ideal in this case and rather resolve this locally

I have rebase it and resolve this locally. However, it seems that some error happens which leads to the Travis CI build could not complete in continuous-integration/travis-ci/pr. I would appreciate it if you could give me a few pointer. @0xc0170

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 3, 2019

This was travis error, restarted it

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 3, 2019

Waiting for @ashok-rao to review and/or @Ronny-Liu

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.

Should the device folder be outside of TARGET_GD32E103VB? There are GD32E1xx common file in the device folder.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 8, 2019

Ci started

@cmonr
Copy link
Contributor

cmonr commented Jan 10, 2019

Restarting jenkins-ci/exporter job.

Windows node closed its connection too soon.

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 11, 2019

Restarting jenkins-ci/exporter job.

Windows node closed its connection too soon.

Hi @cmonr, I found there is a conflicting file(target.json), I have resolved it and force-pushed gd32e10x branch which means CI test may be re-executed. Sorry for the trouble you've taken.

@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

@ChazJin Wow, what timing!
I had just finished doing the rebase on my side and was about to push to get this into CI tonight, thanks!

Commits look good, although I'm puzzled why Travis CI hasn't started. Will start CI once Travis has started and passed.

@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

Trying a thing.

@cmonr cmonr closed this Jan 11, 2019
@cmonr cmonr removed the needs: CI label Jan 11, 2019
@cmonr cmonr reopened this Jan 11, 2019
@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

Aaand Travis CI picked that up!

...it's been having issues lately...

@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 11, 2019

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

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

@cmonr
Copy link
Contributor

cmonr commented Jan 11, 2019

NOTICE

This PR is on hold because an issue in the master branch was found when this was rebased.
Once the issue is fixed, this PR will need to be rebased, and will immediately enter CI.

@mprse
Copy link
Contributor

mprse commented Jan 11, 2019

NOTICE
This PR is on hold because an issue in the master branch was found when this was rebased.
Once the issue is fixed, this PR will need to be rebased, and will immediately enter CI.

PR which adds boot/isr stack size configuration option and unifies boot/isr stack size (PR #9092) has been merged after GD32_F450ZI target has been added (PR #9232).
This caused the CI build failures on GD32_F450ZI/ARM (ARM_LIB_STACK symbol is now reqired ).

I have adapted linker scripts of the new target(GD32_F450ZI) in PR #9350. This should fix the CI issue.

Please adapt linker scripts in this PR. As a reference what has changed you can check PR #8039 which provides framework for configuring boot stack size and example update of the linker scripts.

Copy link
Contributor

@mprse mprse left a comment

Choose a reason for hiding this comment

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

Linker scripts looks good!

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 11, 2019

NOTICE
This PR is on hold because an issue in the master branch was found when this was rebased.
Once the issue is fixed, this PR will need to be rebased, and will immediately enter CI.

PR which adds boot/isr stack size configuration option and unifies boot/isr stack size (PR #9092) has been merged after GD32_F450ZI target has been added (PR #9232).
This caused the CI build failures on GD32_F450ZI/ARM (ARM_LIB_STACK symbol is now reqired ).

I have adapted linker scripts of the new target(GD32_F450ZI) in PR #9350. This should fix the CI issue.

Please adapt linker scripts in this PR. As a reference what has changed you can check PR #8039 which provides framework for configuring boot stack size and example update of the linker scripts.

Thank you very much. I just adapt linker scripts which refers to your revision and if there are any problems, please point them out. Thanks again for your support. @mprse

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 11, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 11, 2019

Test run: SUCCESS

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

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.

9 participants