Skip to content

tests: build_system: intc_nxp_irqsteer.c build warning #88996

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

Closed
hakehuang opened this issue Apr 24, 2025 · 3 comments · Fixed by #89026
Closed

tests: build_system: intc_nxp_irqsteer.c build warning #88996

hakehuang opened this issue Apr 24, 2025 · 3 comments · Fixed by #89026
Labels
bug The issue is a bug, or the PR is fixing a bug

Comments

@hakehuang
Copy link
Collaborator

Describe the bug

[67/153] Building C object zephyr/lib/libc/picolibc/CMakeFiles/lib__libc__picolibc.dir/exit.c.obj[68/153] Building C object zephyr/lib/libc/picolibc/CMakeFiles/lib__libc__picolibc.dir/locks.c.obj[69/153] Building C object zephyr/lib/libc/picolibc/CMakeFiles/lib__libc__picolibc.dir/stdio.c.obj[70/153] Linking C static library zephyr/lib/libc/picolibc/liblib__libc__picolibc.a[71/153] Building C object zephyr/lib/libc/common/CMakeFiles/lib__libc__common.dir/source/stdlib/abort.c.obj[72/153] Building C object zephyr/lib/libc/common/CMakeFiles/lib__libc__common.dir/source/stdlib/malloc.c.obj[73/153] Linking C static library zephyr/lib/libc/common/liblib__libc__common.a[74/153] Building C object zephyr/drivers/interrupt_controller/CMakeFiles/drivers__interrupt_controller.dir/intc_nxp_irqsteer.c.objFAILED: zephyr/drivers/interrupt_controller/CMakeFiles/drivers__interrupt_controller.dir/intc_nxp_irqsteer.c.obj xpansion-to-defined -Wno-unused-but-set-variable -Werror=implicit-int -fno-pic -fno-pie -fno-asynchronous-unwind-tables -fno-reorder-functions --param=min-pagesize=0 -fno-defer-pop -fmacro-prefix-map=/home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr/tests/misc/test_build=CMAKE_SOURCE_DIR -fmacro-prefix-map=/home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/home/jenkins/agent/workspace/zephyr_downstream_build_daily=WEST_TOPDIR -ffunction-sections -fdata-sections -mlongcalls -specs=picolibc.specs -std=c99 -MD -MT zephyr/drivers/interrupt_controller/CMakeFiles/drivers__interrupt_controller.dir/intc_nxp_irqsteer.c.obj -MF zephyr/drivers/interrupt_controller/CMakeFiles/drivers__interrupt_controller.dir/intc_nxp_irqsteer.c.obj.d -o zephyr/drivers/interrupt_controller/CMakeFiles/drivers__interrupt_controller.dir/intc_nxp_irqsteer.c.obj -c /home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c/home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c: In function 'z_soc_irq_is_enabled':/home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c:558:24: error: 'enabled' may be used uninitialized [-Werror=maybe-uninitialized] 558 | return enabled; | ^~~~~~~/home/jenkins/agent/workspace/zephyr_downstream_build_daily/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c:552:14: note: 'enabled' was declared here 552 | bool enabled; | ^~~~~~~cc1: all warnings being treated as errorsninja: build stopped: subcommand failed.

Please also mention any information which could help others to understand
the problem you're facing:

  • What target platform are you using?
  • imx8mp_evk/mimx8ml8/adsp
  • What have you tried to diagnose or workaround this issue?
  • git bisect
  • Is this a regression? If yes, have you been able to "git bisect" it to a
    specific commit?

#eaa1addb5eba17022fd24c0ae530a15923a062d4

drivers: intc: irqstr: refactor level 1 interrupt recounting

So far, it has been assumed that only level 2 interrupts can be shared
via the `CONFIG_SHARED_INTERRUPTS` option, but this is not true. In the
case of i.MX95, for instance, level 1 interrupt 143 is shared among EDMA
channels 30 and 31.

Due to the previous assumption, the irqsteer driver currently performs
reference counting for all level 2 interrupts aggregated by each
dispatcher and, of course, for the level 1 interrupts the dispatchers are
attached to. For instance, assuming a machine with 100 level 1 interrupts
and 1 irqsteer dispatcher attached to line 50 this would mean reference
counting is performed solely for line 50 (and the level 2 interrupts MUX'd
into this line).

Going back to i.MX95, since there's no dispatcher attached to IRQ line 143
that means there's no reference counting for it. In turn, this means that
the IRQ line can be disabled accidentally on a channel release() operation
while the other channel is active.

To protect against such cases, refactor the level1 interrupt reference
counting. Now, reference counting is performed for _all_ level 1
interrupts.

Additionally, simplify the locking logic. Ideally, there would be a lock
for each dispatcher protecting the level 2 interrupts and 1 global lock
protecting the level 1 interrupts. Instead of this approach (which is a
bit more complex), simply use a global lock for all interrupts. If finer
granularity is required then it can be added later on.

Signed-off-by: Laurentiu Mihalcea <[email protected]>

To Reproduce

west twist -p imx8mp_evk/mimx8ml8/adsp -j 12 -T tests/misc/test_build -s "buildsystem.debug.build"

Expected behavior

build pass

Impact

intc code logic

Logs and console output

see above

Environment (please complete the following information):

  • OS: (e.g. Linux, MacOS, Windows)
  • Toolchain (e.g Zephyr SDK, ...): 0.17.0
  • Commit SHA or Version used: v4.1.0-2607-gf9220146e62
@hakehuang hakehuang added the bug The issue is a bug, or the PR is fixing a bug label Apr 24, 2025
@dbaluta
Copy link
Collaborator

dbaluta commented Apr 24, 2025

@hakehuang this looks like a false positive

Warning is:

/home/daniel/work/repos/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c:558:24: error: 'enabled' may be used uninitialized [-Werror=maybe-uninitialized]
  558 |                 return enabled;
      |                        ^~~~~~~
/home/daniel/work/repos/zephyr/drivers/interrupt_controller/intc_nxp_irqsteer.c:552:14: note: 'enabled' was declared here
  552 |         bool enabled;

Relevant code is:

547 int z_soc_irq_is_enabled(unsigned int irq)
548 {
549 »       uint32_t parent_irq;
550 »       int i;
551 »       const struct irqsteer_config *cfg;
552 »       bool enabled;
553 
554 »       if (irq_get_level(irq) == 1) {
555 »       »       K_SPINLOCK(&irqstr_lock) {
556 »       »       »       enabled = l1_irq_refcnt[irq];                                                                                                                               
557 »       »       }
558 »       »       return enabled;
559 »       }

dbaluta added a commit to nxp-upstream/zephyr that referenced this issue Apr 24, 2025
Move initialization of 'enabled' variable together with declaration.
This fixes the following compiler error:
error: 'enabled' may be used uninitialized [-Werror=maybe-uninitialized]

This is not really an error but the compiler is tricked by the
K_SPINLOCK() macro.

Fixes: zephyrproject-rtos#88996
Signed-off-by: Daniel Baluta <[email protected]>
@hakehuang
Copy link
Collaborator Author

hakehuang commented Apr 24, 2025

@hakehuang this looks like a false positive

@dbaluta this is reported by tool-chain, and seeing below the for loop can be skipped.

#define K_SPINLOCK(lck)                                                                            \
	for (k_spinlock_key_t __i K_SPINLOCK_ONEXIT = {}, __key = k_spin_lock(lck); !__i.key;      \
	     k_spin_unlock((lck), __key), __i.key = 1)

@dbaluta
Copy link
Collaborator

dbaluta commented Apr 24, 2025

@hakehuang I have sent a fix. The issue is also discussed here: #78066

kartben pushed a commit that referenced this issue Apr 24, 2025
Move initialization of 'enabled' variable together with declaration.
This fixes the following compiler error:
error: 'enabled' may be used uninitialized [-Werror=maybe-uninitialized]

This is not really an error but the compiler is tricked by the
K_SPINLOCK() macro.

Fixes: #88996
Signed-off-by: Daniel Baluta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants