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

init: adjust the SYS_INIT dev field init to play nice with older comp (alternative to PR68118) #68125

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Jan 25, 2024

…ilers

Fix a build error with certain older Cadence XCC toolchains. These are used e.g. for Intel Tiger Lake products for the audio DSP (thhe oldest platform supported in Zephyr upstream for the audio DSP).

Error:
/work/zephyr/lib/os/p4wq.c:216: error: unknown field ‘dev’ specified in initializer /work/zephyr/lib/os/p4wq.c:216: warning: missing braces around initializer /work/zephyr/lib/os/p4wq.c:216: warning: (near initialization for ‘__init_static_init.’)

Compiler version XCC RG-2017.8-linux (xt-xcc version 12.0.8)

Fixes: 2438dbb ("init: add missing initialization of dev pointer in SYS_INIT macro")

@zephyrbot zephyrbot added Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. area: Device Model labels Jan 25, 2024
@barnas-michal
Copy link
Collaborator

Looks like you need to change the commit message, but this fix works for me. Thanks

gmarull
gmarull previously approved these changes Jan 25, 2024
@kv2019i kv2019i added the platform: Intel ADSP Intel Audio platforms label Jan 25, 2024
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 25, 2024

Hmm, it seems ARM compiler in SDK is not happy with this version:

https://github.com/zephyrproject-rtos/zephyr/actions/runs/7657030858/job/20866968820?pr=68125

 /__w/zephyr/zephyr/include/zephyr/init.h:208:76: error: either all initializer clauses should be designated or none of them should be
  208 |                 Z_INIT_ENTRY_NAME(name) = {.init_fn = {.sys = (init_fn_)}, { .dev = NULL }}

@kv2019i kv2019i marked this pull request as draft January 25, 2024 17:32
@kv2019i kv2019i force-pushed the 202401-fix-intel-tgl-xcc-build-ver2 branch from 403c685 to 3aac160 Compare January 25, 2024 18:58
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 25, 2024

V2 uploaded:

  • drop designated initialized altogether and just use C89 as the lowest common denominator

@kv2019i kv2019i requested a review from barnas-michal January 25, 2024 19:03
@kv2019i kv2019i changed the title init: adjust the SYS_INIT dev field init to play nice with older comp… init: adjust the SYS_INIT dev field init to play nice with older comp (alternative to PR68118) Jan 25, 2024
…ilers

Fix a build error with certain older Cadence XCC toolchains. These
are used e.g. for Intel Tiger Lake products for the audio DSP (thhe oldest
platform supported in Zephyr upstream for the audio DSP).

To keep all compilers happy, use C89 style initializers.

Error:
lib/os/p4wq.c:216: error: unknown field ‘dev’ specified in initializer
lib/os/p4wq.c:216: warning: missing braces around initializer
lib/os/p4wq.c:216: warning: (near initialization for
  ‘__init_static_init.<anonymous>’)

Compiler version XCC RG-2017.8-linux (xt-xcc version 12.0.8)

Fixes: 2438dbb ("init: add missing initialization of dev pointer
in SYS_INIT macro")
Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i kv2019i force-pushed the 202401-fix-intel-tgl-xcc-build-ver2 branch from 3aac160 to d3c8f8f Compare January 25, 2024 19:12
@kv2019i kv2019i marked this pull request as ready for review January 25, 2024 19:55
@carlescufi carlescufi merged commit 19a33c7 into zephyrproject-rtos:main Jan 26, 2024
20 checks passed
@marc-hb
Copy link
Collaborator

marc-hb commented Feb 21, 2024

Hmm, it seems ARM compiler in SDK is not happy with this version:
https://github.com/zephyrproject-rtos/zephyr/actions/runs/7657030858/job/20866968820?pr=68125
/__w/zephyr/zephyr/include/zephyr/init.h:208:76: error: either all initializer clauses should be designated or none of them should be

This is not a toolchain issue. This is because C and C++ are different languages with fairly different designated initializers.

Designated initializers were added to C99. They came to C++ 20 years later in
P0329R0 and in a much more restricted form. C99 designated initializers are "anything goes".

https://isocpp.org/files/papers/p0329r0.pdf
https://en.cppreference.com/w/c/language/struct_initialization
https://en.cppreference.com/w/cpp/language/aggregate_initialization
https://stackoverflow.com/questions/51026674/are-non-trivial-designated-initializers-going-to-be-supported-in-the-future-pos

cc: @nordic-krch

marc-hb added a commit to marc-hb/zephyr that referenced this pull request Feb 24, 2024
As seen in the PR zephyrproject-rtos#68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use #ifdef __cplusplus to restore
them in SYS_INIT_NAMED() thanks to a small braces difference between C
and C++.

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb
Copy link
Collaborator

marc-hb commented Feb 24, 2024

marc-hb added a commit to marc-hb/zephyr that referenced this pull request Feb 26, 2024
As seen in the PR zephyrproject-rtos#68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use an #ifdef to restore them in
SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

Signed-off-by: Marc Herbert <[email protected]>
marc-hb added a commit to marc-hb/zephyr that referenced this pull request Mar 8, 2024
As seen in the PR zephyrproject-rtos#68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use an #ifdef to restore them in
SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

Signed-off-by: Marc Herbert <[email protected]>
dleach02 pushed a commit that referenced this pull request Mar 12, 2024
As seen in the PR #68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use an #ifdef to restore them in
SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

Signed-off-by: Marc Herbert <[email protected]>
ashiroji pushed a commit to ashiroji/zephyr that referenced this pull request Mar 19, 2024
As seen in the PR zephyrproject-rtos#68125 discussion, commit 19a33c7 ("init: adjust
the SYS_INIT dev field init to play nice with older compilers") entirely
threw away designated initializers in SYS_INIT_NAMED() to avoid
compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two
different languages and this is especially true with respect to
designated initializers.

Designated initializers provide safer and more readable code, especially
in their much stricter C++ version. So use an #ifdef to restore them in
SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

Signed-off-by: Marc Herbert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Device Model platform: Intel ADSP Intel Audio platforms Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants