Skip to content

Conversation

@gmarull
Copy link
Contributor

@gmarull gmarull commented Oct 25, 2022

Use the new call signature: int (*init_fn)(void);

NOTE: This should not be merged before zephyrproject-rtos/zephyr#51217 reaches Zephyr main and this project updates Zephyr revision.

@sofci
Copy link
Collaborator

sofci commented Oct 25, 2022

Can one of the admins verify this patch?

reply test this please to run this test once

@gkbldcig
Copy link
Collaborator

Can one of the admins verify this patch?

@lgirdwood
Copy link
Member

@gmarull thanks - I will run the CI after the Zephyr PR is merged. @kv2019i @mwasko fyi.

@lgirdwood lgirdwood added this to the v2.4 milestone Oct 25, 2022
@lgirdwood
Copy link
Member

@gmarull can you pls ping this PR when the Zephyr dependency is merged. It looks like things are fine here.

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

this is good, and we'll need one more small change to

static int zephyr_##name##_init(const struct device *dev) \
{ \
ARG_UNUSED(dev); \
- I'll mark this "request changes" not to forget that

@lgirdwood
Copy link
Member

@gmarull Zephyr PR not currently merged, any update ?

@gmarull
Copy link
Contributor Author

gmarull commented Jan 27, 2023

@gmarull Zephyr PR not currently merged, any update ?

it's been delayed until the 3.4 release.

@lgirdwood
Copy link
Member

@gmarull Zephyr PR not currently merged, any update ?

it's been delayed until the 3.4 release.

Ack, pls ping after merge and we can also include a west update to this PR too.

@kv2019i kv2019i removed this from the v2.4 milestone Feb 17, 2023
@nashif
Copy link
Contributor

nashif commented Apr 12, 2023

@gmarull can you updater this PR?

@gmarull
Copy link
Contributor Author

gmarull commented Apr 12, 2023

Updated PR, pending west.yml Zephyr SHA update (to be done after zephyrproject-rtos/zephyr#51217 is merged).

@gmarull
Copy link
Contributor Author

gmarull commented Apr 19, 2023

@gmarull Can you update the PR to use latest Zephyr main tip to take in zephyrproject-rtos/zephyr#57000 , tests should now pass.

done

@btian1
Copy link
Contributor

btian1 commented Apr 19, 2023

Can this PR fix CI reported build errors like:
/zep_workspace/zephyr/include/zephyr/init.h:156:44: error: initialization of 'int ()(void)' from incompatible pointer type 'int ()(const struct device *)' [-Werror=incompatible-pointer-types]
156 | .init_fn = {.sys = (init_fn_)},
| ^
/zep_workspace/zephyr/include/zephyr/init.h:137:9: note: in expansion of macro 'SYS_INIT_NAMED'
137 | SYS_INIT_NAMED(init_fn, init_fn, level, prio)
| ^~~~~~~~~~~~~~
/zep_workspace/sof/src/init/init.c:350:1: note: in expansion of macro 'SYS_INIT'
350 | SYS_INIT(sof_init, POST_KERNEL, 99);
| ^~~~~~~~
/zep_workspace/zephyr/include/zephyr/init.h:156:44: note: (near initialization for '_init_sof_init.init_fn.sys')
156 | .init_fn = {.sys = (init_fn
)},
| ^
/zep_workspace/zephyr/include/zephyr/init.h:137:9: note: in expansion of macro 'SYS_INIT_NAMED'
137 | SYS_INIT_NAMED(init_fn, init_fn, level, prio)
| ^~~~~~~~~~~~~~
/zep_workspace/sof/src/init/init.c:350:1: note: in expansion of macro 'SYS_INIT'
350 | SYS_INIT(sof_init, POST_KERNEL, 99);
| ^~~~~~~~

@paulstelian97
Copy link
Collaborator

Does CI check every commit? Because I did notice that this may break bisect indeed.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 19, 2023

@paulstelian97 wrote:

Does CI check every commit? Because I did notice that this may break bisect indeed.

CI doesn't check, but with bisect you will hit the problem if you build the tree from middle of this 3 patch series. But given this is a build-time warning only (fails CI but local build is ok), I'm willing to give this is a pass. Squashed commits are not nice either (but sometimes just mandatory).

@gmarull
Copy link
Contributor Author

gmarull commented Apr 19, 2023

squashed to preserve bisectability.

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

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

Given how small the patch is I'd say nobody should mind that this was squashed. Looks good now!

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 19, 2023

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 20, 2023

@wszypelt @keqiaozhang @greg-intel we are really struggling with the CI on this one. Now the regression on Zephyr side should be fixed, but I'm seeing lot of failures in both SOF driver CI and the Intel FW CI. I can't fully decipher what's going on, it seems even the builds failed here (the rimage issue?). I re-kicked the driver CI already once, but still failing...

@tmleman
Copy link
Contributor

tmleman commented Apr 20, 2023

@kv2019i we need this one too zephyrproject-rtos/zephyr#56790
TGL will not build without that.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 20, 2023

@tmleman wrote:

@kv2019i we need this one too zephyrproject-rtos/zephyr#56790 TGL will not build without that.

Thanks! I just realized the same thing locally, it's definitely this one.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 20, 2023

@gmarull So one more update would be needed to take zephyrproject-rtos/zephyr#56790 in (was just merged to Zephyr).

@gmarull
Copy link
Contributor Author

gmarull commented Apr 20, 2023

@gmarull So one more update would be needed to take zephyrproject-rtos/zephyr#56790 in (was just merged to Zephyr).

done

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 20, 2023

@gmarull wrote:

@gmarull So one more update would be needed to take zephyrproject-rtos/zephyr#56790 in (was just merged to Zephyr).

done

Thanks and sorry for the hassle. These are not anyway related to the PR and we should have caught the Zephyr side problems earlier.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 21, 2023

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 21, 2023

Pains me to report but we have still failures:

These are not normal, we have multiple basic cases failing again.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 21, 2023

Two more regressions identified -> zephyrproject-rtos/zephyr#57127
@gmarull No need to update,let me do a full test cycle with a working Zephyr version. Given so many problems, there could be more lurking in, so let me find them all before we proceed with this.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 21, 2023

Got a clean test run now with #7491

So once zephyrproject-rtos/zephyr#57127 is merged, we can retry updating.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 21, 2023

@gmarull zephyrproject-rtos/zephyr#57127 updated, one more update? Should pass the CI now (as #7491 passed).

Update Zephyr head, and use the new call signature:
int (*init_fn)(void);

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
@kv2019i
Copy link
Collaborator

kv2019i commented Apr 24, 2023

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 24, 2023

One boot-log fail in https://sof-ci.01.org/sofpr/PR6480/build6378/devicetest/index.html and lack of test machines in https://sof-ci.01.org/sofpr/PR6480/build6379/devicetest/index.html . Given other tests are ok, and I got green runs with the draft PR #7491 , I think we are finally good to go with this.

Thank you @gmarull for the many updates!

@kv2019i kv2019i merged commit ca68ba6 into thesofproject:main Apr 24, 2023
@gmarull gmarull deleted the sys-init-update branch April 24, 2023 16:14
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.