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

dts: spi: add more dts nodes and fixup defines, convert boards to using dts #5883

Merged
merged 7 commits into from Feb 20, 2018
Merged

dts: spi: add more dts nodes and fixup defines, convert boards to using dts #5883

merged 7 commits into from Feb 20, 2018

Conversation

dwagenk
Copy link
Contributor

@dwagenk dwagenk commented Jan 29, 2018

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.

  • Spi nodes in <board>.dts file are populated matching boards existing pinmux default configuration.
  • Selecting SPI on those boards defaults to enabling SPI_1 in Kconfig. Other SPIs are disabled by default.

Fixes #5836

@codecov-io
Copy link

codecov-io commented Jan 29, 2018

Codecov Report

Merging #5883 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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     3137

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 e84d1a4...50a5bd7. Read the comment docs.

@galak
Copy link
Collaborator

galak commented Jan 29, 2018

Looks like some dts error:

Error: nucleo_f091rc.dts.pre.tmp:190.1-6 Label or path spi2 not found

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

dts/arm/st/stm32f0.dtsi doesn't have spi2 node. Causes issues with building nucleo_f091rc.dts

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 29, 2018

@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 dwagenk requested a review from erwango as a code owner January 30, 2018 00:04
@dwagenk dwagenk changed the title dts: spi: add more fixup defines and convert boards to using dts dts: spi: add more dts nodes and fixup defines, convert boards to using dts Jan 30, 2018
Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

Some SPIx ports are not available on all SoCs. It is better to move them under stm32fxxx.dtsi.

label = "SPI_2";
};

spi3: spi@40003C00 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

SPI3 is not available on all STM32F1 family boards (e.g. stm32f103rb based).

label = "SPI_3";
};

spi4: spi@40013C00 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

SPI4 is not available on all STM32F3 family boards.

};


spi5: spi@40015000 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

SPI5 is not available on all STM32F4 family boards.

label = "SPI_5";
};

spi6: spi@40015400 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

SPI6 is not available on all STM32F4 family boards

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 30, 2018

@ydamigos thanks for the comments.

This would mean adding many some more stm32fYnnnXz.dtsi files? Because right now except the F103 series the dtsi files are not more specific than stm32fYnnn.dtsi. Looking at the series overview tables on ST website theres quite some variations that would mean we'd have to split dtsi files up until having a stm32fYnnnXz.dtsi file for each SOCs.
Appearently for some SOCs it would need to be even more specific, because the difference lies in the 2nd last position that we have Xed out almost everywhere:

SOC SPIs
STM32F048C6 2x
STM32F048G6 1x

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.
Is there another way, to ensure the build fails if a spi node is activated, that isn't supported by the SOC, maybe through things undefined in stm32cube?

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 30, 2018

Just noticed, that in drivers/spi/Kconfig the SPI_STM32 config is not allowed for stm32f1 series, does this mean the current stm32 spi driver doesn't support f1 series yet?
If so I'd remove the dts and fixup entries for spi on stm31f1 again.

@erwango
Copy link
Member

erwango commented Jan 30, 2018

This would mean adding many some more stm32fYnnnXz.dtsi files?

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.
It is possible to deviate from this rule and model, but then we won't be compatible any more with documentation, tools, and people that assume we follow device tree generic model. We'll face issue in future with generic tools and methods . So this is at risk.
Regarding the amount of work, there are > 900 variations of STM32, so it is a significant load in every description model anyway. But not all of them will be ported into Zephyr.

@erwango
Copy link
Member

erwango commented Jan 30, 2018

does this mean the current stm32 spi driver doesn't support f1 series yet?

Yes, it is being addressed by @ydamigos in #5413

@ydamigos
Copy link
Collaborator

This would mean adding many some more stm32fYnnnXz.dtsi files?

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.
It is possible to deviate from this rule and model, but then we won't be compatible any more with documentation, tools, and people that assume we follow device tree generic model. We'll face issue in future with generic tools and methods . So this is at risk.
Regarding the amount of work, there are > 900 variations of STM32, so it is a significant load in every description model anyway. But not all of them will be ported into Zephyr.

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.

@galak
Copy link
Collaborator

galak commented Jan 30, 2018

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;

@erwango
Copy link
Member

erwango commented Jan 31, 2018

We could delete nodes in the stm32fYnnnXz.dtsi to match what a given SoC has.

I agree this method is appealing somehow. Though, I'm not quite in favor of it for few reasons:
*as discussed in much earlier thread, it won't work for some IPs (IPy of STM32 series Fz could have different addresses, hence it is not possible to define IPy in a unique way and delete it). But then it will be complicated to check/review for which IP it is fine and for which IP this method will not work.
*We'll have a bunch of dtsi/dts files with only /delete-node/ lines, not sure it will help user understand what is happening in dts (for those who are not familiar with it)
*This is not the approach used currently in STM32 Linux, so it prevents future use of common repo for dtsi files

@galak
Copy link
Collaborator

galak commented Jan 31, 2018

I'm good either way we want to handle things. Just point out the delete-node feature if it might be useful.

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 31, 2018

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:

We should configure the SPIx interfaces in the stm32fYnnnXz.dtsi files for the supported boards.

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?

  • stm32f0.dtsi
  • stm32f091.dtsi (most files right now)
  • stm32f091Xc.dtsi
  • stm32f091rc.dtsi

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:

IPy of STM32 series Fz could have different addresses, hence it is not possible to define IPy in a unique way and delete it

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?

@erwango
Copy link
Member

erwango commented Feb 1, 2018

So how far should we spread the files in dts/arm/st dir out by default?

stm32f048cX.dtsi and stm32f048gX.dtsi would be fine.

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?

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.

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 9, 2018

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

Theoretically it [the fixup addresses] should [be split up further, to reflect different addresses that can exist inside a SoC family], but I hope we'll get rid of them befoire we get in any trouble

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):

  • stm32f4.dtsi declares i2c3, but it is not supported on all F4 SoCs
  • stm32f3.dtsi declares i2c2, but it is not supported on all F3 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

Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

all stm32f4 boards supported now by Zephyr have a SPI2 port. We could leave it as it is for now

Copy link
Contributor Author

@dwagenk dwagenk Feb 10, 2018

Choose a reason for hiding this comment

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

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

status = "ok";
};

&spi2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

status = "ok";
};

&spi2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we enable SPI2 port here, we should also enable it in Kconfig.defconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we enable SPI3 port here, we should also enable it in Kconfig.defconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the node, since the corresponding pinmux defines are missing.
See https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/10601/3/tests

Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.


/ {
soc {
spi4: spi@40013400 {
Copy link
Member

Choose a reason for hiding this comment

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

SPI4 is present in stm32f401, move to stm32f401.dtsi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

left it as is, stm32f401 Socs mostly don't have spi4

interrupts = <84 5>;
status = "disabled";
label = "SPI_4";
};
Copy link
Member

Choose a reason for hiding this comment

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

What about SPI5 and SPI6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done and added comment and example stm32f429vX.dtsi for /delete-node/

interrupts = <84 5>;
status = "disabled";
label = "SPI_4";
};
};
Copy link
Member

Choose a reason for hiding this comment

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

What about SPI5 and SPI6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added in stm32f469iX.dtsi

@tbursztyka
Copy link
Collaborator

@dwagenk looks like I did some redundant work (#6172) with your pr: I should drop the nucleo_f334rb part I did. Could you rebase your PR on top of master to resolve the conflict? I'd like to rebase my PR on top of yours then.


/ {
soc {
spi2: spi@40003800 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This node could be moved inside stm32f103Xb.dtsi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, but with a comment, that it needs to be removed in stm32f103tb.dtsi

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

@erwango:
For the "missing" SPIs:
I followed the overview at http://www.st.com/en/microcontrollers/stm32f4-series.html wich also lists stm32f401 SoCs that only have 3 SPIs, so I didn't add it in stm32f401.dtsi. Same for SPI5 and SPI6 for stm32f429 and stm32f469.
I know those SoCs are not present in zephyr right now, but I'd rather propose that, whoever wants to use spi4 on a stm32f401 soc adds the corresponding stm32f401Xy.dtsi file and adds it there and vice versa for stm32f429 and stm32f469.
After the discussion in the beginning of this PR I don't like adding nodes in places, where they are OK right now, but would need to be moved when adding more SoCs later.

@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.

@tbursztyka
Copy link
Collaborator

@dwagenk Sure, I'll follow your updates. I have also to apply changes on mine. thanks

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

@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)

tests/kernel/mem_protect/obj_validation/test and samples/mpu/mem_domain_apis_test/test
fail on nucleo_f401re and nucleo_l476rg

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.

@erwango
Copy link
Member

erwango commented Feb 14, 2018

@dwagenk : Ok I had miss these variations across f4 series.
Indeed, I had missed that within F429/F469, a minority of SoCs support only 4 SPI vs 6 in rest of the sub series. This might be a good occasion to introduce "delete-node"
I don't like "delete-node" to be applied as a general rule, but I think it could be useful to handle exceptions to the general rule.
Hence, there would be:

  • stm32f429.dtsi defining 6 SPI
    -- 6 SPI defined
  • stm32f429vX.dtsi
    -- includes stm32f429.dtsi
    -- delete-node SPI5
    -- delete-node SPI6

When used only to handle exceptions, I think it could be useful and allow to reduce amount of dtsi files.

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

@erwango:
should I introduce the stm32f429vX.dtsi right here then? Or will it be added when someone adds that SoC?

@erwango
Copy link
Member

erwango commented Feb 14, 2018

should I introduce the stm32f429vX.dtsi right here then? Or will it be added when someone adds that SoC?

@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.
Besides, I'm raising an issue to document these rules (#6199)

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

recheck

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

@erwango @ydamigos: can you take a look at the changes, especially the /delete-node example and comments?

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 14, 2018

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.

@ydamigos
Copy link
Collaborator

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.

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 15, 2018

@andrewboie or @agross-linaro can you have a look and drop a hint on how to resolve this?

@ydamigos
Copy link
Collaborator

Enabling CONFIG_APPLICATION_MEMORY seems to fix it.

@ydamigos
Copy link
Collaborator

@dwagenk PR #6223 fixes it

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 15, 2018

@ydamigos: thanks, it passes again.

Can we get this merged now? @erwango

#address-cells = <1>;
#size-cells = <0>;
reg = <0x40003800 0x400>;
interrupts = <26 5>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you mention in #6238, I am just leaving a comment here to get this fixed before merging.

#address-cells = <1>;
#size-cells = <0>;
reg = <0x40003800 0x400>;
interrupts = <26 5>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you mention in #6238, I am just leaving a comment here to get this fixed before merging.

@dwagenk
Copy link
Contributor Author

dwagenk commented Feb 19, 2018

rebased, included @ydamigos pr #6254 in the spot of commit history, where it made sense, adressed issue #6238.

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

One last point to discuss, otherwise looks good

@@ -5,7 +5,7 @@
*/

/dts-v1/;
#include <st/stm32f469.dtsi>
#include <st/stm32f469iX.dtsi>
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

This would be fine, thanks.

Daniel Wagenknecht and others added 7 commits February 20, 2018 07:53
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 <wagenknecht.daniel@gmail.com>
Some stm32f4 SoCs don't support spi2, so remove spi2
device-tree node from stm32f4.dtsi file.

Signed-off-by: Daniel Wagenknecht <wagenknecht.daniel@gmail.com>
STM32F0 family has only 2 interrupt priority bits.

Fixes #6238

Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>
Add SPI nodes to existing dtsi files.

Signed-off-by: Daniel Wagenknecht <wagenknecht.daniel@gmail.com>
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 <wagenknecht.daniel@gmail.com>
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 <wagenknecht.daniel@gmail.com>
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 <wagenknecht.daniel@gmail.com>
@galak galak merged commit 1f43e7b into zephyrproject-rtos:master Feb 20, 2018
@dwagenk dwagenk deleted the stm32_spi_add_boards branch February 21, 2018 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spi: stm32: convert remaining boards that support SPI to using dts
6 participants