-
Notifications
You must be signed in to change notification settings - Fork 8.1k
drivers: bluetooth: silabs: Add hardware-dependent feature detection #97994
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,6 +1,60 @@ | ||||
| # Copyright (c) 2025 Silicon Laboratories Inc. | ||||
| # | ||||
| # SPDX-License-Identifier: Apache-2.0 | ||||
|
|
||||
| config BT_SILABS_SIWX91X | ||||
| bool "Silabs SiWx91x Bluetooth interface" | ||||
| default y | ||||
| depends on DT_HAS_SILABS_SIWX91X_BT_HCI_ENABLED | ||||
| select SILABS_SIWX91X_NWP | ||||
| select ENTROPY_GENERATOR | ||||
| help | ||||
| Use Silicon Labs Wiseconnect 3.x Bluetooth library to connect to the controller. | ||||
|
|
||||
| # SiWx917 BLE controller currently does not support HCI Command: Host Number of Completed Packets | ||||
| configdefault BT_HCI_ACL_FLOW_CONTROL | ||||
| default n if BT_SILABS_SIWX91X | ||||
|
|
||||
| config BT_SILABS_EFR32 | ||||
| bool "Silabs EFR32 HCI driver" | ||||
| default y | ||||
| depends on DT_HAS_SILABS_BT_HCI_EFR32_ENABLED | ||||
| depends on ZEPHYR_HAL_SILABS_MODULE_BLOBS || BUILD_ONLY_NO_BLOBS | ||||
| depends on SOC_GECKO_HAS_RADIO | ||||
| select SOC_GECKO_USE_RAIL | ||||
| select PSA_CRYPTO | ||||
| select SILABS_SISDK_PROTOCOL_CRYPTO | ||||
| select HAS_BT_CTLR | ||||
|
|
||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep it for the readability. |
||||
| # Features supported by all EFR32 devices | ||||
| select BT_CTLR_PHY_UPDATE_SUPPORT | ||||
| select BT_CTLR_PER_INIT_FEAT_XCHG_SUPPORT | ||||
| select BT_CTLR_DATA_LEN_UPDATE_SUPPORT | ||||
| select BT_CTLR_EXT_REJ_IND_SUPPORT | ||||
| select BT_CTLR_CHAN_SEL_2_SUPPORT | ||||
| select BT_CTLR_CONN_RSSI_SUPPORT | ||||
| select BT_CTLR_ADV_EXT_SUPPORT | ||||
| select BT_CTLR_PRIVACY_SUPPORT | ||||
| select BT_CTLR_SYNC_PERIODIC_SUPPORT | ||||
| select BT_CTLR_SYNC_TRANSFER_RECEIVER_SUPPORT | ||||
| select BT_CTLR_SYNC_TRANSFER_SENDER_SUPPORT | ||||
|
|
||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep it for the readability. |
||||
| # Features supported by some EFR32 devices | ||||
| select BT_CTLR_PHY_2M_SUPPORT if HAS_HW_EFR32_RADIO_BLE_2M | ||||
| select BT_CTLR_PHY_CODED_SUPPORT if HAS_HW_EFR32_RADIO_BLE_CODED | ||||
| select BT_CTLR_CHANNEL_SOUNDING_SUPPORT if HAS_HW_EFR32_RADIO_CS | ||||
| select BT_CTLR_DF_SUPPORT if HAS_HW_EFR32_RADIO_CTE_TX | ||||
| select BT_CTLR_DF_CTE_TX_SUPPORT if HAS_HW_EFR32_RADIO_CTE_TX | ||||
| select BT_CTLR_DF_CTE_RX_SUPPORT if HAS_HW_EFR32_RADIO_CTE_RX | ||||
| select BT_CTLR_DF_CTE_RX_SAMPLE_1US_SUPPORT if HAS_HW_EFR32_RADIO_CTE_RX | ||||
| select BT_CTLR_DF_ANT_SWITCH_1US_SUPPORT if HAS_HW_EFR32_RADIO_CTE_TX | ||||
| select BT_CTLR_DF_ANT_SWITCH_2US_SUPPORT if HAS_HW_EFR32_RADIO_CTE_TX | ||||
| select BT_SILABS_EFR32_HIGH_POWER if HAS_HW_EFR32_RADIO_TX_HIGH_POWER | ||||
|
|
||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep it for the readability. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no thanks, remove, not allowed under https://docs.zephyrproject.org/latest/contribute/style/kconfig.html |
||||
| help | ||||
| Use Silicon Labs binary Bluetooth library to connect to the | ||||
| controller. | ||||
|
|
||||
| menu "EFR32 Bluetooth Controller Configuration" | ||||
| depends on BT_SILABS_EFR32 | ||||
|
|
||||
|
|
@@ -60,11 +114,11 @@ config BT_SILABS_EFR32_MAX_QUEUED_ADV_REPORTS | |||
| Additional advertising reports are dropped. | ||||
|
|
||||
| config BT_SILABS_EFR32_HIGH_POWER | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need this when you have it in dts, and it has it's own kconfig below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a declaration. DT just selects it if hw supports it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a symbol in Kconfig which is auto generated, you don't need a duplicate symbol |
||||
| bool "High power transmission" | ||||
| bool | ||||
| help | ||||
| Normally the transmit power is limited to 10 dBm. Enable this option | ||||
| to allow the controller to transmit at higher power levels. Note that | ||||
| this feature is not available on all devices. | ||||
| Normally the transmit power is limited to 10 dBm. This option | ||||
| is automatically enabled on devices with high power PA support | ||||
| and allows the controller to transmit at higher power levels. | ||||
|
|
||||
| config BT_SILABS_EFR32_HIGH_POWER_AFH | ||||
| bool "Adaptive frequency hopping for high power transmitters" | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # Copyright (c) 2025 Nordic Semiconductor ASA | ||
| # Copyright (c) 2025 Silicon Laboratories Inc. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| description: | | ||
| Base binding for Bluetooth LE radio hardware capabilities. | ||
| These properties describe hardware-supported Bluetooth features based on | ||
| the Bluetooth Core Specification. They are read-only and indicate what | ||
| the hardware is capable of, not what features are currently enabled. | ||
| properties: | ||
| # Bluetooth 5.0 | ||
| ble-2mbps-supported: | ||
| type: boolean | ||
| description: | | ||
| 2 Mbps PHY support (Bluetooth 5.0 specification). | ||
| ble-coded-phy-supported: | ||
| type: boolean | ||
| description: | | ||
| Coded PHY support for Long Range (Bluetooth 5.0 specification). | ||
| # Bluetooth 5.1 | ||
| ble-cte-tx-supported: | ||
| type: boolean | ||
| description: | | ||
| Constant Tone Extension transmission for Direction Finding in Angle | ||
| of Departure mode (Bluetooth 5.1 specification). | ||
| ble-cte-rx-supported: | ||
| type: boolean | ||
| description: | | ||
| Constant Tone Extension reception with IQ sampling for Direction Finding | ||
| in Angle of Arrival mode. Requires ble-cte-tx-supported capability | ||
| (Bluetooth 5.1 specification). | ||
| # Bluetooth 6.0 | ||
| ble-cs-supported: | ||
| type: boolean | ||
| description: | | ||
| BLE Channel Sounding feature support (Bluetooth 6.0 specification). |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -108,7 +108,7 @@ description: | | |||||
| compatible: "nordic,nrf-radio" | ||||||
|
|
||||||
| include: [base.yaml] | ||||||
| include: [base.yaml, ble-radio.yaml] | ||||||
|
|
||||||
| properties: | ||||||
| reg: | ||||||
|
|
@@ -212,25 +212,8 @@ properties: | |||||
| If set, indicates that the radio hardware supports the IEEE 802.15.4 | ||||||
| mode. | ||||||
| ble-2mbps-supported: | ||||||
| type: boolean | ||||||
| description: | | ||||||
| If set, indicates that the radio hardware supports the 2 Mbps BLE mode. | ||||||
| ble-coded-phy-supported: | ||||||
| type: boolean | ||||||
| description: | | ||||||
| If set, indicates that the radio hardware supports coded BLE PHY. | ||||||
| tx-high-power-supported: | ||||||
| ble-tx-high-power-supported: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Isnt this use of front-end-module, or Tx gain in the multi-protocol radio, right? There is nothing Bluetooth Technology related here, right? If this is specific to Bluetooth Technology LE Power Class 1 feature, then lets name it accordingly and update the description. |
||||||
| type: boolean | ||||||
| description: | | ||||||
| If set, indicates that the radio hardware supports high TX power | ||||||
| settings. | ||||||
| cs-supported: | ||||||
| type: boolean | ||||||
| description: | | ||||||
| If set, the radio hardware supports the BLE Channel Sounding feature. | ||||||
| This property should be treated as read-only and should not be overridden; | ||||||
| the correct value is provided for your target's SoC already. | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ description: | | |
| compatible: "silabs,series2-radio" | ||
|
|
||
| include: [base.yaml] | ||
| include: [base.yaml, ble-radio.yaml] | ||
|
|
||
| properties: | ||
| reg: | ||
|
|
@@ -56,3 +56,12 @@ properties: | |
| property is not set, no sub-GHz PA is enabled. | ||
| enum: | ||
| - highest | ||
|
|
||
| ble-tx-high-power-supported: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even though this isn't based on any spec, but since there's a property with the identical name in the nRF binding, perhaps we'd still want this in the common base binding to avoid duplication (with a mention in the description that the meaning is implementation-specific?). Thoughts from @cvinayak? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see https://github.com/zephyrproject-rtos/zephyr/pull/97994/files#r2451718718
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not put it into a common base, exactly because it is not in the spec and it is very vendor specific There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hence, requesting to revert back to |
||
| type: boolean | ||
| description: | | ||
| If set, indicates that the radio hardware supports high TX power settings. | ||
| This is a vendor-specific capability that enables transmission beyond | ||
| standard power levels (typically >10 dBm). | ||
| This property should be treated as read-only and should not be overridden; | ||
| the correct value is provided for your target's SoC already. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -488,7 +488,7 @@ | |
| status = "disabled"; | ||
| ble-2mbps-supported; | ||
| ble-coded-phy-supported; | ||
| cs-supported; | ||
| ble-cs-supported; | ||
| dfe-supported; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ieee802154-supported; | ||
| interrupts = <44 NRF_DEFAULT_IRQ_PRIORITY>; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exactly is wrong? It pass the compliance test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ipankr all Kconfig definitions should start at the beginning of the line. Furthermore, I thought
configdefaultwas only valid forKconfig.defconfigfiles, but seems like there's no check for that either.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no Kconfig option compliance check, not sure where you got that from. There is one check which checks spacing, it does not check any validity of Kconfig files, and another that checks for "Enable" in the name, and another that checks if it is loadable, that is the extent of testing, e.g. a 700 line file will pass CI