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

Copy rimage toml config files to SOF repository #7280

Closed
wants to merge 1 commit into from

Conversation

aiChaoSONG
Copy link
Collaborator

This patch moves rimage/config to sof/config.
Firmware manifest configuration files should stay
with SOF instead of rimage tool.

Link: #7270

If you don't think sof/config is a good place, please comment the place you think is good.

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 15, 2023

I'd like to finish zephyrproject-rtos/zephyr#54700 first.

There's also the question about whether it's good idea to keep growing the role of a user-space tool written in a low-level, unsafe and unreliable language. The extended manifest is merely concatenated to the signed image.

@aiChaoSONG
Copy link
Collaborator Author

aiChaoSONG commented Mar 15, 2023

I think this patch will impact the commit zephyrproject-rtos/zephyr@810b0f2, but I think this one should go first, because in your commit, you use sof/rimage/config as the default dir, which is going to change very soon. this patch continues using west sign which I don't think you changed any option for it.

I don't think there is big conflict with zephyrproject-rtos/zephyr/pull/54700, as it is approved by many people, this patch can wait.

@kv2019i
Copy link
Collaborator

kv2019i commented Mar 15, 2023

@aiChaoSONG Thanks for doing this! Not sure why System/merge/build failed, but otherwise CI looks very promising.

@marc-hb Ack on #54700 but given it didn't make it to SOF2.5 and this PR is going to be very labour-intensive to keep up-to-date (as we have a continuous stream of rimage PRs to these toml files), I'd like to timebox it so that if #54700 is not done this week, we'll switch the priorities and do the rimage move first. This will have immediate benefits as the wave of rimage PRs is jus increasing.

@aiChaoSONG
Copy link
Collaborator Author

SOFCI TEST

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 16, 2023

- [RFC] Move rimage toml config files to SOF repository
+ [RFC] Copy rimage toml config files to SOF repository

I guess the removal will come later to ease the transition?

What is the transition strategy? Don't forget there are two use cases:

zephyr/west.yml -> sof -> rimage submodule

sof/west.yml    -> sof
                -> rimage module

This patch copies rimage/config to sof/config,
and use toml files in sof/config for signing.

Firmware manifest configuration files should stay
with SOF instead of rimage tool.

Deletion of rimage/config will be done later.

Link: thesofproject#7270

Signed-off-by: Chao Song <[email protected]>
@aiChaoSONG aiChaoSONG changed the title [RFC] Move rimage toml config files to SOF repository Copy rimage toml config files to SOF repository Mar 21, 2023
@aiChaoSONG
Copy link
Collaborator Author

Update:

  • commit message revision, move -> copy because we don't delete rimage/config yet
  • sync with rimage main, add newly merged rimage changes.

@aiChaoSONG
Copy link
Collaborator Author

What is the transition strategy? Don't forget there are two use cases:

  • I think zephyr/west.yml doesn't use rimage.
  • sof/west.yml, once the PR merged, we are using tomls in sof/config. we can keep the rimage commit currently used as long as there are no changes to rimage itself, or we can update to later rimage main.

@kv2019i
Copy link
Collaborator

kv2019i commented Mar 21, 2023

FYI @nashif . Zephyr's tests might require adjustments if we stop updating the toml files in rimage repository.

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 21, 2023

I think zephyr/west.yml doesn't use rimage.

Internal zephyr CI does use rimage, it's required to test anything on any Intel hardware.

@gkbldcig
Copy link
Collaborator

Can one of the admins verify this patch?

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 24, 2023

@aiChaoSONG This can proceed now, but we need to keep the toml files in rimage as well to allow for a smooth transition.

@aiChaoSONG
Copy link
Collaborator Author

Close as we don't want a simple toml file moving, instead, we want to split platform config and algorithm config

@aiChaoSONG aiChaoSONG closed this Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants