Skip to content

Conversation

@thomasstenersen
Copy link
Member

An external project extending the Zephyr RTOS and its drivers may have
subsystems that must use its own specific driver(s) when active. One
example is the nRF5x NVMC that must be scheduled in between radio
operations. A subsystem may also be dependent on its own drivers for
security, real-time and/or because of hardware constrains.

In order to not introduce non-Zephyr specific code into the Zephyr tree,
an option is added to disable the in-tree drivers in Zephyr. Because
Kconfig does not support a good way of de-selecting other symbols, a
variable on the form <DRIVER>_OUT_OF_TREE_DRIVER is added as a
dependency for each <DRIVER>. For example, the out-of-tree subsystem
will select FLASH_OUT_OF_TREE_DRIVER to disable the in-tree driver. A
solution for issue #8181 would open up for a more general solution,
however #8181 requires significant effort.

Support for out-of-tree drivers is added to Nordic drivers for
clock_control, entropy and flash.

See #13631 for earlier discussion.

Signed-off-by: Thomas Stenersen [email protected]

@thomasstenersen thomasstenersen requested a review from nashif as a code owner March 14, 2019 10:03
@carlescufi carlescufi added the dev-review To be discussed in dev-review meeting label Mar 14, 2019
@codecov-io
Copy link

codecov-io commented Mar 14, 2019

Codecov Report

Merging #14509 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #14509   +/-   ##
=======================================
  Coverage   51.96%   51.96%           
=======================================
  Files         309      309           
  Lines       45576    45576           
  Branches    10554    10554           
=======================================
  Hits        23683    23683           
  Misses      17083    17083           
  Partials     4810     4810

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8aba2d5...b127536. Read the comment docs.

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

This is a workaround so the following items need to be addressed:

  • Open an issue describing the more general problem and link to it in the commit an dPR
  • Change the names so that they are NRF-only
  • Depend on SOC_COMPATIBLE_NRF

Copy link
Contributor

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

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

@aescolar filled me in a bit re. the dev. meeting.

I think it's better to keep this as a local one-off hack until it's needed more broadly (hopefully it won't be). Just be careful so that the one-off doesn't get copied to a bunch of places. :)

@thomasstenersen
Copy link
Member Author

The shippable failed, but when skimming through the log it looked like xtensa things failed. I rebased with changes from master and pushed again. @carlescufi is there anything missing?

@nashif
Copy link
Member

nashif commented Mar 15, 2019

I propose to drop the out-of-tree naming and call it alternate or something along those lines. This concept should also work with 2 compatible drivers in the same tree.

@carlescufi
Copy link
Member

carlescufi commented Mar 15, 2019

@thomasstenersen @nashif I agree with the proposal, I never liked OUT_OF_TREE. I suggest:

CLOCK_CONTROL_NRF_ALT or CLOCK_CONTROL_NRF_REPLACE or CLOCK_CONTROL_NRF_FORCE_ALT

or we could use the compatible concept from DT:
CLOCK_CONTROL_NRF_COMPATIBLE, or CLOCK_CONTROL_NRF_ALT_COMPAT or CLOCK_CONTROL_NRF_USE_COMPAT

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Waiting for agreement on naming

@carlescufi carlescufi added this to the v1.14.0 milestone Mar 15, 2019
@thomasstenersen
Copy link
Member Author

If you are OK with CLOCK_CONTROL_NRF_FORCE_ALT I will use that, @nashif.

@thomasstenersen
Copy link
Member Author

@nashif @carlescufi I changed the name to _FORCE_ALT.

Copy link
Member

Choose a reason for hiding this comment

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

Can you update the descriptions so that they reflect "force alternative implementation"?

Copy link
Member

Choose a reason for hiding this comment

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

i.e. not refer to out-of-tree but rather keep it focused on what it actually does

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

An external project extending the Zephyr RTOS and its drivers may have
subsystems that must use its own specific driver(s) when active. One
example is the nRF5x NVMC that must be scheduled in between radio
operations. A subsystem may also be dependent on its own drivers for
security, real-time and/or because of hardware constrains.

In order to not introduce non-Zephyr specific code into the Zephyr tree,
an option is added to disable the in-tree drivers in Zephyr. Because
Kconfig does not support a good way of de-selecting other symbols, a
variable on the form `<DRIVER>_FORCE_ALT` is added as a
dependency for each `<DRIVER>`. For example, the out-of-tree subsystem
will select `FLASH_NRF_FORCE_ALT` to disable the in-tree driver. A
solution for issue zephyrproject-rtos#8181 would open up for a more general solution,
however zephyrproject-rtos#8181 requires significant effort.

Support for out-of-tree drivers is added to Nordic drivers for
clock_control, entropy and flash.

A generic solution for this is desired. Issue zephyrproject-rtos#14527 is tracking that
progress.

Signed-off-by: Thomas Stenersen <[email protected]>
@thomasstenersen
Copy link
Member Author

Shippable failed (timed out apparently?), rebased on top of master to try this again.

@galak galak added the TSC Topics that need TSC discussion label Mar 18, 2019
@galak galak merged commit db90e24 into zephyrproject-rtos:master Mar 20, 2019
@thomasstenersen thomasstenersen deleted the out_of_tree_config branch March 20, 2019 17:24
@pabigot pabigot removed the dev-review To be discussed in dev-review meeting label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Kconfig TSC Topics that need TSC discussion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants