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 1 commit
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
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
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
17 changes: 8 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 @@ -45,24 +51,17 @@ config FLASH_PAGE_LAYOUT
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