Skip to content

samples: drivers: gpio: custom DTS binding demo #25596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

samples: drivers: gpio: custom DTS binding demo #25596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 3.13.1)

find_package(Zephyr HINTS $ENV{ZEPHYR_BASE})
project(gpio_custom_dts_binding)

target_sources(app PRIVATE src/main.c)
62 changes: 62 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
.. _gpio-custom-dts-binding-sample:

GPIO with custom Devicetree binding
###################################

Overview
********

In Zephyr, all hardware-specific configuration is described in the devicetree.

Consequently, also GPIO pins are configured in the devicetree and assigned to
a specific purpose using a compatible.

This is in contrast to other embedded environments like Arduino, where e.g.
the direction (input / output) of a GPIO pin is configured in the application
firmware.

For typical use cases like LEDs or buttons, the existing ``gpio-leds`` or
``gpio-keys`` compatibles can be used.

This sample demonstrates how to use a GPIO pin for other purposes with a
custom dts binding.

We assume that a load with high current demands should be switched on or off
via a MOSFET. The custom DTS binding for the power output controlled via a
GPIO pin is specified in the file ``dts/bindings/power-output.yaml``. The gate
driver for the MOSFET would be connected to the pin as specified in the
``.overlay`` file in the boards folder.

Building and Running
********************

For each board that should be supported, a ``.overlay`` file has to be defined
in the ``boards`` subfolder.

Building and Running for ST Nucleo L073RZ
=========================================
The sample can be built and executed for the
:ref:`nucleo_l073rz_board` as follows:

.. zephyr-app-commands::
:zephyr-app: samples/drivers/gpio/custom_dts_binding
:board: nucleo_l073rz
:goals: build flash
:compact:

For demonstration purposes, we use the GPIO pin of the built-in LED.
Copy link
Member

@henrikbrixandersen henrikbrixandersen May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@erwango erwango May 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

@martinjaeger martinjaeger May 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@mnkp mnkp Jun 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


To build for another board, change "nucleo_l073rz" above to that board's name.

Sample output
=============

The GPIO pin should be switched to active level after one second.

The following output is printed (pin number and port may differ):

.. code-block:: console

Initializing pin 5 on port GPIOA with inactive level.
Waiting one second.
Setting pin to active level.
13 changes: 13 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/boards/nucleo_l073rz.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright (c) 2020 Martin Jäger / Libre Solar
*
* SPDX-License-Identifier: Apache-2.0
*/

/ {
load_switch: load_switch {
compatible = "power-output";
/* using built-in LED pin for demonstration */
gpios = <&gpioa 5 GPIO_ACTIVE_HIGH>;
};
};
11 changes: 11 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/dts/bindings/power-output.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright (c) 2020 Martin Jäger / Libre Solar
# SPDX-License-Identifier: Apache-2.0

description: GPIO pin to switch a power output on or off
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@martinjaeger martinjaeger May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@pabigot pabigot May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@henrikbrixandersen henrikbrixandersen May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


compatible: "power-output"

properties:
gpios:
type: phandle-array
required: true
1 change: 1 addition & 0 deletions samples/drivers/gpio/custom_dts_binding/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CONFIG_GPIO=y
14 changes: 14 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
sample:
name: GPIO driver sample with custom DTS binding
tests:
sample.drivers.gpio.custom_dts_binding:
tags: GPIO
platform_whitelist: nucleo_l073rz
depends_on: gpio
harness: console
harness_config:
type: multi_line
regex:
- "Initializing pin ([0-9]*) on port (.*) with inactive level."
- "Waiting one second."
- "Setting pin to active level."
38 changes: 38 additions & 0 deletions samples/drivers/gpio/custom_dts_binding/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2020 Libre Solar Technologies GmbH
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr.h>
#include <sys/printk.h>
#include <drivers/gpio.h>

#define LOAD_SWITCH DT_NODELABEL(load_switch)

#if DT_NODE_EXISTS(LOAD_SWITCH)
#define LOAD_SWITCH_PORT DT_GPIO_LABEL(LOAD_SWITCH, gpios)
#define LOAD_SWITCH_PIN DT_GPIO_PIN(LOAD_SWITCH, gpios)
#define LOAD_SWITCH_FLAGS DT_GPIO_FLAGS(LOAD_SWITCH, gpios)
#else
#error "Overlay for power output node not properly defined."
#endif

void main(void)
{
struct device *switch_dev = device_get_binding(LOAD_SWITCH_PORT);

printk("Initializing pin %d on port %s with inactive level.\n",
LOAD_SWITCH_PIN, LOAD_SWITCH_PORT);

gpio_pin_configure(switch_dev, LOAD_SWITCH_PIN,
LOAD_SWITCH_FLAGS | GPIO_OUTPUT_INACTIVE);

printk("Waiting one second.\n");

k_sleep(K_MSEC(1000));

printk("Setting pin to active level.\n");

gpio_pin_set(switch_dev, LOAD_SWITCH_PIN, 1);
}