[POC/RFC/WIP] periph/gpio: Proposal for the change of the GPIO API

[gschorcht]

Contribution description

Based on the discussion and the ideas of @maribu and @kaspar030 discussed in PR #12333, this PR provides a proposal and a proof of concept for a change to the GPIO API. The design goals of the change are:

1. The API should allow access to the GPIO pins of any type of GPIO hardware components. These are in particular the GPIO ports of the MCU as well as of GPIO expander devices.
2. The definition and the access to the GPIO pins should always be the same, regardless of the type of GPIO hardware component. This means that the pins are always defined with the macro GPIO_PIN(port, pin). The access is always done with the functions gpio_*.
3. The implementation of the access to the pins of a GPIO port should be hardware-oriented, i.e. port-oriented and not pin-oriented. This makes it possible to set/clear a number of GPIOs with only one hardware access.
4. The changes of the API should not require any changes to existing applications.

For that purpose, the GPIO API is divided into a low-level API and a high-level API:

1. The low-level API provides functions for port-oriented access to the GPIO pins. It must be implemented by any hardware component that provides GPIO pins. The port-oriented functions of the low-level API are

   read(const gpio_port_t *port)
   set(const gpio_port_t *port, gpio_mask_t pins)
   clear(const gpio_port_t *port, gpio_mask_t pins)
   toggle(const gpio_port_t *port, gpio_mask_t pins)
   write(const gpio_port_t *port, gpio_mask_t values)
   

   The functions of the low-level API are used via a gpio_driver_t type driver. This driver defines the interfaces of the low-level functions and contains references to these functions of the respective hardware component. It is thus used for mapping the low-level function calls to the implementation of the hardware component.

   The low-level API implementation of the MCU has to be done in the functions gpio_cpu_*. For performance reasons these functions are used to access the GPIO ports of the MCU via a special gpio_driver_t instance, the gpio_cpu_driver driver.

2. The high-level API is the previous GPIO API and remains unchanged. It is intended for use by the application and allows pin-oriented access to the GPIO pins.

   However, the implementation of the high-level API is no longer realized directly by the MCU. Rather, the functions of the high-level API are implemented as static inline functions that use the functions of the low-level API of the respective GPIO hardware component.

Implementation Details

A GPIO pin of type gpio_t is now defined by a GPIO port of type gpio_port_t and a pin number. gpio_t can't be overridden anymore.

The GPIO port of type gpio_port_t can be either

• a register address of MCU-GPIO ports that is used directly by the low-level functions of the MCU or
• a pointer to a device data structure of type gpio_dev_t which defines any GPIO hardware component, e.g. a GPIO expander device.

The device data structure of type gpio_dev_t contains

• a device descriptor which holds the state and the parameters of a GPIO device, as well as
• an associated driver of type gpio_driver_t for using the device where the driver which maps the function calls of the low-level API to the implementation of the GPIO device.

In case of MCU GPIO ports, the port defines just the register address. The mapping to the functions of the low-level API is realized by the special driver instance gpio_cpu_driver which is created automatically from the MCU low-level functions gpio_cpu_*.

Port-oriented functions of low-level API

read(const gpio_port_t *port)
set(const gpio_port_t *port, gpio_mask_t pins)
clear(const gpio_port_t *port, gpio_mask_t pins)
toggle(const gpio_port_t *port, gpio_mask_t pins)
write(const gpio_port_t *port, gpio_mask_t values)

use a mask or multiple bits that represent the pins of a GPIO port. The width of this mask has to be the maximum width of all different GPIO ports used in the system. For this purpose, each component that provides GPIO ports (MCUs and GPIO device drivers) must enable the corresponding pseudo module that specifies the width of its GPIO ports: gpio_mask_8bit, gpio_mask_16bit, gpio_mask_32bit.

Due to the structured GPIO data type, the comfort functions gpio_is_equal and gpio_is_undef are introduced to check whether GPIOs are equal or undefined.

MCU changes

The following changes are required for each MCU:

• The MCU has to implement the low-level functions gpio_cpu_* according to the interface defined in gpio_driver_t. In most cases, only a very small number of changes of existing MCU implementations are required.
• Depending on the width of the MCU GPIO ports, the pseudomodule gpio_mask_8bit, gpio_mask_16bit or gpio_mask_32bit has to be enabled.
• The MCU has to provide platform specific definitions for GPIO handling in a new file cpu/*/include/gpio_arch.h. These are:
  • GPIO_CPU_REG_MASK as the part of a given address that uniquely identifies the address as a register address.
  • GPIO_CPU_REG_GPIO that identifies an address masked with GPIO_CPU_REG_MASK as GPIO register address.
    See for example the file cpu/stm32_common/include/gpio_arch.h.
• The MCU peripheral configuration in cpu/*/include/periph_cpu.h must define
  • a comma-separated list of MCU port register addresses called GPIO_CPU_PORTS. The index of the port with the register address must match the defined ordinal number of the port. This Definition is used to generate the port list gpio_ports[].
  • PORT_EXT as last definition in the port enumeration type. This will make possible to use port names for GPIO expander devices.
    See for example the file cpu/stm32l0/include/periph_cpu.h.

All defined ports are stored in array gpio_ports.
The MCU and GPIO expander drivers have to implement function according to the prototypes defined as members of the GPIO device driver gpio_driver_t.

The PR provides the changes for STM32 and ATmega MCUs as examples. The changes of all peripheral drivers of these MCUs are already changed an tested.

Board Changes

Many boards should work without changes. Only boards that use direct register access for GPIOs, e.g. to handle the LEDs, have to be changed.

Driver Changes

Many drivers should work without changes. Only drivers that compare a pin directly to GPIO_UNDEF need to be changed. The comparison has to be replaced by the function gpio_is_undef.

Testing procedure

Sinec the PR provides the changes for STM32 and ATmega MCUs as examples, Nucleo-64 boards and ATmega boards can be used to try the concept, for example:

make BOARD=nucleo-f411re -C tests/periph_gpio
USEMODULE=extend_gpio make BOARD=nucleo-f411re -C tests/periph_gpio

The latter one enables the possibility to use GPIO expander devices with small code size increase and small performance decrease.

Issues/PRs references

Initial ideas of a GPIO extension API were discussed in issue #9690 and PRs #9860 and #9958.
An updated version of a GPIO extension API was provided and discussed in #12333.
With the availability of a GPIO extension API, the drivers in PRs #10430 and #10688 could used.

[benpicco]

This looks pretty solid. I especially like how you made the compiler be able to optimize most of it away in the common case :)

I see that updating all the ports & drivers will be a larger undertaking, so is there some fall-back possible so we don't have to update everything at once?

[kaspar030]

This looks pretty solid. I especially like how you made the compiler be able to optimize most of it away in the common case :)

Yeah! +100. And @gschorcht, thanks for your persistence and patience!!

[maribu]

OK. Lets sum up the pros and cons:

• 👍👍👍 this finally allows to use GPIOs regardless of whether the GPIOs a peripheral or external ones
• 👍 this provides access to whole GPIO ports, a feature lacking in the current GPIO API
• 👍 the internal API reflects the inner workings of most GPIO hardware very well, which eases porting RIOT to other devices
• 👍 the GPIO pin API is now shared code implemented on top of the GPIO port access API, which allows to share code
• 👎 the implementation increases ROM size
  • The increased ROM size is due to the additional features provided and justified by them
• 👎 the time to access GPIO pins using the GPIO pin API without LTO is a bit slow
  • With LTO this is likely no longer the case
  • Using the GPIO port API directly is at least as fast and sometimes faster then the current GPIO API; the very few uses cases requiring fast GPIO access can use this API instead
  • A high level GPIO API is objectively not the right tool for "as fast as possible" GPIO access

So to me, the pros clearly outweight the cons and this is the route to go.

[maribu]

I see that updating all the ports & drivers will be a larger undertaking, so is there some fall-back possible so we don't have to update everything at once?

I see two ways to tackle this:

1. This could be provided as alternative separate implementation.

   • The old GPIO API is called periph_gpio, we could call the new just gpio (as it is not tied to peripheral GPIOs) and place the header in gpio.h. The old header periph/gpio.h could just be kept for now.
   • The CPU backends would be implemented (as separate files without interfering with the current API and implementations) one PR at a time
   • Once for every MCU in RIOT a backend is provided for the new API, a compatibility layer in periph/gpio.h could be written and the old implementations be dropped
   • All users of the old API can be converted to natively use the new API instead of the compatibility layer
   • The compatibility layer is marked as deprecated
   • A hook is added to make static-tests to check that no new users of the deprecated compatibility layer sneaks into the upstream code base

2. Develop this as a RIOT branch

   • We do not need to care about breaking stuff in that branch
   • The GPIO implementations and users can be updated one PR targeting that branch at a time
   • Once the branch is fully converted to the new API and everything works, a big PR merging that branch into master can be created without to much worrying that this will break anything

[gschorcht]

I will resume my work on this in next few weeks. I think, I have a possible approach.

1. We define in periph/gpio.h the current API as well as the new API embedded in

   #ifndef PERIPH_GPIO_NEW
   ... /* current API */
   #else /* PERIPH_GPIO_NEW */
   ... /* new API */
   #endif /* PERIPH_GPIO_NEW */
   

   Platforms that implement the new API use the pseudomodule periph_gpio_new.
2. We implement the new API platform by platform.
3. Once the new API is implemented on all platforms, we can remove the pseudomodule and the current API in periph/gpio.h.

[fjmolinas]

The ci has been frozen on starting this build for a while I will cancel it and re-trigger it, see if that fixes it:

[kaspar030]

The ci has been frozen on starting this build for a while I will cancel it and re-trigger it, see if that fixes it:

something's odd with the disque instance on riot-ci, I'm on it.

[maribu]

I really would like to have this in. And now with the release branch just having detached from master, we have the longest period of time before the next release is going out. To me, this is the best time to push for this, as we will have the longest possible period of time for testing it before the feature gets shipped.

The approach of v5 seems to be settled upon. However, I would like to change the approach of getting this in, as a complete ad-hoc replacement of all GPIO implementations and all users IMHO just does not scale. Thus, I propose to do it similar to how it is planned for ztimer:

• Add this in addition to periph_gpio. And name it differently, e.g. let's say gpio_ng. (Also: No need to have periph still in the modules name or in the include path of its header, the new gpio_ng is no longer specific to periph stuff
  • Add a periph_gpio_on_gpio_ng module for backward compatibility. This should just be a slim compatibility layer on top of gpio_ng that will just always choose the periph GPIOs
• Get support for gpio_ng one MCU implementation at a time. Both implementation and reviewing could be better distributed upon multiple people to reduce the bottleneck
  • The old periph_gpio could pull in periph_gpio_on_gpio_ng for MCUs that have gpio_ng support, so no need to maintain two GPIO implementations simultaniously
• In parallel, some drivers should be updated to the gpio_ng API
  • This is the best way to verify that the API is usable
  • Merging this should however be postponed until all MCUs support gpio_ng. Otherwise, those drivers would become unusable for for MCUs that still only provide the old periph_gpio

[gschorcht]

I really would like to have this in. And now with the release branch just having detached from master, we have the longest period of time before the next release is going out. To me, this is the best time to push for this, as we will have the longest possible period of time for testing it before the feature gets shipped.

Thank you for picking up the ball again 😄 I still have it on my ToDo list and I would also like to get it in. I agree that it's now the best time to restart the work on it. My plan was to open a new PR with approach V5 next days (or next weeks 😟).

However, I would like to change the approach of getting this in, as a complete ad-hoc replacement of all GPIO implementations and all users IMHO just does not scale. Thus, I propose to do it similar to how it is planned for ztimer:

It is not really clear to me why a different approach is necessary. In fact, the approach proposed in this PR is probably very similar to that you suggested. It isn't an ad-hoc approach. It will let coexist old periph_gpio and new periph_gpio_ext MCU implementations transparently. MCU implementations can be done MCU by MCU.

As a proof of this concept I only changed the implementations for AVR and STM32 and kept the others untouched. Murdock was happy with compilation including all tests and all existing drivers for all platforms.

What are the steps of the approach proposed and already realized by this PR?

1. Add GPIO comparison/undefined macros/functions to periph_gpio high level API that have to be used (part of this PR) by MCU implementations and drivers.
2. Change all comparisons in existing code by these comparison/undef macros/functions (already part of this PR) what is straight forward.
3. Define a new pseudomodule periph_gpio_ext (ext stands for extendable GPIO API, could also be called periph_gpio_ng).
4. Define the new low level API that is only used when periph_gpio_ext is provided. The high level API remains unchanged.
5. If a MCU implements the new low level API, it provides the feature periph_gpio_ext. Only in this case the new low level API is used. Otherwise the legacy MCU API is used.

Since all drivers and all existing applications always use the high level API (with the exception of the wx281x driver), the migration is completely transparent and the MCU implementation can be done MCU by MCU. The only change that will become necessary at the users side is that they have to replace their comparisons of GPIOs by the comparison macros/functions.

From my point of view, a realistic goal for the next release could be to define the new low level API and to provide the MCU implementations for AVR and STM32.

[maribu]

It will let coexist old periph_gpio and new periph_gpio_ext MCU implementations transparently.

Perfect :-)

From my point of view, a realistic goal for the next release could be to define the new low level API and to provide the MCU implementations for AVR and STM32.

Sounds good. Can you split this into three PRs? One for the basic API, and one for each implementation? I think if more people help out with it, it should also be possible to get some implementations more into master. Maybe we should add a tracking issue for the implementations as check-box list with assigned coders. That could prevent people from accidentally doing the same work without contributing to each other.

(Btw: I have boards featuring the following MCU familys at hand: AVR, STM32 (F0, F1, F3, F4, F7), nRF51, nRF52, ESP8266, ESP32, and SAM3. So I could review / test / implement for some of those.)

[gschorcht]

Sounds good. Can you split this into three PRs? One for the basic API, and one for each implementation? I think if more people help out with it, it should also be possible to get some implementations more into master. Maybe we should add a tracking issue for the implementations as check-box list with assigned coders. That could prevent people from accidentally doing the same work without contributing to each other.

Sure, that was my plan 😉 as I wrote in #12877 (comment):

Once we have agreed to continue in this way, I will open a tracking issue. I might split this PR into
several PRs that implement the proposal step by step.

Since this PR served as POC/RFC, it was better to have all commits in one PR.

(Btw: I have boards featuring the following MCU familys at hand: AVR, STM32 (F0, F1, F3, F4, F7), nRF51, nRF52, ESP8266, ESP32, and SAM3. So I could review / test / implement for some of those.)

That would be great. These are exactly the same platforms I have, except for STM32 F0, F3 and F7.

[aabadie]

@gschorcht, it would be good if we could move forward here. With @fjmolinas, we have a lot of hardware for testing (stm32f0/l0/l1/l4/g4/wb, sam0, kinetis, nrf, esp, efm32).

[gschorcht]

@gschorcht, it would be good if we could move forward here. With @fjmolinas, we have a lot of hardware for testing (stm32f0/l0/l1/l4/g4/wb, sam0, kinetis, nrf, esp, efm32).

I will resume the work on in in next days.

[gschorcht]

I'm going to resume my work on this PR now.

All approaches tested here use some preprocessor conditionals to optimize the call of the low-level MCU functions regarding ROM size, RAM size and performance when no GPIO extenders are used.

I wonder if we should really make such a difference or if the low-level MCU functions should always be called the same way, regardless if GPIO extenders are used or not, to ensure the same timing. Having always the same timing could be important when implementing bit-banging protocols.

[benpicco]

I wonder if we should really make such a difference or if the low-level MCU functions should always be called the same way, regardless if GPIO extenders are used or not, to ensure the same timing. Having always the same timing could be important when implementing bit-banging protocols.

Both GPIO expanders and bit-banging are rather niche use cases. Let's not over-complicate / degrade things for the unlikely case that both are used at the same time.

[gschorcht]

bit-banging are rather niche use cases.

Both ESP32 as well as ESP8266 are using a bit-banging implementation 😉 ESP8266 because there is no hardware controller, the ESP32 because the hardware implemention is buggy and supports only I2C_SPEED_FAST while bit-banging software implementation allows I2C_FAST_PLUS.

[maribu]

Having always the same timing could be important when implementing bit-banging protocols.

Well, I once tried to implement bit-banging the single-wire protocol used by the WS281x RGB-LEDs. I was able to get this working on STM32F4 and STM32F7 (and it also works on ESP32), even though the generated signal was actually out of spec. But on Cortex M0 and Cortex M0+, the GPIO API does not allow matching the timing constraints well enough, as the overhead of the gpio_set() and gpio_clear() calls could no longer be ignored. So I ended up proposing a new API for this use case: #12015

(With that API the WS281x driver works reliable on a bluepill and the timings where indeed within spec.)

So: With the API not being useful for bit-banging with strict timing constraints anyway, I don't think we should worry about this here.

[maribu]

Both ESP32 as well as ESP8266 are using a bit-banging implementation

When ported to the GPIO ABC API, those could move to drivers/ and made available for other platforms as well. I2C is surprisingly messy when clock stretching is used, and I bet that some other platforms have hardware bugs related to this. A portable big-banging solution would be quite nice.

(And also when more I2C buses are needed, a software solution would be quite handy as well.)

[gschorcht]

@aabadie When I tried to rebase this PR, I realized that the GPIO port definitions for STM32 MCUs had been restructured. In previous versions, there was a definition of available ports as an enumeration for each MCU in cpu/stm*/include/periph_cpu.h. Now you only have one definition of available ports in the cpu/stm32/include/periph_cpu.h file, which uses a cascade of per-processor conditionals.

For the proposed GPIO API, we need a definition of GPIO_PORT_EXT and a MCU specific port table definition GPIO_CPU_PORTS, for example for STM32L0

RIOT/cpu/stm32l0/include/periph_cpu.h

Lines 39 to 60 in 23783b7

	enum {
	PORT_A = 0, /**< port A */
	PORT_B = 1, /**< port B */
	PORT_C = 2, /**< port C */
	PORT_D = 3, /**< port D */
	PORT_E = 4, /**< port E */
	PORT_H = 7, /**< port H */
	GPIO_EXT_PORT = 8 /**< first GPIO extender port */
	};
	/**
	* @brief Available ports on the STM32L0 family as GPIO register definitions
	*/
	#define GPIO_CPU_PORTS \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_A)), \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_B)), \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_C)), \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_D)), \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_E)), \
	GPIO_CPU_PORT(0), \
	GPIO_CPU_PORT(0), \
	GPIO_CPU_PORT(GPIO_CPU_PORT_ADDR(PORT_H)),

GPIO_PORT_EXT was just defined as the last value the enumeration of avialable ports.

Do you think it would be better to define GPIO_PORT_EXT and GPIO_CPU_PORTS cpu/stm32/include/periph_cpu.h using the same preprocessor conditionals or should we define them for each CPU in cpu/stm32/include/*/periph_cpu.h?

[gschorcht]

Closed in favor of PR #14602