power_domain_intel_adsp.c: revert recent INIT_PRIORITY change

[marc-hb]

This is a partial revert of one-line from commit 06cfbd4 ("drivers: power_domain: Introduce a gpio monitor driver") which not just introduced a new driver (no problem with that) but also changed the initialization priority of another, unrelated and existing power domain driver without even trying to compile it:

• drivers: power_domain: Introduce a gpio monitor driver #61166 (comment)

west config manifest.project-filter -- +sof
west update

west build -b intel_adsp_ace20_lnl modules/audio/sof/app/

ERROR: /soc/ssp@28100 POST_KERNEL 43 < /soc/dfpmccu@71b00/io0_domain  51
ERROR: /soc/ssp@29100 POST_KERNEL 44 < /soc/dfpmccu@71b00/io0_domain  51
ERROR: /soc/ssp@2a100 POST_KERNEL 45 < /soc/dfpmccu@71b00/io0_domain  51
ERROR: /soc/ssp@2b100 POST_KERNEL 46 < /soc/dfpmccu@71b00/io0_domain  51
ERROR: /soc/ssp@2c100 POST_KERNEL 47 < /soc/dfpmccu@71b00/io0_domain  51
ERROR: /soc/ssp@2d100 POST_KERNEL 48 < /soc/dfpmccu@71b00/io0_domain  51

Also note a reviewer (@ceolin) expressed concerns about this unrelated change but it was ignored:

• drivers: power_domain: Introduce a gpio monitor driver #61166 (comment)

Using CONFIG_KERNEL_INIT_PRIORITY_DEFAULT here may be "bad" for some reason(s) and maybe it should be changed in the future, but it's nothing compared to breaking compilation of code that has been validated for months and been released in production
(https://github.com/thesofproject/sof-bin/releases/tag/v2023.09)

So the very urgent thing is to very quickly revert to the previous state to unblock development. Then we can discuss what is the better thing to do here.

[marc-hb]

Corresponding compilation failure in SOF daily tests:
https://github.com/thesofproject/sof/actions/runs/6726741244/job/18283515270

Addition to Zephyr CI:

• [SKIP SOF-TEST] app/sample.yml: add MTL and LNL to Zephyr CI thesofproject/sof#8429

[fabiobaltieri]

but also changed the initialization priority of another, unrelated and existing power domain driver without even trying to compile it:

The change should have had a standalone commit, but the more important things:

• the CI should have caught it (don't think anything here built that file: https://github.com/zephyrproject-rtos/zephyr/actions/runs/6726760180/job/18283626965)
• someone from sof should have been cc'd once the PR got uploaded with a change to that file

Is this a case of the "Intel Platforms (Xtensa)" maintainer area missing an entry for drivers/*/*intel_adsp*?

[marc-hb]

the CI should have caught it

As just mentioned, fix already submitted in:

• [SKIP SOF-TEST] app/sample.yml: add MTL and LNL to Zephyr CI thesofproject/sof#8429

someone from sof should have been cc'd once the PR got uploaded with a change to that file

True, maybe the filepath matching needs some fine-tuning. This being said:

• Github's notification system is way too spammy (for instance no "cooldown", please upvote https://github.com/orgs/community/discussions/12504) so that PR could have been missed anyway.
• Especially likely to be missed considering the PR was not about audio. It only had a small audio change sneaked in.
• @ceolin did note the issue yet his review comment was missed.

It's quite subjective of course but for me the main cause was not in the lack of code reviews (which did not help) but in submitting a line change that was not even compile-tested. No one can be expected to buy hardware to test changes but I think compilation of changed code is a fair requirement! I always insert garbage temporarily in modified code to make sure I'm actually compiling my changes. The best way to test something is always to break it first, otherwise it's very common with Kconfig explosion and others to accidentally not test anything (example: missing thesofproject/sof#8429!). As another example: error handling is almost never tested.

cc: @semihalf-jakiela-albert

[fabiobaltieri]

It's quite subjective of course but for me the main cause was not in the lack of code reviews (which did not help) but in submitting a line change that was not even compile-tested.

Yeah, what I mean is that the CI should try and build every file that has been modified, but it didn't. The whole build time priority check thing was meant to block these at CI build time so they don't get through in any case. I'm not familiar with the test plan generation and why it did not hit any cavs platforms for the change, but I think it should have, @nashif is that something you could look into?

[marc-hb]

@fabiobaltieri

As just mentioned, fix already submitted in:

• [SKIP SOF-TEST] app/sample.yml: add MTL and LNL to Zephyr CI thesofproject/sof#8429

[fabiobaltieri]

@marc-hb oh, sorry I see what you mean now, I saw it was in the sof repo and thought it was downstream only, forgot that it has tests that are run in the upstream CI, thanks for persisting :-)

[marc-hb]

To be honest I'm not sure this twister file belongs to the SOF repo :-) It was moved with all the rest which does belong to the SOF repo:

• Move the SOF app from zephyr to where it belongs thesofproject/sof#6004
• update SOF tree in manifest and remove sample #50243

[fabiobaltieri]

No idea, I see a tests/boards/intel_adsp/ssp still in the main repo but it seems to be limited to intel_adsp_cavs25, besides that nothing seems to be enabling CONFIG_DAI, but I guess it's fine if the module test runs.