STM32WB - Update CubeDriver from v1.0.0 to v1.4.0 #12283

jeromecoutant · 2020-01-20T17:01:19Z

Summary of changes

Goal is to update mbed-os file from the latest STM32CubeWB MCU Firmware Package:
https://www.st.com/en/embedded-software/stm32cubewb.html
https://github.com/STMicroelectronics/STM32CubeWB

Impact of changes

Several changes are proposed in several commits:

new directory structure as agreed in TARGET_STM: FW driver files refactor proposition #12201
BLE driver files coming from STM32Cube package are moved to TARGET_STM/TARGET_STM32WB/STM32Cube_FW/STM32_WPAN
STM32W50xx MCU are introduced
BLE is considered as mandatory feature for this STM32 family

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 regression for mbed-os tests and the 3 tool chains
HearRate BLE example OK:
https://github.com/ARMmbed/mbed-os-example-ble/tree/master/BLE_HeartRate

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ARMmbed/team-st-mcd

- BLE feature is mandatory - remove clock source selection - license alignment - startup file from Cube delivery - linker script alignement

+ astyle

jeromecoutant · 2020-01-20T17:07:56Z

@donatieng

ciarmcom · 2020-01-20T18:00:37Z

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

donatieng

I've looked at the Bluetooth part and carried some tests and it looks good to me, thank you @jeromecoutant

LMESTM

Single question: maybe one header file (hw_if.h) is not needed ? ?

LMESTM · 2020-01-23T08:38:15Z

targets/TARGET_STM/TARGET_STM32WB/STM32Cube_FW/hw_if.h

@@ -0,0 +1,250 @@
+/* USER CODE BEGIN Header */


Is this file needed in our MBED port ?

Yes, it can be removed.
Just need to move 2 include lines.

Pull request has been modified.

LMESTM

Good job - thanks Jerome !

adbridge

Some minor comments/questions...

adbridge · 2020-01-23T13:34:54Z

targets/TARGET_STM/TARGET_STM32WB/STM32Cube_FW/CMSIS/stm32wbxx.h

@@ -62,14 +62,14 @@
   In this case, these drivers will not be included and the application code will
   be based on direct access to peripherals registers
   */
-  #define USE_HAL_DRIVER
+  /*#define USE_HAL_DRIVER */


Why is this commented out rather than removed ?

All files in this STM32Cube_FW directory come from the ST driver FW delivery.
Before we were patching few files during mbed import.
Now I remove all differences between official ST files and mbed imported files.

adbridge · 2020-01-23T13:46:00Z

targets/TARGET_STM/TARGET_STM32WB/STM32Cube_FW/STM32WBxx_HAL_Driver/stm32wbxx_hal_flash.c

-  {
-    CLEAR_BIT(FLASH->CR, clearbit);
+    /* No Procedure on-going */
+    /* Nothing to do, but check error if any */


This comment as it stands in this place makes it look like there is some missing error code in this block? I assume the intention is that you are just documenting that there is nothing to do in the default else case and it will just fall through to the error checking below ?

adbridge · 2020-01-23T14:03:31Z

targets/TARGET_STM/TARGET_STM32WB/STM32Cube_FW/STM32WBxx_HAL_Driver/stm32wbxx_hal_sai.c

+  {
+     /* SAI_I2S_MSBJUSTIFIED or SAI_I2S_LSBJUSTIFIED */
+    hsai->FrameInit.FSPolarity = SAI_FS_ACTIVE_HIGH;
+    hsai->FrameInit.FSOffset   = SAI_FS_FIRSTBIT;


I assume the protocol can only ever be one of the 3 specified values ?

adbridge · 2020-01-23T14:03:59Z

targets/TARGET_STM/TARGET_STM32WB/STM32Cube_FW/STM32WBxx_HAL_Driver/stm32wbxx_hal_sai.c

+  else
+  {
+    /* SAI_PCM_LONG */
+    hsai->FrameInit.ActiveFrameLength = 13;


Same question here wrt protocol values ?

adbridge · 2020-01-24T11:36:46Z

CI started

mbed-ci · 2020-01-24T13:33:17Z

Test run: SUCCESS

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

donatieng · 2020-01-24T15:41:51Z

Hi @adbridge, it would be great to have this included in the upcoming release as discussed with @MarceloSalazar

mergify · 2020-01-25T11:55:12Z

This PR does not contain release version label after merging.

bulislaw · 2020-02-04T16:24:05Z

This is a huge change for a patch release. Makes me a bit uncomfortable but If @ARMmbed/team-st-mcd is confident and ok with merging it to 15.15.1 lets do it.

adbridge · 2020-02-19T16:18:41Z

@bulislaw this didn't get marked for 5.15.1 in the review meeting so looks like it didn't make it to that release. @jeromecoutant is this really important for the 5.15 branch ?

adbridge · 2020-04-08T09:53:48Z

@jeromecoutant as you haven't indicated otherwise I'm going to assume this is currently not seen as important for the 5.15 branch/releases. If this is not the case, please raise a corresponding PR against mbed-os-5.15 and it will be considered for inclusion there.

jeromecoutant added 10 commits January 20, 2020 16:10

STM32WB - New directory structure

8c76a43

STM32WB - BLE restructure

8f6171f

STM32WB cleanup

339846a

- BLE feature is mandatory - remove clock source selection - license alignment - startup file from Cube delivery - linker script alignement

STM32WB Enable BLE HCIDriver for all targets

b74756d

+ astyle

STM32Cube_FW_WB_V1.4.0 - CMSIS part

d6e4b15

STM32Cube_FW_WB_V1.4.0 - HAL_DRIVER part

08184d7

STM32Cube_FW_WB_V1.4.0 - STM32_WPAN part

b4f3b07

STM32Cube_FW_WB_V1.4.0 - template part

c39a13d

STM32Cube_FW_WB_V1.4.0 - STM32WB50xx part

7a5da61

STM32Cube_FW_WB_V1.4.0 - STM32WB55xx part

3657f90

ciarmcom requested review from a team January 20, 2020 18:00

ciarmcom added the needs: review label Jan 20, 2020

jeromecoutant requested review from LMESTM and a team and removed request for a team January 21, 2020 08:05

donatieng previously approved these changes Jan 22, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Jan 22, 2020

LMESTM reviewed Jan 23, 2020

View reviewed changes

STM32WB remove extra file

25da13b

LMESTM approved these changes Jan 23, 2020

View reviewed changes

adbridge reviewed Jan 23, 2020

View reviewed changes

adbridge approved these changes Jan 24, 2020

View reviewed changes

mergify bot added ready for merge and removed needs: CI labels Jan 24, 2020

adbridge merged commit ceaf562 into ARMmbed:master Jan 25, 2020

adbridge added Release review required and removed ready for merge labels Jan 25, 2020

jeromecoutant deleted the PR_STM32WB branch January 27, 2020 13:12

adbridge added the release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 label Feb 19, 2020

adbridge removed the Release review required label Apr 8, 2020

STM32WB - Update CubeDriver from v1.0.0 to v1.4.0 #12283

STM32WB - Update CubeDriver from v1.0.0 to v1.4.0 #12283

Uh oh!

Conversation

jeromecoutant commented Jan 20, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

jeromecoutant commented Jan 20, 2020

Uh oh!

ciarmcom commented Jan 20, 2020

Uh oh!

donatieng left a comment

Choose a reason for hiding this comment

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

adbridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adbridge commented Jan 24, 2020

Uh oh!

mbed-ci commented Jan 24, 2020

Uh oh!

donatieng commented Jan 24, 2020

Uh oh!

mergify bot commented Jan 25, 2020

Uh oh!

bulislaw commented Feb 4, 2020

Uh oh!

adbridge commented Feb 19, 2020

Uh oh!

adbridge commented Apr 8, 2020

Uh oh!

Uh oh!