Skip to content

Conversation

mcatee-infineon
Copy link
Contributor

  • 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

Copy link

github-actions bot commented Sep 11, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_infineon zephyrproject-rtos/hal_infineon@f78b8f8 zephyrproject-rtos/hal_infineon@3ef39bd (master) zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-hal_infineon DNM (manifest) This PR should not be merged (controlled by action-manifest) Binary Blobs Modified labels Sep 11, 2025
@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch from 7adb05e to ed61e48 Compare September 11, 2025 21:22
Copy link
Contributor

@nordicjm nordicjm left a 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

Comment on lines 7 to 12
config SOC_FAMILY_INFINEON_CAT1
bool

config SOC_FAMILY_INFINEON_EDGE
bool
select SOC_FAMILY_INFINEON_CAT1
Copy link
Contributor

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

Copy link
Contributor Author

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.

bool "pse84 m55 core is enabled during setup"
default n
help
This option enables the m55 core on pse84. It is enabled during
Copy link
Contributor

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

Copy link
Contributor Author

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?

Comment on lines 12 to 13
config CORTEX_M_SYSTICK
default y
Copy link
Contributor

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

Copy link
Contributor Author

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

Comment on lines 30 to 33
config BUILD_OUTPUT_ADJUST_LMA
depends on XIP
depends on CPU_CORTEX_M33
default "0x60000000 - $(dt_node_reg_addr_hex,$(dt_nodelabel_path,flash0))"
Copy link
Contributor

Choose a reason for hiding this comment

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

as above

Copy link
Contributor Author

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.

Comment on lines 33 to 35
# Main Stack Size
CONFIG_MAIN_STACK_SIZE=2048

Copy link
Contributor

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

Copy link
Contributor Author

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(
Copy link
Contributor

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

Copy link
Contributor Author

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nordicjm @tomi-font.

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.

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)
Copy link
Contributor

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

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
set(pdl_drv_dir ${pdl_dir}/drivers)
set(pdl_drv_dir ${pdl_dir}/drivers)

fix all

Copy link
Contributor Author

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


Copy link
Contributor

Choose a reason for hiding this comment

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

no?

Copy link
Contributor Author

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.

@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch from ed61e48 to f2693ca Compare September 12, 2025 22:21
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
Copy link
Contributor

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

Copy link
Contributor Author

@mcatee-infineon mcatee-infineon Sep 15, 2025

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?

Copy link
Contributor

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

Copy link
Contributor Author

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.

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
Copy link
Contributor

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

Copy link
Contributor

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?

Copy link
Contributor Author

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)
Copy link
Contributor

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

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

not indented

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

no tabs in cmake

Copy link
Contributor Author

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(
Copy link
Contributor

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)
Copy link
Contributor

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

Copy link
Contributor Author

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.

@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch from f2693ca to ace5681 Compare September 15, 2025 23:10
@karthi012
Copy link
Contributor

IMO, this PR has lot of issues,

  • Firstly the commits are not in order, driver changes needs to be commited before the consuming the driver, try to make each commit clear and build/compilable one.
  • Clock driver is messy, don't know why clk divider requires a node and driver.
  • Pinctrl for all infineon based soc are looks same, don't know why redefinition, or am i missing something?

@mcatee-infineon
Copy link
Contributor Author

IMO, this PR has lot of issues,

  • Firstly the commits are not in order, driver changes needs to be commited before the consuming the driver, try to make each commit clear and build/compilable one.
  • Clock driver is messy, don't know why clk divider requires a node and driver.
  • Pinctrl for all infineon based soc are looks same, don't know why redefinition, or am i missing something?

Hello @karthi012.

  • I can reorder the commits in the approximate order of: modules, driver changes, soc, dts, boards, overlays
  • For this and some other Infineon boards, the peri-div essentially functions as its own block and can be configured to clock different resources. (i.e. scb, tcpwm, sdio). Setting it up as a separate node/driver makes the most sense for its usecase.
  • I am not quite sure what you are talking about here. If you are speaking on the pinctrl driver, the modifications in this PR are to add support for the kit_pse84_eval and address some structural feedback. If you are speaking on the pinctrl_soc.h file, it is similar to other Infineon socs in Zephyr, but not exactly the same.

@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch 2 times, most recently from c55295c to 5a6af38 Compare October 8, 2025 22:46
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)
Copy link
Contributor

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?

# 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)
Copy link
Contributor

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)

Copy link
Contributor Author

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}
Copy link
Contributor

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

Copy link
Contributor Author

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.")
Copy link
Contributor

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

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@mcatee-infineon mcatee-infineon Oct 9, 2025

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)

@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch from 5a6af38 to 7e7627f Compare October 9, 2025 18:57
- 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]>
@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch 2 times, most recently from 88904b1 to 0c4e9da Compare October 9, 2025 22:07
@mcatee-infineon
Copy link
Contributor Author

@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]>
@mcatee-infineon mcatee-infineon force-pushed the PR/add-kit_pse84_eval-basic-support branch from 0c4e9da to 18ea244 Compare October 10, 2025 00:49
Copy link

Copy link
Contributor

@nordicjm nordicjm 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, thanks for fixing issues

@parthitce parthitce dismissed their stale review October 10, 2025 15:45

Requested changes needs to addressed as separate incremental PR with cleanup's for the all the clock consumer and producers. Unblocking this PR.

@cfriedt cfriedt merged commit c47e606 into zephyrproject-rtos:main Oct 10, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.