-
Notifications
You must be signed in to change notification settings - Fork 8k
Add kit_pse84_eval basic board and functionality support #95874
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 kit_pse84_eval basic board and functionality support #95874
Conversation
mcatee-infineon
commented
Sep 11, 2025
- add dts, soc, and board files for ifx kit_pse84_eval board
- update pinctrl, clock_control, and uart drivers for kit_pse84_eval board
- update hal_infineon module for kit_pse84_eval board
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
7adb05e
to
ed61e48
Compare
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 PR is wholly non zephyr coding style compliant
soc/infineon/edge/Kconfig.soc
Outdated
config SOC_FAMILY_INFINEON_CAT1 | ||
bool | ||
|
||
config SOC_FAMILY_INFINEON_EDGE | ||
bool | ||
select SOC_FAMILY_INFINEON_CAT1 |
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.
you can't have a family selecting a family, this needs to match up with the soc.yml 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.
This should now be modified to match what other vendors are doing.
soc/infineon/edge/pse84/Kconfig
Outdated
bool "pse84 m55 core is enabled during setup" | ||
default n | ||
help | ||
This option enables the m55 core on pse84. It is enabled during |
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 is wrongly formatted and has things like default n
for bools
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.
modifications to indentation and removal of "default n" complete. Was there anything else?
soc/infineon/edge/pse84/Kconfig
Outdated
config CORTEX_M_SYSTICK | ||
default y |
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 would go in the Kconfig.defconfig 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.
This has now been moved
soc/infineon/edge/pse84/Kconfig
Outdated
config BUILD_OUTPUT_ADJUST_LMA | ||
depends on XIP | ||
depends on CPU_CORTEX_M33 | ||
default "0x60000000 - $(dt_node_reg_addr_hex,$(dt_nodelabel_path,flash0))" |
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.
as above
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 has also been moved.
# Main Stack Size | ||
CONFIG_MAIN_STACK_SIZE=2048 | ||
|
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 is never set in defconfig files
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 is now removed from the three defconfig files.
|
||
if(SB_CONFIG_BOARD_KIT_PSE84_EVAL_PSE846GPS2DBZC4A_M33) | ||
|
||
ExternalZephyrProject_Add( |
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.
what is any of this
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 is a method for the bootflow of the board m33_secure -> m33_nonsecure -> m55. The --sysbuild command sets CONFIG values for the soc to run a late hook, and setup the next stage. This was done as opposed to standalone apps, as per #90834 (comment).
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.
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.
It seems other vendors are doing something similar under soc/
, though with samples/basic/minimal
, and using BOARD ${launcher_board}
instead of hardcoding the board target.
So this doesn't necessarily seem totally wrong, but honestly I'm not qualified to review this.
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.
other thing is for allowing to use flpr, this here is for tfm however and is not how other tfm-based board targets work
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.
The current implementation of the flow for the board's cores is to move from
m33_s -> m33_ns -> m55, in general samples and tests.
We created the sysbuild.cmake file to allow for the building of dependencies. i.e. m55 needs m33_ns/m33_s and m33_ns needs m33_s. The idea we had was to allow for the building and programming of each individual target, if desired. Meanwhile, if someone wanted to build a target with its "dependencies", they could use the "--sysbuild" flag. The intention is to use our specific secure application for the m33_s.
This is similar to the "Building the Secure firmware using Zephyr" section of the index.rst for the nrf5340dk board, without the merging of binaries. We are using the CONFIG_TRUSTED_EXECUTION_SECURE and CONFIG_TRUSTED_EXECUTION_NONSECURE to differentiate between the m33_s and m33_ns. However, we are not using CONFIG_BUILD_WITH_TFM.
modules/hal_infineon/CMakeLists.txt
Outdated
endif() | ||
|
||
if (CONFIG_SOC_FAMILY_INFINEON_CAT1 AND NOT CONFIG_SOC_FAMILY_PSOC6_LEGACY) | ||
if (CONFIG_SOC_FAMILY_INFINEON_CAT1 AND NOT CONFIG_SOC_FAMILY_PSOC6_LEGACY AND NOT CONFIG_SOC_FAMILY_INFINEON_EDGE) |
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.
no spaces in cmake between if and (, and there is a double space
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 should be resolved.
set(pdl_dir ${ZEPHYR_HAL_INFINEON_MODULE_DIR}/mtb-pdl-cat1) | ||
endif() | ||
|
||
set(pdl_drv_dir ${pdl_dir}/drivers) |
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.
set(pdl_drv_dir ${pdl_dir}/drivers) | |
set(pdl_drv_dir ${pdl_dir}/drivers) |
fix 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.
The extra whitespace should be removed now.
west.yml
Outdated
- name: babblesim | ||
url-base: https://github.com/BabbleSim | ||
|
||
|
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.
no?
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.
Haha. This extra line is no longer present.
ed61e48
to
f2693ca
Compare
zephyr_compile_definitions(FLASH_BOOT) | ||
|
||
set_property(GLOBAL APPEND PROPERTY extra_post_build_commands | ||
COMMAND edgeprotecttools image-metadata --image ${PROJECT_BINARY_DIR}/zephyr.hex --output ${PROJECT_BINARY_DIR}/zephyr.hex --erased-val 0xff --header-size 0x400 --slot-size 0x130000 --hex-addr 0x60100000 |
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.
no to overwriting an existing file, you can create a new one but do not overwrite a file that the build system has produced
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 is not fixed in the current update, but will be addressed in the next update with something like a zephyr-signed.hex as output and flash target. Would that work?
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.
you need to get the bin name correctly since that is a Kconfig then you can set it to that + .signed.hex
yes
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.
@nordicjm. This should be resolved now.
soc/infineon/edge/pse84/Kconfig
Outdated
This option enables the m55 core on pse84. It is enabled during | ||
initialization of the cm33 core in its nonsecure state. | ||
|
||
config SOC_PSE84_M33_NS_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.
@tomi-font for comment, this does not look like how tfm is handled in zephyr
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.
Yeah, the one thing I'm wondering about is, what is done with the S/NS split here? There's no TF-M involved here, so @mcatee-infineon what is the goal with this?
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.
The idea was to build and flash the S/NS independently. There are more details in the comment on the related thread.
#95874 (comment)
return return_result; | ||
} | ||
|
||
cy_rslt_t cy_mpc_Init(void) |
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 is still not snake case
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 should be resolved now.
zephyr_library_sources_ifdef(CONFIG_USE_INFINEON_UART ${pdl_drv_dir}/source/cy_scb_uart.c) | ||
zephyr_library_sources_ifdef(CONFIG_USE_INFINEON_FLASH ${pdl_drv_dir}/source/cy_flash.c) | ||
if(CONFIG_SOC_FAMILY_INFINEON_EDGE) | ||
zephyr_code_relocate(FILES ${pdl_drv_dir}/source/cy_scb_uart.c LOCATION RAM) |
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.
not indented
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 should be resolved now.
zephyr_include_directories(${edge_dir}/devices/include) | ||
|
||
zephyr_library_sources(${edge_dir}/system_edge.c) | ||
if(CONFIG_BOARD_KIT_PSE84_EVAL_PSE846GPS2DBZC4A_M33) |
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.
no tabs in cmake
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 should be fixed with a move to the 2 space indentation
|
||
if(SB_CONFIG_BOARD_KIT_PSE84_EVAL_PSE846GPS2DBZC4A_M33) | ||
|
||
ExternalZephyrProject_Add( |
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.
uint32_t system_clock; /* ifx_cat1_clock_block */ | ||
}; | ||
|
||
__WEAK void cycfg_ClockStartupError(uint32_t error) |
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.
as above, these are not snake case
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 should be fixed now.
f2693ca
to
ace5681
Compare
IMO, this PR has lot of issues,
|
Hello @karthi012.
|
c55295c
to
5a6af38
Compare
modules/hal_infineon/CMakeLists.txt
Outdated
if(CONFIG_SOC_FAMILY_INFINEON_EDGE) | ||
if(CONFIG_CPU_CORTEX_M33 AND CONFIG_TRUSTED_EXECUTION_SECURE) | ||
# Use secure device in cy_device_headers | ||
zephyr_compile_definitions(COMPONENT_SECURE_DEVICE) |
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 is still not fixed?
modules/hal_infineon/CMakeLists.txt
Outdated
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
if(CONFIG_HAS_XMCLIB OR CONFIG_SOC_FAMILY_PSOC6_LEGACY OR CONFIG_SOC_FAMILY_INFINEON_CAT1) | ||
if(CONFIG_HAS_XMCLIB OR CONFIG_SOC_FAMILY_PSOC6_LEGACY OR CONFIG_SOC_FAMILY_INFINEON_CAT1 OR CONFIG_SOC_FAMILY_INFINEON_EDGE) |
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.
SOC_FAMILY_INFINEON_EDGE
this Kconfig is referenced in the 1st commit and added in the 6th commit, they are out of order - see #95874 (comment)
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.
That makes sense it. It is rearranged now to where builds are functioning in each commit, aside from the commit renaming files. We have had requests in the past to keep commits renaming files separate from the related functional changes.
The modules commit has been split in two. The first half is still the first commit and it just updates the hash for hal_infineon in the west.yml. The second is after the edge soc is added, and it contains the updates needed for the board.
dt_reg_size(header_size PATH ${m33s_header}) | ||
|
||
set_property(GLOBAL APPEND PROPERTY extra_post_build_commands | ||
COMMAND edgeprotecttools image-metadata --image ${unsigned_hex} --output ${signed_hex} --erased-val 0xff --hex-addr ${header_addr} --header-size ${header_size} |
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.
2 space indent for cmake, split up into multiple lines, max line size is 100
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 should be fixed now.
find_program(EDGEPROTECTTOOLS edgeprotecttools OPTIONAL) | ||
|
||
if(${EDGEPROTECTTOOLS} STREQUAL "EDGEPROTECTTOOLS-NOTFOUND") | ||
message(WARNING "Could not find edgeprotecttools\nThis will result in a nonfunctional secure cm33. Please consult index.rst for kit_pse84_eval board.") |
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.
as above, can remove the \n
and have "<blah>"
on one line then "<blah2>"
on next line and it will output like that
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 should be fixed now.
zephyr_linker_sources_ifdef(CONFIG_CPU_CORTEX_M55 RWDATA rwdata.ld) | ||
|
||
# Edge family defines | ||
zephyr_compile_definitions_ifdef(CONFIG_CPU_CORTEX_M33 COMPONENT_CM33) |
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.
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.
These defines have now been moved and reimplemented in the same way as those previously discussed. #95874 (comment)
5a6af38
to
7e7627f
Compare
- update hal_infineon revision in preparation for new kit_pse84_eval board Signed-off-by: McAtee Maxwell <[email protected]>
- add basic soc files to support ifx pse84 soc - add files needed for setting up the m55 Signed-off-by: McAtee Maxwell <[email protected]>
88904b1
to
0c4e9da
Compare
@nordicjm This comment should be resolved as far as I see. #95874 (comment) |
- update moudle CMakeLists.txt files for new assets - add infineon_kconfig.h file for definitions needed by soc Signed-off-by: McAtee Maxwell <[email protected]>
- add devicetree support for pse84 devices Signed-off-by: McAtee Maxwell <[email protected]>
- rename ifx clock_control drivers - rename infineon,cat1-peri-div yaml - rename ifx clock dt-binding .h file Signed-off-by: McAtee Maxwell <[email protected]>
- add support for kit_pse84_eval board - refactor infineon,fixed-clock binding - refactor infineon,fixed-factor binding - refactor infinein,peri-div binding Signed-off-by: McAtee Maxwell <[email protected]>
- add drive-strength capability for kit_pse84_eval Signed-off-by: McAtee Maxwell <[email protected]>
- update ifx uart driver for kit_pse84_eval board Signed-off-by: McAtee Maxwell <[email protected]>
- add needed board files for kit_pse84_eval board Signed-off-by: McAtee Maxwell <[email protected]>
- add kit_pse84_eval overlays for gpio_basic_api testsuite Signed-off-by: McAtee Maxwell <[email protected]>
0c4e9da
to
18ea244
Compare
|
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.
Looks good, thanks for fixing issues
Requested changes needs to addressed as separate incremental PR with cleanup's for the all the clock consumer and producers. Unblocking this PR.