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

Shift DTS before Kconfig #12108

Merged
merged 9 commits into from
Feb 1, 2019
Merged

Conversation

galak
Copy link
Collaborator

@galak galak commented Dec 14, 2018

This is a WIP PR to get feedback on the direction and code associated with moving DTS generation before Kconfig.

The general idea is that Kconfig can utilize DTS generated values for defaults or other purposes going forward, and that there will NOT be any Kconfig based symbols or ifdef's in dts files.

To keep some functionality with dts_fixup.h files working we need to split the DTS generation into 2 pieces. The first will preprocess the .dts file, compile dts, and generate the generated_dts_board.conf. The second pass will run after Kconfig since we need to know CONFIG_SOC to find all the dts_fixup.h files. The second pass will just generate the generated_dts_board.h

resolves #7302

@galak galak added area: Devicetree DNM This PR should not be merged (Do Not Merge) labels Dec 14, 2018
@galak galak self-assigned this Dec 14, 2018
@galak galak requested review from ulfalizer and mbolivar December 14, 2018 17:13
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to build test this, but it doesn't seem to have a solution for our use case for selecting nodes in an overlay based on kconfig:

https://github.com/foundriesio/dm-hawkbit-mqtt/blob/master/common.overlay

Is there an equivalent in DT i'm missing?

@galak
Copy link
Collaborator Author

galak commented Dec 14, 2018

I tried to build test this, but it doesn't seem to have a solution for our use case for selecting nodes in an overlay based on kconfig:

https://github.com/foundriesio/dm-hawkbit-mqtt/blob/master/common.overlay

Is there an equivalent in DT i'm missing?

I'm not aware of any way, need to look into how true DTS overlay work more. For us to be able to truly have DTS not require Kconfig, what you are doing based on SoC and CONFIG_ defines isn't something we can support.

I do think it would be interesting and useful to expand the DTC tooling to allow for conditional nodes based on some property being set.

@carlescufi
Copy link
Member

When building standard "hello_world" on Windows I get:

C:\Users\Carles\src\zephyr\samples\hello_world\b>ninja
[65/138] Building C object zephyr/CMakeFiles/zephyr.dir/soc/arm/nordic_nrf/nrf52/mpu_regions.c.obj
In file included from C:/Users/Carles/src/zephyr/soc/arm/nordic_nrf/nrf52/mpu_regions.c:8:0:
C:/Users/Carles/src/zephyr/samples/hello_world/b/zephyr/include/generated/autoconf.h:6:34: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define CONFIG_SRAM_BASE_ADDRESS 0x536870912
                                  ^
../../../include/arch/arm/cortex_m/mpu/arm_mpu.h:44:11: note: in definition of macro 'MPU_REGION_ENTRY'
   .base = _base, \
           ^~~~~
C:/Users/Carles/src/zephyr/soc/arm/nordic_nrf/nrf52/mpu_regions.c:28:5: note: in expansion of macro 'CONFIG_SRAM_BASE_ADDRESS'
     CONFIG_SRAM_BASE_ADDRESS,
     ^~~~~~~~~~~~~~~~~~~~~~~~
[133/138] Linking C executable zephyr\zephyr_prebuilt.elf
FAILED: zephyr/zephyr_prebuilt.elf
cmd.exe /C "cd . && C:\gccarmemb\bin\arm-none-eabi-gcc.exe    zephyr/CMakeFiles/zephyr_prebuilt.dir/misc/empty_file.c.obj  -o zephyr\zephyr_prebuilt.elf  -T zephyr/linker.cmd -Wl,-Map=C:/Users/Carles/src/zephyr/samples/hello_world/b/zephyr/zephyr.map -u_OffsetAbsSyms -u_ConfigAbsSyms -Wl,--whole-archive app/libapp.a zephyr/libzephyr.a zephyr/arch/arm/core/libarch__arm__core.a zephyr/arch/arm/core/cortex_m/libarch__arm__core__cortex_m.a zephyr/arch/arm/core/cortex_m/mpu/libarch__arm__core__cortex_m__mpu.a zephyr/lib/libc/minimal/liblib__libc__minimal.a zephyr/boards/arm/nrf52840_pca10059/libboards__arm__nrf52840_pca10059.a zephyr/subsys/bluetooth/common/libsubsys__bluetooth__common.a zephyr/subsys/bluetooth/host/libsubsys__bluetooth__host.a zephyr/subsys/bluetooth/controller/libsubsys__bluetooth__controller.a zephyr/subsys/net/libsubsys__net.a zephyr/drivers/gpio/libdrivers__gpio.a zephyr/drivers/serial/libdrivers__serial.a zephyr/drivers/entropy/libdrivers__entropy.a -Wl,--no-whole-archive zephyr/kernel/libkernel.a zephyr/CMakeFiles/offsets.dir/arch/arm/core/offsets/offsets.c.obj -L"c:/gccarmemb/bin/../lib/gcc/arm-none-eabi/7.2.1/thumb/v7e-m" -LC:/Users/Carles/src/zephyr/samples/hello_world/b/zephyr -lgcc -Wl,--print-memory-usage -mthumb -mcpu=cortex-m4 -nostdlib -static -no-pie -Wl,-X -Wl,-N -Wl,--gc-sections -Wl,--build-id=none -Wl,--orphan-handling=warn -mabi=aapcs -march=armv7e-m && cd ."
Memory region         Used Size  Region Size  %age Used
           FLASH:       47476 B         1 MB      4.53%
            SRAM:       11177 B       256 KB      4.26%
        IDT_LIST:         137 B         2 KB      6.69c:/gccarmemb/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/bin/ld.exe: zephyr\zephyr_prebuilt.elf section `bss' will not fit in region `SRAM'
c:/gccarmemb/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/bin/ld.exe: region `SRAM' overflowed by 4294716329 bytes
collect2.exe: error: ld returned 1 exit status
%
ninja: build stopped: subcommand failed.

@galak galak force-pushed the wip-dt-before-kconfig branch 2 times, most recently from 3853249 to 217abfa Compare December 14, 2018 20:37
@carlescufi
Copy link
Member

Works fine on Windows now

@mbolivar
Copy link
Contributor

mbolivar commented Dec 14, 2018

I do think it would be interesting and useful to expand the DTC tooling to allow for conditional nodes based on some property being set.

Is the ELCE discussion still plan of record?

#10821

In particular these two:

- Build system needs to look by default for board, arch and soc .conf and .overlay files
- Find the exact arch and soc name to locate SoC-specific overlays, requires a 2-pass DT approach: First pass to find out arch and soc, second pass is actually DT (PR #9925)

We could definitely work with that.

It would be nice to stop cluttering the top level directory of each application with $board.conf and $board.overlay files, by the way.

Picking these up automatically for example would be nice:

  • overlays/arch-$ARCH.overlay
  • overlays/soc-family-$SOC_FAMILY.overlay (if applicable)
  • overlays/soc-series-$SOC_SERIES.overlay (if applicable)
  • overlays/soc-$SOC.overlay
  • overlays/board-$BOARD.overlay

That of course would require all of those values to be determined before running DTS, or during the first run of DTS, as discussed.

Edit: and similarly for "conf mixins", of course.

@codecov-io
Copy link

codecov-io commented Dec 14, 2018

Codecov Report

Merging #12108 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12108      +/-   ##
==========================================
- Coverage   53.53%   53.51%   -0.02%     
==========================================
  Files         239      239              
  Lines       27052    26983      -69     
  Branches     6521     6518       -3     
==========================================
- Hits        14482    14440      -42     
+ Misses       9900     9879      -21     
+ Partials     2670     2664       -6
Impacted Files Coverage Δ
subsys/net/lib/sockets/sockets_internal.h 28.57% <0%> (-71.43%) ⬇️
subsys/net/ip/net_context.c 60.38% <0%> (-2.34%) ⬇️
subsys/net/lib/sockets/sockets.c 49.14% <0%> (-1.43%) ⬇️
subsys/shell/shell.c 25.25% <0%> (ø) ⬆️
include/net/net_context.h 78.84% <0%> (ø) ⬆️
subsys/logging/log_core.c 37.8% <0%> (+0.15%) ⬆️
subsys/net/ip/net_if.c 61.37% <0%> (+0.31%) ⬆️
subsys/net/ip/tcp.c 54.37% <0%> (+0.38%) ⬆️
subsys/net/ip/net_pkt.c 67.58% <0%> (+0.61%) ⬆️
subsys/net/lib/dns/resolve.c 34.8% <0%> (+1.22%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66b9e95...ee63a78. Read the comment docs.

@SebastianBoe
Copy link
Collaborator

The second pass will run after Kconfig since we need to know CONFIG_SOC to find all the dts_fixup.h files.

Can we deduce the SOC from the BOARD in a similar manner to how we deduce the ARCH from the BOARD?

I'm concerned that a two-pass DT approach will be difficult to reason about.

@mnkp
Copy link
Member

mnkp commented Dec 17, 2018

Can we deduce the SOC from the BOARD in a similar manner to how we deduce the ARCH from the BOARD?

A related Slack comment:

one idea is if we match the board dir layout to soc dir so boards/arm/nrf52_pca10040 would become boards/arm/nordic_nrf/nrf52/nrf52_pca10040
-- @galak

But we also need to extract part number information. One approach is discussed here #12073. It would be best to come up with a solution that covers all our needs.

arch/arc/Kconfig Outdated Show resolved Hide resolved
Copy link
Collaborator

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small suggestions.

scripts/kconfig/kconfigfunctions.py Outdated Show resolved Hide resolved
scripts/kconfig/kconfigfunctions.py Outdated Show resolved Hide resolved
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jan 30, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@galak galak force-pushed the wip-dt-before-kconfig branch from 94a2f33 to 3b89b5b Compare January 30, 2019 18:29
@galak galak dismissed nashif’s stale review January 30, 2019 18:31

Add requested docs

@galak
Copy link
Collaborator Author

galak commented Jan 30, 2019

@dbkinder can you review the docs again?

@galak
Copy link
Collaborator Author

galak commented Jan 30, 2019

@dbkinder can you look at the docs again?

@mbolivar
Copy link
Contributor

@galak I've submitted a PR for the west commands which hopefully should make it clear why CONFIG_BOOTLOADER_MCUBOOT is a big usability win:

#12903

I would like to work with you to make sure we can keep the usability of building, signing, and flashing MCUboot child images in Zephyr even if DTS comes before Kconfig. Do you have time for that this week/next?

Thanks!

@nashif nashif dismissed dbkinder’s stale review January 31, 2019 13:19

addressed, @dbkinder please review

@SebastianBoe
Copy link
Collaborator

@galak : I believe you've posted a PR somewhere that runs DTC multiple times, is that still on the table?

Or does this replace that PR?

@galak
Copy link
Collaborator Author

galak commented Jan 31, 2019

@galak : I believe you've posted a PR somewhere that runs DTC multiple times, is that still on the table?

Or does this replace that PR?

I think that's still on the table but for different reasons.

galak and others added 9 commits January 31, 2019 17:24
Expose PROJECT_BINARY_DIR to kconfig since we will we looking for
generated files from dts pass in the future and need to know the
location of those files.

Signed-off-by: Kumar Gala <[email protected]>
Based on work from Sebastian Bøe and updated to current tree.

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Kumar Gala <[email protected]>
As we want to remove dts dependency on Kconfig, we had a case based on
CONFIG_BOOTLOADER_MCUBOOT.  From a DTS point of view that was just
getting the chosen property 'zephyr,code-partition' set.  We can easily
move this to the actual dts files and remove the mcuboot.overlay.

Signed-off-by: Kumar Gala <[email protected]>
As a step to completing removing use of Kconfig defines in dts files we
need to change how we configure/set CONFIG_FLASH_LOAD_{OFFSET,SIZE}.

Previously we would set them based on how the chosen property
'zephyr,code-partition' was set to.  If 'zephyr,code-partition' wasn't
set we would default the values to 0.  We had some generic overlay logic
which would define 'zephyr,code-partition' based on
CONFIG_BOOTLOADER_MCUBOOT.

Going forward if the DTS has 'zephyr,code-partition' set we will
generate DT_CODE_PARTITION_OFFSET & DT_CODE_PARTITION_SIZE defines.  We
will leave it to Kconfig to set CONFIG_FLASH_LOAD_OFFSET &
CONFIG_FLASH_LOAD_SIZE.

We introduce a python script that allows Kconfig to extract data from
the DTS and thus we can utilize DT_CODE_PARTITION_OFFSET and
DT_CODE_PARTITION_SIZE values to set defaults for
CONFIG_FLASH_LOAD_OFFSET & CONFIG_FLASH_LOAD_SIZE.

Signed-off-by: Kumar Gala <[email protected]>
Signed-off-by: Carles Cufi <[email protected]>
Export a new KCONFIG_DOC_MODE environment variable when building the doc
and invoking Kconfig, so that the functions that expect a build folder
can accordingly return a hardcoded value.

Signed-off-by: Carles Cufi <[email protected]>
Add docs on the kconfigfunctions that we support.  We only expose some
functions related to device tree currently.

Signed-off-by: Kumar Gala <[email protected]>
dts will now generate DT_SRAM_BASE_ADDRESS, DT_SRAM_SIZE,
DT_FLASH_BASE_ADDRESS, and DT_FLASH_SIZE defines.  Kconfig can utilize
these defines to set defaults for the CONFIG_ variants.

Signed-off-by: Kumar Gala <[email protected]>
Move the how enabling of CONFIG_XIP impacts CONFIG_FLASH_SIZE and
CONFIG_FLASH_BASE_ADDRESS to Kconfig instead of dts.

Signed-off-by: Kumar Gala <[email protected]>
The dts files on these boards had some CONFIG_ defines related to which
memory should be used to hold code.  We move this choice out of DTS and
back into Kconfig.

As such, we removed the default setting of 'zephyr,flash' and just
map

CONFIG_CODE_ITCM to:
	DT_NXP_IMX_RT_ITCM_0_SIZE
	DT_NXP_IMX_RT_ITCM_0_BASE_ADDRESS

CONFIG_CODE_{QSPI,HYPERFLASH} to:
	DT_NXP_IMX_FLEXSPI_402A8000_SIZE_1
	DT_NXP_IMX_FLEXSPI_402A8000_BASE_ADDDRESS_1

for the mimxrt1050_evk, we remove the default setting of 'zephyr,sram'
and just map:

CONFIG_DATA_DTCM to:
	DT_NXP_IMX_DTCM_0_SIZE
	DT_NXP_IMX_DTCM_0_BASE_ADDRESS

CONFIG_DATA_SDRAM to:
	DT_MMIO_SRAM_80000000_SIZE
	DT_MMIO_SRAM_80000000_BASE_ADDRESS

Signed-off-by: Kumar Gala <[email protected]>
@galak galak force-pushed the wip-dt-before-kconfig branch from 39cead0 to ee63a78 Compare January 31, 2019 23:27
@galak galak merged commit c3ba733 into zephyrproject-rtos:master Feb 1, 2019
@galak galak deleted the wip-dt-before-kconfig branch February 1, 2019 16:16
@carlescufi carlescufi mentioned this pull request Feb 6, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DTS metadata should be made available to Kconfig