Skip to content
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

boards: intel: ish: Improve Simics support #82657

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

golowanow
Copy link
Member

Improve Simics support for boards/intel/ish/intel_ish_5_8_0 for better integration with the simulator.

Improve Simics support for `boards/intel/ish/intel_ish_5_8_0`
for better integration with the simulator.

Signed-off-by: Dmitrii Golovanov <[email protected]>
@golowanow golowanow marked this pull request as ready for review December 6, 2024 22:25
@zephyrbot zephyrbot added platform: Intel ISH Intel Corporation, Integrated Sensor Hub area: Build System labels Dec 6, 2024
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm.

A minor observation / improvement proposal but nothing blocking.

--batch-mode
${SIMICS_SCRIPT}
${SIMICS_ARGS}
$ENV{SIMICS_EXTRA_ARGS}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we use EXTRA_SIMICS_ARGS to be aligned with other Zephyr CMake settings, like EXTRA_CONF_FILE, EXTRA_CFLAGS, etc. ?

Also, using zephyr_get(EXTRA_SIMICS_ARGS ...) will allow users to choose freely between sysbuild, CMake var -DEXTRA_SIMICS_ARGS=<val>, or environment variable and not force them into using env vars.

Copy link
Member Author

@golowanow golowanow Dec 12, 2024

Choose a reason for hiding this comment

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

SIMICS_EXTRA_ARGS got its name as one of many other SIMICS_* env variables which belong to Simics integration on the host level; its purpose - to pass through some custom platform parameters from user/CI to the simulator runtime through all the layers, so it is not in Zephyr naming domain.
Currently there is no need to ingest values there from other sources (CMake etc.) with zephyr_get(), but may be helpful to do that later (good idea, thanks @tejlmand).

@kartben kartben merged commit c3cb5bd into zephyrproject-rtos:main Dec 12, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System platform: Intel ISH Intel Corporation, Integrated Sensor Hub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants