Skip to content

Add GD32_F450ZI as new target #9232

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 10, 2019
Merged

Add GD32_F450ZI as new target #9232

merged 5 commits into from
Jan 10, 2019

Conversation

ChazJin
Copy link
Contributor

@ChazJin ChazJin commented Jan 3, 2019

Description

Pull request type

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

Reviewers

CC @Ronny-Liu @0xc0170 @ashok-rao. I would appreciate if you could review it when you are free.

@ciarmcom ciarmcom requested review from 0xc0170, Ronny-Liu and a team January 3, 2019 12:00
@ciarmcom
Copy link
Member

ciarmcom commented Jan 3, 2019

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

@bulislaw
Copy link
Member

bulislaw commented Jan 3, 2019

Can we have a complete set of tests for all 3 compilers?

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 4, 2019 •
edited
Loading

Can we have a complete set of tests for all 3 compilers?

OK. The mbed test results based on MBED OS 5.11 are as follows(WIN10, run "mbed test -t ARM/GCC_ARM/IAR -m GD32_F450ZI" directly).
GD32F450ZI_ARM_5_11.txt
GD32F450ZI_GCC_ARM_5_11.txt
GD32F450ZI_IAR_5_11.txt

Ronny-Liu
Ronny-Liu reviewed Jan 4, 2019
View reviewed changes
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.

@ChazJin Do you have low power timing resource provided by hardware ?

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 4, 2019 •
edited
Loading

@ChazJin Do you have low power timing resource provided by hardware ?

@Ronny-Liu Low power timing is not supported on hardware for GD32F450ZI, so LPTICKER is not added to target.json for the time being.

Ronny-Liu
Ronny-Liu approved these changes Jan 4, 2019
View reviewed changes
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.

@ChazJin Do you have low power timing resource provided by hardware ?

Ronny-Liu
Ronny-Liu reviewed Jan 4, 2019
View reviewed changes
Copy link
Contributor

@Ronny-Liu Ronny-Liu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@ChazJin Any user button defined on this board? I don't see this in pinName.
I saw KEY1 KEY2 defined. You'd better still define standardized name BUTTON1, BUTTON2, etc...

0xc0170
0xc0170 requested changes Jan 4, 2019
View reviewed changes
@0xc0170 0xc0170 requested a review from a team January 7, 2019 09:46
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 7, 2019

@ARMmbed/mbed-os-crypto Please review TRNG HAL implementation

yanesca
yanesca approved these changes Jan 7, 2019
View reviewed changes
Copy link
Contributor

@yanesca yanesca left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Ronny-Liu
Ronny-Liu suggested changes Jan 8, 2019
View reviewed changes
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.

Why is the device folder placed under TARGET_GD32F450ZI? There are files named gd32f4xx.h, etc , which should be f4xx common file, should they be outside of TARGET_GD32F450ZI?

@NirSonnenschein
Copy link
Contributor

Hi @ChazJin,
The Travis failures are caused by an issue fixed on master a few hours ago, and this change should be propagated here via rebase. We can start CI only after travis is fixed, could you please rebase this PR?
Sorry for the trouble

@ChazJin
Copy link
Contributor Author

ChazJin commented Jan 9, 2019 •
edited
Loading

Hi @ChazJin,
The Travis failures are caused by an issue fixed on master a few hours ago, and this change should be propagated here via rebase. We can start CI only after travis is fixed, could you please rebase this PR?
Sorry for the trouble

Hi @NirSonnenschein, I just used git rebase upstream/master and git push -f to my remote branch. I would like to ask is there anything inappropriate in doing so? If so, please correct my approach. Thanks a lot!

PS: I think our other PR(#9203) faces the same problem. Whether it is in the same operation?

@ChazJin ChazJin mentioned this pull request Jan 9, 2019
Merged
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 9, 2019

Looks fine after rebase (Travis passed), will start CI now

@cmonr
Copy link
Contributor

cmonr commented Jan 9, 2019

@ChazJin Can you rebase this PR?

A Travis CI fix was brought in yesterday.

@cmonr
Copy link
Contributor

cmonr commented Jan 9, 2019

#9232 (comment)

Disregard. Really irritated that GitHub caches PR pages.

@mbed-ci
Copy link

mbed-ci commented Jan 10, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 10, 2019

One target failure not related for this PR, we will restart tests soon

@cmonr
Copy link
Contributor

cmonr commented Jan 10, 2019

Restarted jenkins-ci/greentea-test job.

@cmonr cmonr merged commit 12980f4 into ARMmbed:master Jan 10, 2019
@0xc0170 0xc0170 mentioned this pull request Jan 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@yanesca yanesca yanesca approved these changes

@Ronny-Liu Ronny-Liu Ronny-Liu approved these changes

Assignees
No one assigned
Labels
release-version: 5.11.2 scope: new-target
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@ChazJin @ciarmcom @bulislaw @0xc0170 @NirSonnenschein @cmonr @mbed-ci @yanesca @Ronny-Liu @adbridge