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

boards: actinius_*: fix board init priority #36906

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

alextsam
Copy link
Contributor

This fixes an issue that surfaced with Zephyr v2.6.0, where the GPIO driver has not completed initialization when attempting to use it during POST_KERNEL with KERNEL_INIT_PRIORITY_DEFAULT.

Signed-off-by: Alex Tsamakos [email protected]

This fixes an issue that surfaced with Zephyr v2.6.0,
where the GPIO driver has not completed initialization
when attempting to use it during POST_KERNEL with
KERNEL_INIT_PRIORITY_DEFAULT.

Signed-off-by: Alex Tsamakos <[email protected]>
@marinjurjevic
Copy link
Contributor

marinjurjevic commented Jul 14, 2021

I tested your change on nrf-sdk v1.6.0 which uses v2.6.0-rc1. I am still getting the same error.
I've got a feeling this is connected with SPM image.
Also, board_actinius_icarus_init is only called during SPM image boot. During app image boot this function is not called.

Shouldn't board.c also be linked in app image? I've tested this with blinky and nrf9160/at_client samples.

@alextsam
Copy link
Contributor Author

I tested your change on nrf-sdk v1.6.0 which uses v2.6.0-rc1. I am still getting the same error.
I've got a feeling this is connected with SPM image.
Also, board_actinius_icarus_init is only called during SPM image boot. During app image boot this function is not called.

Shouldn't board.c also be linked in app image? I've tested this with blinky and nrf9160/at_client samples.

It is actually linked to both the SPM and app. I am suspecting that you say that the function is not called in the app because you don't see a message about it, is that correct? The at_client has logging turned off; if you add "CONFIG_LOG=y" in your prj.conf you should then see this:

*** Booting Zephyr OS build v2.6.0-rc1-ncs1  ***
The AT host sample started
[00:00:00.201,538] <inf> board_control: eSIM is selected

@marinjurjevic
Copy link
Contributor

Yes, you are correct, logging was turned off. Thank you for clarifying.

This still leaves SPM image printing on boot:

E: Could not get GPIO Device Binding

This is is probably be due to CONFIG_GPIO=n in spm image prj.conf.

@alextsam
Copy link
Contributor Author

This is is probably be due to CONFIG_GPIO=n in spm image prj.conf.

Indeed, this is because the SPM project is overriding CONFIG_GPIO and turns it off. I don't think there is a nice fix for that; either we have to stop the board_init from being included in the secure image, or your firmware project needs to add a conf for the SPM to override CONFIG_GPIO back to y

@marinjurjevic
Copy link
Contributor

marinjurjevic commented Jul 14, 2021

either we have to stop the board_init from being included in the secure image

Does zephyr (or sdk-nrf) have support for conditional inclusion of board.c? I am not all that much familiar with the build system, but I can see there zephyr_library_sources(board.c) in boards CMakeLists.txt which indicates there is no such mechanism at the moment.

For this particular problem this is fine, as we can ignore the failed SIM selection in SPM, as long as it's correctly initialized in the app image which I tested and it is.

SYS_INIT(board_actinius_icarus_init, POST_KERNEL,
CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
/* Needs to happen after GPIO driver init */
SYS_INIT(board_actinius_icarus_init, POST_KERNEL, 99);
Copy link
Member

Choose a reason for hiding this comment

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

Suggest adding a new Kconfig to make this configurable instead of hardcoding the value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO this operation should not be made configurable as the dependency is that it runs after the GPIO driver init, similar to #33740. Would that be ok?

@MaureenHelm MaureenHelm merged commit 094a8ac into zephyrproject-rtos:main Aug 2, 2021
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.

4 participants