init: remove the need for a dummy device pointer in SYS_INIT functions

[gmarull]

NOTE: This is a breaking change! All SYS_INIT functions will need
to be converted to the new signature. See the script offered in c5c637c.

The init infrastructure, found in init.h, is currently used by:

• SYS_INIT: to call functions before main
• DEVICE_*: to initialize devices

They are all sorted according to an initialization level + a priority.
SYS_INIT calls are really orthogonal to devices, however, the required
function signature requires a const struct device *dev as a first
argument. The only reason for that is because the same init machinery is
used by devices, so we have something like:

struct init_entry {
	int (*init)(const struct device *dev);
	/* only set by DEVICE_*, otherwise NULL */
	const struct device *dev;
}

As a result, we end up with such weird/ugly pattern:

static int my_init(const struct device *dev)
{
	/* always NULL! add ARG_UNUSED to avoid compiler warning */
	ARG_UNUSED(dev);
	...
}

This is really a result of poor internals isolation. This patch proposes
a to make init entries more flexible so that they can accept sytem
initialization calls like this:

static int my_init(void)
{
	...
}

This is achieved using a union:

union init_function {
	/* for SYS_INIT, used when init_entry.dev == NULL */
	int (*sys)(void);
	/* for DEVICE*, used when init_entry.dev != NULL */
	int (*dev)(const struct device *dev);
};

struct init_entry {
	/* stores init function (either for SYS_INIT or DEVICE*) */
	union init_function init_fn;
	/* stores device pointer for DEVICE*, NULL for SYS_INIT. Allows
	 * to know which union entry to call.
	 */
	const struct device *dev;
}

This solution does not increase ROM usage, and allows to offer clean
public APIs for both SYS_INIT and DEVICE*. Note that however, init
machinery keeps a coupling with devices.

NOTE: This is a breaking change! All SYS_INIT functions will need
to be converted to the new signature. See the script offered in c5c637c.

[fabiobaltieri]

Drop RFC from the PR title?

[marc-hb]

Since this commit SOF compilation with -Werror fails as below.

For exact reproduction steps stay tuned.

In file included from /zep_workspace/sof/zephyr/include/rtos/init.h:9, 
448 from /zep_workspace/sof/src/init/init.c:13: 
449/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] 
450 156 | .init_fn = {.sys = (init_fn_)}, \ 
451 | ^ 
452/zep_workspace/zephyr/include/zephyr/init.h:137:9: note: in expansion of macro 'SYS_INIT_NAMED' 
453 137 | SYS_INIT_NAMED(init_fn, init_fn, level, prio) 
454 | ^~~~~~~~~~~~~~ 
455/zep_workspace/sof/src/init/init.c:350:1: note: in expansion of macro 'SYS_INIT' 
456 350 | SYS_INIT(sof_init, POST_KERNEL, 99); 
457 | ^~~~~~~~ 
458/zep_workspace/zephyr/include/zephyr/init.h:156:44: note: (near initialization for '__init_sof_init.init_fn.sys') 
459 156 | .init_fn = {.sys = (init_fn_)}, \ 
460 | ^ 
461/zep_workspace/zephyr/include/zephyr/init.h:137:9: note: in expansion of macro 'SYS_INIT_NAMED' 
462 137 | SYS_INIT_NAMED(init_fn, init_fn, level, prio) 
463 | ^~~~~~~~~~~~~~ 
464/zep_workspace/sof/src/init/init.c:350:1: note: in expansion of macro 'SYS_INIT' 
465 350 | SYS_INIT(sof_init, POST_KERNEL, 99); 
466 | ^~~~~~~~ 
467cc1: all warnings being treated as errors

[marc-hb]

For exact reproduction steps stay tuned.

OK that was really easy, nothing special required:

west build -b intel_adsp_cavs25 ../modules/audio/sof/app -- -DEXTRA_CFLAGS=-Werror

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_)},          

Yet https://github.com/zephyrproject-rtos/zephyr/actions/runs/4678432008/jobs/8288559016 did not care.

Twister used to compile with -Werror, I know because a long time ago I added it to CMAKE_AFLAGS in 8a603da. I don't know when this was lost but it was a pretty big loss IMHO.

Twister also failed to catch this regression: #56701 (demoed again in #56791)

cc: @nashif, @galak

[marc-hb]

Just spotted this sorry "NOTE: This is a breaking change! All SYS_INIT functions will need..."

The 267 comments put me off.

Still, dropping -Werror is not the solution to "support" backwards incompatible changes in CI.

[nashif]

Since this commit SOF compilation with -Werror fails as below.

there is a PR submitted to SOF to deal with this.

[nashif]

Twister also failed to catch this regression: #56701 (demoed again in #56791)

catch it where? File a bug please instead of commenting in merged PRs

[dbaluta]

@gmarull any idea why SOF build are failing? I think this is related to your change.

https://github.com/thesofproject/sof/actions/runs/4686511690/jobs/8304685904?pr=7429

[marc-hb]

@dbaluta for SOF the API change was submitted a long time ago by @gmarull here:

• zephyr: update SYS_INIT calls thesofproject/sof#6480

Next time a similar situation arises, corresponding module changes like this one will hopefully be present in the module branch used by Zephyr first (and required by Zephyr CI). Among others this should preserve bisectability on both sides (as long as west update is invoked at every step)

[gmarull]

@dbaluta for SOF the API change was submitted a long time ago by @gmarull here:

• zephyr: update SYS_INIT calls thesofproject/sof#6480

Next time a similar situation arises, corresponding module changes like this one will hopefully be present in the module branch used by Zephyr first (and required by Zephyr CI). Among others this should preserve bisectability on both sides (as long as west update is invoked at every step)

As a breaking change, the update needs to happen in multiple steps here. First in Zephyr fork, then upstream, then sync Zephyr fork. I don't see any better way if we allow Zephyr glue code in modules that have an upstream counterpart and that are tested in Zephyr CI.