samples: drivers: gpio: custom DTS binding demo

[martinjaeger]

For people new to Zephyr it's not obvious how to switch on or off a pin that's not an LED, as there is no generic digital output or input DTS binding.

As this question is asked quite frequently on Slack, this sample tries to help understanding of the concept. It should allow to point people directly to a solution instead of explaining the different steps involved to define custom DTS bindings.

[henrikbrixandersen]

I think this sample can be improved by not using the LED GPIO pin, as using the LED GPIO pin leaves out one important step: Having to setup the pinmux.

[erwango]

This statement depends on platform. On STM32, this is not needed (only required for PMW LED).
But I agree, ideally this sample should not require additional change.

EDIT: that said, having a LED to see the effect of the code is a benefit (more than the console print)

[martinjaeger]

Ah, ok, was not aware that the pinmux setup is necessary for other platforms. Can you suggest a board that requires pinmux setup and that one of you has available for testing? I can add an additional overlay for that. (assuming those other platforms handle pinmux via dts and not with the pinmux.c file as for STM32)

Edit: I suggest to continue using the LED, but still add a (redundant) pinmux configuration to the overlay so that users don't forget it. And I can also mention it in the Readme.

[mnkp]

I think this sample can be improved by not using the LED GPIO pin, as using the LED GPIO pin leaves out one important step: Having to setup the pinmux.

Setting pinmux is currently only required on NXP platform and hopefully this will soon change. As part of the work on supporting pinmux configuration in DTS we want the driver to be responsible for configuring the pins. The same approach is used by Linux.

I propose to leave this part as is.

[mbolivar-nordic]

I appreciate the effort that went into this sample and I think the demonstration of how to make a custom binding is worthwhile, so I am not against merging it at all.

However, it doesn't really address my concern that this is a lot of boilerplate just to define a pin. People have expressed concerns to me that moving things like "use this pin" from Kconfig to DT takes an amount of extra effort that feels unnecessary, compared to just defining a string symbol which is the GPIO device name, and an int symbol which is the pin number.

I tend to agree that this feels like a lot of extra boilerplate. I wonder if there's some way we can propose a convention for miscellaneous application-specific GPIOs for people in a hurry who don't want to go to the trouble of creating a binding just for a pin.

Thanks again for this patch.

[martinjaeger]

Yeah so there are basically 3 options:

1. With custom DTS binding as in this sample.
2. Abuse the existing gpio-leds binding for digital outputs (or gpio-keys for inputs). I guess that's what many people do (like me some time ago).
3. Manually define GPIO device name and pin number as you suggested (either via Kconfig or hardcoded).

I'm still not totally against introducing more generic DTS bindings like gpio-outputs and gpio-inputs in master branch to avoid option 2.

[pabigot]

A recent change to devicetree tooling allowed generation of nodes for which there was no compatible, but that generation did not include any properties on such nodes (which I still think is weird).

Do we have enough special-casing in the tooling to support something like:

/ {
  app {
    power-gpios = <&gpioa 5 GPIO_ACTIVE_HIGH>;
  };
};

then look it up by DT_PATH(app, power_gpios)? At least for GPIO we can reasonably say that if the property ends in -gpios we can assume it's a GPIO phandle+specifier. @galak?

[mbolivar-nordic]

Do we have enough special-casing in the tooling

That doesn't work today:

/* Special property macros: */
#define DT_N_S_app_REG_NUM 0
#define DT_N_S_app_IRQ_NUM 0
#define DT_N_S_app_STATUS_okay 1

/* (No generic property macros) */

But I think it's a pretty interesting idea, actually

[henrikbrixandersen]

I think a sample like this is very instructive in showing how custom GPIO consumer bindings are intended to be described using DTS in Zephyr.

For the "Arduino-like" approach, I really don't see we need to add anything. I mean, there is absolutely nothing wrong with people writing non-portable, out-of-tree code using e.g. device_get_binding("GPIO_1") and hard-coding the pin numbers for a given GPIO. Perhaps we just need to mention that this is indeed possible within the Zephyr documentation - but at the same time mention the benefits of not going down that road?

I'm afraid that even if we add tooling and/or simplified bindings for describing simple GPIOs, we will still be met with questions as to why this needs to be so complicated (even though it does not need to if you do not need the abstractions and portability).

[martinjaeger]

@henrikbrixandersen I still don't completely agree that we should not have generic GPIO pin binding, but we do have generic UARTs and other peripherals predefined that you can reference using an alias. Let me give an example of an actual project:

We've got controller boards in different power levels (and different pin configurations or even different MCUs). All boards have the same basic functions and use the same application firmware. However, you can compile them to use different communication modules, which are attached to a UEXT connector present on each board.

If the GSM comms board is attached, it communicates through UART and requires to use some of the GPIO pins (which could otherwise be used for SPI) for the reset pin and an interrupt input. If we attach a LoRa comms board, the same pins are used for actual SPI peripheral.

We can of course create custom DTS bindings for gsm-board and lora-board which include the entire setup necessary for this board (e.g. UART, reset pin and interrupt pin). This would be the cleanest approach for sure.

However, a more simple approach could be to add some generic GPIO DTS nodes like uext_pin3 and use the DT macros to reference to that pin in the same way as I can already reference to an alias uext-uart to the correct UART peripheral wired to that connector. In contrast to calling device_get_binding("GPIOA"), this would still keep the application portable without requiring to write a custom DTS binding.

[jameswalmsley]

Hi, I think having a basic and simple gpio binding definition would be great.. so that we don't need to overload gpio-keys and gpio-led.

I've added the following yaml file to our code-base, but would be awesome if this just existed:

 # Copyright (c) 2020 James Walmsley
 # SPDX-License-Identifier: Apache-2.0

 description: GPIO PIN parent node

 compatible: "gpio-pin"

 child-binding:
     description: GPIO PIN child node
     properties:
        gpios:
           type: phandle-array
           required: true
        label:
           required: false
           type: string
           description: Human readable string describing the device (used by Zephyr for API name)

Happy to turn this into a proper PR if that would be a more acceptable approach?

[pabigot]

I've added #25945 as another possible solution path. I would really like to be able to provide some basic properties in devicetree that I could fetch via path, without defining a binding for them. This seems to be more consistent with what can be done in Linux.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[martinjaeger]

@pabigot, @mbolivar-nordic, @carlescufi and @henrikbrixandersen: Do you think we should close this PR because the new approach as discussed in #25945 will be used for any custom GPIO in the future?

Otherwise I would try to address the review comments and get this merged soon, as we keep getting this question on Slack.

[pabigot]

I believe the broader solution described in #25945 (comment) is the right path forward. We just need to get resources to make progress on it.

[mbolivar-nordic]

Let's discuss at dev-review. I think there's an argument for merging it as a stopgap measure until we have something better, to make it easier to answer the FAQ.

[galak]

We've now merged #27937 as the means to handle this. Closing this PR now.