Skip to content

Conversation

@lpawelcz
Copy link
Contributor

Hi, this PR supersedes #13. It is a minimal code that should be added to hal_silabs module in order to get initial support for EFR32BG22 SoCs. Added code is free of MSLA license which was a problem in #13.
Although it is a minimal version of the previous PR, we still need to import code and headers for sleeptimer, power_manager, hfxo_manager together with device_init because we do need all of that for our initial support PR in Zephyr.

Origin: Silicon Labs Gecko SDK
URL:
https://www.silabs.com/products/development-tools/software/simplicity-studio Version: 3.1.2
Purpose: board configuration files are added.
License: Zlib
Maintained-by: External

Co-authored-by: Pawel Czarnecki [email protected]
Signed-off-by: Sateesh Kotapati [email protected]
Signed-off-by: Paweł Czarnecki [email protected]

@fkokosinski
Copy link
Member

Hi @chrta @mnkp @galak, could you take a look at these changes?

@fkokosinski
Copy link
Member

Hi @chrta @mnkp @galak, sorry for pinging you again, but could you take a look at these changes?

@lpawelcz
Copy link
Contributor Author

@fkokosinski When updating HAL code to version 4.1.1 of Gecko SDK I forgot to bump https://github.com/antmicro/hal_silabs/blob/bg22-minimal/gecko/service/device_init/src/brd4184b/sl_device_init_clocks.c file. This is now fixed.

Copy link
Member

@fkokosinski fkokosinski left a comment

Choose a reason for hiding this comment

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

LGTM

@sateeshkotapati
Copy link
Contributor

sateeshkotapati commented Nov 22, 2022

Changes look good to me.

galak
galak previously requested changes Nov 30, 2022
Copy link

@galak galak 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 remove the board code. Any board code should be handled in Zephyr.

@lpawelcz
Copy link
Contributor Author

lpawelcz commented Dec 1, 2022

can you remove the board code. Any board code should be handled in Zephyr.

@galak I removed the whole board directory. In order to do that I had to create additional header for defines used in HAL for hfxo initialization in Zephyr PR under board directory. This header is included in HAL CMakeLists.txt through ZEPHYR_BASE environment variable. We can see similar solution in hal_atmel .

Please let me know if that change is OK.

Comment on lines 24 to 26
if (${CONFIG_BOARD} STREQUAL "efr32bg_sltb010a")
set(SILABS_GECKO_BOARD "brd4184b")
endif()
Copy link

Choose a reason for hiding this comment

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

To me it looks like setting SILABS_GECKO_BOARD only effects which sl_device_init_clocks.c is compiled in.
To me the content in brd4184b/sl_device_init_clocks.c seems to be specific to SoC and not board specific.

We defined a new board which means ${CONFIG_BOARD} will not be efr32bg_stlb010a. That means without altering this CMakeLists.txt our project can not compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I removed the board specific code (sl_device_init_clocks.c) from HAL and implemented a separate clock init function in Zephyr PR as part of the board support. Now SILABS_GECKO_BOARD is not used anywhere so I deleted it. I believe that, from zephyr perspective, things like clock configuration are specific to given platform, not to whole SoC. That is why I placed the code in board init file.

@kgugala kgugala requested review from galak and removed request for chrta December 6, 2022 15:14
${ZEPHYR_BASE}/boards/arm/${CONFIG_BOARD}
)
Copy link

Choose a reason for hiding this comment

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

I think BOARD_ROOT is a better fit here. But this variable is a list.
A potential solution could be:

Suggested change
${ZEPHYR_BASE}/boards/arm/${CONFIG_BOARD}
)
)
foreach(root ${BOARD_ROOT})
if(IS_DIRECTORY "${root}/boards/arm/${CONFIG_BOARD}")
zephyr_include_directories(${root}/boards/arm/${CONFIG_BOARD})
endif()
endforeach()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, that will allow us to handle out-of-tree development. Thank you for this, I added the change to the PR.

sateeshkotapati and others added 6 commits December 7, 2022 09:59
Origin: Silicon Labs Gecko SDK
URL:
https://www.silabs.com/products/development-tools/software/simplicity-studio
Version: 3.1.2
Purpose: board configuration files are added.
License: Zlib
Maintained-by: External

Co-authored-by: Pawel Czarnecki <[email protected]>
Signed-off-by: Sateesh Kotapati <[email protected]>
Signed-off-by: Paweł Czarnecki <[email protected]>
Origin: Silicon Labs Gecko SDK
URL:
https://www.silabs.com/products/development-tools/software/simplicity-studio
Version: 4.1.1
Purpose: update HAL version from 3.1.2 to 4.1.1
License: Zlib
Maintained-by: External

Signed-off-by: Paweł Czarnecki <[email protected]>
It seems that throwing preprocessor error when
__ARM_FEATURE_CMSE is not defined is not needed.

Signed-off-by: Pawel Czarnecki <[email protected]>
@lpawelcz
Copy link
Contributor Author

lpawelcz commented Dec 7, 2022

@galak I addressed your comment about removing the board code. Please have a look at this PR.

@fkokosinski fkokosinski dismissed galak’s stale review December 14, 2022 09:36

Hi @galak, I'm dismissing your review because it looks like your comment had been addressed, and this PR has been stale for a while.

Copy link
Contributor

@sateeshkotapati sateeshkotapati left a comment

Choose a reason for hiding this comment

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

Looks good to me

@stephanosio stephanosio merged commit 9dcaa76 into zephyrproject-rtos:master Dec 20, 2022
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.

6 participants