zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.69k stars 6.53k forks source link

[HWMv2] Failed to use same name for include dtsi file for two different architectures #70849

Closed dbaluta closed 5 months ago

dbaluta commented 6 months ago

In a system with 2 asymmetrical cores like imxrt685 (cm33 and hifi4 dsp) one would create an include dtsi like this:

./zephyr/dts/arm/nxp/nxp_rt6xx.dtsi
./zephyr/dts/xtensa/nxp/nxp_rt6xx.dtsi

The problem appears when we try to include this file in a board dts like this:

~/work/$ cat zephyr/boards/nxp/mimxrt685_evk/mimxrt685_evk_mimxrt685s_adsp.dts 

#include <nxp/nxp_rt6xx.dtsi>

Because the include path for processor contains all the arch directories: dts/;dts/x86;dts/xtensa;dts/sparc;dts/riscv;dts/posix;dts/nios2;dts/mips;dts/arm64;dts/arm;dts/arc

The preprocessor will get the first include file found, which is not always the correct one.

The problems comes from the way HWMv2 decides to compute the include directores:

# cat zephyr/cmake/modules/pre_dt.cmake

  if(HWMv1)
    set(arch_include dts/${ARCH})
  else()
    foreach(arch ${ARCH_V2_NAME_LIST})
      list(APPEND arch_include dts/${arch})
    endforeach()
  endif()

So instead of just appendind current dts/${arch} it include all architectures existent in the include path.

I think this is a bug and not intended behavior. As an workaround we can always use different include names (like prefix them with the core name _cm33 or _adsp but I think this is a limitation and should be taken into consideration for fixing.

github-actions[bot] commented 6 months ago

Hi @dbaluta! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

57300 commented 6 months ago

Because the include path for processor contains all the arch directories: dts/;dts/x86;dts/xtensa;dts/sparc;dts/riscv;dts/posix;dts/nios2;dts/mips;dts/arm64;dts/arm;dts/arc

Note that dts/ is also in this list, so another possible workaround is to use:

#include <arm/nxp/nxp_rt6xx.dtsi>

I think it makes sense to indicate the target arch like this anyway.

A similar pattern was already used for filenames shared between arm and arm64 subdirectories - armv8-r.dtsi and r8a77951.dtsi - and this predates HWMv2. The only exception is viper-a72.dtsi, but I just opened #70969 to address it.

tejlmand commented 5 months ago

this is not a bug.

A board in Zephyr is no longer organized under an architecture, because this doesn't make sense for a multi-SoC board, where each SoC may be of different architectures, nor does it make sense for a board with a multi-core SoC, where each core might be of different architectures, such as the imxrt685.

This means that at the time of pre-processing, we cannot know the architecture and therefore not limit the include folders, and from there it follows that the header filename must be unique or otherwise ensured to be uniquely identified by having the arch in the include directive.

For the initial work in HWMv2 we left out the zephyr/dts folder structure in the cleanup, but as @57300 mention, if we long term want to keep the arch separation in the dts folder, then it would make sense that the devicetree itself has the knowledge of the architecture in its #include <{arch}/{path}> directive. But that would require additional treewide changes, and before doing so we should make sure the going forward structure of dts folder has been decided.

aescolar commented 5 months ago

Closing as not a bug. Please reopen if you disagree.