Shields: Allow shields conditional configuration #20934

erwango · 2019-11-22T10:14:11Z

Today shields Kconfig configuration (set of Kconfig symbols defined in <shield>_defconfig files) is applied unconditionally when a specific shield is selected by user (or indirectly in a sample/test via cmake configuration or with a yaml configuration when running sanitycheck).

This must be changed. Indeed, for application portability matter, board default configuration guidelines states that features activation is application responsibility and should not be done on board side (and shields are on board side).
This is applied for boards and should then be aslo applied for shields, so applications can rely on the fact that a HW component can be configured the same way, either it is directly available on the board or added via a shield.

To make shield conditional configuration possible:

Get cmake variable SHIELD_AS_LIST (user selection of shields) available in Kconfig
Set a SHIELD_MY_SHIELDX symbol, matching the state of user activation of my_shieldx in boards/shields/my_shieldx/Kconfig.shield
Parse newly created Kconfig.shield in boards/Kconfig (which by the way enable the selection of shields in menuconfig)
Set Kconfig shield configuration in Kconfig.defconfig under feature condition
Parse shields/boards/*.defconfig to get boards secific shields configuration scaling in this new scheme
Convert shields x_nucleo_idb05a1 and wnc_m14a2a to this new scheme. Eventually all shields will be converted as part of this PR if RFC get positive feedback.

EDIT: 11-27-19: Pushed a new version using a more readable Kconfig function (shields_list_contains)

EDIT: 12-10-19: Pushed a new version which updates all shields

TODO:

Complete shields convertion
Update shields documentation

Fixes #17310

zephyrbot · 2019-11-22T10:22:47Z

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

SebastianBoe · 2019-11-22T10:47:41Z

Kconfig.zephyr

@@ -22,6 +22,8 @@ endmenu
 # Note: $ARCH and $BOARD_DIR might be glob patterns.
 source "$(BOARD_DIR)/Kconfig.defconfig"

+source "boards/shields/*/Kconfig.defconfig"


Are the enabled shields known pre-Kconfig?

If so we want to limit the sourced files to just those enabled.

Are the enabled shields known pre-Kconfig?

Yes and no.
When running a west build command this is known... But user can modify shields selection using menuconfig.

If so we want to limit the sourced files to just those enabled.

If we do so (need to figure it out how, though), we prevent the ability to select shields in menuconfig.
So we need to chose between this and build performance (which, I guess, is your point).

But user can modify shields selection using menuconfig.

Never mind then, thank you for the explanation.

dbkinder · 2019-11-22T22:27:05Z

doc/guides/kconfig/index.rst

 For example, we can get the default value for a Kconfig symbol
 from the devicetree.
+See the Python docstrings in ``scripts/kconfig/kconfigfunctions.py`` for more


OK for now, but we can also use the sphinx autodoc capabilities (as done for west).

dbkinder

OK on the docs, but we should take advantage of the sphinx autodoc capabilities (as done with west), to extract and process docstrings.

doc/guides/kconfig/index.rst

scripts/kconfig/kconfigfunctions.py

ulfalizer · 2019-11-22T22:56:43Z

scripts/kconfig/kconfigfunctions.py

@@ -335,6 +335,15 @@ def dt_compat_enabled(kconf, _, compat):
    return "n"


+def cmake_in_list(kconf, _, elem, list):


Could call it in_cmake_list() maybe. I read cmake_in_list as "CMake is in the list" at first and was a bit confused.

I though about it, but finally opted to use cmake as namespace to follow logic used for other dt functions (dt_foo, ..).
Up to you!

How about cmake_list_contains?

scripts/kconfig/kconfigfunctions.py

boards/shields/wnc_m14a2a/boards/nrf52840_pca10056.defconfig

ulfalizer

For def_bool stuff.

erwango · 2019-11-25T16:20:33Z

@ulfalizer, Thanks for the review. I've pushed a new version fixing the points you reported. About:

Could call it in_cmake_list() maybe. I read cmake_in_list as "CMake is in the list" at first and was a bit confused.

I answered, let me know what you prefer.

mbolivar

Looks like a much better way to do this. +1 from me for the approach, nice work!

mbolivar · 2019-11-25T18:56:02Z

scripts/kconfig/kconfigfunctions.py

@@ -335,6 +335,15 @@ def dt_compat_enabled(kconf, _, compat):
    return "n"


+def cmake_in_list(kconf, _, elem, list):


How about cmake_list_contains?

erwango · 2019-11-26T08:31:31Z

@mbolivar, @ulfalizer, thanks for reviews. For the remaining controversial point, what about cmake_shields_list_contains (less generic, but clearer and cleaner) ? So in Kconfig.shields files we'll use :

config SHIELD_WNC_M14A2A
	bool "WNC-M14A2A LTE-M Modem"
	default $(cmake_shields_list_contains,wnc_m14a2a)

We can even drop the cmake and use:

	default $(shields_list_contains,wnc_m14a2a)

mbolivar · 2019-11-26T13:44:38Z

Either cmake_shields_list_contains or shields_list_contains sounds good to me.

Add Kconfig.shield file for x_nucleo_iks01ax shields. Signed-off-by: Erwan Gouriou <[email protected]>

erwango · 2019-12-16T08:09:48Z

@ulfalizer, can you review once more ?

jfischer-no

Thanks, just one comment.

jfischer-no · 2019-12-16T23:51:47Z

boards/shields/waveshare_epaper/Kconfig.defconfig

+	default 250 if SHIELD_WAVESHARE_EPAPER_GDEH0213B72
+
+config LVGL_VER_RES
+	default 128


Should be:

default 128 if SHIELD_WAVESHARE_EPAPER_GDEH029A1 default 120 if SHIELD_WAVESHARE_EPAPER_GDEH0213B1 default 120 if SHIELD_WAVESHARE_EPAPER_GDEH0213B72

Fixed, thanks

Move wavehare epaper shields to conditional Kconfig. As part of this change, direct drivers Kconfig symbols enabling (SPI, SSD16XX) are removed as they are application responsibility. GPIO is removed as well as it should be 'y' by default on all boards according to Default board configuration guidelines. Signed-off-by: Erwan Gouriou <[email protected]>

Factorize definitions for ssd1306_128x32 and sd1306_128x64 shields and move to conditional Kconfig. As part of this change, direct drivers Kconfig symbols enabling (I2C, SSD1306) are removed as they are application responsibility. Also disabling SSD16XX is removed as SSD16XX should not be enabled by default. Signed-off-by: Erwan Gouriou <[email protected]>

Move sparkfun_sara_r4 to conditional Kconfig. As part of this change, remove disco_l475_iot1 specific configuration as it is already part of board default configuration. Signed-off-by: Erwan Gouriou <[email protected]>

Move link_board_eth to conditional Kconfig. Signed-off-by: Erwan Gouriou <[email protected]>

Move link_board_can to conditional Kconfig. As part of this change, some board specific symbols are moved to reel_board specific file. Signed-off-by: Erwan Gouriou <[email protected]>

Add Kconfig.shield to frdm_kw41z. Signed-off-by: Erwan Gouriou <[email protected]>

Move shield to Kconfig.shield format. Signed-off-by: Erwan Gouriou <[email protected]>

Move dfrobot_can_bus_v2_0 to Kcondig.shield foramt and as part of this change introduce nrf52_pca10040 board specific configuration. Signed-off-by: Erwan Gouriou <[email protected]>

Move adafruit_2_8_tft_touch_v2 to Kconfig.defconfig format. As part of this change Kconfig flags SPI and DISPLAY are removed from the shield configuration as they are part of application configuration. Signed-off-by: Erwan Gouriou <[email protected]>

Update doc with new Kconfig.shield files. Signed-off-by: Erwan Gouriou <[email protected]>

erwango · 2019-12-20T08:11:26Z

@ulfalizer, when you have some time, a new review would be welcome

ulfalizer

Some nits.

Did you consider putting the shield stuff in the main board Kconfig.defconfig file by the way (e.g. after a # Shield configuration comment)? It's a bit confusing that we have so many Kconfig files.

ulfalizer · 2019-12-23T14:28:10Z

boards/Kconfig

@@ -23,6 +23,9 @@ source "$(BOARD_DIR)/Kconfig.board"

 endchoice

+# Parse shields references
+# Don't do it as a menuconfig, as shield selection is a CMake feature.
+rsource "shields/*/Kconfig.shield"


FWIW, I prefer source over rsource where rsource isn't needed, because it makes the path easier to grep for.

The globbing kinda breaks grepping already though.

ulfalizer · 2019-12-23T14:34:36Z

boards/shields/frdm_cr20a/Kconfig.defconfig

+# Copyright (c) 2019, NXP
+# Copyright (c) 2019 Linaro Limited
+# SPDX-License-Identifier: Apache-2.0
+#


Inconsistent header. Make it

# Copyright (c) 2019, NXP # Copyright (c) 2019 Linaro Limited # SPDX-License-Identifier: Apache-2.0 ...

ulfalizer · 2019-12-23T14:35:09Z

boards/shields/frdm_cr20a/boards/frdm_k64f.defconfig

+# Copyright (c) 2019 Linaro Limited
+#
+# SPDX-License-Identifier: Apache-2.0
+#


Make it

# Copyright (c) 2019, NXP # Copyright (c) 2019 Linaro Limited # SPDX-License-Identifier: Apache-2.0

ulfalizer · 2019-12-23T14:36:26Z

boards/shields/waveshare_epaper/Kconfig.defconfig

+# Copyright (c) 2019 Linaro Limited
+#
+# SPDX-License-Identifier: Apache-2.0
+#


ulfalizer · 2019-12-23T14:42:47Z

Also wondering if Kconfig.shield could be merged with the main board Kconfig file, to cut down on things a bit there.

SebastianBoe · 2019-12-31T10:58:04Z

Also wondering if Kconfig.shield could be merged with the main board Kconfig file, to cut down on things a bit there.

Wouldn't that require O(boards*shields) snippets of shield code instead of O(shields) ?

Use conditional Kconfig, as introduced by PR zephyrproject-rtos#20934, for ST7789V based shields. Signed-off-by: Jan Van Winkel <[email protected]>

Use conditional Kconfig, as introduced by PR #20934, for ST7789V based shields. Signed-off-by: Jan Van Winkel <[email protected]>

erwango added RFC Request For Comments: want input from the community area: Kconfig area: Shields Shields (add-on boards) labels Nov 22, 2019

erwango added this to the v2.2.0 milestone Nov 22, 2019

erwango requested review from mbolivar, ulfalizer, nashif, SebastianBoe, mike-scott and galak November 22, 2019 10:14

erwango requested a review from dbkinder as a code owner November 22, 2019 10:14

zephyrbot added area: Boards area: Documentation area: Build System labels Nov 22, 2019

erwango force-pushed the shield_defKconfig branch from f6c2e38 to 2feb272 Compare November 22, 2019 10:26

jfischer-no self-requested a review November 22, 2019 10:37

SebastianBoe reviewed Nov 22, 2019

View reviewed changes

SebastianBoe approved these changes Nov 22, 2019

View reviewed changes

dbkinder reviewed Nov 22, 2019

View reviewed changes

dbkinder approved these changes Nov 22, 2019

View reviewed changes

ulfalizer reviewed Nov 22, 2019

View reviewed changes

boards/shields/wnc_m14a2a/boards/nrf52840_pca10056.defconfig Outdated Show resolved Hide resolved

ulfalizer suggested changes Nov 22, 2019

View reviewed changes

erwango force-pushed the shield_defKconfig branch from 2feb272 to 6b560fa Compare November 25, 2019 12:55

mbolivar approved these changes Nov 25, 2019

View reviewed changes

erwango force-pushed the shield_defKconfig branch from 6b560fa to 3afd4ac Compare November 27, 2019 10:16

boards/shields: x_nucleo_iks01ax: move to conditional Kconfig

5d12789

Add Kconfig.shield file for x_nucleo_iks01ax shields. Signed-off-by: Erwan Gouriou <[email protected]>

erwango force-pushed the shield_defKconfig branch from 378bd43 to 31f52d9 Compare December 11, 2019 15:05

vanwinkeljan approved these changes Dec 16, 2019

View reviewed changes

jfischer-no requested changes Dec 16, 2019

View reviewed changes

erwango added 10 commits December 18, 2019 10:35

boards/shields: sparkfun_sara_r4: Move to conditional Kconfig

cd9dfcb

Move sparkfun_sara_r4 to conditional Kconfig. As part of this change, remove disco_l475_iot1 specific configuration as it is already part of board default configuration. Signed-off-by: Erwan Gouriou <[email protected]>

boards/shields: link_board_eth: Move to conditional Kconfig

9d93f02

Move link_board_eth to conditional Kconfig. Signed-off-by: Erwan Gouriou <[email protected]>

boards/shields: link_board_can: Move to conditional Kconfig

e86adc4

Move link_board_can to conditional Kconfig. As part of this change, some board specific symbols are moved to reel_board specific file. Signed-off-by: Erwan Gouriou <[email protected]>

boards/shields: frdm_kw41z: Move to conditional Kconfig

62faa39

Add Kconfig.shield to frdm_kw41z. Signed-off-by: Erwan Gouriou <[email protected]>

boards/shields: fdrm_cr20a: Move to conditional Kconfig

a9620f8

Move shield to Kconfig.shield format. Signed-off-by: Erwan Gouriou <[email protected]>

boards/shields: dfrobot_can_bus_v2_0: Move to Kconfig.shield format

4191c5c

Move dfrobot_can_bus_v2_0 to Kcondig.shield foramt and as part of this change introduce nrf52_pca10040 board specific configuration. Signed-off-by: Erwan Gouriou <[email protected]>

doc/guides: shields: Document shield conditional configuration

a778c21

Update doc with new Kconfig.shield files. Signed-off-by: Erwan Gouriou <[email protected]>

erwango force-pushed the shield_defKconfig branch from 31f52d9 to a778c21 Compare December 18, 2019 09:35

vanwinkeljan mentioned this pull request Dec 18, 2019

Added generic shield for ST7789V display controller #20570

Merged

jfischer-no approved these changes Dec 18, 2019

View reviewed changes

jfischer-no requested a review from ulfalizer December 23, 2019 14:25

ulfalizer approved these changes Dec 23, 2019

View reviewed changes

nashif merged commit 9055cfe into zephyrproject-rtos:master Jan 2, 2020

vanwinkeljan mentioned this pull request Jan 4, 2020

Allow conditional config for ST7789V shields #21697

Merged

nashif pushed a commit that referenced this pull request Jan 6, 2020

boards/shields: st7789v: Use conditional Kconfig

6c5477d

Use conditional Kconfig, as introduced by PR #20934, for ST7789V based shields. Signed-off-by: Jan Van Winkel <[email protected]>

erwango mentioned this pull request May 20, 2020

shields: mikroe_eth_click config should be made conditional #25501

Closed

erwango deleted the shield_defKconfig branch January 27, 2022 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shields: Allow shields conditional configuration #20934

Shields: Allow shields conditional configuration #20934

erwango commented Nov 22, 2019 •

edited

Loading

zephyrbot commented Nov 22, 2019 •

edited

Loading

SebastianBoe Nov 22, 2019

erwango Nov 22, 2019 •

edited

Loading

SebastianBoe Nov 22, 2019

dbkinder Nov 22, 2019

dbkinder left a comment

ulfalizer Nov 22, 2019 •

edited

Loading

erwango Nov 25, 2019

mbolivar Nov 25, 2019

ulfalizer left a comment

erwango commented Nov 25, 2019

mbolivar left a comment

mbolivar Nov 25, 2019

erwango commented Nov 26, 2019 •

edited

Loading

mbolivar commented Nov 26, 2019

erwango commented Dec 16, 2019

jfischer-no left a comment

jfischer-no Dec 16, 2019 •

edited

Loading

erwango Dec 18, 2019

erwango commented Dec 20, 2019

ulfalizer left a comment

ulfalizer Dec 23, 2019

ulfalizer Dec 23, 2019

ulfalizer Dec 23, 2019

ulfalizer Dec 23, 2019

ulfalizer commented Dec 23, 2019

SebastianBoe commented Dec 31, 2019 •

edited

Loading

		@@ -335,6 +335,15 @@ def dt_compat_enabled(kconf, _, compat):
		return "n"


		def cmake_in_list(kconf, _, elem, list):

Shields: Allow shields conditional configuration #20934

Shields: Allow shields conditional configuration #20934

Conversation

erwango commented Nov 22, 2019 • edited Loading

zephyrbot commented Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

erwango Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbkinder left a comment

Choose a reason for hiding this comment

ulfalizer Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulfalizer left a comment

Choose a reason for hiding this comment

erwango commented Nov 25, 2019

mbolivar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erwango commented Nov 26, 2019 • edited Loading

mbolivar commented Nov 26, 2019

erwango commented Dec 16, 2019

jfischer-no left a comment

Choose a reason for hiding this comment

jfischer-no Dec 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erwango commented Dec 20, 2019

ulfalizer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulfalizer commented Dec 23, 2019

SebastianBoe commented Dec 31, 2019 • edited Loading

erwango commented Nov 22, 2019 •

edited

Loading

zephyrbot commented Nov 22, 2019 •

edited

Loading

erwango Nov 22, 2019 •

edited

Loading

ulfalizer Nov 22, 2019 •

edited

Loading

erwango commented Nov 26, 2019 •

edited

Loading

jfischer-no Dec 16, 2019 •

edited

Loading

SebastianBoe commented Dec 31, 2019 •

edited

Loading