Skip to content

Conversation

shontal1005
Copy link

@shontal1005 shontal1005 commented May 7, 2025

Adds support for Silicon Labs EFR32ZG28 SoC and BRD4401C Radio Board.

Copy link

github-actions bot commented May 7, 2025

Hello @shontal1005, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

kartben
kartben previously requested changes May 7, 2025
Copy link
Contributor

@kartben kartben 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 missing a documentation page
see https://github.com/zephyrproject-rtos/zephyr/blob/76e1fc7713a4a3f2b50c497afddec0364a34c10b/doc/templates/board.tmpl or other SiLabs boards for examples

Thanks!

@jerome-pouiller
Copy link
Contributor

jerome-pouiller commented May 7, 2025

The commit is large. Would you mind to split it in two parts (or more)? Maybe one commit for the soc and one commit for the board?

BTW, don't this PR require any change on the hal? If yes, you need to update west.yml.

compatible = "fixed-factor-clock";
clocks = <&lfrco>;
};
wdog0clk: wdog0clk {
Copy link
Contributor

Choose a reason for hiding this comment

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

peripherals need to be disabled in the base dts file, then enabled as-needed by boards

Copy link
Author

Choose a reason for hiding this comment

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

It seems that on all other platforms the peripherals are not disabled

Comment on lines 24 to 25
config SOC_GECKO_SDID
default 235 if SOC_SERIES_EFR32ZG28
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is an existing symbol, put it in Kconfig.defconfig

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand, it seems like all the other platforms do the same thing as I did

Copy link
Contributor

Choose a reason for hiding this comment

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

Then they need to move Kconfig adds new symbols, if you are changing the default of an existing symbol then it goes in Kconfig.defconfig

Copy link
Contributor

Choose a reason for hiding this comment

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

@shontal1005, do you want to carry this change or you prefer I open a PR?

Copy link
Author

Choose a reason for hiding this comment

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

If the change I made is what you meant, I can open another PR.

@github-actions github-actions bot added area: UART Universal Asynchronous Receiver-Transmitter area: ADC Analog-to-Digital Converter (ADC) labels May 11, 2025
@github-actions github-actions bot requested review from anangl and dcpleung May 11, 2025 20:05
@rettichschnidi rettichschnidi removed their request for review May 12, 2025 07:40
nordicjm
nordicjm previously approved these changes Aug 18, 2025
asmellby
asmellby previously approved these changes Aug 21, 2025
Copy link
Contributor

@asmellby asmellby left a comment

Choose a reason for hiding this comment

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

Thanks! The HAL PR is merged, please update west.yml to the new revision (c4d2f7bcfb591bc59b7b9f800a10017f62de23cf)

Copy link
Contributor

@asmellby asmellby left a comment

Choose a reason for hiding this comment

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

Please rewrite the commit that changed the west.yml hal_silabs reference instead of adding an additional commit. References to PRs in the HAL tree shouldn't remain in the history after the merge. See https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-requirements.

asmellby
asmellby previously approved these changes Aug 27, 2025
west.yml Outdated
@@ -235,7 +235,7 @@ manifest:
groups:
- hal
- name: hal_silabs
revision: 13343bccf850eb7b6541f6e71a8d2a880209850b
revision: 0b58229357750b8bd0ce52e514c54ace6abcfeba
Copy link
Contributor

Choose a reason for hiding this comment

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

You no longer need to update the hal pointer, since this revision has already been included through another PR. You would notice if this PR was rebased. But as long as it's the same, it doesn't hurt.

Copy link
Author

Choose a reason for hiding this comment

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

I have rebased the PR since the revision was changed

Add support for Silicon Labs EFR32ZG28 SoC.

Signed-off-by: Shontal Biton <[email protected]>
Add support for Silicon Labs BRD4401C (a.k.a xG28-RB4401C) Radio Board.

Signed-off-by: Shontal Biton <[email protected]>
Added support for xg28 in eeprom test.

Signed-off-by: Shontal Biton <[email protected]>
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Clock Control area: DMA Direct Memory Access area: EEPROM area: Pinctrl area: UART Universal Asynchronous Receiver-Transmitter platform: Silabs Silicon Labs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants