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

Add support for Actinius Icarus SoM Development Kit #51790

Conversation

alextsam
Copy link
Contributor

This adds support for the Actinius Icarus SoM DK (www.actinius.com/icarus-som-dk) and patches other Actinius boards for silencing a build-time warning.

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

@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch 2 times, most recently from f9b9a8a to ed82c99 Compare October 31, 2022 00:44
@alextsam
Copy link
Contributor Author

@gmarull @carlescufi can this make it into Zephyr before the upcoming release?

boards/arm/actinius_icarus/board.c Outdated Show resolved Hide resolved
boards/arm/actinius_icarus/board_ns.c Outdated Show resolved Hide resolved
boards/arm/actinius_icarus_bee/board.c Outdated Show resolved Hide resolved
boards/arm/actinius_icarus_som_dk/board.c Outdated Show resolved Hide resolved
boards/arm/actinius_icarus_som_dk/board_ns.c Outdated Show resolved Hide resolved
@carlescufi
Copy link
Member

@alextsam please address review comments

@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch 3 times, most recently from e813f62 to a33ed6c Compare December 10, 2022 20:00
@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch 3 times, most recently from 24abc20 to 90854f4 Compare December 15, 2022 12:46
@alextsam alextsam requested review from mbolivar-nordic and removed request for gmarull December 15, 2022 14:56
Comment on lines +6 to +8
zephyr_library()

add_subdirectory(${ZEPHYR_BASE}/boards/common/actinius actinius_common)
Copy link
Member

Choose a reason for hiding this comment

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

@tejlmand can you please review this?

boards/arm/actinius_icarus_som_dk/CMakeLists.txt Outdated Show resolved Hide resolved
boards/common/actinius/Kconfig Outdated Show resolved Hide resolved
@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch 2 times, most recently from 2efa27d to 3415549 Compare January 8, 2023 03:28
@alextsam alextsam requested a review from gmarull January 8, 2023 03:52
boards/arm/actinius_icarus_som_dk/arduino_connector.dtsi Outdated Show resolved Hide resolved
Comment on lines 123 to 124
irq-gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>,
<&gpio0 28 GPIO_ACTIVE_HIGH>;
Copy link
Member

Choose a reason for hiding this comment

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

align properly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I improved the indentation, but these cannot align completely without either using spaces or by pushing the gpio of the first line further, which in my opinion looks weird

boards/common/actinius/actinius_board_common.c Outdated Show resolved Hide resolved
@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch 2 times, most recently from cead00e to 2b6c30b Compare January 10, 2023 15:46
@alextsam alextsam requested a review from gmarull January 10, 2023 16:18
@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch from 2b6c30b to 086b613 Compare January 12, 2023 16:48
@alextsam alextsam requested review from mbolivar-nordic and removed request for gmarull January 19, 2023 14:45
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Can you fix the commit logs? Looks like you have some extra signed-off-by lines due to some squashing. One question about error handling -- is this really what you mean?

Generally looks fine to me, thanks for your persistence!

#endif

#if DT_HAS_COMPAT_STATUS_OKAY(actinius_charger_enable)
result = actinius_board_set_charger_enable();
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't a success here overwrite an error on line 75?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the commit log, thanks, I missed this earlier.

Regarding the error handling, basically it's relying on logging here; maybe we should even remove the return as this function is only used with SYS_INIT and -please correct me if I am wrong- there is no way to get the return value from the kernel.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should even remove the return

No objections from my side if it's OK to proceed despite errors on your hardware

This adds support for the nRF9160-based Icarus SoM DK
(development kit) from Actinius and a common library for
board init code that is common to multiple boards
from Actinius.

Signed-off-by: Alex Tsamakos <[email protected]>
This changes all Actinius boards to use the helper lib in
`boards/common/actinius` for setting up common init values
such as SIM Select and Charger Enable.

Signed-off-by: Alex Tsamakos <[email protected]>
@alextsam alextsam force-pushed the feature/add-dk-and-fix-cmake-wrn branch from 086b613 to 2565bcc Compare January 20, 2023 16:09
@mbolivar-nordic mbolivar-nordic merged commit c348fb5 into zephyrproject-rtos:main Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants