periph/timer: fix Kconfig menu title

[gschorcht]

Contribution description

This PR is a very small fix of inconsistent peripheral driver entry in Kconfig for periph/timer.

In Kconfig menu (Top) → Drivers → Peripherals drivers all entries start with the peripheral name in alphabetical order with only one exception, the timer entry. This entry is called Configure timer peripheral driver:

[ ] CPU unique ID
[ ] EEPROM peripheral driver
[ ] Flashpage peripheral driver  ----
[*] GPIO peripheral driver  --->
[ ] HWRNG peripheral driver
[ ] PWM peripheral driver
[*] Power Management (PM) peripheral driver
[*]     Auto initialize Power Management (PM) peripheral
[ ] Quadrature Decoder (QDEC) peripheral driver
[ ] RTC peripheral driver  ----
[ ] SPI peripheral driver  ----
[*] UART peripheral driver  --->
[*] Configure timer peripheral driver  --->

This is confusing and doesn't help to find the right entry. This PR

1. changes the entry to Timer peripheral driver and
2. corrects the alphabetical order.

Testing procedure

Use command

TEST_KCONFIG=1 make -C tests/periph_timer menuconfig

and check the output. in menu (Top) → Drivers → Peripherals drivers. It should be with this PR:

[ ] CPU unique ID
[ ] EEPROM peripheral driver
[ ] Flashpage peripheral driver  ----
[*] GPIO peripheral driver  --->
[ ] HWRNG peripheral driver
[ ] PWM peripheral driver
[*] Power Management (PM) peripheral driver
[*]     Auto initialize Power Management (PM) peripheral
[ ] Quadrature Decoder (QDEC) peripheral driver
[ ] RTC peripheral driver  ----
[ ] SPI peripheral driver  ----
[*] Timer peripheral driver  --->
[*] UART peripheral driver  --->

Issues/PRs references

[gschorcht]

@leandrolanzieri The following question arose in relation to periph/timer's Kconfig file when I tried to move ESP32 specifc timer configuration to cpu/esp32/periph/Kconfig.timer.

Why does the CPU-specific timer configuration depend on KCONFIG_USEMODULE_PERIPH_TIMER here

RIOT/drivers/periph_common/Kconfig.timer

Lines 43 to 46 in c4c50c5

	# Include CPU specific configurations
	if KCONFIG_USEMODULE_PERIPH_TIMER
	osource "$(RIOTCPU)/$(CPU)/periph/Kconfig.timer"
	endif

and when is KCONFIG_USEMODULE_PERIPH_TIMER is enabeld?

It was your suggestion in PR #14848 (comment) to add this whith the note that it isn't a conditional inclusion. But in in fact it seems to be conditional:

  config MODULE_ESP_HW_COUNTER
        bool "Enable hardware counter as low-level timer peripheral"
        depends on HAS_ESP_HW_COUNTER(=y) && MODULE_PERIPH_TIMER(=y) && TEST_KCONFIG(=y) && KCONFIG_USEMODULE_PERIPH_TIMER(=n)

The entry doesn't become visible because KCONFIG_USEMODULE_PERIPH_TIMER is false.

[aabadie]

Not necessarily related to this PR which is focusing on Timer, but the alphabetical order is still wrong: PWM config should be after Power Management config.

[leandrolanzieri]

@leandrolanzieri The following question arose in relation to periph/timer's Kconfig file when I tried to move ESP32 specifc timer configuration to cpu/esp32/periph/Kconfig.timer.

Why does the CPU-specific timer configuration depend on KCONFIG_USEMODULE_PERIPH_TIMER here

This should be used only for configuration symbols (i.e., CFLAGS), not modules. You can find documentation on how we are handling configurations during the transition phase here.

and when is KCONFIG_USEMODULE_PERIPH_TIMER is enabeld?

On this table you can find the reserved prefixes. The KCONFIG_USEMODULE_ symbols are used to enable the configuration of modules when not using TEST_KCONFIG, as they depend on USEMODULE_-prefixed symbols, which are set to y based on the USEMODULE list generated from Makefile.dep.

It was your suggestion in PR #14848 (comment) to add this whith the note that it isn't a conditional inclusion. But in in fact it seems to be conditional:

  config MODULE_ESP_HW_COUNTER
        bool "Enable hardware counter as low-level timer peripheral"
        depends on HAS_ESP_HW_COUNTER(=y) && MODULE_PERIPH_TIMER(=y) && TEST_KCONFIG(=y) && KCONFIG_USEMODULE_PERIPH_TIMER(=n)

The entry doesn't become visible because KCONFIG_USEMODULE_PERIPH_TIMER is false.

Kconfig does not have conditional inclusion (in your case it is clear the file is sourced). When you wrap a source with a condition, the result is that all symbols inside will depend on it. To avoid the issue you are having then, I propose we remove the if around the source, and instead move this dependency to each of the individual files.

[gschorcht]

This should be used only for configuration symbols (i.e., CFLAGS), not modules. You can find documentation on how we are handling configurations during the transition phase here.

Hm, but in case of Kconfig.i2c it works also for modules. We modelled the handling of esp_i2c_hw and esp_i2c_sw exactly in that way. To be honest, it is a bit strange and confusing that a ESP specific timer configuration appears directly in main menu:

    ESP32 specific configurations  --->
[ ] Use hardware counter
    ESP configurations  --->
[ ] Simulate ESP with QEMU
[*] RIOT Core  ----

Therefore, I tried to move the configuration for ESP counter based timers to the (Top) → Drivers → Peripherals drivers menu an renamed the entry title a bit to have the configuration in timers menu and it becomes clear from the title that it is ESP related.

So the question is why do we handle timers configuration in different way than I2C configuration.

On this table you can find the reserved prefixes. The KCONFIG_USEMODULE_ symbols are used to enable the configuration of modules when not using TEST_KCONFIG, as they depend on USEMODULE_-prefixed symbols, which are set to y based on the USEMODULE list generated from Makefile.dep.

Yes, I have seen this table, but I did not really get the description of KCONFIG_USEMODULE_ from this table.

[leandrolanzieri]

This should be used only for configuration symbols (i.e., CFLAGS), not modules. You can find documentation on how we are handling configurations during the transition phase here.

Hm, but in case of Kconfig.i2c it works also for modules. We modelled the handling of esp_i2c_hw and esp_i2c_sw exactly in that way. To be honest, it is a bit strange and confusing that a ESP specific timer configuration appears directly in main menu:

    ESP32 specific configurations  --->
[ ] Use hardware counter
    ESP configurations  --->
[ ] Simulate ESP with QEMU
[*] RIOT Core  ----

Therefore, I tried to move the configuration for ESP counter based timers to the (Top) → Drivers → Peripherals drivers menu an renamed the entry title a bit to have the configuration in timers menu and it becomes clear from the title that it is ESP related.

So the question is why do we handle timers configuration in different way than I2C configuration.

We shouldn't, it's probably something I overlooked. It would be great if you could fix this on this PR by removing the if wrapping the source and moving the dependency to the specific files instead, as I described above.

On this table you can find the reserved prefixes. The KCONFIG_USEMODULE_ symbols are used to enable the configuration of modules when not using TEST_KCONFIG, as they depend on USEMODULE_-prefixed symbols, which are set to y based on the USEMODULE list generated from Makefile.dep.

Yes, I have seen this table, but I did not really get the description of KCONFIG_USEMODULE_ from this table.

The original reason for these symbols, is that when we started migrating configuration symbols some users didn't want to be forced to use Kconfig to configure every module. These symbols are user switches, to enable the particular configuration of a given used module.

[gschorcht]

We shouldn't, it's probably something I overlooked. It would be great if you could fix this on this PR by removing the if wrapping the source and moving the dependency to the specific files instead, as I described above.

OK, I will extend the PR a bit. Would it be ok also to move "Sumulate ESP with QEMU" into the "ESP configurations" menu with this PR? I would rename the PR a bit to something "cleanup ..."

    ESP32 specific configurations  --->
[ ] Use hardware counter
    ESP configurations  --->
[ ] Simulate ESP with QEMU
[*] RIOT Core  ----

[leandrolanzieri]

We shouldn't, it's probably something I overlooked. It would be great if you could fix this on this PR by removing the if wrapping the source and moving the dependency to the specific files instead, as I described above.

OK, I will extend the PR a bit. Would it be ok also to move "Sumulate ESP with QEMU" into the "ESP configurations" menu with this PR? I would rename the PR a bit to something "cleanup ..."

    ESP32 specific configurations  --->
[ ] Use hardware counter
    ESP configurations  --->
[ ] Simulate ESP with QEMU
[*] RIOT Core  ----

Sure!

[gschorcht]

@leandrolanzieri Hm, I'm still not completely satisfied with the way configurations that depend on other configurations are structured in the case of Timer. For example, I have changed the hardware counter selection for ESP32 so that it is displayed in Driver → Peripheral driver below Timer peripheral driver:

[ ] SPI peripheral driver  ----
[ ] Timer peripheral driver  ----
-*- UART peripheral driver  --->

Once it is enabled, hardware counter selction appears. The output changes to

[ ] SPI peripheral driver  ----
[*] Timer peripheral driver  --->
[ ] Enable hardware counter as low-level timer peripheral (NEW)
-*- UART peripheral driver  --->

Since Timer peripheral driver is a menu anyway, wouldn't it be better if this CPU specific timer driver configuration would be part of that menu, or at least indented to make it clear that it is a timer driver configuration? For example:

[ ] SPI peripheral driver  ----
[*] Timer peripheral driver  --->
[ ]     Enable hardware counter as low-level timer peripheral (NEW)
-*- UART peripheral driver  --->

The same we have for EFM32 if xtimer module is used.

[ ] SPI peripheral driver  ----
[*] Timer peripheral driver  --->
[ ] Configure timer peripheral driver  ----
-*- UART peripheral driver  --->

[leandrolanzieri]

Since Timer peripheral driver is a menu anyway, wouldn't it be better if this CPU specific timer driver configuration would be part of that menu, or at least indented to make it clear that it is a timer driver configuration?

This should happen if they are defined right after the symbol. Is your example based on the current state of the PR? It does not render like that to me.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[aabadie]

Looks good and is correctly addressing the initial problem.

ACK

[aabadie]

bors merge

[bors]

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• Murdock
• static-tests
• tools-build-success