Zephyr: RFC: Enable device by using dts, not Kconfig

Created on 16 Oct 2018  路  7Comments  路  Source: zephyrproject-rtos/zephyr

The most drivers use Kconfig to select which device compiled and which not currently. eg:
(CONFIG_GPIO_STM32_PORTA is a variable defined in Kconfig)

#ifdef CONFIG_GPIO_STM32_PORTA
GPIO_DEVICE_INIT_STM32(a, A);
#endif /* CONFIG_GPIO_STM32_PORTA */

#ifdef CONFIG_GPIO_STM32_PORTB
GPIO_DEVICE_INIT_STM32(b, B);
#endif /* CONFIG_GPIO_STM32_PORTB */

#ifdef CONFIG_GPIO_STM32_PORTC
GPIO_DEVICE_INIT_STM32(c, C);
#endif /* CONFIG_GPIO_STM32_PORTC */

And the 'status = "ok"' in dts seems useless because the drivers doesn't need the defines generated from dts. Drivers use the peripheral's base address, irq number and other informations provide by HAL.

#define GPIO_DEVICE_INIT_STM32(__suffix, __SUFFIX)          \
    GPIO_DEVICE_INIT("GPIO" #__SUFFIX, __suffix,            \
             GPIO##__SUFFIX##_BASE, STM32_PORT##__SUFFIX,   \
             LL_APB2_GRP1_PERIPH_AFIO |         \
             STM32_PERIPH_GPIO##__SUFFIX,           \
             STM32_CLOCK_BUS_GPIO)

In the above code, GPIOA_BASE,_STM32_PERIPH_GPIOA... are provided by stm32cube.

How about replace this informations by using the marcos generated from dts? Remove all Kconfig variables such as CONFIG_GPIO_STM32_PORTB, use the peripheral base address generated from dts to determined which device should compiled.

Here is a example in drivers/spi/spi-sam.c:

#define SPI_SAM_DEFINE_CONFIG(n)                    \
    static const struct spi_sam_config spi_sam_config_##n = {   \
        .regs = (Spi *)CONFIG_SPI_##n##_BASE_ADDRESS,       \
        .periph_id = CONFIG_SPI_##n##_PERIPHERAL_ID,        \
        .pins = PINS_SPI##n,                    \
    }

#define SPI_SAM_DEVICE_INIT(n)                      \
    SPI_SAM_DEFINE_CONFIG(n);                   \
    static struct spi_sam_data spi_sam_dev_data_##n = {     \
        SPI_CONTEXT_INIT_LOCK(spi_sam_dev_data_##n, ctx),   \
        SPI_CONTEXT_INIT_SYNC(spi_sam_dev_data_##n, ctx),   \
    };                              \
    DEVICE_AND_API_INIT(spi_sam_##n,                \
                CONFIG_SPI_##n##_NAME,          \
                &spi_sam_init, &spi_sam_dev_data_##n,   \
                &spi_sam_config_##n, POST_KERNEL,       \
                CONFIG_SPI_INIT_PRIORITY, &spi_sam_driver_api)

#if CONFIG_SPI_0_BASE_ADDRESS
SPI_SAM_DEVICE_INIT(0);
#endif

#if CONFIG_SPI_1_BASE_ADDRESS
SPI_SAM_DEVICE_INIT(1);
#endif

So we don't need select which spi port we used in Kconfig, can use dts select spi port directly.

&spi0 {
    status = "ok";
};
Enhancement Devicetree

Most helpful comment

I would definitely go for 3) as well.

I would like to stress, however, that it must still be possible to disable a driver (not its instances, those should be handled through devicetree) through Kconfig.

All 7 comments

Hi @qianfan-Zhao

Thanks for this proposal. Indeed STM32 GPIO driver should be moved to dts information usage.
I've raised https://github.com/zephyrproject-rtos/zephyr/issues/10629 to follow the change. You're welcome to do it if interested.

About using using dts to select instances to activate, there has been discussions about that (you could check issue https://github.com/zephyrproject-rtos/zephyr/issues/8499 which lists all the open points) already and for now decision is to keep it as Kconfig responsibility, mainly because there there is a UI available for Kconfig (menuconfig) and not for dts.

I'm reviving this old issue to continue discussion on how to handle per-instance Kconfigs.

I can see the following options:

  1. Keep per-instance Kconfig options. Do not do any devicetree integration at all.
  2. Keep per-instance Kconfig options whenever an SoC driver maintainer wants them. These should default to 'y' if the right node label is enabled. We cannot use DT_INST instance numbers here because their relationship to nodes is undefined. This also likely means the device driver will not use DT_INST APIs unless we do something more clever to establish this relationship.
  3. Remove per-instance Kconfig options as a general policy; let node status properties control which device instances get created.

Thoughts on these? Did I miss anything?

Edit: option 3. also relates to https://github.com/zephyrproject-rtos/zephyr/issues/19448 (which in turn relates to https://github.com/zephyrproject-rtos/zephyr/issues/24318). I'm being intentionally vague when I say "let node status properties control" -- we need to consider if going with 3. requires refinements to the way status works.

cc @carlescufi @MaureenHelm @henrikbrixandersen @erwango @mnkp @galak @anangl @pabigot @nashif

I feel like the ship has sailed on #1. I'd argue that #2 should not be allowed and that we are moving towards #3 already.

For example - here's a rough idea for serial drivers, the majority of cases that are under 15 lines are just singletons to enable/disable the driver.

[galak@spiff zephyr]$ wc -l drivers/serial/Kconfig*
  128 drivers/serial/Kconfig
    8 drivers/serial/Kconfig.altera_jtag
   27 drivers/serial/Kconfig.cc13xx_cc26xx
   10 drivers/serial/Kconfig.cc32xx
   12 drivers/serial/Kconfig.cmsdk_apb
   11 drivers/serial/Kconfig.esp32
   14 drivers/serial/Kconfig.gecko
   13 drivers/serial/Kconfig.imx
   14 drivers/serial/Kconfig.leuart_gecko
   13 drivers/serial/Kconfig.litex
   12 drivers/serial/Kconfig.mcux
   18 drivers/serial/Kconfig.mcux_flexcomm
   18 drivers/serial/Kconfig.mcux_lpsci
   12 drivers/serial/Kconfig.mcux_lpuart
   11 drivers/serial/Kconfig.miv
   10 drivers/serial/Kconfig.msp432p4xx
   70 drivers/serial/Kconfig.native_posix
  353 drivers/serial/Kconfig.nrfx
   42 drivers/serial/Kconfig.ns16550
    7 drivers/serial/Kconfig.nsim
   23 drivers/serial/Kconfig.pl011
   23 drivers/serial/Kconfig.psoc6
  104 drivers/serial/Kconfig.rtt
   39 drivers/serial/Kconfig.rv32m1_lpuart
   19 drivers/serial/Kconfig.sam0
   72 drivers/serial/Kconfig.sifive
   39 drivers/serial/Kconfig.stellaris
   14 drivers/serial/Kconfig.stm32
   13 drivers/serial/Kconfig.uart_sam
   12 drivers/serial/Kconfig.usart_sam
   12 drivers/serial/Kconfig.xlnx

I would definitely go for 3) as well.

I would like to stress, however, that it must still be possible to disable a driver (not its instances, those should be handled through devicetree) through Kconfig.

I would like to stress, however, that it must still be possible to disable a driver (not its instances, those should be handled through devicetree) through Kconfig.

+1

I would definitely go for 3) as well.

I would like to stress, however, that it must still be possible to disable a driver (not its instances, those should be handled through devicetree) through Kconfig.

+1

DEV-REVIEW: In general no issues with the general direction here (option #3), need to deal with some issues associated with mis-matched configuration between removal of Kconfig and what dts enables, also some power mgmt issues in that we might enable things sub-optimally now (we can look to use reference count here as well as warning checks for various controllers enabled w/o any deps).

@mbolivar-nordic to add general policy to DTS docs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rosterloh picture rosterloh  路  4Comments

Nukersson picture Nukersson  路  4Comments

DeltaEvo picture DeltaEvo  路  4Comments

snej picture snej  路  5Comments

karstenkoenig picture karstenkoenig  路  4Comments