Skip to content

Bluetooth: Add missing depends on for stack size Kconfig #90048

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cvinayak
Copy link
Collaborator

Add missing depends on for stack size Kconfig values.

@cvinayak cvinayak marked this pull request as ready for review May 16, 2025 12:14
@github-actions github-actions bot added area: Bluetooth HCI Bluetooth HCI Driver area: Bluetooth area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels May 16, 2025
@jhedberg
Copy link
Member

jhedberg commented May 16, 2025

Thanks for this! It's been annoying me as well that these show up in .config even though they don't get used for anything. However, seems like there might be some #ifdefs or similar missing somewhere, based on the CI failure.

@cvinayak
Copy link
Collaborator Author

Thanks for this! It's been annoying me as well that these show up in .config even though they don't get used for anything. However, seems like there might be some #ifdefs or similar missing somewhere, based on the CI failure.

There are few more of these unused stack sizes... will get to them too only I get current CI happy.

default 256
help
Stack size for the HCI driver's TX thread.

config BT_DRV_RX_STACK_SIZE
int
depends on BT_H4 || BT_H5 || BT_SPI || BT_HCI_RAW_H4 || BT_AMBIQ_HCI || BT_STM32_IPM || HCI_NXP_RX_THREAD
Copy link
Member

Choose a reason for hiding this comment

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

A more maintainable/scalable solution for something like this is to define a virtual option that you depend on, and then each driver (that needs this) selects that virtual option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it make more sense to have each of the drivers define their own Kconfig variable? Is there a reason it makes sense to have a shared variable?

Copy link
Member

Choose a reason for hiding this comment

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

That's a fair point. Driver specific options should be fine as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wouldn't it make more sense to have each of the drivers define their own Kconfig variable? Is there a reason it makes sense to have a shared variable?

Not sure of the history behind the common BT_DRV_RX_STACK_SIZE being used, but some of the vendors already has driver specific rx stack size values defined, that is being assigned to this shared Kconfig.

@github-actions github-actions bot requested a review from wopu-ot May 19, 2025 10:13
@cvinayak cvinayak force-pushed the github_stack_size_depends branch 5 times, most recently from 40a4410 to 133f446 Compare May 19, 2025 12:42
Add missing depends on for stack size Kconfig values.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak cvinayak force-pushed the github_stack_size_depends branch from 133f446 to 1b9bca2 Compare May 19, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Classic Bluetooth Classic (BR/EDR) area: Bluetooth Controller area: Bluetooth HCI Bluetooth HCI Driver area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth platform: Ambiq Ambiq platform: NXP Drivers NXP Semiconductors, drivers platform: Silabs Silicon Labs platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants