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

intel_adsp: ptl: Fix SOF compilation without PM #9203

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

pjdobrowolski
Copy link
Contributor

PTL has CONFIG_PM disabled. SOF should still build without this configuration flag. This patch fixes the compilation issue.

Copy link
Contributor

@gbernatxintel gbernatxintel 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 to me

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@pjdobrowolski are we pending some other patch hence the DNM ?

PTL has CONFIG_PM disabled. SOF should still build without
this configuration flag. This patch fixes the compilation issue.

Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>
@kv2019i
Copy link
Collaborator

kv2019i commented Jun 13, 2024

@pjdobrowolski Same question Liam, why the DNM in title? I don't see any hard dependencies and it would seem we can merge this right away.

@pjdobrowolski
Copy link
Contributor Author

Because it is commit taken from #9185 only for CI purpose.

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 13, 2024

Because it is commit taken from #9185 only for CI purpose.

You're confusing "Continuous Integration" and "PR tests". The latter is only a small part of the former. There are also daily tests which run only on merged code. Also, merged code is "tested" by all developers who update their branches. "Integrating" == merging (smaller) bits of code.

Also, merging this one can make 9185 smaller
https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs

PS: I wrote "Continuous Integration" in 9185 and recommended to extract this.

@abonislawski
Copy link
Member

PTL has CONFIG_PM disabled

Its good to fix build without this kconfig but the question is why it is disabled? @jxstelter @tmleman

@pjdobrowolski pjdobrowolski changed the title [CI][DNM] intel_adsp: ptl: Fix SOF compilation without PM intel_adsp: ptl: Fix SOF compilation without PM Jun 14, 2024
@pjdobrowolski
Copy link
Contributor Author

@marc-hb removed dnm, ready to merge.

@jxstelter
Copy link
Contributor

PTL has CONFIG_PM disabled

Its good to fix build without this kconfig but the question is why it is disabled? @jxstelter @tmleman

Originally during PTL development we started with CONFIG_PM disabled but the fix was just allowing to compile with this config flag cleared, By default it should be set anyway for PTL.

@pjdobrowolski
Copy link
Contributor Author

@lgirdwood @kv2019i I think that PR is ready to merge.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

The git message is still not very accurate as this has nothing to do with "ptl" and this touches multiple places in the code, so the "intel_adsp:" prefix is not accurate either. But I guess one gets the point from the commit message, so let me go ahead and merge this. I think this is useful capability to be able to build without CONFIG_PM.

@kv2019i kv2019i merged commit 6102aac into thesofproject:main Jun 14, 2024
44 of 46 checks passed
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.

9 participants