Skip to content
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

modules: hal_nordic: nRF 802.15.4 customizable asserts #1411

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

ankuns
Copy link
Contributor

@ankuns ankuns commented Dec 4, 2023

Recent nRF 802.15.4 Radio Driver provides assert abstraction layer. The assert abstraction layer is implemented in Zephyr in following ways depending on the NRF_802154_ASSERT_CHOICE Kconfig choice.

NRF_802154_ASSERT_ZEPHYR_MINIMAL (default) gives ability to still perform run-time checking of the nRF 802.15.4 Radio Driver operation with minimum memory overhead and configurable behavior on fault detection regardless of the CONFIG_ASSERT Kconfig option value.

NRF_802154_ASSERT_ZEPHYR gives ability to use asserts provided and configurable by Zephyr including the ability to turn off the run-time checking of the nRF 802.15.4 Radio Driver operation.

Related JIRA ticket: KRKNWK-18116

Comment on lines +14 to +18
#ifdef CONFIG_USERSPACE
/* User threads aren't allowed to induce kernel panics; generate
* an oops instead.
*/
if (k_is_user_context()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why k_panic() is used here at all? Isn't k_oops() a better choice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced a driver fault is considered a kernel fault. But I won't block this PR because of this.

Comment on lines +14 to +18
#ifdef CONFIG_USERSPACE
/* User threads aren't allowed to induce kernel panics; generate
* an oops instead.
*/
if (k_is_user_context()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced a driver fault is considered a kernel fault. But I won't block this PR because of this.

@ankuns ankuns force-pushed the pr/nrf802154_asserts branch from 7c0d57e to ed3e823 Compare December 4, 2023 12:24
modules/hal_nordic/Kconfig Outdated Show resolved Hide resolved
@ankuns ankuns force-pushed the pr/nrf802154_asserts branch from ed3e823 to 8fa697c Compare December 4, 2023 12:57
Copy link
Contributor

@LuDuda LuDuda 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 awesome! Thank you for contribution! LGTM 👍

modules/hal_nordic/Kconfig Outdated Show resolved Hide resolved
modules/hal_nordic/Kconfig Outdated Show resolved Hide resolved
Recent nRF 802.15.4 Radio Driver provides assert abstraction layer.
The assert abstraction layer is implemented in Zephyr in following
ways depending on the `NRF_802154_ASSERT_CHOICE` Kconfig choice.

`NRF_802154_ASSERT_ZEPHYR_MINIMAL` (default) gives ability to still
perform run-time checking of the nRF 802.15.4 Radio Driver operation
with minimum memory overhead and configurable behavior on fault
detection regardless of the `CONFIG_ASSERT` Kconfig option value.

`NRF_802154_ASSERT_ZEPHYR` gives ability to use asserts provided and
configurable by Zephyr including the ability to turn off the run-time
checking of the nRF 802.15.4 Radio Driver operation.

Signed-off-by: Andrzej Kuroś <[email protected]>
(cherry picked from commit f5337fc)
@ankuns ankuns force-pushed the pr/nrf802154_asserts branch from 42cc3aa to 46b1970 Compare December 11, 2023 09:59
@rlubos rlubos merged commit e4511cf into nrfconnect:main Dec 11, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants