boards/esp32-wt32-sc01-plus: fix I2C driver selection in Kconfig #19945
Conversation
cpu/esp32/periph/Kconfig.i2c
Outdated
| @@ -10,6 +10,7 @@ if TEST_KCONFIG | |||
| choice ESP32_I2C_IMPLEMENTATION | |||
| bool "I2C implementation" | |||
| depends on MODULE_PERIPH_I2C | |||
| default MODULE_ESP_I2C_HW if BOARD_ESP32S3_WT32_SC01_PLUS | |||
There was a problem hiding this comment.
It looks strange to have a board related dependency resolution at cpu level. I would rather add the following in boards/esp32s3_wt32_sc01_plus/Kconfig:
select MODULE_ESP_I2C_HW if MODULE_PERIPH_I2C
@MrKevinWeiss, any opinion?
There was a problem hiding this comment.
It looks strange to have a board related dependency resolution at cpu level.
Agreed and I tried it exactly in that way but it threw an error:
=== [ATTENTION] Testing Kconfig dependency modelling ===
[genconfig.py]:ERROR-Treating Kconfig warnings as errors:
[genconfig.py]:ERROR-=> warning: the choice symbol MODULE_ESP_I2C_HW (defined at cpu/esp32/periph/Kconfig.i2c:21) is selected by the following symbols, but select/imply has no effect on choice symbols
- BOARD_ESP32S3_WT32_SC01_PLUS (defined at boards/esp32s3-wt32-sc01-plus/Kconfig:11)
There was a problem hiding this comment.
Would it be a possible approach to define a symbol BOARD_REQUIRES_ESP_I2C_HW which selects MODULE_ESP_I2C_HW as default for the ESP32_I2C_IMPLEMENTATION and to set it by that board?
There was a problem hiding this comment.
In this case you should actually use a default choice override in the board.
You cannot select choice options directly in kconfig
in boards/esp32s3_wt32_sc01_plus/Kconfig:
choice ESP32_I2C_IMPLEMENTATION
default MODULE_ESP_I2C_HW
There was a problem hiding this comment.
boards/common/esp32x/Kconfig:
+ config BOARD_REQUIRES_ESP_I2C_HW
+ bool
+ help
+ Indicates that the board requires the I2C hardware implementation.cpu/esp32/periph/Kconfig:
choice ESP32_I2C_IMPLEMENTATION
bool "I2C implementation"
depends on MODULE_PERIPH_I2C
+ default MODULE_ESP_I2C_HW if BOARD_REQUIRES_ESP_I2C_HWboards/esp32s3-wt32-sc01-plus/Kconfig:
config BOARD_ESP32S3_WT32_SC01_PLUS
bool
default y
+ select BOARD_REQUIRES_ESP_I2C_HW|
Maybe it is time to change the default I2C driver for the ESP32-S3 to the hardware driver, which had some problems on the ESP32 but seems to work fine on the ESP32-S3. It would have to be tested a bit more. |
cpu/esp32/periph/Kconfig.i2c
Outdated
| @@ -10,6 +10,7 @@ if TEST_KCONFIG | |||
| choice ESP32_I2C_IMPLEMENTATION | |||
| bool "I2C implementation" | |||
| depends on MODULE_PERIPH_I2C | |||
| default MODULE_ESP_I2C_HW if BOARD_ESP32S3_WT32_SC01_PLUS | |||
There was a problem hiding this comment.
In this case you should actually use a default choice override in the board.
You cannot select choice options directly in kconfig
in boards/esp32s3_wt32_sc01_plus/Kconfig:
choice ESP32_I2C_IMPLEMENTATION
default MODULE_ESP_I2C_HW
7b50f2d to
282873c
Compare
Ok, I did it in that way. I squashed it directly. |
282873c to
7a94c74
Compare
|
bors merge |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page.
|
|
Thanks for pointing out how it has to be realized and merging it. |
Contribution description
This PR fixes the
Kconfigmismatch for the I2C peripheral driver selection for theesp32s3-wt32-sc01-plusboard.Testing procedure
fails w/o this PR
and should succeed with this PR
Issues/PRs references