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

MIMXRT1050: Update to EVK Rev B #7105

Merged
merged 1 commit into from
Jun 22, 2018
Merged

Conversation

mmahadevan108
Copy link
Contributor

Description

  1. Add the IVT header to the binary as this is required for boot up
    This was earlier added by the DAPLink firmware. As it is no longer
    handled in DAPLink, the header needs to be added inside mbed.
  2. Update drivers
  3. Uses the SDRAM for data section, stack and heap
  4. EVK Rev A boards would continue to work, however you would need to update the DAPLink Interface firmware to the latest. Binary is available for download from www.nxp.com/opensda

Pull request type

[ ] Fix
[X] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@mmahadevan108
Copy link
Contributor Author

cc @maclobdell @0xc0170

@mmahadevan108
Copy link
Contributor Author

All mbed tests pass except for the below with GCC_ARM toolchain:

I am seeing a weird behavior when running the mbed test ‘tests-mbed_platform-error_handling’ using GCC_ARM toolchain.

1527872989.41][CONN][RXD] >>> Running case #3: 'Test error context capture'...
1527872989.47][CONN][INF] found KV pair in stream: {{__testcase_start;Test error context capture}}, queued...
1527872989.51][CONN][RXD] :127::FAIL: Expected 2147490068 Was 2147490068
1527872989.57][CONN][INF] found KV pair in stream: {{__testcase_finish;Test error context capture;0;1}}, queued...
1527872989.65][CONN][RXD] >>> 'Test error context capture': 0 passed, 1 failed with reason 'Assertion Failed'

As seen above the expected and actual seem to match, however the test is throwing this as an error. This is only seen with GCC_ARM toolchain.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 11, 2018

I am seeing a weird behavior when running the mbed test ‘tests-mbed_platform-error_handling’ using GCC_ARM toolchain.

How can we reproduce this error ? Is this error only visible after this update or also current master branch ?
cc @SenRamakri

0xc0170
0xc0170 previously approved these changes Jun 11, 2018
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.

LGTM but failing tests should be fixed

@mmahadevan108
Copy link
Contributor Author

I need some guidance on the failure. I do not see it with ARM and IAR toolchains.

Also I do not see if on other platforms with GCC_ARM.

@cmonr
Copy link
Contributor

cmonr commented Jun 12, 2018

@mmahadevan108 A shot in the dark. The failure might be a comparison between ((uint32_t) -1) and ((char) -1), or something equivalent, and htrun is up-casting the value.

@mmahadevan108
Copy link
Contributor Author

mmahadevan108 commented Jun 12, 2018

I add the below change and see the tests pass with GCC_ARM

diff --git a/TESTS/mbed_platform/error_handling/main.cpp b/TESTS/mbed_platform/error_handling/main.cpp
index 008033c3d..a09493d23 100644
--- a/TESTS/mbed_platform/error_handling/main.cpp
+++ b/TESTS/mbed_platform/error_handling/main.cpp
@@ -124,7 +124,7 @@ void test_error_context_capture()
mbed_error_status_t status = mbed_get_last_error_info( &error_ctx );
TEST_ASSERT(status == MBED_SUCCESS);
TEST_ASSERT_EQUAL_UINT(error_value, error_ctx.error_value);
- TEST_ASSERT_EQUAL_UINT(osThreadGetId(), error_ctx.thread_id);
+ TEST_ASSERT_EQUAL_UINT((uint32_t)osThreadGetId(), error_ctx.thread_id);

cmonr added a commit that referenced this pull request Jun 13, 2018
Discovered via #7105.
If both values are negative values, they are casted in such a way that -1 != -1. This small commit fixes that.
@mmahadevan108
Copy link
Contributor Author

Any update on this PR?

@cmonr
Copy link
Contributor

cmonr commented Jun 14, 2018

@mmahadevan108 My understanding was that this was waiting on #7202 to come in first.

@cmonr
Copy link
Contributor

cmonr commented Jun 15, 2018

@mmahadevan108 Aaand it's in! Rebase when you can and we can get CI started.

adbridge pushed a commit that referenced this pull request Jun 15, 2018
Discovered via #7105.
If both values are negative values, they are casted in such a way that -1 != -1. This small commit fixes that.
@mmahadevan108
Copy link
Contributor Author

Thanks. I have rebased and updated the PR. I can confirm that all tests passed with all toolchains.

cmonr
cmonr previously approved these changes Jun 15, 2018
@cmonr
Copy link
Contributor

cmonr commented Jun 15, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 15, 2018

Build : SUCCESS

Build number : 2358
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7105/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Jun 16, 2018

@mbed-ci
Copy link

mbed-ci commented Jun 16, 2018

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 18, 2018

/morph test
/morph export-build

cmonr
cmonr previously approved these changes Jun 18, 2018
Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

LGTM. Just some non-blockign comments.

*/

/* TEXT BELOW IS USED AS SETTING FOR TOOLS *************************************
!!GlobalInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. First time I've seen YAML used inside of a source file.

@@ -1,8 +1,11 @@
/*
* The Clear BSD License
Copy link
Contributor

Choose a reason for hiding this comment

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

Making a note here that this is compatible with our Apache 2 licensing.

https://spdx.org/licenses/BSD-3-Clause-Clear.html

/*************************************
* DCD Data
*************************************/
const uint8_t dcd_data[] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there any particular reason why this wasn't bunched up into multi-entry lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, maybe it was separated out for clarity. I do not want to make changes to this file as it comes from the SDK team and is used by the ROM code to initialize SDRAM during boot.

@mbed-ci
Copy link

mbed-ci commented Jun 18, 2018

Build : SUCCESS

Build number : 2373
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7105/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Jun 18, 2018

@mmahadevan108
Copy link
Contributor Author

I undid the change to this file to revert back to the previous value. However exporter still fails for IAR, any idea why?

@mbed-ci
Copy link

mbed-ci commented Jun 19, 2018

@mmahadevan108
Copy link
Contributor Author

@cmonr Anything else needed for this PR?

@cmonr
Copy link
Contributor

cmonr commented Jun 20, 2018

@mmahadevan108 We're still looking into this. Assuming that the MIMXRT1052xxx6A is supported by IAR 7, I don't see yet why this is still failing.

@mmahadevan108
Copy link
Contributor Author

mmahadevan108 commented Jun 20, 2018

Support for MIMXRT1052xxx6A was added in IAR 8.20.1. Was this exporter test added recently, it was not highlighted when the initial commit for MXRT was added in Dec 2017.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 21, 2018

Nothing new, I reviewed that PR back in February, seems like the example did not do anything there but passed as result, see http://mbed-os.s3-eu-west-1.amazonaws.com/builds/exporter/5826/PASS/MIMXRT1050_EVK/iar/060daa99c93aaa7ed40318c8e168f15bb12938d8_exporter_build_log_MIMXRT1050_EVK_iar.txt (@studavekar please review). Make iar compiled though.

Support for MIMXRT1052xxx6A was added in IAR 8.20.1

This is the issue. If that is true, this would become dependent on updating to the IAR 8.

@cmonr
Copy link
Contributor

cmonr commented Jun 21, 2018

@mmahadevan108 I think the reason that this wasn't originally an issue was because as of right now, exporting to IAR for this particular target is disabled for IAR. At least, this is what is indicated by running mbed export --supported |grep "MIMXRT1050_EVK\|Platform"

Support for MIMXRT1052xxx6A was added in IAR 8.20.1

If this support wasn't also included in IAR 7, then we can't take in the part of the changeset that specifies the target MCU. Otherwise we'll keep on seeing

Updating build tree...
WTD: Unavailable option selection: 'NXP MIMXRT1052xxx6A'.

I think if the change in tools/export/iar/iar_definitions.json is reverted, this PR should be able to pass the IAR export text by reverting to the current behavior of not actually being able to export to IAR 7.

1. Add the IVT header to the binary as this is required for boot up
   This was earlier added by the DAPLink firmware. As it is no longer
   handled in DAPLink, the header needs to be added inside mbed.
2. Update drivers

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
@mmahadevan108
Copy link
Contributor Author

Updated PR to remove IAR exporter support.

@cmonr
Copy link
Contributor

cmonr commented Jun 21, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 22, 2018

Build : SUCCESS

Build number : 2428
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7105/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Jun 22, 2018

Exporter Build : SUCCESS

Build number : 2057
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7105/

@mbed-ci
Copy link

mbed-ci commented Jun 22, 2018

@cmonr cmonr merged commit 24daf18 into ARMmbed:master Jun 22, 2018
@mmahadevan108 mmahadevan108 deleted the mxrt_add_ivt branch July 4, 2018 13:16
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.

4 participants