-
Notifications
You must be signed in to change notification settings - Fork 7.7k
boards: Add support for PG23/PG28 Pro Kit #93285
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
base: main
Are you sure you want to change the base?
Conversation
Hello @janchri, and thank you very much for your first pull request to the Zephyr project! |
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
c557aae
to
1f89892
Compare
0eae543
to
dcf914d
Compare
default 875000 | ||
depends on LOG_BACKEND_SWO | ||
|
||
if SOC_GECKO_USE_RAIL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if block for RAIL isn't needed on PG23, since it doesn't have a radio.
@@ -0,0 +1,6 @@ | |||
board: | |||
name: xg23_pk2504a | |||
full_name: PG28 Pro Kit PK2504A Rev. A03 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: s/PG28/PG23/
|
||
eusart1_default: eusart1_default { | ||
group0 { | ||
pins = <EUSART1_TX_PC1>; //<EUSART1_TX_PB5>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out pinout should be removed (Zephyr requires C-style comments, but the alternate pin selection should not be here in the first place).
But more fundamentally: PC1/PC2 seem to be connected to the LCD, and aren't available as breakout pins. Should EUSART1 be configured at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed eusart1
|
||
&gpiob { | ||
status = "okay"; | ||
// VCOM Isolation. Set PB1 to HIGH to enable VCOM_{RX,TX}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++ style comment
status = "okay"; | ||
regulator-boot-on; | ||
regulator-initial-mode = <SILABS_DCDC_MODE_BUCK>; | ||
silabs,pfmx-peak-current-milliamp = <80>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In SiSDK, the config option SL_DEVICE_INIT_DCDC_PFMX_IPKVAL_OVERRIDE
is not set on this board. This line should be removed to make the configuration match SiSDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please generate a complete clock config header using the script in hal_silabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated the xg28 using the python script. Had to change a few clock configuration in xg28.dtsi file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename efr32zg23
to xg23
and add the PG23 config to that directory rather than introducing a new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this. Please, take a look if did not corrupt anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please introduce a directory xg28
for the pg28 content, in order to avoid needing to rename it when more xg28 families are added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the folder.
|
||
config NUM_IRQS | ||
# must be >= the highest interrupt number used | ||
default 75 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pg28 has 79 external interrupts, not 75
west.yml
Outdated
@@ -235,7 +237,8 @@ manifest: | |||
groups: | |||
- hal | |||
- name: hal_silabs | |||
revision: 190a144a16bed9a938a94543ed5bbc70c0552e0f | |||
remote: hal_silabs_pr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please point to the PR in the upstream using the pull/95/head
revision, don't introduce your personal remote. That PR will also need to be rebased.
Thank a lot for the feedback @asmellby |
Adding xg23_pk2504a and xg28_pk2506a boards files. Signed-off-by: Christoph Jans <[email protected]>
Add device tree and support files for xg23 and xg28 dev Kit boards. Signed-off-by: Christoph Jans <[email protected]>
Introducing the efm32pg23 and efm32pg28 Series 2 Silabs chips. Signed-off-by: Christoph Jans <[email protected]>
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
|
||
leds { | ||
compatible = "gpio-leds"; | ||
led0: led_0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline missing lines 37, 50, etc.
&gpiob { | ||
status = "okay"; | ||
/* VCOM Isolation. Set PB1 to HIGH to enable VCOM_{RX,TX}. */ | ||
board-controller-enable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
145 newline missing
|
||
leds { | ||
compatible = "gpio-leds"; | ||
led0: led_0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix in whole PR
config SOC_SERIES_EFM32PG23 | ||
bool | ||
select SOC_FAMILY_SILABS_S2 | ||
select SOC_SERIES_XG23 | ||
help | ||
Silicon Labs EFM32PG23 Series MCU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seemingly I've not noticed this before, there should only be one SOC_SERIES_* Kconfig, not one selecting another, and that series needs to match up with what is in the soc.yml file @jerome-pouiller please remove existing usage in tree and fix it up in another PR
config SOC | ||
default "efr32zg23b020f512im48" if SOC_PART_NUMBER_EFR32ZG23B020F512IM48 | ||
default "efm32pg23b310f512im48" if SOC_PART_NUMBER_EFM32PG23B310F512IM48 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this select is done based on a SOC_X Kconfig, not a part number Kconfig, so:
config SOC
default "efr32zg23b020f512im48" if SOC_EFR32ZG23B020F512IM48
Again, matching up with what is in a soc.yml file
This PR adds support for two development kits from Silicon Labs:
For the xG23-PK2504A, RTT has been configured due to issues when accessing the console via EUSART0/1. Tested manually on basic samples (hello world, blinky, and button).
This PR builds on the work carried out for xg24_ek2703a and other ports from the dev Kit folder.
Depends on: zephyrproject-rtos/hal_silabs#95