dts: spi: add more dts nodes and fixup defines, convert boards to using dts #5883
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5883 +/- ##
=======================================
Coverage 53.16% 53.16%
=======================================
Files 412 412
Lines 40117 40117
Branches 7726 7726
=======================================
Hits 21327 21327
Misses 15653 15653
Partials 3137 3137Continue to review full report at Codecov.
|
|
Looks like some dts error: Error: nucleo_f091rc.dts.pre.tmp:190.1-6 Label or path spi2 not found |
|
@galak there seem to be some more missing spi nodes in the dts/arm/st/stm32XX.dtsi files. I'll look into adding those as well |
dwagenk
changed the title
dts/arm/st/stm32f1.dtsi
Outdated
| label = "SPI_2"; | ||
| }; | ||
|
|
||
| spi3: spi@40003C00 { |
There was a problem hiding this comment.
SPI3 is not available on all STM32F1 family boards (e.g. stm32f103rb based).
dts/arm/st/stm32f3.dtsi
Outdated
| label = "SPI_3"; | ||
| }; | ||
|
|
||
| spi4: spi@40013C00 { |
There was a problem hiding this comment.
SPI4 is not available on all STM32F3 family boards.
dts/arm/st/stm32f4.dtsi
Outdated
| }; | ||
|
|
||
|
|
||
| spi5: spi@40015000 { |
There was a problem hiding this comment.
SPI5 is not available on all STM32F4 family boards.
dts/arm/st/stm32f4.dtsi
Outdated
| label = "SPI_5"; | ||
| }; | ||
|
|
||
| spi6: spi@40015400 { |
There was a problem hiding this comment.
SPI6 is not available on all STM32F4 family boards
|
@ydamigos thanks for the comments. This would mean adding many some more
I'm not saying we should keep everything in the soc family level .dtsi file, I'm just trying to grasp the dimension of work that splitting it up so every SOC dts only sees the nodes it should see. Your approach would ensure, that the build fails, if I (wrongly) define a spi node in a board level dts file, that isn't supported by the SOC (e.g. SPI_3 on a stm32f103rb based board), thus preventing illegal memory access. |
|
Just noticed, that in |
Device tree proposes a description model of HW configuration each SoC, so yes, dts representation should be faithful to each SoC and we need to create dtsi files when needed. Then thanks to inheritance property, there might be only little variations in each files. |
We should configure the SPIx interfaces in the stm32fYnnnXz.dtsi files for the supported boards. We could add a new stm32fYnnnXz.dtsi (if it is needed), when we add a new board. |
|
We could delete nodes in the stm32fYnnnXz.dtsi to match what a given SoC has. Thus we reduce duplication/error and just prune the tree as needed for a given SoC. We can do something like: /delete-node/ &spi2; |
I agree this method is appealing somehow. Though, I'm not quite in favor of it for few reasons: |
|
I'm good either way we want to handle things. Just point out the delete-node feature if it might be useful. |
|
I've understood the reasoning for keeping device tree completely consistent with every soc/board and not define nodes, that might not be present. So I'll throw out most of the definitions in .dtsi files i introduced in this pr. Two more questions:
Sometimes there might be differences even between two SoCs that need to include the same stm32fYnnnXz.dtsi file (e.g STM32F048C6 and STM32F048G6). So how far should we spread the files in dts/arm/st dir out by default?
And whenever a Board is added, that needs finer grained distinction between the files, it has to add another finer layer, move some of the nodes and maybe even change wich file an already existing board includes. And:
This would have an effect on fixup definitions as well? But I guess since the fixup defines should be obsolete sooner or later, my approach of defining most of them now is OK? |
stm32f048cX.dtsi and stm32f048gX.dtsi would be fine.
Theoretically it should, but I hope we'll get rid of them befoire we get in any trouble. So let's keep things simple for now on that side, your approach is ok. |
|
So I changed this PR to include the spi nodes in the right places of dtsi file hierarchy. But I still generate all the fixups with the adresses, that seem to be the SOC families default addresses. As discussed with @erwango
Please also look at the new dtsi include hierarchy of stm32f405.dtsi, I don't have enough overview of the SoC families to be sure it is correct now. Some dts conflicts I noted along the way (not spi related, so outside the scope of this PR):
|
There was a problem hiding this comment.
When we enable a port in boards dts file we should also enable it in Kconfig.defconfig.
stm32f4.dtsi declares i2c3, but it is not supported on all F4 SoCs
stm32f1.dtsi declares i2c2, but it is not supported on all F1 SoCs
stm32f0.dtsi declares i2c2, but it is not supported on all F0 SoCs
all boards supported now by Zephyr support the I2Cx ports mentioned above. We could leave it as it is for now.
stm32f3.dtsi declares i2c2, but it is not supported on all F3 SoCs
We should move I2C2 port inside stm32f303xc, stm32f373xc dtsi files as stm32f334x8 doesn't support this port. I will prepare a patch.
| @@ -135,16 +135,6 @@ | |||
| status = "disabled"; | |||
| label = "SPI_1"; | |||
| }; | |||
|
|
|||
| spi2: spi@40003800 { | |||
There was a problem hiding this comment.
all stm32f4 boards supported now by Zephyr have a SPI2 port. We could leave it as it is for now
There was a problem hiding this comment.
I moved SPI2 to stm32f401.dtsi, which is included directly or through other includes by every stm32f4 board that is supported by Zephyr right now.
Following the principle from previous discussion on this PR
dts representation should be faithful to each SoC
So I'd rather move it now, then have to do it later, when new SoCs are introduced
| status = "ok"; | ||
| }; | ||
|
|
||
| &spi2 { |
There was a problem hiding this comment.
If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.
| status = "ok"; | ||
| }; | ||
|
|
||
| &spi2 { |
There was a problem hiding this comment.
If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.
| status = "ok"; | ||
| }; | ||
|
|
||
| &spi2 { |
There was a problem hiding this comment.
If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.
There was a problem hiding this comment.
I removed the node, since the corresponding pinmux defines are missing.
See https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/10601/3/tests
| status = "ok"; | ||
| }; | ||
|
|
||
| &spi3 { |
There was a problem hiding this comment.
If we enable SPI3 port here, we should also enable it in Kconfig.defconfig.
There was a problem hiding this comment.
I removed the node, since the corresponding pinmux defines are missing.
See https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/10601/3/tests
|
|
||
| / { | ||
| soc { | ||
| spi4: spi@40013400 { |
There was a problem hiding this comment.
SPI4 is present in stm32f401, move to stm32f401.dtsi
There was a problem hiding this comment.
left it as is, stm32f401 Socs mostly don't have spi4
| interrupts = <84 5>; | ||
| status = "disabled"; | ||
| label = "SPI_4"; | ||
| }; |
There was a problem hiding this comment.
done and added comment and example stm32f429vX.dtsi for /delete-node/
dts/arm/st/stm32f469.dtsi
Outdated
| interrupts = <84 5>; | ||
| status = "disabled"; | ||
| label = "SPI_4"; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
added in stm32f469iX.dtsi
dts/arm/st/stm32f103Xe.dtsi
Outdated
|
|
||
| / { | ||
| soc { | ||
| spi2: spi@40003800 { |
There was a problem hiding this comment.
This node could be moved inside stm32f103Xb.dtsi.
There was a problem hiding this comment.
done, but with a comment, that it needs to be removed in stm32f103tb.dtsi
|
@erwango: @ydamigos: same reasoning for not including SPI2 in stm32f103Xb. @tbursztyka: rebased, but I' might have to integrate further changes, depending on ydamigos and erwango's replies. |
|
@dwagenk Sure, I'll follow your updates. I have also to apply changes on mine. thanks |
|
@galak can you look into the sanitycheck errors? it seems to be unrelated to my changes or at least in some area I didn't tocuh (although it only effects boards I touched)
FAILED: : && ccache /opt/sdk/zephyr-sdk-0.9.2/sysroots/x86_64-pokysdk-linux/usr/bin/arm-zephyr-eabi/arm-zephyr-eabi-gcc zephyr/CMakeFiles/app_sizing_prebuilt.dir/misc/empty_file.c.obj -o zephyr/app_sizing_prebuilt.elf -T zephyr/linker_app_sizing.cmd -Wl,-Map=/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/nucleo_f401re/tests/kernel/mem_protect/obj_validation/test/zephyr/zephyr.map -u_OffsetAbsSyms -u_ConfigAbsSyms -e__start -Wl,--start-group -Wl,--whole-archive libapp.a zephyr/libzephyr.a zephyr/boards/arm/nucleo_f401re/libboards__arm__nucleo_f401re.a zephyr/tests/ztest/libtests__ztest.a -Wl,--no-whole-archive zephyr/kernel/libkernel.a zephyr/CMakeFiles/offsets.dir/arch/arm/core/offsets/offsets.c.obj -Wl,--end-group -L"/opt/sdk/zephyr-sdk-0.9.2/sysroots/armv5-zephyr-eabi/usr/lib/arm-zephyr-eabi/6.2.0/armv7e-m" -L/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/nucleo_f401re/tests/kernel/mem_protect/obj_validation/test/zephyr -lgcc -nostartfiles -nodefaultlibs -nostdlib -static -no-pie -Wl,--fatal-warnings -Wl,-X -Wl,-N -Wl,--gc-sections -Wl,--build-id=none && cd /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/nucleo_f401re/tests/kernel/mem_protect/obj_validation/test/zephyr && /usr/bin/python3 /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/scripts/gen_alignment_script.py --output ./include/generated/app_data_alignment.ld --kernel /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/nucleo_f401re/tests/kernel/mem_protect/obj_validation/test/zephyr/app_sizing_prebuilt.elf
Traceback (most recent call last):
File "/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/scripts/gen_alignment_script.py", line 79, in <module>
main()
File "/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/scripts/gen_alignment_script.py", line 62, in main
app_ram_size = syms['__app_last_address_used'] - \
KeyError: '__app_last_address_used'
ninja: build stopped: subcommand failed. |
|
@dwagenk : Ok I had miss these variations across f4 series.
When used only to handle exceptions, I think it could be useful and allow to reduce amount of dtsi files. |
|
@erwango: |
@dwagenk : I was about to say "If not used right now don't introduce it", but challenge is to remember this proposal when the need will come. Then I propose you introduce it, as an example of "delete-node" usage, so we can refer to it for upcoming SoC ports. |
|
recheck |
|
Can somebody help me with the CI failures? When I try to run the tests localy (e.g. kernel/mem_protect/obj_validation for nucleo_l476rg) no tests are run due to filters. |
I believe it has something to do with CONFIG_STM32_ARM_MPU_ENABLE=y and PR #4974 being merged. |
|
@andrewboie or @agross-linaro can you have a look and drop a hint on how to resolve this? |
|
Enabling CONFIG_APPLICATION_MEMORY seems to fix it. |
dts/arm/st/stm32f072.dtsi
Outdated
| #address-cells = <1>; | ||
| #size-cells = <0>; | ||
| reg = <0x40003800 0x400>; | ||
| interrupts = <26 5>; |
There was a problem hiding this comment.
As you mention in #6238, I am just leaving a comment here to get this fixed before merging.
dts/arm/st/stm32f091.dtsi
Outdated
| #address-cells = <1>; | ||
| #size-cells = <0>; | ||
| reg = <0x40003800 0x400>; | ||
| interrupts = <26 5>; |
There was a problem hiding this comment.
As you mention in #6238, I am just leaving a comment here to get this fixed before merging.
| @@ -5,7 +5,7 @@ | |||
| */ | |||
|
|
|||
| /dts-v1/; | |||
| #include <st/stm32f469.dtsi> | |||
| #include <st/stm32f469iX.dtsi> | |||
There was a problem hiding this comment.
Is that really required to introduce stm32f469iX here, as 6 SPI seems to be a majority on F469
469aX, 469bX, 469iX, 469nX (12 SoCs) : 6 SPI
469vX, 469zX (5 SoCs): 4 SPI
There was a problem hiding this comment.
deepends on the guidelines we want to introduce.
I think for 429 (where we now introduced the /delete-node/ example) it was a quite clear case:
1 (V) vs 5 (ABINZ) series, 5 vs 23 SoCs only has 4 SPIs, others have 6
But for 469 it's not that clear:
2 (VZ) vs 4(ABIN) series, 5 vs 12 SoCs
Where should we draw this line? Having to delete nodes when introducing new SoCs seems like something that should be avoided where possible.
There was a problem hiding this comment.
Where should we draw this line?
Agreed we're in a grey zone where the line moves depending on the criteria in use.
My point (/criteria) is to limit the number of .dtsi files we'll have to create (and hence maintain).
Then:
-if we introduce a iX.dtsi, we'll need aX, bX and nX : 5 stm32f469 dtsi variants
-if we keep 6 SPI variants in f469.dtsi, we'll required xV and xZ : 3 stm32f469 dtsi variants
I hope this rule is objective enough to be applied easily
There was a problem hiding this comment.
OK, I'd then propose doing it as in stm32f429.dtsi:
- Introduce the nodes, but put a comment next to them, that lists where exceptions are necessary.
- Whoever introduces a new SoC needs to check the include hierarchy and see if any of the file they include requires deleting a node further down the hierarchy
That should of course be noted in #6199
OK? I'll go ahead and change it in this PR then
stm32f405 is not an expansion of stm32f411, since stm32f411 has more SPIs than stm32f405. Fix this by including stm32f401.dtsi in stm32f405.dtsi (instead of stm32f411.dtsi). Signed-off-by: Daniel Wagenknecht <[email protected]>
Some stm32f4 SoCs don't support spi2, so remove spi2 device-tree node from stm32f4.dtsi file. Signed-off-by: Daniel Wagenknecht <[email protected]>
STM32F0 family has only 2 interrupt priority bits. Fixes #6238 Signed-off-by: Yannis Damigos <[email protected]>
Add SPI nodes to existing dtsi files. Signed-off-by: Daniel Wagenknecht <[email protected]>
Most STM32F429 SoCs have 6 SPIs, but STM32F429Vx SoCs only have 4 SPIs. This is one of the rare conditions where device-tree directive /delete-node/ should be used. Add spi5 and spi6 node to stm32f429.dtsi. Create file stm32f429vX.dtsi to delete those nodes and document usage of /delete-node/ directive. Signed-off-by: Daniel Wagenknecht <[email protected]>
Add SPI fixup defines on STM32 SoC family level for all SPIs that are supported on one or more SOCs of that SoC family. Signed-off-by: Daniel Wagenknecht <[email protected]>
Configure SPI using DT for the following STM32 boards: 96b_neonkey nucleo_f091rc nucleo_f334r8 nucleo_f401re nucleo_l432kc nucleo_l476rg SPI nodes in <board>.dts file are populated matching boards existing pinmux default configuration and enabled in Kconfig.defconfig if SPI is enabled. For nucleo_l476rg board SPI2 and SPI3 nodes are not yet added, because of missing pinmux defines. Fixes #5836 Signed-off-by: Daniel Wagenknecht <[email protected]>
This adds the remaining SPI related fixup defines to STM32 dts.fixup files at SOC family level. I added fixup defines for all SPIs that might be present on a SOC of that specific STM32 SOC family. Same for SPI dts-nodes.
Also I added SPI configuration via device tree to all STM32 Boards, that already have SPI in their pinmux.c.
Fixes #5836