Skip to content

Commit dd585f9

Browse files
maribumaribu
committed
cpu/esp32/gpio_ll: fix & cleanup
- `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.
1 parent c2bd865 commit dd585f9

3 files changed

Lines changed: 78 additions & 75 deletions

File tree

‎cpu/esp32/include/gpio_ll_arch.h‎

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -22,73 +22,100 @@
2222
#define GPIO_LL_ARCH_H
2323

2424
#include "gpio_arch.h"
25+
#include "irq.h"
26+
#include "soc/gpio_reg.h"
2527
#include "soc/soc.h"
26-
#include "soc/gpio_struct.h"
2728

2829
#ifdef __cplusplus
2930
extern "C" {
3031
#endif
3132

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

34-
#define GPIO_PORT(num) ((gpio_port_t)(&_esp32_ports[num]))
35-
#define GPIO_PORT_NUM(port) (((_esp32_port_t*)port == _esp32_ports) ? 0 : 1)
36-
37-
/* GPIO port descriptor type */
38-
typedef struct {
39-
volatile uint32_t *out; /* address of GPIO_OUT/GPIO_OUT1 */
40-
volatile uint32_t *out_w1ts; /* address of GPIO_OUT_W1TS/GPIO_OUT1_W1TC */
41-
volatile uint32_t *out_w1tc; /* address of GPIO_OUT_W1TC/GPIO_OUT1_W1TC */
42-
volatile uint32_t *in; /* address of GPIO_IN/GPIO_IN1 */
43-
volatile uint32_t *enable; /* address of GPIO_ENABLE/GPIO_ENABLE1 */
44-
volatile uint32_t *enable_w1ts; /* address of GPIO_ENABLE_W1TS/GPIO_ENABLE1_W1TS */
45-
volatile uint32_t *enable_w1tc; /* address of GPIO_ENABLE_W1TC/GPIO_ENABLE1_W1TC */
46-
volatile uint32_t *status_w1tc; /* address of GPIO_STATUS_W1TC/GPIO_STATUS1_W1TC */
47-
} _esp32_port_t;
48-
49-
/* GPIO port table */
50-
extern const _esp32_port_t _esp32_ports[];
35+
#define GPIO_PORT(num) (num)
36+
#if GPIO_PORT_NUMOF > 1
37+
# define GPIO_PORT_NUM(port) (port)
38+
#else
39+
# define GPIO_PORT_NUM(port) 0
40+
#endif
5141

5242
static inline uword_t gpio_ll_read(gpio_port_t port)
5343
{
44+
static_assert(GPIO_PORT_NUMOF < 3);
45+
volatile uword_t *in = (uint32_t *)GPIO_IN_REG;
5446
/* return 0 for unconfigured pins, the current level at the pin otherwise */
55-
const _esp32_port_t *p = (_esp32_port_t *)port;
56-
return *p->in;
47+
#if GPIO_PORT_NUM > 1
48+
if (GPIO_PORT_NUM(port) != 0) {
49+
in = (uint32_t *)GPIO_IN1_REG;
50+
}
51+
#endif
52+
53+
return *in;
5754
}
5855

5956
static inline uword_t gpio_ll_read_output(gpio_port_t port)
6057
{
61-
/* return output register bits */
62-
const _esp32_port_t *p = (_esp32_port_t *)port;
63-
return *p->out;
58+
static_assert(GPIO_PORT_NUMOF < 3);
59+
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
60+
#if GPIO_PORT_NUM > 1
61+
if (GPIO_PORT_NUM(port) != 0) {
62+
out = (uint32_t *)GPIO_OUT1_REG;
63+
}
64+
#endif
65+
66+
return *out;
6467
}
6568

6669
static inline void gpio_ll_set(gpio_port_t port, uword_t mask)
6770
{
68-
/* set output register bits for configured pins in the mask */
69-
const _esp32_port_t *p = (_esp32_port_t *)port;
70-
*p->out_w1ts = mask;
71+
static_assert(GPIO_PORT_NUMOF < 3);
72+
volatile uword_t *out_w1ts = (uint32_t *)GPIO_OUT_W1TS_REG;
73+
if (GPIO_PORT_NUM(port) != 0) {
74+
#if GPIO_PORT_NUM > 1
75+
out_w1ts = (uint32_t)GPIO_OUT1_W1TS;
76+
#endif
77+
}
78+
79+
*out_w1ts = mask;
7180
}
7281

7382
static inline void gpio_ll_clear(gpio_port_t port, uword_t mask)
7483
{
75-
/* clear output register bits for configured pins in the mask */
76-
const _esp32_port_t *p = (_esp32_port_t *)port;
77-
*p->out_w1tc = mask;
84+
static_assert(GPIO_PORT_NUMOF < 3);
85+
volatile uword_t *out_w1tc = (uint32_t *)GPIO_OUT_W1TC_REG;
86+
if (GPIO_PORT_NUM(port) != 0) {
87+
#if GPIO_PORT_NUM > 1
88+
out_w1tc = (uint32_t)GPIO_OUT1_W1TC;
89+
#endif
90+
}
91+
92+
*out_w1tc = mask;
7893
}
7994

8095
static inline void gpio_ll_toggle(gpio_port_t port, uword_t mask)
8196
{
82-
/* toggle output register bits for configured pins in the mask */
83-
const _esp32_port_t *p = (_esp32_port_t *)port;
84-
*p->out ^= mask;
97+
static_assert(GPIO_PORT_NUMOF < 3);
98+
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
99+
#if GPIO_PORT_NUM > 1
100+
if (GPIO_PORT_NUM(port) != 0) {
101+
out = (uint32_t *)GPIO_OUT1_REG;
102+
}
103+
#endif
104+
unsigned irq_state = irq_disable();
105+
*out ^= mask;
106+
irq_restore(irq_state);
85107
}
86108

87109
static inline void gpio_ll_write(gpio_port_t port, uword_t value)
88110
{
89-
/* write output register bits for configured pins in the mask */
90-
const _esp32_port_t *p = (_esp32_port_t *)port;
91-
*p->out = value;
111+
static_assert(GPIO_PORT_NUMOF < 3);
112+
volatile uword_t *out = (uint32_t *)GPIO_OUT_REG;
113+
#if GPIO_PORT_NUM > 1
114+
if (GPIO_PORT_NUM(port) != 0) {
115+
out = (uint32_t *)GPIO_OUT1_REG;
116+
}
117+
#endif
118+
*out = value;
92119
}
93120

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

109136
static inline void * gpio_port_unpack_addr(gpio_port_t port)
110137
{
111-
const _esp32_port_t *p = (_esp32_port_t *)port;
138+
if (port <= 1) {
139+
return NULL;
140+
}
112141

113-
/* return NULL if port is one of the port descriptors in GPIO port table */
114-
#if GPIO_PORT_NUMOF > 1
115-
return ((p == &_esp32_ports[0]) ||
116-
(p == &_esp32_ports[1])) ? NULL : (void *)port;
117-
#else
118-
return (p == _esp32_ports) ? NULL : (void *)port;
119-
#endif
142+
return (void *)port;
120143
}
121144

122145
static inline bool is_gpio_port_num_valid(uint_fast8_t num)

‎cpu/esp32/periph/gpio_ll.c‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
#include "hal/gpio_hal.h"
3535
#include "hal/gpio_types.h"
3636
#include "gpio_ll_arch.h"
37-
#include "soc/gpio_reg.h"
37+
#include "soc/gpio_struct.h"
3838

3939
#include "esp_idf_api/gpio.h"
4040

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

5656
static gpio_conf_t _gpio_conf[GPIO_PIN_NUMOF] = { };
5757

58-
const _esp32_port_t _esp32_ports[GPIO_PORT_NUMOF] = {
59-
{
60-
.out = (uint32_t *)GPIO_OUT_REG,
61-
.out_w1ts = (uint32_t *)GPIO_OUT_W1TS_REG,
62-
.out_w1tc = (uint32_t *)GPIO_OUT_W1TC_REG,
63-
.in = (uint32_t *)GPIO_IN_REG,
64-
.enable = (uint32_t *)GPIO_ENABLE_REG,
65-
.enable_w1ts = (uint32_t *)GPIO_ENABLE_W1TS_REG,
66-
.enable_w1tc = (uint32_t *)GPIO_ENABLE_W1TC_REG,
67-
.status_w1tc = (uint32_t *)GPIO_STATUS_W1TC_REG,
68-
},
69-
#if GPIO_PORT_NUMOF > 1
70-
{
71-
.out = (uint32_t *)GPIO_OUT1_REG,
72-
.out_w1ts = (uint32_t *)GPIO_OUT1_W1TS_REG,
73-
.out_w1tc = (uint32_t *)GPIO_OUT1_W1TC_REG,
74-
.in = (uint32_t *)GPIO_IN1_REG,
75-
.enable = (uint32_t *)GPIO_ENABLE1_REG,
76-
.enable_w1ts = (uint32_t *)GPIO_ENABLE1_W1TS_REG,
77-
.enable_w1tc = (uint32_t *)GPIO_ENABLE1_W1TC_REG,
78-
.status_w1tc = (uint32_t *)GPIO_STATUS1_W1TC_REG,
79-
}
80-
#endif
81-
};
82-
8358
int gpio_ll_init(gpio_port_t port, uint8_t pin, gpio_conf_t conf)
8459
{
85-
assert(port);
86-
assert(GPIO_PORT_NUM(port) < GPIO_PORT_NUMOF);
60+
assert(is_gpio_port_num_valid(port));
8761
assert(pin < GPIO_PORT_PIN_NUMOF(port));
8862

8963
gpio_t gpio = GPIO_PIN(GPIO_PORT_NUM(port), pin);

‎cpu/esp32/periph/gpio_ll_irq.c‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,8 @@ extern void IRAM gpio_int_handler(void* arg);
5555
int gpio_ll_irq(gpio_port_t port, uint8_t pin, gpio_irq_trig_t trig,
5656
gpio_ll_cb_t cb, void *arg)
5757
{
58-
assert(port);
58+
assert(is_gpio_port_num_valid(port));
5959
assert(arg);
60-
assert(GPIO_PORT_NUM(port) < GPIO_PORT_NUMOF);
6160
assert(pin < GPIO_PORT_PIN_NUMOF(port));
6261

6362
unsigned state = irq_disable();
@@ -138,13 +137,20 @@ void gpio_ll_irq_unmask(gpio_port_t port, uint8_t pin)
138137

139138
void gpio_ll_irq_unmask_and_clear(gpio_port_t port, uint8_t pin)
140139
{
141-
_esp32_port_t *p = (_esp32_port_t *)port;
142140
gpio_t gpio = GPIO_PIN(GPIO_PORT_NUM(port), pin);
143141

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

147-
*p->status_w1tc = BIT(pin);
145+
volatile uint32_t *status_w1tc = (uint32_t *)GPIO_STATUS_W1TC_REG;
146+
#if GPIO_PORT_NUMOF > 1
147+
if (GPIO_PORT_NUM(port) != 0) {
148+
status_w1tc = (uint32_t *)GPIO_STATUS1_W1TC_REG;
149+
}
150+
#endif
151+
152+
*status_w1tc = BIT(pin);
153+
148154
if (esp_idf_gpio_intr_enable(gpio) == ESP_OK) {
149155
gpio_int_enabled_table[gpio] = true;
150156
}

0 commit comments

Comments
 (0)