STM32F7 update drivers version to CUBE V1.15.0 #11711

jeromecoutant · 2019-10-18T13:39:57Z

Description

ST Cube drivers version is updated from V1.10.0 to V1.15.0,
which is the latest official version from:
https://www.st.com/en/embedded-software/stm32cubef7.html

ST CI tests OK

Pull request type

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

Reviewers

Release Notes

@ARMmbed/team-st-mcd

ciarmcom · 2019-10-18T15:00:30Z

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

0xc0170 · 2019-10-21T10:03:37Z

What is changed in 4c6031b ?
Does it need a review @ARMmbed/mbed-os-crypto team?

jeromecoutant · 2019-10-21T10:48:04Z

What is changed in 4c6031b ?
Does it need a review @ARMmbed/mbed-os-crypto team?

ST CRYPTO HAL API has changed for STM32F7 family,
so I had to differentiate functions per family in order to not break other targets.

See #10422

0xc0170 · 2019-10-21T11:24:47Z

👍

This is pending crypto team review

Patater · 2019-10-23T15:05:37Z

features/mbedtls/targets/TARGET_STM/TARGET_STM32F4/aes_alt.c

@@ -0,0 +1,364 @@
+/*
+ *  Hardware aes implementation for STM32F4 STM32F7 and STM32L4 families


Would a newer version of this aes_alt.c file work for both TARGET_STM32F4 and TARGET_STM32L4? It seems silly to have two different nearly identical copies of the same file in different places in the Mbed OS tree. I'd prefer we use ifdefs to handle the minor differences between these targets as needed. Same goes for other files added by this PR that are nearly the same as existing files.

Also, please ensure it is very clear which version of the ST SDK these files are from, for easier maintenance.

Patater · 2019-10-23T15:14:50Z

features/mbedtls/targets/TARGET_STM/TARGET_STM32F7/aes_alt.h

@@ -0,0 +1,503 @@
+/**
+ * \file aes_alt.h


This file is very similar to features/mbedtls/targets/TARGET_STM/TARGET_STM32L4/aes_alt.h. It looks like L4 is just an older copy of aes.h from Mbed TLS (XTS is missing, for example, so the file is quite old). Can we move that L4 file so it is picked up and used for both targets instead of adding duplication?

Patater · 2019-10-23T15:16:57Z

features/mbedtls/targets/TARGET_STM/TARGET_STM32L4/aes_alt.h

@@ -0,0 +1,307 @@
+/*
+ *  aes_alt.h AES block cipher


This file is identical to features/mbedtls/targets/TARGET_STM/TARGET_STM32F4/aes_alt.h. Can you think of a way to use a single file for both targets? That'd be far more maintainable. Thanks.

Patater · 2019-10-23T15:32:39Z

targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_cryp.c

+
+      /* Process unlocked */
+      __HAL_UNLOCK(hcryp);
+    }    


Do we need to __HAL_UNLOCK(hcryp); when status != HAL_OK? If not, why not?

Same goes HAL_CRYP_AESCTR_Decrypt() and other similar functions.

ST_INTERNAL_REF 75115

Patater · 2019-10-23T15:34:23Z

targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_cryp.c

+
+    default:
+      hcryp->ErrorCode |= HAL_CRYP_ERROR_NOT_SUPPORTED;
+      return HAL_ERROR;


Do we need to __HAL_UNLOCK(hcryp); before returning here?

0xc0170 · 2019-10-25T11:15:46Z

This also will require rebase to resolve a conflict

https://www.st.com/en/embedded-software/stm32cubef7.html

jeromecoutant · 2019-10-31T16:58:27Z

@Patater
Thanks for the review for mbedtls part.
I am waiting for some internal feedback for this...
So as current mbedtls tests are OK with this current version, are you OK to approve this PR?
I promise to push updates soon :-)

@0xc0170
Rebase and conflicts solved

0xc0170 · 2019-11-04T14:52:19Z

Restarted CI

mbed-ci · 2019-11-04T16:20:03Z

Test run: SUCCESS

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

0xc0170 · 2019-11-05T07:27:57Z

@Patater
Thanks for the review for mbedtls part.
I am waiting for some internal feedback for this...
So as current mbedtls tests are OK with this current version, are you OK to approve this PR?
I promise to push updates soon :-)

@Patater Please review

Patater · 2019-11-05T08:39:12Z

There are no Mbed TLS tests that do multithreading in Mbed OS, unfortunately, so we can't depend on them to catch these sorts of issues. Passing tests doesn't mean the code is good enough for merge into Mbed OS. The issues found in review need resolution.

jeromecoutant · 2019-11-05T08:55:32Z

The issues found in review need resolution

I agree, but in a second step.
Issues you pointed are from driver files. You can't bock all other feature :-(

0xc0170 · 2019-11-05T09:14:12Z

Issues you pointed are from driver files. You can't bock all other feature :-(

If that is the case, these requested changes could go into a new issue (I can see internal ticket was pasted here) and this proceed, @Patater ? We are talking about device/stm32f7xx_hal_cryp.c file

jeromecoutant · 2019-11-05T09:22:15Z

We are talking about device/stm32f7xx_hal_cryp.c file

source: https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/STM32F7xx_HAL_Driver/Src/stm32f7xx_hal_cryp.c

jeromecoutant · 2019-11-05T10:56:18Z

@Patater I tried to patch the driver file before ST owner feedback.
Please review
Thx

Patater · 2019-11-06T18:50:29Z

OK, thanks. So, is this still V1.15.0 or is it now V1.15.1?

jeromecoutant · 2019-11-07T07:59:20Z

OK, thanks. So, is this still V1.15.0 or is it now V1.15.1?

I am not responsible for ST Cube drive delivery :-)

0xc0170 · 2019-11-07T08:26:02Z

CI started

mbed-ci · 2019-11-07T10:15:45Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 2
Build artifacts

This was referenced Oct 18, 2019

STM32F7 HAL_cryp API compatibility break with cube 1.15 onwards #10422

Closed

Building for DISCO_F746NG on ARM toolchain results in a link warning #11045

Closed

ciarmcom requested review from a team October 18, 2019 15:00

ciarmcom added the needs: review label Oct 18, 2019

0xc0170 added the release-version: 5.14.2 label Oct 21, 2019

0xc0170 approved these changes Oct 21, 2019

View reviewed changes

Patater reviewed Oct 23, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Oct 25, 2019

jeromecoutant added 6 commits October 31, 2019 17:43

F7 ST CUBE V1.10.0 => V1.15.0

8ac9189

https://www.st.com/en/embedded-software/stm32cubef7.html

STM32F7 updates for new driver version

52bfd0c

STM32F7 refactor common files

4f788ad

STM32F7 MBEDTLS AES_ALT update for new HAL

76248b4

STM32F7 ARM SCT file update to define correct RAM_SIZE

356de44

STM32F7 targets.json cleaning

a246164

jeromecoutant force-pushed the PULL_REQUEST_CUBE_UPDATE_F7_V1.15.0 branch from ed62c8c to a246164 Compare October 31, 2019 16:52

0xc0170 requested a review from Patater November 4, 2019 14:52

0xc0170 added needs: CI and removed needs: work labels Nov 4, 2019

0xc0170 added needs: review and removed needs: CI labels Nov 5, 2019

0xc0170 added needs: work and removed needs: review labels Nov 5, 2019

STM32F7 HAL CRYPT patch to add missing UNLOCK

7847ad7

Patater approved these changes Nov 6, 2019

View reviewed changes

0xc0170 added ready for merge needs: CI and removed needs: work ready for merge labels Nov 7, 2019

0xc0170 added ready for merge and removed needs: CI labels Nov 7, 2019

0xc0170 merged commit 383cf19 into ARMmbed:master Nov 7, 2019

0xc0170 removed the ready for merge label Nov 7, 2019

jeromecoutant deleted the PULL_REQUEST_CUBE_UPDATE_F7_V1.15.0 branch November 7, 2019 11:45

jeromecoutant mentioned this pull request Nov 15, 2019

STM32F7: linker scripts updates #11870

Merged

0xc0170 mentioned this pull request Nov 19, 2019

Release candidate for mbed-os-5.14.2 #11894

Merged

adbridge added release-version: 5.15.0-rc1 and removed release-version: 5.14.2 labels Nov 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM32F7 update drivers version to CUBE V1.15.0 #11711

STM32F7 update drivers version to CUBE V1.15.0 #11711

jeromecoutant commented Oct 18, 2019

ciarmcom commented Oct 18, 2019

0xc0170 commented Oct 21, 2019

jeromecoutant commented Oct 21, 2019

0xc0170 commented Oct 21, 2019

Patater Oct 23, 2019

Patater Oct 23, 2019

Patater Oct 23, 2019

Patater Oct 23, 2019

jeromecoutant Nov 4, 2019

Patater Oct 23, 2019

0xc0170 commented Oct 25, 2019

jeromecoutant commented Oct 31, 2019

0xc0170 commented Nov 4, 2019

mbed-ci commented Nov 4, 2019

0xc0170 commented Nov 5, 2019

Patater commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

0xc0170 commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

Patater commented Nov 6, 2019

jeromecoutant commented Nov 7, 2019

0xc0170 commented Nov 7, 2019

mbed-ci commented Nov 7, 2019

		@@ -0,0 +1,364 @@
		/*
		* Hardware aes implementation for STM32F4 STM32F7 and STM32L4 families

STM32F7 update drivers version to CUBE V1.15.0 #11711

STM32F7 update drivers version to CUBE V1.15.0 #11711

Conversation

jeromecoutant commented Oct 18, 2019

Description

Pull request type

Reviewers

Release Notes

ciarmcom commented Oct 18, 2019

0xc0170 commented Oct 21, 2019

jeromecoutant commented Oct 21, 2019

0xc0170 commented Oct 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Oct 25, 2019

jeromecoutant commented Oct 31, 2019

0xc0170 commented Nov 4, 2019

mbed-ci commented Nov 4, 2019

0xc0170 commented Nov 5, 2019

Patater commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

0xc0170 commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

jeromecoutant commented Nov 5, 2019

Patater commented Nov 6, 2019

jeromecoutant commented Nov 7, 2019

0xc0170 commented Nov 7, 2019

mbed-ci commented Nov 7, 2019