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 uVisor support for the DISCO_F429ZI #3397

Merged
merged 3 commits into from
Jan 16, 2017

Conversation

AlessandroA
Copy link
Contributor

Note: Please do not merge yet. Our tests need to be updated to test the new target.

This PR adds support for uVisor on the DISCO_F429ZI target.

Changelog:

  • Port of uVisor to DISCO_F429ZI, by @adustm.
  • New version of uVisor, v0.26.2, needed to enable the new target.
  • Update to other supported linker scripts, to make them consistent with our porting guide.

@0xc0170 @meriac @sg-

@mazimkhan Please use my branch https://github.com/AlessandroA/mbed-os/tree/stm32f4_support to test the new target locally. If those tests succeed you can report back here and I'll remove the WIP tag.

@bridadan
Copy link
Contributor

bridadan commented Dec 8, 2016

Went ahead and added the big fat do not merge label to match your description 😄

@mazimkhan
Copy link

mazimkhan commented Dec 8, 2016

@AlessandroA I get following error while compiling for profile disco_f429zi_gcc_arm_r_1_1_2

err.txt

@AlessandroA AlessandroA changed the title WIP: Add support for the DISCO_F429ZI WIP: Add uVisor support for the DISCO_F429ZI Dec 9, 2016
@adustm
Copy link
Member

adustm commented Dec 15, 2016

Hello @AlessandroA
Unfortunately, mbed-os tests are failing with this PR (or did I launch them wrong ?)
I have done the following :

mbed import https://github.com/AlessandroA/mbed-os
cd mbed-os

(at this point I am on origin/master)
mbed test -m DISCO_F429ZI -t GCC_ARM
It gives 138 tests OK (the complete list ok)

git checkout stm32f4_support
(at this point I am on your Pull Request)
mbed test -m DISCO_F429ZI -t GCC_ARM
It gives TIMEOUT ERROR for every tests

Can you tell me if there is something I did wrong ? and if not, is it possible for you to check that, please ?
Thanks in advance,
Kind regards

@AlessandroA
Copy link
Contributor Author

Hi @adustm, I will look into that this afternoon and will report back here. Thanks for reporting the issue!

@AlessandroA
Copy link
Contributor Author

Hi @adustm just a quick update. We found a couple of issues with the tests and we are debugging them at the moment. We plan to release support for the target of this PR with a restricted set of tests, and then work to increase the test coverage. I will update this PR when we are ready to review and merge.

@adustm
Copy link
Member

adustm commented Jan 3, 2017

Hi Alessandro,
Happy 2017 to you.
Could you give me some news ? Is there any plan for this ?
Kind regards
Armelle

@sg-
Copy link
Contributor

sg- commented Jan 9, 2017

@AlessandroA any update? If it needs much more work we should close and re-open when ready.

adustm and others added 3 commits January 10, 2017 13:15
This commit improves consistency between different platforms' linker
scripts. In particular, we use "__UVISOR_SRAM_START" instead of
"__UVISOR_BSS_START" as the uVisor BSS sections might be outside of the
SRAM (for example, when using a tightly-coupled memory).
* Add support for uVisor own SRAM.
    * This enables targets where uVisor sits in a TCM.
* Distinguish between uVisor and public SRAMs.
@AlessandroA
Copy link
Contributor Author

Hi @adustm , @sg- , I updated this PR. The current status is:

  • 75% of uVisor tests passing.
  • 1 example out of 3 working (the other 2 fail after a couple of seconds).

If all non-uVisor-specific tests succeed for this target, my suggestion is to merge this PR. The uVisor-specific issues will be solved in the next weeks, as some of the tasks we are working on address the underlying problems. That will require a new release of uVisor anyway.

@adustm
Copy link
Member

adustm commented Jan 10, 2017

Hi @AlessandroA, @sg-
Thank you for the update.
You did not mention the status of mbed test -m DISCO_F429ZI -t GCC_ARM (regression tests). Did you try it again ?
Cheers,

@AlessandroA
Copy link
Contributor Author

@adustm those tests fail for me even with the HEAD of master, so I am not sure if the issue is with my board/tools. If they are enabled, I'd rather wait for the results of the CI job which is running in this PR.

@adustm
Copy link
Member

adustm commented Jan 10, 2017

Hi @AlessandroA
I had no idea that the master HEAD was wrong at that time... but I won't spend time to confirm it. I passed the tests with your branch and without it, as I previously did in the past.
mbed_lib_rev133 for mbed os 2 tests:
python ./mbed/tools/singletest.py --auto -f DISCO_F429ZI --tc GCC_ARM
[...]Result: 32 OK / 1 NOT_SUPPORTED
mbed-os HEAD of master :
results for mbed test -m DISCO_F429ZI -t GCC_ARM

those tests fail for me even with the HEAD of master, so I am not sure if the issue is with my board/tools.

I just launched mbed test -m DISCO_F429ZI -t GCC_ARM it on HEAD of mbed-os master and it gives :
'[...]mbedgt: test suite results: 48 OK'

Then I did fetch your branch git checkout AlessandroA/stm32f4_support
and lanched again mbed test -m DISCO_F429ZI -t GCC_ARM
'[...]mbedgt: test suite results: 48 OK'

As a conclusion : the porting of uvisor your did for DISCO_F429ZI is now fine for me, causing no dammage. 👍

Your suggestion :

If all non-uVisor-specific tests succeed for this target, my suggestion is to merge this PR. The uVisor-specific issues will be solved in the next weeks, as some of the tasks we are working on address the underlying problems. That will require a new release of uVisor anyway.

is fine for me also. 👍

@adustm
Copy link
Member

adustm commented Jan 10, 2017

@bcostm

@adustm
Copy link
Member

adustm commented Jan 12, 2017

Hello @sg- , @bridadan
what do you think about @AlessandroA 's idea:

my suggestion is to merge this PR. The uVisor-specific issues will be solved in the next weeks, as some of the tasks we are working on address the underlying problems. That will require a new release of uVisor anyway.

Could the Wip title and the do-not-merge label be removed ?
Kind regards
Armelle

@AlessandroA AlessandroA changed the title WIP: Add uVisor support for the DISCO_F429ZI Add uVisor support for the DISCO_F429ZI Jan 12, 2017
@bridadan
Copy link
Contributor

@adustm We can definitely get a test run on it! The NUCLEO_F429ZI has some failing tests on master when heap and stack stats are enabled (the always are in CI). But I believe this issue is being worked through here: #3319

/morph test

@adustm
Copy link
Member

adustm commented Jan 12, 2017

Thanks @bridadan .
You are right about the issue you have with the heap&stack issue. I never see this error on my side.

This PR is for DISCO_F429ZI by the way, not NUCLEO
(I plan the NUCLEO asap after the merge of this one)
cheers

@bridadan
Copy link
Contributor

This PR is for DISCO_F429ZI by the way, not NUCLEO

True, though I think the problem is present in both (as well as a few other ST targets)? I haven't actually tried it on the DISCO though. Shouldn't block this PR from coming in though.

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1389

All builds and test passed!

@bridadan
Copy link
Contributor

CI looks ok, I'll leave the final call up to @0xc0170 or @sg-

@sg-
Copy link
Contributor

sg- commented Jan 13, 2017

LGTM 👍

@0xc0170 0xc0170 merged commit c14d715 into ARMmbed:master Jan 16, 2017
aisair pushed a commit to aisair/mbed that referenced this pull request Apr 30, 2024
Ports for Upcoming Targets

3571: DISCO_F769NI introduction ARMmbed/mbed-os#3571
3605: Add DELTA_DFCM_NNN50 platform ARMmbed/mbed-os#3605
3640: [MAX32630FTHR] Adding new platform ARMmbed/mbed-os#3640

Fixes and Changes

3397: Add uVisor support for the DISCO_F429ZI ARMmbed/mbed-os#3397
3573: fix failing RTC initialization for MTS_DRAGONFLY_F411RE ARMmbed/mbed-os#3573
3575: Dev stm factorize gpio ARMmbed/mbed-os#3575
3584: STM32: make PeripheralPins.h a common file ARMmbed/mbed-os#3584
3583: STM32F7 Cube FW new release v1.5.1 ARMmbed/mbed-os#3583
3578: Target system - Inherit names from target parents ARMmbed/mbed-os#3578
3599: K22F: Enable TRNG ARMmbed/mbed-os#3599
3614: STM32: make PortNames.h a common file ARMmbed/mbed-os#3614
3617: EFM32GG: Fix GCC_ARM linker script ARMmbed/mbed-os#3617
3618: STM32: Move types definitions to a common file ARMmbed/mbed-os#3618
3631: F3 CUBE update V1.7.0 ARMmbed/mbed-os#3631
3635: STM32 I2C : Fix bug in i2c_byte_read function ARMmbed/mbed-os#3635
3651: Max32630 - fix LED4 ARMmbed/mbed-os#3651
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.

8 participants