Skip to content

ext: hal: nordic: Import soc/nrfx_coredep.h from nrfx 1.3.1 #11644

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

Merged

Conversation

anangl
Copy link
Member

@anangl anangl commented Nov 26, 2018

This is a follow-up to commit b45fcc6.
The nrfx import is complemented with the file containing
a core-dependent delay function. The intention is to allow it
to be used in a custom k_busy_wait() implementation.

Origin: nrfx
License: BSD 3-Clause
URL: https://github.com/NordicSemiconductor/nrfx/tree/v1.3.1
commit: d4ebe15f58de1442e3eed93b40d13930e7785903
Purpose: Provide peripheral drivers for Nordic SoCs
Maintained-by: External

Signed-off-by: Andrzej Głąbek [email protected]

This is a follow-up to commit b45fcc6.
The nrfx import is complemented with the file containing
a core-dependent delay function. The intention is to allow it
to be used in a custom k_busy_wait() implementation.

Origin: nrfx
License: BSD 3-Clause
URL: https://github.com/NordicSemiconductor/nrfx/tree/v1.3.1
commit: d4ebe15f58de1442e3eed93b40d13930e7785903
Purpose: Provide peripheral drivers for Nordic SoCs
Maintained-by: External

Signed-off-by: Andrzej Głąbek <[email protected]>
@zephyrbot
Copy link
Collaborator

Found the following issues, please fix and resubmit:

License/Copyright issues

  • ext/hal/nordic/nrfx/soc/nrfx_coredep.h is not apache-2.0 licensed: bsd-new

@pabigot
Copy link
Collaborator

pabigot commented Nov 26, 2018

@anangl Please consider adding

  zephyr_include_directories(nrfx/soc)

to CMakeLists.txt alongside the other paths so the new file can be accessed in the same way as the others from nrfx.

@anangl
Copy link
Member Author

anangl commented Nov 26, 2018

@anangl Please consider adding

  zephyr_include_directories(nrfx/soc)

to CMakeLists.txt alongside the other paths so the new file can be accessed in the same way as the others from nrfx.

This file is intended to be accessed as <soc/nrfx_coredep.h> (or <nrfx/soc/nrfx_coredep.h> if in a given context it would be more appropriate, for instance to avoid confusion with some other soc/ folder), similarly to nrfx HAL files which should be (and mostly are) accessed as <hal/nrf_*.h>.

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11644   +/-   ##
=======================================
  Coverage   48.22%   48.22%           
=======================================
  Files         279      279           
  Lines       43285    43285           
  Branches    10357    10357           
=======================================
  Hits        20875    20875           
  Misses      18270    18270           
  Partials     4140     4140

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 38efc2d...24531d9. Read the comment docs.

@pabigot
Copy link
Collaborator

pabigot commented Nov 26, 2018

OK. It's a bit confusing to have hal (and maybe mdk) in the cmake include list if the intended access for these files is to be relative to the nrfx directory root.

@carlescufi carlescufi added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Nov 26, 2018
@carlescufi carlescufi merged commit dc7e8a9 into zephyrproject-rtos:master Nov 26, 2018
@anangl anangl deleted the import_nrfx_coredep_1_3_1 branch November 26, 2018 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants