Skip to content

Commit

Permalink
drivers/periph_gpio_ll: change API to access GPIO ports
Browse files Browse the repository at this point in the history
The API was based on the assumption that GPIO ports are mapped in
memory sanely, so that a `GPIO_PORT(num)` macro would work allow for
constant folding when `num` is known and still be efficient when it
is not.

Some MCUs, however, will need a look up tables to efficiently translate
GPIO port numbers to the port's base address. This will prevent the use
of such a `GPIO_PORT(num)` macro in constant initializers.

As a result, we rather provide `GPIO_PORT_0`, `GPIO_PORT_1`, etc.
macros for each GPIO port present (regardless of MCU naming scheme), as
well as `GPIO_PORT_A`, `GPIO_PORT_B`, etc. macros if (and only if) the
MCU port naming scheme uses letters rather than numbers.

These can be defined as macros to the peripheral base address even when
those are randomly mapped into the address space. In addition, a C
function `gpio_port()` replaces the role of the `GPIO_PORT()` and
`gpio_port_num()` the `GPIO_PORT_NUM()` macro. Those functions will
still be implemented as efficient as possible and will allow constant
folding where it was formerly possible. Hence, there is no downside
for MCUs with sane peripheral memory mapping, but it is highly
beneficial for the crazy ones.

There are also two benefits for the non-crazy MCUs:
1. We can now test for valid port numbers with `#ifdef GPIO_PORT_<NUM>`
    - This directly benefits the test in `tests/periph/gpio_ll`, which
      can now provide a valid GPIO port for each and every board
    - Writing to invalid memory mapped I/O addresses was treated as
      triggering undefined behavior by the compiler and used as a
      optimization opportunity
2. We can now detect at compile time if the naming scheme of the
   MCU uses letters or numbers, and produce more user friendly output.
    - This is directly applied in the test app
  • Loading branch information
maribu committed Apr 29, 2024
1 parent 06edfa8 commit 9dc939a
Show file tree
Hide file tree
Showing 24 changed files with 528 additions and 148 deletions.
19 changes: 10 additions & 9 deletions boards/common/stm32/include/stm32_leds.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
#ifndef STM32_LEDS_H
#define STM32_LEDS_H

/* GPIO_PORT() macro. This define even works when GPIO LL is not in used */
/* Using gpio_ll_arch for the gpio_port() function. This even works when
* GPIO LL is not in used */
#include "gpio_ll_arch.h"
#include "kernel_defines.h"

Expand All @@ -36,7 +37,7 @@ extern "C" {
* @{
*/
#if defined(LED0_PORT_NUM) && defined (LED0_PIN_NUM)
# define LED0_PORT ((GPIO_TypeDef *)GPIO_PORT(LED0_PORT_NUM))
# define LED0_PORT ((GPIO_TypeDef *)gpio_port(LED0_PORT_NUM))
# define LED0_PIN GPIO_PIN(LED0_PORT_NUM, LED0_PIN_NUM)
# define LED0_MASK (1 << LED0_PIN_NUM)
# if IS_ACTIVE(LED0_IS_INVERTED)
Expand All @@ -50,7 +51,7 @@ extern "C" {
#endif

#if defined(LED1_PORT_NUM) && defined (LED1_PIN_NUM)
# define LED1_PORT ((GPIO_TypeDef *)GPIO_PORT(LED1_PORT_NUM))
# define LED1_PORT ((GPIO_TypeDef *)gpio_port(LED1_PORT_NUM))
# define LED1_PIN GPIO_PIN(LED1_PORT_NUM, LED1_PIN_NUM)
# define LED1_MASK (1 << LED1_PIN_NUM)
# if IS_ACTIVE(LED1_IS_INVERTED)
Expand All @@ -64,7 +65,7 @@ extern "C" {
#endif

#if defined(LED2_PORT_NUM) && defined (LED2_PIN_NUM)
# define LED2_PORT ((GPIO_TypeDef *)GPIO_PORT(LED2_PORT_NUM))
# define LED2_PORT ((GPIO_TypeDef *)gpio_port(LED2_PORT_NUM))
# define LED2_PIN GPIO_PIN(LED2_PORT_NUM, LED2_PIN_NUM)
# define LED2_MASK (1 << LED2_PIN_NUM)
# if IS_ACTIVE(LED2_IS_INVERTED)
Expand All @@ -78,7 +79,7 @@ extern "C" {
#endif

#if defined(LED3_PORT_NUM) && defined (LED3_PIN_NUM)
# define LED3_PORT ((GPIO_TypeDef *)GPIO_PORT(LED3_PORT_NUM))
# define LED3_PORT ((GPIO_TypeDef *)gpio_port(LED3_PORT_NUM))
# define LED3_PIN GPIO_PIN(LED3_PORT_NUM, LED3_PIN_NUM)
# define LED3_MASK (1 << LED3_PIN_NUM)
# if IS_ACTIVE(LED3_IS_INVERTED)
Expand All @@ -92,7 +93,7 @@ extern "C" {
#endif

#if defined(LED4_PORT_NUM) && defined (LED4_PIN_NUM)
# define LED4_PORT ((GPIO_TypeDef *)GPIO_PORT(LED4_PORT_NUM))
# define LED4_PORT ((GPIO_TypeDef *)gpio_port(LED4_PORT_NUM))
# define LED4_PIN GPIO_PIN(LED4_PORT_NUM, LED4_PIN_NUM)
# define LED4_MASK (1 << LED4_PIN_NUM)
# if IS_ACTIVE(LED4_IS_INVERTED)
Expand All @@ -106,7 +107,7 @@ extern "C" {
#endif

#if defined(LED5_PORT_NUM) && defined (LED5_PIN_NUM)
# define LED5_PORT ((GPIO_TypeDef *)GPIO_PORT(LED5_PORT_NUM))
# define LED5_PORT ((GPIO_TypeDef *)gpio_port(LED5_PORT_NUM))
# define LED5_PIN GPIO_PIN(LED5_PORT_NUM, LED5_PIN_NUM)
# define LED5_MASK (1 << LED5_PIN_NUM)
# if IS_ACTIVE(LED5_IS_INVERTED)
Expand All @@ -120,7 +121,7 @@ extern "C" {
#endif

#if defined(LED6_PORT_NUM) && defined (LED6_PIN_NUM)
# define LED6_PORT ((GPIO_TypeDef *)GPIO_PORT(LED6_PORT_NUM))
# define LED6_PORT ((GPIO_TypeDef *)gpio_port(LED6_PORT_NUM))
# define LED6_PIN GPIO_PIN(LED6_PORT_NUM, LED6_PIN_NUM)
# define LED6_MASK (1 << LED6_PIN_NUM)
# if IS_ACTIVE(LED6_IS_INVERTED)
Expand All @@ -134,7 +135,7 @@ extern "C" {
#endif

#if defined(LED7_PORT_NUM) && defined (LED7_PIN_NUM)
# define LED7_PORT ((GPIO_TypeDef *)GPIO_PORT(LED7_PORT_NUM))
# define LED7_PORT ((GPIO_TypeDef *)gpio_port(LED7_PORT_NUM))
# define LED7_PIN GPIO_PIN(LED7_PORT_NUM, LED7_PIN_NUM)
# define LED7_MASK (1 << LED7_PIN_NUM)
# if IS_ACTIVE(LED7_IS_INVERTED)
Expand Down
102 changes: 89 additions & 13 deletions cpu/atmega_common/include/gpio_ll_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,96 @@ extern "C" {
* whenever the port number is known at compile time.
*/

#ifdef PORTH
#define GPIO_PORT(num) \
((num >= PORT_H) ? \
(ATMEGA_GPIO_BASE_H + ((num) - PORT_H) * ATMEGA_GPIO_SIZE) : \
(ATMEGA_GPIO_BASE_A + (num) * ATMEGA_GPIO_SIZE))
#define GPIO_PORT_NUM(port) \
(((port) >= ATMEGA_GPIO_BASE_H) ? \
(((port) - ATMEGA_GPIO_BASE_H) / ATMEGA_GPIO_SIZE + PORT_H) : \
(((port) - ATMEGA_GPIO_BASE_A) / ATMEGA_GPIO_SIZE))
#else
#define GPIO_PORT(num) (ATMEGA_GPIO_BASE_A + (num) * ATMEGA_GPIO_SIZE)
#define GPIO_PORT_NUM(port) (((port) - ATMEGA_GPIO_BASE_A) / ATMEGA_GPIO_SIZE)
#ifdef PINA
/* We sadly cannot use PINA, as PINA is technically (in terms of C spec lingo)
* not constant and would trigger:
* initializer element is not constant
* Hence, we the defines are a bit more involved to yield proper constants
* suitable for initializers.
*/
# define GPIO_PORT_A (ATMEGA_GPIO_BASE_A)
# define GPIO_PORT_0 GPIO_PORT_A
#endif

#ifdef PINB
# define GPIO_PORT_B (ATMEGA_GPIO_BASE_A + 1 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_1 GPIO_PORT_B
#endif

#ifdef PINC
# define GPIO_PORT_C (ATMEGA_GPIO_BASE_A + 2 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_2 GPIO_PORT_C
#endif

#ifdef PIND
# define GPIO_PORT_D (ATMEGA_GPIO_BASE_A + 3 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_3 GPIO_PORT_D
#endif

#ifdef PINE
# define GPIO_PORT_E (ATMEGA_GPIO_BASE_A + 4 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_4 GPIO_PORT_E
#endif

#ifdef PINF
# define GPIO_PORT_F (ATMEGA_GPIO_BASE_A + 5 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_5 GPIO_PORT_F
#endif

#ifdef PING
# define GPIO_PORT_G ATMEGA_GPIO_BASE_A + 6 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_6 GPIO_PORT_G
#endif

/* There is a larger gap between PING and PINH to allow other peripherals to
* also be mapped into the fast I/O memory area. */
#ifdef PINH
# define GPIO_PORT_H (ATMEGA_GPIO_BASE_H)
# define GPIO_PORT_7 GPIO_PORT_H
#endif

#ifdef PINI
# define GPIO_PORT_I (ATMEGA_GPIO_BASE_H + 1 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_8 GPIO_PORT_I
#endif

#ifdef PINJ
# define GPIO_PORT_J (ATMEGA_GPIO_BASE_H + 2 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_9 GPIO_PORT_J
#endif

#ifdef PINK
# define GPIO_PORT_K (ATMEGA_GPIO_BASE_H + 3 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_10 GPIO_PORT_K
#endif

#ifdef PINL
# define GPIO_PORT_L (ATMEGA_GPIO_BASE_H + 4 * ATMEGA_GPIO_SIZE)
# define GPIO_PORT_11 GPIO_PORT_L
#endif

static inline gpio_port_t gpio_port(uword_t num)
{
#ifdef PINH
if (num >= PORT_H) {
return ATMEGA_GPIO_BASE_H + ((num - PORT_H) * ATMEGA_GPIO_SIZE);
}
#endif

return ATMEGA_GPIO_BASE_A + (num * ATMEGA_GPIO_SIZE);
}

static inline uword_t gpio_port_num(gpio_port_t port)
{
#ifdef PINH
if ((port) >= ATMEGA_GPIO_BASE_H) {
return (port - ATMEGA_GPIO_BASE_H) / ATMEGA_GPIO_SIZE + PORT_H;
}
#endif

return (port - ATMEGA_GPIO_BASE_A) / ATMEGA_GPIO_SIZE;
}

static inline uword_t gpio_ll_read(gpio_port_t port)
{
atmega_gpio_port_t *p = (void *)port;
Expand Down Expand Up @@ -158,7 +234,7 @@ static inline void gpio_ll_write(gpio_port_t port, uword_t value)

static inline gpio_port_t gpio_get_port(gpio_t pin)
{
return GPIO_PORT(pin >> 4);
return gpio_port(pin >> 4);
}

static inline uint8_t gpio_get_pin_num(gpio_t pin)
Expand Down
6 changes: 3 additions & 3 deletions cpu/atmega_common/include/periph_cpu_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,16 @@ typedef uint8_t gpio_t;
#ifdef GPIO_PORT_DESCENDENT
#ifdef _AVR_ATTINY1634_H_INCLUDED
/* the only one that requires particular treatment! */
#define ATMEGA_GPIO_BASE_A (0x2F)
#define ATMEGA_GPIO_BASE_A 0x2F
#else
/* all other port descendent, including :
- _AVR_IO8534_ (only have port A but with 0x1B address) ;
- _AVR_IOAT94K_H_ (only have ports D and E) ;
- _AVR_IOTN28_H_ (only have ports A and D). */
#define ATMEGA_GPIO_BASE_A (0x39)
#define ATMEGA_GPIO_BASE_A 0x39
#endif /* _AVR_ATTINY1634_H_INCLUDED */
#else /* !GPIO_PORT_DESCENDENT */
#define ATMEGA_GPIO_BASE_A (0x20)
#define ATMEGA_GPIO_BASE_A 0x20
#endif /* GPIO_PORT_DESCENDENT */
/**
* @brief Base of the GPIO port G register as memory address
Expand Down
8 changes: 4 additions & 4 deletions cpu/atmega_common/periph/gpio_ll_irq.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static void clear_pending_irqs(uint8_t exti)

void gpio_ll_irq_mask(gpio_port_t port, uint8_t pin)
{
uint8_t exti = atmega_pin2exti(GPIO_PORT_NUM(port), pin);
uint8_t exti = atmega_pin2exti(gpio_port_num(port), pin);
#if defined(EIMSK)
EIMSK &= ~(1 << exti);
#elif defined(GICR)
Expand All @@ -72,7 +72,7 @@ void gpio_ll_irq_mask(gpio_port_t port, uint8_t pin)

void gpio_ll_irq_unmask(gpio_port_t port, uint8_t pin)
{
uint8_t exti = atmega_pin2exti(GPIO_PORT_NUM(port), pin);
uint8_t exti = atmega_pin2exti(gpio_port_num(port), pin);
#if defined(EIMSK)
EIMSK |= 1 << exti;
#elif defined(GICR)
Expand All @@ -82,7 +82,7 @@ void gpio_ll_irq_unmask(gpio_port_t port, uint8_t pin)

void gpio_ll_irq_unmask_and_clear(gpio_port_t port, uint8_t pin)
{
uint8_t exti = atmega_pin2exti(GPIO_PORT_NUM(port), pin);
uint8_t exti = atmega_pin2exti(gpio_port_num(port), pin);
clear_pending_irqs(exti);
#if defined(EIMSK)
EIMSK |= 1 << exti;
Expand Down Expand Up @@ -118,7 +118,7 @@ static void set_trigger(uint8_t exti, gpio_irq_trig_t trig)
int gpio_ll_irq(gpio_port_t port, uint8_t pin, gpio_irq_trig_t trig,
gpio_ll_cb_t cb, void *arg)
{
int port_num = GPIO_PORT_NUM(port);
int port_num = gpio_port_num(port);
assert((trig != GPIO_TRIGGER_LEVEL_HIGH) && cb);
if (!atmega_has_pin_exti(port_num, pin)) {
return -ENOTSUP;
Expand Down
76 changes: 71 additions & 5 deletions cpu/efm32/include/gpio_ll_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,72 @@ extern "C" {

#ifndef DOXYGEN /* hide implementation specific details from Doxygen */

/* Note: The pin count may be defined as zero to indicate the port not existing.
* Hence, don't to `#if defined(foo)` but only `#if foo`
*/
#if _GPIO_PORT_A_PIN_COUNT
# define GPIO_PORT_A 0
# define GPIO_PORT_0 GPIO_PORT_A
#endif

#if _GPIO_PORT_B_PIN_COUNT
# define GPIO_PORT_B 1
# define GPIO_PORT_1 GPIO_PORT_B
#endif

#if _GPIO_PORT_C_PIN_COUNT
# define GPIO_PORT_C 2
# define GPIO_PORT_2 GPIO_PORT_C
#endif

#if _GPIO_PORT_D_PIN_COUNT
# define GPIO_PORT_D 3
# define GPIO_PORT_3 GPIO_PORT_D
#endif

#if _GPIO_PORT_E_PIN_COUNT
# define GPIO_PORT_E 4
# define GPIO_PORT_4 GPIO_PORT_E
#endif

#if _GPIO_PORT_F_PIN_COUNT
# define GPIO_PORT_F 6
# define GPIO_PORT_6 GPIO_PORT_F
#endif

#if _GPIO_PORT_G_PIN_COUNT
# define GPIO_PORT_G 7
# define GPIO_PORT_7 GPIO_PORT_G
#endif

#if _GPIO_PORT_H_PIN_COUNT
# define GPIO_PORT_H 8
# define GPIO_PORT_8 GPIO_PORT_H
#endif

#if _GPIO_PORT_I_PIN_COUNT
# define GPIO_PORT_I 9
# define GPIO_PORT_9 GPIO_PORT_I
#endif

#if _GPIO_PORT_J_PIN_COUNT
# define GPIO_PORT_J 10
# define GPIO_PORT_10 GPIO_PORT_J
#endif

#if _GPIO_PORT_K_PIN_COUNT
# define GPIO_PORT_K 11
# define GPIO_PORT_11 GPIO_PORT_K
#endif

/* We could do
*
#define GPIO_PORT(num) (GPIO->P[num])
#define GPIO_PORT_NUM(port) ((port - &GPIO->P))
* static inline gpio_port_t gpio_port(uword_t num)
* {
* return GPIO->P[num];
* }
*
* which forks for some operations, but at latest when _ll_set needs to fan out
* which works for some operations, but at latest when _ll_set needs to fan out
* for some EFM32 families to
*
#if defined(_GPIO_P_DOUTSET_MASK)
Expand All @@ -78,9 +138,15 @@ extern "C" {
* an implementation for other EFM32 families. For the time being, the
* suboptimal-but-works-for-all version is the best we have.
*/
static inline gpio_port_t gpio_port(uword_t num)
{
return num;
}

#define GPIO_PORT(num) (num)
#define GPIO_PORT_NUM(port) (port)
static inline uword_t gpio_port_num(gpio_port_t port)
{
return port;
}

static inline uword_t gpio_ll_read(gpio_port_t port)
{
Expand Down

0 comments on commit 9dc939a

Please sign in to comment.