Skip to content

Commit

Permalink
cpu/esp32/gpio_ll: fix & cleanup
Browse files Browse the repository at this point in the history 
- `gpio_ll_toggle()` now is race-free
- avoid using a look up table but branch to the two different registers
  in the `gpio_ll*()` functions
    - in most cases the GPIO port is a compile time constant and the
      dead branch is eliminated by the optimizer, making this vastly
      more efficient
    - some MCUs do only have a single port, in which case
      `GPIO_PORT_NUM(port)` is known to return `0` even if `port` is
      not known, resulting in one of the branch being eliminated as
      dead branch no matter what
    - in case it really is unknown at compile time which port to work
      on, the branch can still be implemented efficiently by the
      compiler e.g. using a conditional move; likely more efficient
      than fetching a value from the look up table.
  • Loading branch information
@maribu
maribu committed Apr 30, 2024
1 parent c2bd865 commit dd585f9
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 75 deletions.
109 changes: 66 additions & 43 deletions cpu/esp32/include/gpio_ll_arch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,73 +22,100 @@
#define GPIO_LL_ARCH_H

#include "gpio_arch.h"
#include "irq.h"
#include "soc/gpio_reg.h"
#include "soc/soc.h"
#include "soc/gpio_struct.h"

#ifdef __cplusplus
extern "C" {
#endif

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

#define GPIO_PORT(num) ((gpio_port_t)(&_esp32_ports[num]))
#define GPIO_PORT_NUM(port) (((_esp32_port_t*)port == _esp32_ports) ? 0 : 1)

/* GPIO port descriptor type */
typedef struct {
volatile uint32_t *out; /* address of GPIO_OUT/GPIO_OUT1 */
volatile uint32_t *out_w1ts; /* address of GPIO_OUT_W1TS/GPIO_OUT1_W1TC */
volatile uint32_t *out_w1tc; /* address of GPIO_OUT_W1TC/GPIO_OUT1_W1TC */
volatile uint32_t *in; /* address of GPIO_IN/GPIO_IN1 */
volatile uint32_t *enable; /* address of GPIO_ENABLE/GPIO_ENABLE1 */
volatile uint32_t *enable_w1ts; /* address of GPIO_ENABLE_W1TS/GPIO_ENABLE1_W1TS */
volatile uint32_t *enable_w1tc; /* address of GPIO_ENABLE_W1TC/GPIO_ENABLE1_W1TC */
volatile uint32_t *status_w1tc; /* address of GPIO_STATUS_W1TC/GPIO_STATUS1_W1TC */
} _esp32_port_t;

/* GPIO port table */
extern const _esp32_port_t _esp32_ports[];
#define GPIO_PORT(num) (num)
#if GPIO_PORT_NUMOF > 1
# define GPIO_PORT_NUM(port) (port)
#else
# define GPIO_PORT_NUM(port) 0
#endif

static inline uword_t gpio_ll_read(gpio_port_t port)
{
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *in = (uint32_t *)GPIO_IN_REG;
/* return 0 for unconfigured pins, the current level at the pin otherwise */
const _esp32_port_t *p = (_esp32_port_t *)port;
return *p->in;
#if GPIO_PORT_NUM > 1
if (GPIO_PORT_NUM(port) != 0) {
in = (uint32_t *)GPIO_IN1_REG;
}
#endif

return *in;
}

static inline uword_t gpio_ll_read_output(gpio_port_t port)
{
/* return output register bits */
const _esp32_port_t *p = (_esp32_port_t *)port;
return *p->out;
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
#if GPIO_PORT_NUM > 1
if (GPIO_PORT_NUM(port) != 0) {
out = (uint32_t *)GPIO_OUT1_REG;
}
#endif

return *out;
}

static inline void gpio_ll_set(gpio_port_t port, uword_t mask)
{
/* set output register bits for configured pins in the mask */
const _esp32_port_t *p = (_esp32_port_t *)port;
*p->out_w1ts = mask;
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *out_w1ts = (uint32_t *)GPIO_OUT_W1TS_REG;
if (GPIO_PORT_NUM(port) != 0) {
#if GPIO_PORT_NUM > 1
out_w1ts = (uint32_t)GPIO_OUT1_W1TS;
#endif
}

*out_w1ts = mask;
}

static inline void gpio_ll_clear(gpio_port_t port, uword_t mask)
{
/* clear output register bits for configured pins in the mask */
const _esp32_port_t *p = (_esp32_port_t *)port;
*p->out_w1tc = mask;
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *out_w1tc = (uint32_t *)GPIO_OUT_W1TC_REG;
if (GPIO_PORT_NUM(port) != 0) {
#if GPIO_PORT_NUM > 1
out_w1tc = (uint32_t)GPIO_OUT1_W1TC;
#endif
}

*out_w1tc = mask;
}

static inline void gpio_ll_toggle(gpio_port_t port, uword_t mask)
{
/* toggle output register bits for configured pins in the mask */
const _esp32_port_t *p = (_esp32_port_t *)port;
*p->out ^= mask;
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
#if GPIO_PORT_NUM > 1
if (GPIO_PORT_NUM(port) != 0) {
out = (uint32_t *)GPIO_OUT1_REG;
}
#endif
unsigned irq_state = irq_disable();
*out ^= mask;
irq_restore(irq_state);
}

static inline void gpio_ll_write(gpio_port_t port, uword_t value)
{
/* write output register bits for configured pins in the mask */
const _esp32_port_t *p = (_esp32_port_t *)port;
*p->out = value;
static_assert(GPIO_PORT_NUMOF < 3);
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
#if GPIO_PORT_NUM > 1
if (GPIO_PORT_NUM(port) != 0) {
out = (uint32_t *)GPIO_OUT1_REG;
}
#endif
*out = value;
}

static inline gpio_port_t gpio_get_port(gpio_t pin)
Expand All @@ -108,15 +135,11 @@ static inline gpio_port_t gpio_port_pack_addr(void *addr)

static inline void * gpio_port_unpack_addr(gpio_port_t port)
{
const _esp32_port_t *p = (_esp32_port_t *)port;
if (port <= 1) {
return NULL;
}

/* return NULL if port is one of the port descriptors in GPIO port table */
#if GPIO_PORT_NUMOF > 1
return ((p == &_esp32_ports[0]) ||
(p == &_esp32_ports[1])) ? NULL : (void *)port;
#else
return (p == _esp32_ports) ? NULL : (void *)port;
#endif
return (void *)port;
}

static inline bool is_gpio_port_num_valid(uint_fast8_t num)
Expand Down
30 changes: 2 additions & 28 deletions cpu/esp32/periph/gpio_ll.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#include "hal/gpio_hal.h"
#include "hal/gpio_types.h"
#include "gpio_ll_arch.h"
#include "soc/gpio_reg.h"
#include "soc/gpio_struct.h"

#include "esp_idf_api/gpio.h"

Expand All @@ -55,35 +55,9 @@ extern bool _gpio_pin_pd[GPIO_PIN_NUMOF];

static gpio_conf_t _gpio_conf[GPIO_PIN_NUMOF] = { };

const _esp32_port_t _esp32_ports[GPIO_PORT_NUMOF] = {
{
.out = (uint32_t *)GPIO_OUT_REG,
.out_w1ts = (uint32_t *)GPIO_OUT_W1TS_REG,
.out_w1tc = (uint32_t *)GPIO_OUT_W1TC_REG,
.in = (uint32_t *)GPIO_IN_REG,
.enable = (uint32_t *)GPIO_ENABLE_REG,
.enable_w1ts = (uint32_t *)GPIO_ENABLE_W1TS_REG,
.enable_w1tc = (uint32_t *)GPIO_ENABLE_W1TC_REG,
.status_w1tc = (uint32_t *)GPIO_STATUS_W1TC_REG,
},
#if GPIO_PORT_NUMOF > 1
{
.out = (uint32_t *)GPIO_OUT1_REG,
.out_w1ts = (uint32_t *)GPIO_OUT1_W1TS_REG,
.out_w1tc = (uint32_t *)GPIO_OUT1_W1TC_REG,
.in = (uint32_t *)GPIO_IN1_REG,
.enable = (uint32_t *)GPIO_ENABLE1_REG,
.enable_w1ts = (uint32_t *)GPIO_ENABLE1_W1TS_REG,
.enable_w1tc = (uint32_t *)GPIO_ENABLE1_W1TC_REG,
.status_w1tc = (uint32_t *)GPIO_STATUS1_W1TC_REG,
}
#endif
};

int gpio_ll_init(gpio_port_t port, uint8_t pin, gpio_conf_t conf)
{
assert(port);
assert(GPIO_PORT_NUM(port) < GPIO_PORT_NUMOF);
assert(is_gpio_port_num_valid(port));
assert(pin < GPIO_PORT_PIN_NUMOF(port));

gpio_t gpio = GPIO_PIN(GPIO_PORT_NUM(port), pin);
Expand Down
14 changes: 10 additions & 4 deletions cpu/esp32/periph/gpio_ll_irq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ extern void IRAM gpio_int_handler(void* arg);
int gpio_ll_irq(gpio_port_t port, uint8_t pin, gpio_irq_trig_t trig,
gpio_ll_cb_t cb, void *arg)
{
assert(port);
assert(is_gpio_port_num_valid(port));
assert(arg);
assert(GPIO_PORT_NUM(port) < GPIO_PORT_NUMOF);
assert(pin < GPIO_PORT_PIN_NUMOF(port));

unsigned state = irq_disable();
Expand Down Expand Up @@ -138,13 +137,20 @@ 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)
{
_esp32_port_t *p = (_esp32_port_t *)port;
gpio_t gpio = GPIO_PIN(GPIO_PORT_NUM(port), pin);

DEBUG("%s gpio=%u port=%u pin=%u\n",
__func__, gpio, GPIO_PORT_NUM(port), pin);

*p->status_w1tc = BIT(pin);
volatile uint32_t *status_w1tc = (uint32_t *)GPIO_STATUS_W1TC_REG;
#if GPIO_PORT_NUMOF > 1
if (GPIO_PORT_NUM(port) != 0) {
status_w1tc = (uint32_t *)GPIO_STATUS1_W1TC_REG;
}
#endif

*status_w1tc = BIT(pin);

if (esp_idf_gpio_intr_enable(gpio) == ESP_OK) {
gpio_int_enabled_table[gpio] = true;
}
Expand Down

0 comments on commit dd585f9

Please sign in to comment.