Generate DTS 'compatible' based compilation flags

[erwango]

This issue aims at proposing a mechanism that could be used to control code generation aside from dts based code generation.

Basic idea

Generate compilation flags based on device tree 'compatible' property to control drivers compilation

Why this is desirable

Code generation gating was removed from codegen proposal (#6762) as this was not fulfilling buid system expectations. I think a mechanism linked to dts based generation is anyway required (since now removed from pure codegen part).
For example, looking to STM32 I2C drivers (SPI would be another example), we have currently 2 compatibles. Each compatible matches with a variation of the driver made to match different versions of the HW IP.
Driver is made of 4 files:

i2c_ll_stm32_v1.c
i2c_ll_stm32_v2.c
i2c_ll_stm32.c
i2c_ll_stm32.h

Compilation of one or the other variation is today controlled with CONFIG_I2C_STM32_V1/V2 flags:

Besides from implementation options in the files:

	/* Send out messages */
#if defined(CONFIG_I2C_STM32_V1)
	LL_I2C_Enable(i2c);
#endif

it will also control which files are compiled:

zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V1
	i2c_ll_stm32_v1.c
	i2c_ll_stm32.c
	)
zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V2
	i2c_ll_stm32_v2.c
	i2c_ll_stm32.c
	)

This allows to embed driver code dedicated to the exact IP variation (unlike Linux drivers that will embed
full code compatible with all the IPs and behave according to the compatible detected at run time).

So, even if device instantiation information is filled from dts, we still need the information of which variation of one driver should be compiled. And it would make sense this is generated at dts parsing,
so that we generate matching compile flags depending on the detected devices compatibles. And this
would not change during the life of the project.

For instance, for following node:

	i2c2: i2c@40005800 {
		compatible = "st,stm32-i2c-v2";

Following flag would be generated:

#define CONFIG_ST_STM32_I2C_V2

CONFIG_ST_STM32_I2C_V2 replacing current CONFIG_I2C_STM32_V2, for i2c_stm32 driver compilation (to control which files are compiled and also implementation speficics inside driver code.

Also, if several compatibles are declared, several flags are generated:

	flash-controller@40022000 {
		compatible = "st,stm32l4-flash-controller", "st,stm32-flash-controller";
		label = "FLASH_CTRL";

#define CONFIG_ST_STM32L4_FLASH_CONTROLLER
#define CONFIG_ST_STM32_FLASH_CONTROLLER

So generic code could be implemented under CONFIG_ST_STM32_FLASH_CONTROLLER while
L4 specific would lie under CONFIG_ST_STM32L4_FLASH_CONTROLLER.

Naming/Prefix:

Note: I've used CONFIG_ as flag prefix, but it could be DTS_ if using _CONFIG creates confusion.

Dependency with Kconfig settings:

Current driver configuration Kconfig flags depends on generic driver type Kconfig symbols (I2C, USB, SPI, ...)
CONFIG_I2C_STM32_V1/V2 depends on I2C_STM32 depending on I2C symbol.
This way, user could compile out I2C driver by deactivating CONFIG_I2C.
Unless this is no more requested for a good reason, this dependency will have to be reproduced somehow.
Current yaml binding property 'id', which provides the generic type of the driver could be used to bind
generated CONFIG_/DTS_ symbols with high level Kconfig symbols.

To me, this would be quite flexible to use and complementary to what we're trying to do with codegen.
Also, this would clean Kconfig files from various depends on that are spread accross Kconfig files today (for instance here)

Generation of the flags would be done after board dts computation and before compilation (obviously), but could be done before or after code generation by codegen, this is fully independant.

[b0661]

@erwango

Note: I've used CONFIG_ as flag prefix, but it could be DTS_ if using _CONFIG creates confusion.

I think DTS_ is the better choice as this is status information from the DTS. I would expect CONFIG_ to name something a dev can choose on.

By the way 'compatible' is available in codegen/edts:

        "/soc/i2c@40005800": {
            "device-id": "/soc/i2c@40005800",
            "compatible": {
                "0": "st,stm32-i2c-v2"
            },

You may access it within device_declare template by ${compatible/0} or by a get property access to edts in the codegen snippet.

[b0661]

@erwango

There are at least two usage scenarios you are adressing:

1. include a file into compilation based on DTS 'compatible' info

• needs some way to inform Kconfig/CMake about the 'comaptible' info. Could be read from then edts json file.

2. include/exclude source code within a file based on DTS 'compatible' info

• already possible by codegen
• to ease the task the devicedeclare codegen module maybe enhanced by a simple function to produce defines that can be used in the source code. Must be done at the beginning of the driver source code vs. device_declare should be at the end.

Waht scenario is your primary target?

You can get rid of 1) if you always include all files of a driver and do some guarding by the codegen generated #ifdef mechanism. Not really nice.

If you go for 1) the Kconfig vars should be updated to make the available devices known to the Kconfig user configuration process. This should generate the appropriate CONFIG_ for driver file inclusion/exclusion.

[erwango]

@b0661

I intend to replace the use of existing flags such as CONFIG_I2C_STM32_V1 which are currently used
in both your scenari 1 and 2.
It would also help to replace the following flags combinations, if adequate compatible is provided

#if defined(CONFIG_SOC_SERIES_STM32F1X) || defined(CONFIG_SOC_SERIES_STM32F4X) \
	|| defined(CONFIG_SOC_SERIES_STM32F2X)
		/* Clear the interrupt */
		LL_USART_ClearFlag_RXNE(UartInstance);
#endif

You can get rid of 1) if you always include all files of a driver and do some guarding by the codegen generated #ifdef mechanism. Not really nice.

Indeed, I prefer to avoid generating code under ifdef and compile it out afterwards.
We will have some, but it should be limited to real devs Kconfig options.

If you go for 1) the Kconfig vars should be updated to make the available devices known to the Kconfig user configuration process. This should generate the appropriate CONFIG_ for driver file inclusion/exclusion.

Well I guess this is not far away from #7302.
but then, if we're generating DTS_ flags, I don't see why additional appropriate CONFIG_ should be
additionally generated.

About 2)

already possible by codegen

Yes, but I don't want to have codegen bit across the driver core code. To me this should be limited to driver instantiation and configuration. I remind of the "Don't do thing with codegen that could be done by preprocessor"

• to ease the task the devicedeclare codegen module maybe enhanced by a simple function to produce defines that can be used in the source code. Must be done at the beginning of the driver source code vs. device_declare should be at the end

to ease the task the devicedeclare codegen module maybe enhanced by a simple function to produce defines that can be used in the source code. Must be done at the beginning of the driver source code vs. device_declare should be at the end.

I don't want to add too much complexity. If 1) and 2) could be solved by the same set of flags (as done today with CONFIG flag) let's go with this.

Things could be done in a different ways for sure. Though I think this solution is simple enough to
be intuitive and don't require too much effort for a newcomer to understand the mechanism:

• a list fo flags is generated based on compatibles defined in board dts.
• flags could be used as CONFIG_ flags for driver implementation
  We already have generated CONFIG_ flags used directly in code, so I think this should even work today.
  If DTS_ prefix is used, there will be additional intrications, but I guess this is part of DTS metadata should be made available to Kconfig #7302 already.

[b0661]

@erwango

but then, if we're generating DTS_ flags, I don't see why additional appropriate CONFIG_ should be
additionally generated.

DTS_xxxx describes a hardware characteristic. CONFIG_xxxx describes a choice made by the dev/ user.

[b0661]

@erwango

I don't want to add too much complexity. If 1) and 2) could be solved by the same set of flags (as done today with CONFIG flag) let's go with this.

You may add the compatible defines definition in scripts/extract/compatible.py the usual way.

[ulfalizer]

Hello,

It would be nice if some other prefix than CONFIG_ could be used, so that Kconfig settings can be distinguished from other settings. That makes it easy to derive what Kconfig symbols source files depend on, and to hunt down references to undefined symbols.

Don't know how it's set up at the moment, but what about DTS_ or the like? Pretty nice and explicit.

[erwango]

Flags dependency was missing in the initial idea.
Issue description edited with the following:

Dependency with Kconfig settings:

Current driver configuration Kconfig flags depends on generic driver type Kconfig symbols (I2C, USB, SPI, ...)
CONFIG_I2C_STM32_V1/V2 depends on I2C_STM32 depending on I2C symbol.
This way, user could compile out I2C driver by deactivating CONFIG_I2C.
Unless this is no more requested for a good reason, this dependency will have to be reproduced somehow.
Current yaml binding property 'id', which provides the generic type of the driver could be used to bind
generated CONFIG_/DTS_ symbols with high level Kconfig symbols.

[erwango]

@b0661 , @ulfalizer

Thanks for feedbacks and hints.
Cf upper comment for a missing point that I just added.

Generated DTS_ flags will have to keep the existing dependency links with existing Kconfig generic symbols (I2C, SPI, USB, ..).
So I think that besides a pure list of #define we'll need to build a Kconfig file with the following assertions:

config ST_STM32_I2C_V2
      depends on I2C

Or an equivalent definitions that allow to reproduce the depedency, a 'C' equivalent being:

#ifdef CONFIG_I2C
#define DTS_ST_STM32_I2C_V2
#endif /* CONFIG_I2C */

Generation should be equivalent in complexity, but this second version should be easier to integrate in build system, and allows the use of DTS_ prefix.

[erwango]

@carlescufi , @SebastianBoe , this proposal is one possible solution to a) in #7302 (comment)

[erwango]

@carlescufi, if you have some cycles, your feedback would be welcome.

[carlescufi]

I think the proposal here, essentially going back to using DT to decide which driver is built, which was, as @erwango mentions, one of the points I made in here. The reason I think this proposal and @erwango's currently suggested solution makes sense is that it is very unlikely that users will want to select a particular driver for a hardware resource in Kconfig. They might want to enable or disable at the Kconfig level, but not select which driver they want. I think it's much more logical, as this solution proposes, to define this at the DTS level, and the proposal here seems sound to me.

A couple of tidbits:

1. Definitely use DTS_ instead of CONFIG_
2. For the case where more than one software driver implementation exists, I think it is reasonable to expect users to select the one they want via compatible and .dts files instead of Kconfig.
3. We need wider agreement here. I am paging @SebastianBoe @galak, @anangl, @Mierunski so that they can also express their opinion.

All in all this sounds good to me @erwango

[erwango]

Implementation of the current feature could use the edts database proposed in #9876

Generated edts.json file contains, among others, following structures:

'compatibles': dict(compatible : sorted list(device-id)),
'device-types': dict(device-types : sorted list(compatible)),

This should be sufficient to generate the list of DTS_ flags and their dependencies
on Zephyr generic device types (USB, SPI, ..)

[galak]

Is this resolved with merging of #9974

[erwango]

@galak

Is this resolved with merging of #9974

Not entirely:

• Use of DTS_ namespace was requested
• We should be able to establish dependency relationship between zephyr Kconfig drivers types (CONFIG_SPI, CONFIG_I2C, ...) and board supported DTS_COMPAT_XXX, so we can control matching drivers compilation thanks to Kconfig symbols like we're able to do today.
• In the other hand same Kconfig flags should ideally be visible to the user only if matching HW is available on board

[erwango]

Once edts database generation is available, I propose to implement the following approach:

In edts, we'll get the following struct:

    "device-types": {
        "uart": [
            "st,stm32-uart",
            "st,stm32-usart"
        ],
        "spi": [
            "st,stm32-spi"
        ],

We should then be able to generate the following:

#ifdef CONFIG_UART
#define DTS_COMPAT_ST_STM32_UART
#define DTS_COMPAT_ST_STM32_USART
#endif /* CONFIG_UART */

#ifdef CONFIG_SPI
#define DTS_COMPAT_ST_STM32_SPI
#endif /* CONFIG_SPI */

Then, we'll also need to provide default settings to Kconfig for UART and SPI:

config UART
       default y

config SPI
       default y

This should be provided in a zephyr/generated/config/board.defconfig that should be parsed by Kconfig system so we can input information (extracted from device tree) that SPI and UART are available on HW.

@ulfalizer @SebastianBoe : any feedback on this proposition?

[carlescufi]

from: #11180
@carlescufi

So in the future we will define:

.h file:
#define DT_COMPAT_STM32_SPI 1
.conf file:
CONFIG_DT_COMPAT_STM32_SPI=y

Then, how will we link both ?

Well, my hope is that #define DT_COMPAT_STM32_SPI won't be needed at all and we'll be able to remove it. Instead of generating that macro, we will enable the Kconfig option CONFIG_DT_COMPAT_STM32_SPI in a generated fragment (.conf). If the code needs to check for that compatible, then Kconfig will have generated #define CONFIG_DT_COMPAT_STM32_SPI 1 as it does for all Kconfig options.

[erwango]

Output of discussion on slack https://zephyrproject.slack.com/archives/C18PLHN8H/p1541680753156800

Proposal:

Do not generate #define DT_COMPAT or #define CONFIG_DT_COMPAT

Depending on node 'compatible' field:

if it's ON, we write the line CONFIG_DT_COMPAT_X=y in the generated .conf.
Then Kconfig will generated #define CONFIG_DT_COMPAT_X 1 in autoconf.h.

If it's OFF, we do not write CONFIG_DT_COMPAT_X=y in the generated conf.
Then Kconfig will not generate the #define

[carlescufi]

@galak @erwango can you please update this issue after the recent merges?

[ioannisg]

@erwango @galak is this going to be completed for 2.0.0?

[galak]

@erwango @galak is this going to be completed for 2.0.0?

I doubt it.

[mbolivar-nordic]

Is this issue able to cover adding support for something like this in CMake?

zephyr_library_sources_if_dt_compat_enabled(nordic,nrf-spim spi_nrfx_spim.c)

We have confusion from the users about how compatibles bind to drivers and why grep doesn't work.

[galak]

Is this issue able to cover adding support for something like this in CMake?

zephyr_library_sources_if_dt_compat_enabled(nordic,nrf-spim spi_nrfx_spim.c)

We have confusion from the users about how compatibles bind to drivers and why grep doesn't work.

I chatted with @tejlmand about doing something like this. I hacked up something that would take the nordic,nrf-spim do a CMAKE regex to convert it to nordic_nrf_spim and do a -DDT_DRV_COMPAT=nordic_nrf_spim when we build spi_nrfx_spim.c

[mbolivar-nordic]

@erwango do you still think this is needed?

Thinking about this a bit more, Kconfig options whose values can be determined from the devicetree can do this with the features in kconfigfunctions.py.

For instance, for following node:

	i2c2: i2c@40005800 {
		compatible = "st,stm32-i2c-v2";

Following flag would be generated:

#define CONFIG_ST_STM32_I2C_V2

You can do this with:

DT_COMPAT_ST_STM32_I2C_V2 := st,stm32-i2c-v2
config ST_STM32_I2C_V2
	def_bool $(dt_compat_enabled,$(DT_COMPAT_ST_STM32_I2C_V2))

Any other features we need can similarly be implemented with kconfigfunctions. Do you have any features covered in this issue that are not already supported in this way?

[erwango]

@mbolivar

You can do this with:

This is exactly what we're doing right now :-)
While this is not as nice as I had though it could have been, this is working indeed.

I think this point can be closed and new requests should be made on the new base we have today.