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

UNO_91H: fix MPU compilation issue #9274

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Conversation

caixue1102
Copy link
Contributor

@caixue1102 caixue1102 commented Jan 7, 2019

Description

Disable MPU as it's not valid configuration for this target UNO target

Pull request type

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

Reviewers

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.

@caixue1102 Fill in also description and PR type in the first comment. This does not disable MPU but is adding EMAC support for a target?

@@ -35,7 +35,8 @@
!defined(TARGET_MTB_ADV_WISE_1530) && \
!defined(TARGET_MTB_USI_WM_BN_BM_22) && \
!defined(TARGET_MTB_MXCHIP_EMW3166) && \
!defined(TARGET_MTB_UBLOX_ODIN_W2)
!defined(TARGET_MTB_UBLOX_ODIN_W2) && \
!defined(TARGET_UNO_91H)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you align this line



/* Interface implementation */
//WiFiInterface::WiFiInterface(EMAC &emac, OnboardNetworkStack &stack) :
Copy link
Contributor

Choose a reason for hiding this comment

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

remove dead code

@@ -0,0 +1,165 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

add license header please

@@ -0,0 +1,333 @@
#include <stdlib.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

license header missing

{
/* Drop this packet */
LWIP_DEBUGF(NETIF_DEBUG, ("low_level_input pbuf_alloc fail, rxlen:%d\n", len));
//LINK_STATS_INC(link.memerr);
Copy link
Contributor

Choose a reason for hiding this comment

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

please clean-up files (no commented out code)

@caixue1102 caixue1102 changed the title Disable MPU fix MPU compilation issue Jan 8, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 8, 2019

@caixue1102 Can you edit the first commit to introduce this changes please? Why __MPU_PRESENT is set to 0 ?

@kyliuxing
Copy link
Contributor

@caixue1102 Can you edit the first commit to introduce this changes please? Why __MPU_PRESENT is set to 0 ?

@0xc0170 UNO_91H didn't support MPU,if set __MPU_PRESENT to 1 will introduce compilation error. Thanks!

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 8, 2019

Please read https://os.mbed.com/docs/v5.11/contributing/workflow.html (Guidelines for GitHub pull requests). Will be helpful for the following contributions (how to describe you changes in Github and commit messages).

Ideally, the fix would contain a new paragraph stating UNO_91H does not support MPU. You can update it, however this time would be fine as it is.

@NirSonnenschein
Copy link
Contributor

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

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 9, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 9, 2019

Test run: SUCCESS

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

@cmonr cmonr merged commit 7addb80 into ARMmbed:master Jan 10, 2019
@0xc0170 0xc0170 changed the title fix MPU compilation issue UNO_91H: fix MPU compilation issue Jan 10, 2019
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.

None yet

6 participants