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 support for MIMXRT1170_EVK #14394

Closed
wants to merge 34 commits into from
Closed

Add support for MIMXRT1170_EVK #14394

wants to merge 34 commits into from

Conversation

s-bruce13
Copy link
Contributor

@s-bruce13 s-bruce13 commented Mar 8, 2021

Summary of changes

Adding Support for the MIMXRT1170_EVK

Impact of changes

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR
tests-mbed_hal-common_ticker*,tests-mbed_hal-us_ticker*
tests-mbed_hal-common_ticker*,tests-mbed_hal-lp_ticker*
build successfully

Reviewers


@mergify mergify bot added the needs: work label Mar 8, 2021
@mergify
Copy link

mergify bot commented Mar 8, 2021

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@ciarmcom
Copy link
Member

ciarmcom commented Mar 8, 2021

@s-bruce13, thank you for your changes.
@ARMmbed/team-nxp @ARMmbed/mbed-os-maintainers please review.

0xc0170
0xc0170 previously requested changes Mar 9, 2021
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.

Please describe your changes in the first section in your first comment (Summary of changes).

How was this tested, can you paste logs to confirm this run all tests?

@mergify mergify bot removed the needs: review label Mar 9, 2021
@mergify mergify bot dismissed 0xc0170’s stale review March 9, 2021 15:27

Pull request has been modified.

@s-bruce13
Copy link
Contributor Author

s-bruce13 commented Mar 9, 2021

I am trying to do pull request by pieces, so I am submitting the serial and timers first. Based on Porting Guide online I did these two tests.
mbed test -t -m -n tests-mbed_hal-common_ticker*,tests-mbed_hal-lp_ticker*
mbed test -t -m -n tests-mbed_hal-common_ticker*,tests-mbed_hal-us_ticker*
Memory map breakdown for built projects (values in Bytes):

name target toolchain static_ram total_flash

Build successes:

  • MIMXRT1170_EVK::GCC_ARM::MBED-BUILD
    mbedgt: greentea test automation tool ver. 1.8.3
    mbedgt: test specification file '.\BUILD\tests\MIMXRT1170_EVK\GCC_ARM\test_spec.json' (specified with --test-spec option)
    mbedgt: using '.\BUILD\tests\MIMXRT1170_EVK\GCC_ARM\test_spec.json' from current directory!
    mbedgt: detecting connected mbed-enabled devices...
    mbedgt: detected 1 device
    mbedgt: processing target 'MIMXRT1170_EVK' toolchain 'GCC_ARM' compatible platforms... (note: switch set to --parallel 1)
    mbedgt: all tests finished!
    mbedgt: shuffle seed: 0.0015540407

Memory map breakdown for built projects (values in Bytes):

name target toolchain static_ram total_flash

Build successes:

  • MIMXRT1170_EVK::GCC_ARM::MBED-BUILD
    mbedgt: greentea test automation tool ver. 1.8.3
    mbedgt: test specification file '.\BUILD\tests\MIMXRT1170_EVK\GCC_ARM\test_spec.json' (specified with --test-spec option)
    mbedgt: using '.\BUILD\tests\MIMXRT1170_EVK\GCC_ARM\test_spec.json' from current directory!
    mbedgt: detecting connected mbed-enabled devices...
    mbedgt: detected 1 device
    mbedgt: processing target 'MIMXRT1170_EVK' toolchain 'GCC_ARM' compatible platforms... (note: switch set to --parallel 1)
    mbedgt: all tests finished!
    mbedgt: shuffle seed: 0.1923834667
    mbedgt: no platform/target matching tests were found!
    mbedgt: no matching platforms were found!
    mbedgt: completed in 0.06 sec

Is there a different test I should be running?

@adbridge
Copy link
Contributor

@s-bruce13 you have modified the default header template by removing sections, please put them back and do not remove in the future - thanks.

@s-bruce13
Copy link
Contributor Author

@adbridge, i apologize i didnt realize i deleted sections. How can I retrieve the original template?

@adbridge
Copy link
Contributor

adbridge commented Mar 10, 2021

@s-bruce13 just click on create new PR and copy from there :) OR I can fix it for you this time

@adbridge
Copy link
Contributor

Fixed

@s-bruce13
Copy link
Contributor Author

Thank you!!

@s-bruce13
Copy link
Contributor Author

@0xc0170 Hello, I posted my test results. I am not sure if I should be running a different test than the ones mentioned in the porting guide. Could you please advice what I need to check?

targets/targets.json Outdated Show resolved Hide resolved
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 11, 2021

CI started

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 11, 2021

Is there a different test I should be running?

Yes, run all the tests, do not specify -n and report back with a log please.

@s-bruce13
Copy link
Contributor Author

@0xc0170 @hugueskamba @gpsimenos ok, thank you so should the last test "continuous-integration/jenkins/pr-head " be able to pass as is? or what needs to happen next for this pull request to be merged?

@gpsimenos
Copy link
Contributor

@0xc0170 @hugueskamba @gpsimenos ok, thank you so should the last test "continuous-integration/jenkins/pr-head " be able to pass as is? or what needs to happen next for this pull request to be merged?

Please rebase your branch as soon as #14457 is merged (should be very soon), and then your PR will be able to pass Jenkins CI.

@s-bruce13
Copy link
Contributor Author

@gpsimenos @0xc0170 @adbridge @hugueskamba
Hello All, I rebased with the pinnames merge that was done a little bit ago, recompiled and tested all is good to start the CI please.

@s-bruce13
Copy link
Contributor Author

@adbridge @0xc0170 @hugueskamba Hello all hope you are well. Is it possible to start the CI?

@adbridge
Copy link
Contributor

@s-bruce13 unfortunately we are having some CI issues currently and the CI team are on Easter vacation so we are restricted to running one PR at a time (this is in the queue).

@s-bruce13
Copy link
Contributor Author

@adbridge thank you for letting me know!

@s-bruce13
Copy link
Contributor Author

@adbridge Hello Anna, hope you are doing well. I just wanted to ask if you had a rough estimate as to a timeline on when the CI will be run based on the queue you mentioned?

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 6, 2021

CI started

0xc0170
0xc0170 previously requested changes Apr 6, 2021
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.

@s-bruce13 Looking at the history, can you rebase to fix some commits ("same" for instance could be squashed, and add better description to some commits). As this is a new target, this could be consolidated into couple of commits.

@mergify mergify bot added needs: work and removed needs: CI labels Apr 6, 2021
@mbed-ci
Copy link

mbed-ci commented Apr 6, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️

@s-bruce13
Copy link
Contributor Author

@0xc0170 Do I have to rebase with the latest release(5..15.7, is that correct)? also how can I confirm why it failed these 4 tests? There is no "failed" folder in the logs for the CI tests. for cloud example, there is a log for the mimxrt1170 in the "PASS" folder. Is this correct? I am and have been running all tests on my end and are passing, did I miss something?

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 6, 2021

Cmake error:

CMake Error at mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_MIMXRT1170/TARGET_EVK/CMakeLists.txt:4 (add_library):
  add_library cannot create target "mbed-evk" because another target with the
  same name already exists.  The existing target is an interface library
  created in source directory
  "/builds/workspace/mbed-os-ci_cmake-example-ARM@3/examples/mbed-os-example-blinky/mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_MIMXRT1050/TARGET_EVK".
  See documentation for policy CMP0002 for more details.

Please fix.

To rebase, just use interactive rebase to squash some commits.

@s-bruce13
Copy link
Contributor Author

@0xc0170 I've been attempting to rebase with the interactive rebase. However it presents merge conflicts and shows commits that are not part of my pull request, so I am unable to squash the commits properly. Should I close this PR and open a new one, with the latest fix?

@mergify mergify bot dismissed 0xc0170’s stale review April 6, 2021 14:32

Pull request has been modified.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 6, 2021

A new PR might be easier if you can't resolve the conflicts (there are couple of merges on your master that should not be there).

@s-bruce13
Copy link
Contributor Author

got it will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants