Skip to content

Conversation

kwd-doodling
Copy link
Contributor

The reason is that this driver needs to call the function 'irq_connect_dynamic()' which is implemented with DYNAMIC_INTERRUPTS.

The reason is that this driver needs to call the function
'irq_connect_dynamic()' which  is implemented with DYNAMIC_INTERRUPTS.

Signed-off-by: Dong Wang <dong.d.wang@intel.com>
@dleach02 dleach02 merged commit af6d790 into zephyrproject-rtos:main Jun 14, 2024
@kartben
Copy link
Contributor

kartben commented Jun 14, 2024

@kwd-doodling compliance check seems to be spotting a cyclic dependency here: https://github.com/zephyrproject-rtos/zephyr/actions/runs/9510083836/job/26214024528#step:10:1
Can you please investigate as this will likely break CI for others? Not sure why CI for this PR didn't catch it tho

@ycsin
Copy link
Member

ycsin commented Jun 14, 2024

This change introduced a Kconfig dependency error when building

west build -b m2gl025_miv -p auto -T zephyr/tests/posix/common/portability.posix.common.static_stack
/home/ycsin/zephyrproject/zephyr/scripts/kconfig/kconfig.py: Dependency loop
===============

DYNAMIC_INTERRUPTS (defined at soc/nxp/imxrt/imxrt5xx/Kconfig.defconfig:104, soc/nxp/imx/imx8m/Kconfig.defconfig.mimx8ml8_adsp:38, soc/ite/ec/it8xxx2/Kconfig.defconfig.series:39, soc/intel/intel_ish/Kconfig.defconfig:28, soc/intel/intel_adsp/ace/Kconfig.defconfig.series:56, soc/intel/intel_adsp/cavs/Kconfig.defconfig.series:37, soc/espressif/common/Kconfig.defconfig:15, arch/Kconfig:448), with definition...

config DYNAMIC_INTERRUPTS
        bool
        default n
        depends on SOC_MIMXRT595S_F1 && SOC_FAMILY_NXP_IMXRT

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on SOC_MIMX8ML8_ADSP && SOC_SERIES_IMX8M && SOC_SERIES_IMX8M && SOC_FAMILY_NXP_IMX

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on SOC_SERIES_IT8XXX2 && SOC_FAMILY_ITE_EC

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on APIC_TIMER_TSC && SOC_FAMILY_INTEL_ISH

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on SOC_SERIES_INTEL_ADSP_ACE && SOC_FAMILY_INTEL_ADSP

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on SOC_SERIES_INTEL_ADSP_CAVS && SOC_FAMILY_INTEL_ADSP

config DYNAMIC_INTERRUPTS
        bool
        default y
        depends on SOC_SERIES_ESP32C3 && SOC_FAMILY_ESPRESSIF_ESP32

config DYNAMIC_INTERRUPTS
        bool "Installation of IRQs at runtime"
        help
          Enable installation of interrupts at runtime, which will move some
          interrupt-related data structures to RAM instead of ROM, and
          on some architectures increase code size.

(select-related dependencies: (BT_STM32WBA && SOC_SERIES_STM32WBAX && SOC_FAMILY_STM32) || (NRF_802154_RADIO_DRIVER && HAS_HW_NRF_RADIO_IEEE802154 && !n && HAS_NORDIC_DRIVERS) || (NRF_802154_RADIO_DRIVER && HAS_HW_NRF_RADIO_IEEE802154 && !n && HAS_NORDIC_DRIVERS && 0) || (SOC_SERIES_CC13X2_CC26X2 && SOC_FAMILY_TI_SIMPLELINK) || (SOC_SERIES_CC13X2X7_CC26X2X7 && SOC_FAMILY_TI_SIMPLELINK) || (SOC_SERIES_CC32XX && SOC_FAMILY_TI_SIMPLELINK) || (SOC_SERIES_MSP432P4XX && SOC_FAMILY_TI_SIMPLELINK) || (SOC_SERIES_AM6X_M4 && SOC_FAMILY_TI_K3) || (SOC_SERIES_RA4M1 && SOC_FAMILY_RENESAS_RA) || SOC_RAPTOR_LAKE || SOC_ELKHART_LAKE || SOC_APOLLO_LAKE || SOC_ALDER_LAKE || (SOC_SERIES_CYW20829 && SOC_FAMILY_INFINEON_CAT1B) || SOC_FAMILY_PSOC6 || (SOC_SERIES_ESP32 && SOC_FAMILY_ESPRESSIF_ESP32) || (SOC_SERIES_ESP32C3 && SOC_FAMILY_ESPRESSIF_ESP32) || (SOC_SERIES_ESP32S2 && SOC_FAMILY_ESPRESSIF_ESP32) || (SOC_SERIES_ESP32S3 && SOC_FAMILY_ESPRESSIF_ESP32) || (GIC_V3_ITS && GIC_V3 && CPU_CORTEX) || (SMBUS_INTEL_PCH && DT_HAS_INTEL_PCH_SMBUS_ENABLED && SMBUS) || (LEON_GPTIMER && DT_HAS_GAISLER_GPTIMER_ENABLED && SYS_CLOCK_EXISTS))

...depends on the choice symbol APIC_TIMER_TSC (defined at drivers/timer/Kconfig.x86:54), with definition...

config APIC_TIMER_TSC
        bool "Local APIC timer using TSC time source"
        select LOAPIC
        select TICKLESS_CAPABLE
        select TIMER_HAS_64BIT_CYCLE_COUNTER
        depends on !SMP && DYNAMIC_INTERRUPTS && <choice>
        help
          If your CPU supports invariant TSC but no TSC deadline capability
          then this choice will rely on the TSC as time source and the
          local APIC in one-shot mode as the timeout event source.
          You must know the ratio of the TSC frequency to the local APIC
          timer frequency.

...depends again on DYNAMIC_INTERRUPTS (defined at soc/nxp/imxrt/imxrt5xx/Kconfig.defconfig:104, soc/nxp/imx/imx8m/Kconfig.defconfig.mimx8ml8_adsp:38, soc/ite/ec/it8xxx2/Kconfig.defconfig.series:39, soc/intel/intel_ish/Kconfig.defconfig:28, soc/intel/intel_adsp/ace/Kconfig.defconfig.series:56, soc/intel/intel_adsp/cavs/Kconfig.defconfig.series:37, soc/espressif/common/Kconfig.defconfig:15, arch/Kconfig:448)

Once this is reverted, you will see another build error, but that error is by me 😎

/home/ycsin/zephyrproject/zephyr/arch/riscv/core/stacktrace.c: In function 'in_kernel_thread_stack_bound':
/home/ycsin/zephyrproject/zephyr/arch/riscv/core/stacktrace.c:41:23: error: 'const struct k_thread' has no member named 'stack_info'
   41 |         start = thread->stack_info.start;
      |                       ^~
In file included from /home/ycsin/zephyrproject/zephyr/include/zephyr/kernel_includes.h:42,
                 from /home/ycsin/zephyrproject/zephyr/include/zephyr/kernel.h:17,
                 from /home/ycsin/zephyrproject/zephyr/arch/riscv/core/stacktrace.c:8:
/home/ycsin/zephyrproject/zephyr/arch/riscv/core/stacktrace.c:42:39: error: 'const struct k_thread' has no member named 'stack_info'
   42 |         end = Z_STACK_PTR_ALIGN(thread->stack_info.start + thread->stack_info.size);
      |                                       ^~
/home/ycsin/zephyrproject/zephyr/include/zephyr/kernel/thread_stack.h:73:71: note: in definition of macro 'Z_STACK_PTR_ALIGN'
   73 | #define Z_STACK_PTR_ALIGN(ptr) ((uintptr_t)z_stack_ptr_align((char *)(ptr)))
      |                                                                       ^~~
/home/ycsin/zephyrproject/zephyr/arch/riscv/core/stacktrace.c:42:66: error: 'const struct k_thread' has no member named 'stack_info'
   42 |         end = Z_STACK_PTR_ALIGN(thread->stack_info.start + thread->stack_info.size);
      |                                                                  ^~
/home/ycsin/zephyrproject/zephyr/include/zephyr/kernel/thread_stack.h:73:71: note: in definition of macro 'Z_STACK_PTR_ALIGN'
   73 | #define Z_STACK_PTR_ALIGN(ptr) ((uintptr_t)z_stack_ptr_align((char *)(ptr)))
      |                                                                       ^~~

dleach02 added a commit to nxp-upstream/zephyr that referenced this pull request Jun 14, 2024
PR zephyrproject-rtos#74127 introduced a dependency loop that appeared to not be caught
by CI.

Signed-off-by: David Leach <david.leach@nxp.com>
@kwd-doodling
Copy link
Contributor Author

Then issue happens after merging of #72843.
They two were submitted and passed CI separately.
It seems better to trigger CI again before merging a patch.

dleach02 added a commit that referenced this pull request Jun 14, 2024
PR #74127 introduced a dependency loop that appeared to not be caught
by CI.

Signed-off-by: David Leach <david.leach@nxp.com>
@marc-hb
Copy link
Contributor

marc-hb commented Jun 14, 2024

They two were submitted and passed CI separately.

This is a well-known CI issue and this is the well-known fix:

https://about.gitlab.com/blog/2020/01/30/all-aboard-merge-trains/
https://github.blog/2024-03-06-how-github-uses-merge-queue-to-ship-hundreds-of-changes-every-day/
https://github.blog/2023-07-12-github-merge-queue-is-generally-available/

I don't know how (in)compatible with Zephyr CI such a fix would be.

PS: this one probably came first: https://chromium.googlesource.com/infra/infra/+/master/doc/users/services/commit_queue/index.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Timer Timer platform: X86 x86 and x86-64 size: XS A PR changing only a single line of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants