Skip to content

Out of tree (driver) configuration support #13631

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Out of tree (driver) configuration support #13631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da850d5
kconfig: Use depend on instead of select to avoid kconfig loop
Feb 21, 2019
b35d39a
kconfig: bluetooth: Let BT_HCI_VS be configurable from controller
Feb 21, 2019
2478ce5
drivers: nrf5: Don't force specific drivers from soc/arm/nordic_nrf
Feb 21, 2019
f92154f
drivers: clock_control: nrf: add choice variable
Feb 21, 2019
8f1e6cb
drivers: add support for forcing out-of-tree drivers
Feb 21, 2019
8e8c9fe
bluetooth: host: Specify controller in BT_HCI_TX_STACK_SIZE defaults
Feb 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion boards/posix/nrf52_bsim/Kconfig.board
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ config BOARD_NRF52_BSIM
select CONSOLE_HAS_DRIVER
select NRF_RTC_TIMER
select CLOCK_CONTROL
select CLOCK_CONTROL_NRF
help
Will produce a console Linux process which can be executed natively.
It needs the BabbleSim simulator both in compile time and to execute
16 changes: 10 additions & 6 deletions drivers/clock_control/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
#
# Clock controller drivers
#

config CLOCK_CONTROL_MUST_USE_OUT_OF_TREE_DRIVER
Copy link
Contributor

@ulfalizer ulfalizer Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe this could be called CLOCK_CONTROL_OUT_OF_TREE_DRIVER or the like, to shorten it a bit. It might be clear anyway that there must be an out-of-tree driver if it's on.

@carlescufi suggested something similar. He's more familiar than me with Bluetooth though.

Copy link
Member Author

@thomasstenersen thomasstenersen Mar 12, 2019

Choose a reason for hiding this comment

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

(@carlescufi) We had a naming discussion here: #13631 (comment)

The reason for the _MUST_USE_ (originally _HAS_) was to keep consistency with other select-able Kconfig symbols. If we keep select-able symbols on the form <noun>_<verb>_<noun> (FOO_HAS_BAR), it is much easier to keep your sanity when working with the Kconfig files.

The name is verbose, but I'd rather sacrifice verbosity for clarity.

Copy link
Member

@carlescufi carlescufi Mar 12, 2019

Choose a reason for hiding this comment

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

If you really want to enforce the <noun>_<verb>_<noun> (which I don't think we could call a convention right now except for the _HAS_ case) then I would go for something like *_SELECT_OOT or REQUIRE_OOT something like that. MUST_USE sounds convoluted and verbose to me.

Copy link
Contributor

@SebastianBoe SebastianBoe Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The original proposal of _HAS_ didn't make sense, but now I'm unsure if we actually considered using an option without HAS. As Ulf says, if the OOT driver is enabled, that does imply that the other drivers can't be. The symbol for an enabled out-of-tree driver would look like:

CLOCK_CONTROL_OUT_OF_TREE_DRIVER

Copy link
Member

@nashif nashif Mar 12, 2019

Choose a reason for hiding this comment

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

hmmm, why do we need to have Kconfigs for this? Are we going to have such a kconfig for each driver subsystem? Are we talking about 2 conflicting drivers and we would select the one out of tree instead of the one in the tree? In short, this looks like a hack to me. Need some documentation or a design proposal explaining how this can be done in a generic way without polluting Kconfig with such options.

Copy link
Contributor

@SebastianBoe SebastianBoe Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Need some documentation or a design proposal explaining how this can be done in a generic way without polluting Kconfig with such options.

Please prove me wrong @ulfalizer; it is unavoidable to pollute the kconfig with these options without resolving #8181, which is not going to happen any time soon.

For your other questions, please see the commit message in 8f1e6cb

Copy link
Member

@carlescufi carlescufi Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The reasoning is described in this commit message: 8f1e6cb, but I think @thomasstenersen might want to update the PR description with a clear definition of the problem and why these additional Kconfig options are required.

Copy link
Member Author

@thomasstenersen thomasstenersen Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Edit: hadn't refreshed the page before commenting. I can copy the following comment to the PR description. It provides an answer to what would be my initial reaction to seeing this PR as well :)

Yes we need this, unfortunately. This is for the case were an out-of-tree component for some reason must use it's own driver. There is no way in Kconfig that I and others have found to effectively de-select a Kconfig choice/config without severely polluting the Zephyr configuration files.

Are we going to have such a kconfig for each driver subsystem?

I would add it at a per use-case/need basis. But this is a generic pattern, yes.

Are we talking about 2 conflicting drivers and we would select the one out of tree instead of the one in the tree?

Yes. And the fact that they are not only conflicting but another subsystem must use the out-of-tree one. We also want this switch to happen automatically, so if I select a different BLE controller in the menuconfig, the correct driver dependencies are resolved without the need for other changes by the user.

In short, this looks like a hack to me.

I would like it for there to be a nicer way of doing this. But as far as I can tell, this is the most pragmatic, least intrusive and nicest way of handling out-of-tree dependencies like this. IMO the only way of actually solving the root problem is to resolve #8181.

Need some documentation or a design proposal [...]

What would such a proposal look like? Could I add more to 8f1e6cb to explain the why and how better?

bool
help
This option can be enabled by an out-of-tree library if it
needs to take control over the clock controller.

menuconfig CLOCK_CONTROL
bool "Hardware clock controller support"
help
Expand All @@ -23,18 +30,15 @@ module = CLOCK_CONTROL
module-str = clock control
source "subsys/logging/Kconfig.template.log_config"

source "drivers/clock_control/Kconfig.nrf"

if !CLOCK_CONTROL_MUST_USE_OUT_OF_TREE_DRIVER
source "drivers/clock_control/Kconfig.nrf"
source "drivers/clock_control/Kconfig.quark_se"

source "drivers/clock_control/Kconfig.stm32"

source "drivers/clock_control/Kconfig.beetle"

source "drivers/clock_control/Kconfig.mcux_ccm"

source "drivers/clock_control/Kconfig.mcux_sim"

source "drivers/clock_control/Kconfig.rv32m1"
endif # !CLOCK_CONTROL_MUST_USE_OUT_OF_TREE_DRIVER

endif # CLOCK_CONTROL
5 changes: 3 additions & 2 deletions drivers/clock_control/Kconfig.nrf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
menuconfig CLOCK_CONTROL_NRF
bool "NRF Clock controller support"
depends on SOC_COMPATIBLE_NRF
default y
help
Enable support for the Nordic Semiconductor nRFxx series SoC clock
driver.

if CLOCK_CONTROL_NRF

choice
choice CLOCK_CONTROL_NRF_SOURCE
prompt "32KHz clock source"
default CLOCK_CONTROL_NRF_K32SRC_XTAL

Expand All @@ -34,7 +35,7 @@ config CLOCK_CONTROL_NRF_K32SRC_BLOCKING
initially start running and automatically switch to crystal when
ready.

choice
choice CLOCK_CONTROL_NRF_ACCURACY
prompt "32KHz clock accuracy"
default CLOCK_CONTROL_NRF_K32SRC_500PPM if CLOCK_CONTROL_NRF_K32SRC_RC
default CLOCK_CONTROL_NRF_K32SRC_20PPM
Expand Down
9 changes: 9 additions & 0 deletions drivers/entropy/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,28 @@
# SPDX-License-Identifier: Apache-2.0
#

config ENTROPY_MUST_USE_OUT_OF_TREE_DRIVER
bool
help
This option can be enabled by an out-of-tree library if it
needs to take control over the entropy driver.

menuconfig ENTROPY_GENERATOR
bool "Entropy Drivers"
help
Include entropy drivers in system config.


if ENTROPY_GENERATOR

if !ENTROPY_MUST_USE_OUT_OF_TREE_DRIVER
source "drivers/entropy/Kconfig.mcux"
source "drivers/entropy/Kconfig.stm32"
source "drivers/entropy/Kconfig.esp32"
source "drivers/entropy/Kconfig.nrf5"
source "drivers/entropy/Kconfig.sam"
source "drivers/entropy/Kconfig.native_posix"
endif # !ENTROPY_MUST_USE_OUT_OF_TREE_DRIVER

config ENTROPY_HAS_DRIVER
bool
Expand Down
2 changes: 1 addition & 1 deletion drivers/entropy/Kconfig.nrf5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ menuconfig ENTROPY_NRF5_RNG
bool "nRF5 RNG driver"
depends on SOC_COMPATIBLE_NRF
select ENTROPY_HAS_DRIVER
default y
help
This option enables the RNG peripheral, which is a random number
generator, based on internal thermal noise, that provides a
Expand Down Expand Up @@ -59,7 +60,6 @@ config ENTROPY_NRF5_ISR_THRESHOLD
buffer goes below this number hardware entropy generation will be
started.


config ENTROPY_NRF5_PRI
int "RNG interrupt priority"
range 0 2 if SOC_SERIES_NRF51X
Expand Down
18 changes: 9 additions & 9 deletions drivers/flash/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ config FLASH_HAS_PAGE_LAYOUT
This option is enabled when the SoC flash driver supports
retrieving the layout of flash memory pages.

config FLASH_MUST_USE_OUT_OF_TREE_DRIVER
bool
help
This option can be enabled by an out-of-tree library if it
needs to take control over the flash memory peripheral.

menuconfig FLASH
bool "Flash hardware support"
help
Expand All @@ -41,27 +47,21 @@ config FLASH_SHELL
config FLASH_PAGE_LAYOUT
bool "API for retrieving the layout of pages"
depends on FLASH_HAS_PAGE_LAYOUT
default y
help
Enables API for retrieving the layout of flash memory pages.

if !FLASH_MUST_USE_OUT_OF_TREE_DRIVER
source "drivers/flash/Kconfig.nrf"

source "drivers/flash/Kconfig.mcux"

source "drivers/flash/Kconfig.nios2_qspi"

source "drivers/flash/Kconfig.gecko"

source "drivers/flash/Kconfig.nor"

source "drivers/flash/Kconfig.qmsi"

source "drivers/flash/Kconfig.stm32"

source "drivers/flash/Kconfig.sam0"

source "drivers/flash/Kconfig.sam"

source "drivers/flash/Kconfig.w25qxxdv"
endif # !FLASH_MUST_USE_OUT_OF_TREE_DRIVER

endif # FLASH
1 change: 1 addition & 0 deletions drivers/flash/Kconfig.nrf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ config SOC_FLASH_NRF
depends on SOC_FAMILY_NRF
select FLASH_HAS_PAGE_LAYOUT
select FLASH_HAS_DRIVER_ENABLED
default y
help
Enables Nordic Semiconductor nRF flash driver.

Expand Down
3 changes: 2 additions & 1 deletion drivers/timer/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ config ALTERA_AVALON_TIMER
config NRF_RTC_TIMER
bool "nRF Real Time Counter (NRF_RTC1) Timer"
default y
depends on CLOCK_CONTROL_NRF
depends on CLOCK_CONTROL
depends on SOC_COMPATIBLE_NRF
select TICKLESS_CAPABLE
help
This module implements a kernel device driver for the nRF Real Time
Expand Down
1 change: 1 addition & 0 deletions samples/bluetooth/ipsp/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ CONFIG_BT=y
CONFIG_BT_DEBUG_LOG=y
CONFIG_BT_SMP=y
CONFIG_BT_PERIPHERAL=y
CONFIG_BT_CENTRAL=y
CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
CONFIG_BT_DEVICE_NAME="Test IPSP node"
CONFIG_NETWORKING=y
Expand Down
1 change: 1 addition & 0 deletions samples/bluetooth/ipsp/prj_dbg.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ CONFIG_BT=y
CONFIG_BT_DEBUG_LOG=y
CONFIG_BT_SMP=y
CONFIG_BT_PERIPHERAL=y
CONFIG_BT_CENTRAL=y
CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
CONFIG_BT_DEVICE_NAME="Test IPSP node"
CONFIG_NETWORKING=y
Expand Down
1 change: 1 addition & 0 deletions samples/bluetooth/ipsp/prj_zep1656.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ CONFIG_BT=y
CONFIG_BT_DEBUG_LOG=y
CONFIG_BT_SMP=y
CONFIG_BT_PERIPHERAL=y
CONFIG_BT_CENTRAL=y
CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
CONFIG_BT_DEVICE_NAME="Test IPSP node"
CONFIG_NETWORKING=y
Expand Down
14 changes: 0 additions & 14 deletions soc/arm/nordic_nrf/Kconfig.defconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,6 @@ source "soc/arm/nordic_nrf/*/Kconfig.defconfig.series"
config BUILD_OUTPUT_HEX
default y

if FLASH

config SOC_FLASH_NRF
default y

endif # FLASH

if ENTROPY_GENERATOR

config ENTROPY_NRF5_RNG
default y

endif # ENTROPY_GENERATOR

if SPI

config GPIO
Expand Down
1 change: 0 additions & 1 deletion soc/arm/nordic_nrf/nrf51/Kconfig.series
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ config SOC_SERIES_NRF51X
select SOC_FAMILY_NRF
select NRF_RTC_TIMER
select CLOCK_CONTROL
select CLOCK_CONTROL_NRF
select HAS_STATE_DEEP_SLEEP_1
select XIP
select HAS_CMSIS
Expand Down
1 change: 0 additions & 1 deletion soc/arm/nordic_nrf/nrf52/Kconfig.series
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ config SOC_SERIES_NRF52X
select SOC_FAMILY_NRF
select NRF_RTC_TIMER
select CLOCK_CONTROL
select CLOCK_CONTROL_NRF
select HAS_STATE_DEEP_SLEEP_1
select XIP
select HAS_CMSIS
Expand Down
1 change: 0 additions & 1 deletion soc/arm/nordic_nrf/nrf91/Kconfig.series
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ config SOC_SERIES_NRF91X
select SOC_FAMILY_NRF
select NRF_RTC_TIMER
select CLOCK_CONTROL
select CLOCK_CONTROL_NRF
select HAS_STATE_DEEP_SLEEP_1
select XIP
select HAS_CMSIS
Expand Down
7 changes: 7 additions & 0 deletions subsys/bluetooth/common/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,15 @@
# SPDX-License-Identifier: Apache-2.0
#

config BT_HAS_HCI_VS
bool
help
This option is set by the Bluetooth controller to indicate support
for the Zephyr HCI Vendor-Specific Commands and Event.

config BT_HCI_VS
bool "Zephyr HCI Vendor-Specific Commands"
depends on BT_HAS_HCI_VS
default y
help
Enable support for the Zephyr HCI Vendor-Specific Commands in the
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/controller/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ choice BT_LL_CHOICE
config BT_LL_SW
bool "Software-based BLE Link Layer"
select BT_RECV_IS_RX_THREAD
select BT_HAS_HCI_VS
select ENTROPY_GENERATOR
select ENTROPY_NRF5_RNG if SOC_COMPATIBLE_NRF
select ENTROPY_NRF5_BIAS_CORRECTION if SOC_COMPATIBLE_NRF
Expand Down
10 changes: 6 additions & 4 deletions subsys/bluetooth/host/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,19 @@ config BT_RX_BUF_LEN
(4 bytes) and the ACL header (also 4 bytes) which yields 73 bytes.

config BT_HCI_TX_STACK_SIZE
# NOTE: This value is derived from other symbols and should not be
# user-configurable. Do not give it a prompt.
int
default 512 if BT_H4
default 512 if BT_H5
default 416 if BT_SPI
default 940 if BT_CTLR && NO_OPTIMIZATIONS
default 1024 if BT_CTLR && BT_CENTRAL
default 640 if BT_CTLR
default 940 if BT_CTLR && (BT_LL_SW || BT_LL_SW_SPLIT) && NO_OPTIMIZATIONS
default 1024 if BT_CTLR && (BT_LL_SW || BT_LL_SW_SPLIT) && BT_CENTRAL
default 640 if BT_CTLR && (BT_LL_SW || BT_LL_SW_SPLIT)
default 512 if BT_USERCHAN
# Even if no driver is selected the following default is still
# needed e.g. for unit tests.
default 512
default 1024
help
Stack size needed for executing bt_send with specified driver

Expand Down
2 changes: 1 addition & 1 deletion subsys/fs/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ config FAT_FILESYSTEM_ELM

config FILE_SYSTEM_NFFS
bool "NFFS file system support"
select FLASH_PAGE_LAYOUT
depends on FLASH_PAGE_LAYOUT
select FS_FLASH_STORAGE_PARTITION
help
Enables NFFS file system support.
Expand Down
10 changes: 5 additions & 5 deletions subsys/net/l2/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ config NET_L2_DUMMY
config NET_L2_BT
bool "Enable Bluetooth support"
depends on NET_IPV6
select BT
select BT_PERIPHERAL
select BT_CENTRAL
select BT_SMP
select BT_L2CAP_DYNAMIC_CHANNEL
depends on BT
depends on BT_PERIPHERAL
depends on BT_CENTRAL
depends on BT_SMP
depends on BT_L2CAP_DYNAMIC_CHANNEL
select NET_6LO
help
Enable Bluetooth driver that send and receives IPv6 packets,
Expand Down
6 changes: 6 additions & 0 deletions tests/net/all/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ CONFIG_NET_L2_IEEE802154_SECURITY=y
CONFIG_NET_L2_IEEE802154_SECURITY_CRYPTO_DEV_NAME="CRYPTO-DEV"
CONFIG_NET_L2_DUMMY=y
CONFIG_NET_L2_ETHERNET=y

CONFIG_BT=y
CONFIG_BT_PERIPHERAL=y
CONFIG_BT_CENTRAL=y
CONFIG_BT_SMP=y
CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y
CONFIG_NET_L2_BT=y
CONFIG_NET_L2_BT_ZEP1656=y
CONFIG_NET_L2_BT_SEC_LEVEL=4
Expand Down