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

ST: STM32WB0: Implement BLE feature #79734

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

HoZHel
Copy link
Contributor

@HoZHel HoZHel commented Oct 11, 2024

Provide BLE support for STM32WB0x:

  • Use hal_stm32 PR to provide the binary blob and required files to build a BLE controller library.
  • Provide STM32WB0 HCI driver

Prior building a Zephyr BLE application, user should run west blobs fetch hal_stm32 to fetch binary blob so as to provide BLE controller support.

@@ -344,7 +344,7 @@ config BT_USER_DATA_LEN_UPDATE
info is available in the connection info.

config BT_AUTO_DATA_LEN_UPDATE
bool "Auto-initiate Data Length Update procedure"
bool "Auto-initiate Data Length Update procedure" if !BT_STM32WB0
depends on BT_DATA_LEN_UPDATE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to this file should not be required, based on the changes to soc/st/stm32/stm32wb0x/Kconfig.defconfig.

Copy link
Contributor Author

@HoZHel HoZHel Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this config has prompt, it is user configurable. So, if in an application, like BT shell (available under tests/bluetooth/shell), it is activated in prj.conf then the default value will be ignored as well as changes in soc/st/stm32/stm32wb0x/Kconfig.defconfig.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. Then is is the case as well for bunch of these options (BT_HCI_ACL_FLOW_CONTROL for instance) and yet this is never used. Anyway, I let this to subsystem maintainers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason these options must remain disabled for this controller? Is it because the controller doesn't support them? Does it incorrectly report support for them when querying its features over HCI? (the host shouldn't initiate such procedures if the controller reports that it doesn't support them).

Either way, having HCI driver specific references in the host Kconfig is not the right way to deal with this. What the right/best way is depends on the answers to my earlier questions.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Oct 15, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_stm32 zephyrproject-rtos/hal_stm32@6f0e5f7 (main) zephyrproject-rtos/hal_stm32#236 zephyrproject-rtos/hal_stm32#236/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest DNM This PR should not be merged (Do Not Merge) labels Oct 15, 2024
Introduce Kconfig parameter for STM32WB0x HCI Bluetooth driver.

Exclude STM32WB0x from activating config BT_HCI_ACL_FLOW_CONTROL.

Signed-off-by: Ali Hozhabri <[email protected]>
Add a yaml file required by STM32WB0x bluetooth HCI driver.

Signed-off-by: Ali Hozhabri <[email protected]>
Add Bluetooth HCI driver for STM32WB0x series.

Modify CMakeLists.txt to compile the driver based on its kconfig parameter.

Signed-off-by: Ali Hozhabri <[email protected]>
Add BLE feature to STM32WB0x series at SOC level.

Signed-off-by: Ali Hozhabri <[email protected]>
Dedicate RAM section on STM32WB0x for BLE part based on the number
of radio tasks and device type.

Signed-off-by: Ali Hozhabri <[email protected]>
…UPDATE

Put the default value for BT_AUTO_PHY_UPDATE and BT_AUTO_DATA_LEN_UPDATE
to "n" at SOC level since they are not supported by the SOC series.

Disable the prompts for BT_AUTO_PHY_UPDATE and BT_AUTO_DATA_LEN_UPDATE
to remove user configurable option for this SOC series.

Signed-off-by: Ali Hozhabri <[email protected]>
Enable BLE feature for Nucleo-WB09KE and Nucleo-WB05KZ.

Dedicate 32KB and 8KB at the end of flash memory to storage partition on
Nucleo-WB09KE and Nucleo-WB05KZ respectively.

Add ble tag to the both devices yaml file.

Signed-off-by: Ali Hozhabri <[email protected]>
Modify west.yaml to update hal_stm32.

Signed-off-by: Ali Hozhabri <[email protected]>
@@ -344,7 +344,7 @@ config BT_USER_DATA_LEN_UPDATE
info is available in the connection info.

config BT_AUTO_DATA_LEN_UPDATE
bool "Auto-initiate Data Length Update procedure"
bool "Auto-initiate Data Length Update procedure" if !BT_STM32WB0
depends on BT_DATA_LEN_UPDATE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason these options must remain disabled for this controller? Is it because the controller doesn't support them? Does it incorrectly report support for them when querying its features over HCI? (the host shouldn't initiate such procedures if the controller reports that it doesn't support them).

Either way, having HCI driver specific references in the host Kconfig is not the right way to deal with this. What the right/best way is depends on the answers to my earlier questions.

default y if !BT_CTLR && !BT_STM32_IPM && !BT_ESP32 && !BT_STM32WBA
default y if !BT_CTLR && !BT_STM32_IPM && !BT_ESP32 && !BT_STM32WBA && !BT_STM32WB0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is starting to get messy. We should probably instead create a virtual option that the HCI drivers select and then here we only depend on that one virtual option. That said, do you realize that BT_CTLR is actually a generic option for describing any local (same memory & CPU) controller? I think we should work toward having all such HCI drivers enable BT_CTRL and associated options. Nordic already enables BT_CTRL for the upstream open source split link layer, as well for their downstream closed source SoftDevice controller, but no other HCI driver does this at the moment.

Comment on lines +102 to +103
static uint8_t legacy_cmd_issued = FALSE, extended_cmd_issued = FALSE;
uint8_t allowed = TRUE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't these bool?

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

Successfully merging this pull request may close these issues.

5 participants