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

STM32F303RE: Activate FLASHIAP #10493

Merged
merged 1 commit into from May 2, 2019

Conversation

Projects
None yet
9 participants
@LMESTM
Copy link
Contributor

commented Apr 26, 2019

Description

As suggested in #10486 by @JanneKiiskila, we may activate FLASHIAP for F303RE.
Flash driver is already in place.

Pull request type

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

Reviewers

Release Notes

Tests results:
+-----------------------+---------------+-----------------------------+--------+--------------------+-------------+
| target                | platform_name | test suite                  | result | elapsed_time (sec) | copy_method |
+-----------------------+---------------+-----------------------------+--------+--------------------+-------------+
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | OK     | 18.39              | default     |
+-----------------------+---------------+-----------------------------+--------+--------------------+-------------+
mbedgt: test suite results: 1 OK
mbedgt: test case report:
+-----------------------+---------------+-----------------------------+-----------------------------------+--------+--------+--------+--------                                                       ------------+
| target                | platform_name | test suite                  | test case                         | passed | failed | result | elapsed                                                       _time (sec) |
+-----------------------+---------------+-----------------------------+-----------------------------------+--------+--------+--------+--------                                                       ------------+
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | FlashIAP - init                   | 1      | 0      | OK     | 0.09                                                                      |
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | FlashIAP - program                | 1      | 0      | OK     | 0.22                                                                      |
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | FlashIAP - program across sectors | 1      | 0      | OK     | 0.16                                                                      |
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | FlashIAP - program errors         | 1      | 0      | OK     | 0.1                                                                       |
| NUCLEO_F303RE-GCC_ARM | NUCLEO_F303RE | tests-mbed_drivers-flashiap | FlashIAP - timing                 | 1      | 0      | OK     | 1.01                                                                      |
+-----------------------+---------------+-----------------------------+-----------------------------------+--------+--------+--------+--------                                                       ------------+
mbedgt: test case results: 5 OK
mbedgt: completed in 30.58 sec
@JanneKiiskila
Copy link
Contributor

left a comment

LGTM, we need this to next Mbed OS 5.12.x patch release, please!
@adbridge

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

@LMESTM, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from ARMmbed/mbed-os-maintainers Apr 26, 2019

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Apr 29, 2019

@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@0xc0170 - merge time!

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@0xc0170 - merge time!

@JanneKiiskila I think more like CI time first...

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

CI started

@jeromecoutant

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Hi

FLASHIAP tests becomes OK with NUCLEO_F303RE.

But FLASHIAP enables also device_key tests, which are FAIL...

@JanneKiiskila could you check this ?

mbed test -t ARM -m NUCLEO_F303RE -v -n features-device_key-tests-device_key-functionality
==> KVMap init issue ?

@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@davidsaada - could you give it a look please?

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@JanneKiiskila @LMESTM I don't have the board here, nor see the failure log, so it will be hard for me to tell why this fails.
That said, the test results posted here are not relevant for the change. They only test the FlashIAP driver, but the change adds the FlashIAP block device. This is best tested with the general block device test.
i.e. run:

mbed test -v -t GCC_ARM -m NUCLEO_F303RE --profile debug -n features-storage-tests-blockdevice-general_block_device

This may give more clues regarding the failure.

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@jeromecoutant Is the "KVMap init issue?" question based on a speculation or on log results you see?
Any logs (preferably with debug profile) will be welcomed.

@jeromecoutant

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

KVMap init issue ?

This is the first debug step :-)
I added a printf at the beginning of some KVMap functions.

  • With DISCO_L475 (test OK), I get the init printf in the test log.
[1556540133.44][CONN][RXD] >>> Running case #1: 'Device Key - long consistency test'...
[1556540133.51][CONN][INF] found KV pair in stream: {{__testcase_start;Device Key - long consistency test}}, queued...
[1556540133.52][CONN][RXD] KVMap init
[1556540133.54][CONN][RXD] KVMap attach
[1556540133.55][CONN][RXD] KVMap config_lookup
...
  • With NUCLEO_F3, I couldn't and test is failing:
[1556539842.21][CONN][RXD] >>> Running case #1: 'Device Key - long consistency test'...
[1556539842.27][CONN][INF] found KV pair in stream: {{__testcase_start;Device Key - long consistency test}}, queued...
[1556539842.29][CONN][RXD] KVMap config_lookup
mbedgt: :95::FAIL: Expected Not-Equal
@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Thanks @jeromecoutant. It seems like there's a problem with using the FlashIAP block device (test fails on getting the internal block device).
Can you run the general block device test as following and post the results?

mbed test -v -t GCC_ARM -m NUCLEO_F303RE --profile debug -n features-storage-tests-blockdevice-general_block_device
@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@jeromecoutant BTW, it could well be that the internal flash of this board is small, not enough for the allocation of KVStore, hence the failure in device key test. Do you have the sector map of this board?
In addition, can you rerun the the FlashIAP test (whose results are posted in the description). It may well be that few of tests there also sense that there's not enough room in the flash for testing. But in this case, the test is skipped and you should see prints telling that.

@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@davidsaada - whare are the space requirements? It has the same amount of flash as Nucleo F411RE, but in a more sensible approach (2 kilobyte blocks, 256 pieces of them).

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@JanneKiiskila Took it offline with @jeromecoutant. Seems like it's not related to space requirements, but to something else we're trying to figure out now.

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@yossi2le

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

I see that this device is missing "device_has": [ "TRNG" ]. DeviceKey needs TRNG in order to work correctly or any other support which enables the MBEDTLS_ENTROPY_NV_SEED macro.

@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

No, it does not have TRNG. It is similar to Nucleo F411, we need to inject the entropy externally.

@yossi2le

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

and are you injecting it from outside?

@yossi2le

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@JanneKiiskila I don't think it is related to the TRNG and injection of the device key. The test should create a dummy ROT if there is no TRNG support. I have to run now but I will be happy to help debugging the issue tomorrow morning.
The only problem I do not have a board here. Do we have a remote one?

@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Yes, it is available in RaaS.

@yossi2le

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Excellent, can you tell me on which RaaS. Please send me the details offline?

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 29, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Apr 30, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

@yossi2le @jeromecoutant What is the status here? I set it to needs work until we get confirmation the tests are fixed

@yossi2le

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

I have submitted a PR #10516 which should solve the issue of device key

@jeromecoutant
Copy link
Contributor

left a comment

Approved as FLASHIAP tests are OK with this PR.

DEVICE_KEY tests will then be OK with #10516

@adbridge adbridge added ready for merge and removed needs: work labels May 1, 2019

@0xc0170 0xc0170 merged commit 58a5f95 into ARMmbed:master May 2, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage Success
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8567 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@JanneKiiskila

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

Excellent, let's get them in and enable stuff. 👍

@0xc0170 0xc0170 removed the ready for merge label May 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.