Skip to content

periph/gpio: support for extension API #12333

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
gschorcht wants to merge 11 commits into from
Closed

periph/gpio: support for extension API #12333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
377d838
periph/gpio: initial GPIO extension API
gschorcht Sep 28, 2019
9934c86
cpu/*: changes to make cppcheck happy
gschorcht Sep 30, 2019
9a95ef9
cpu/*: changes for GPIO extension API
gschorcht Sep 28, 2019
af28d82
cpu/atmega_common: change gpio_t size
gschorcht Sep 28, 2019
04b6405
drivers/extend: initial GPIO extension API
gschorcht Sep 28, 2019
cf2714c
drivers/extend: initial doc
gschorcht Sep 28, 2019
54128bf
drivers/extend: GPIO Makefile support
gschorcht Sep 28, 2019
160ff7e
tests/periph_gpio_coll: test for collission
gschorcht Sep 28, 2019
7d28f3e
tests/extend_gpio: GPIO extension test
gschorcht Sep 28, 2019
edc275a
fixup! periph/gpio: initial GPIO extension API
gschorcht Nov 4, 2019
174bfa7
fixup! drivers/extend: initial GPIO extension API
gschorcht Nov 4, 2019
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
4 changes: 2 additions & 2 deletions boards/common/esp8266/include/periph_conf_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ extern "C" {
#define SPI0_SCK_GPIO GPIO14 /**< HSPI / SPI_DEV(0) SCK pin */

#ifndef SPI0_CS0_GPIO
#define SPI0_CS0_GPIO GPIO15 /**< HSPI / SPI_DEV(0) CS default pin, only used when cs
parameter in spi_acquire is GPIO_UNDEF */
#define SPI0_CS0_GPIO GPIO15 /**< HSPI / SPI_DEV(0) CS default pin, only used when cs
parameter in spi_acquire is GPIO_UNDEF */

@maribu maribu Oct 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unrelated. (If you split this up into a separate PR, I'll ACK and merge that right away.)

#endif
#endif /* defined(MODULE_PERIPH_SPI) || defined(DOXYGEN) */
/** @} */
Expand Down
6 changes: 3 additions & 3 deletions cpu/atmega_common/include/periph_cpu_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ extern "C" {
* @{
*/
#define HAVE_GPIO_T
typedef uint8_t gpio_t;
typedef uint16_t gpio_t;

@maribu maribu Oct 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is going to have a performance penalty, as the ATmegas are 8bit. I think the defines in gpio_ext_h already are using sizeof(gpio_t), so would it be possible to keep that uint8_t for 8 bit platforms? (If the cost is that only one GPIO extender could be used there, I bet that users of the ATmegas would take that trade-off. And if not, they could use CFLAGS += -include <header_with_custom_gpio_t_definition.h> -DHAVE_GPIO_T to use more GPIO extenders.)

But if this increases maintenance effort and/or lines of code more than a bit, I'd say using 16bit on ATmesags is fine. (The Arduino community are also using int (=16 bit) for digitalRead() and digitalWrite() - likely due to not paying attention when defining the API. And their user base seems to be not upset with that.)

@gschorcht gschorcht Oct 14, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I know. But ATmega 2560 defines 10 ports and GPIO_CPU_PIN(x, y) is defined by ((x << 4) | y). That is, with uint8_t there is no space left for the extension flag.

@maribu maribu Oct 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe then 8 bit unless ATmega2560 is used?

@gschorcht gschorcht Oct 14, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be possible in that way.

/** @} */
#endif

/**
* @brief Definition of a fitting UNDEF value
*/
#define GPIO_UNDEF (0xff)
#define GPIO_UNDEF (0xffff)

/**
* @brief Define a CPU specific GPIO pin generator macro
*/
#define GPIO_PIN(x, y) ((x << 4) | y)
#define GPIO_PIN(x, y) ((x << 4) | y)

/**
* @brief Override the GPIO flanks
Expand Down
34 changes: 17 additions & 17 deletions cpu/atmega_common/periph/gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ static inline uint16_t _pin_addr(gpio_t pin)
return (_port_addr(pin) - 0x02);
}

int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_cpu_init(gpio_t pin, gpio_mode_t mode)
{
uint8_t pin_mask = (1 << _pin_num(pin));

Expand All @@ -241,38 +241,38 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
return 0;
}

int gpio_read(gpio_t pin)
int gpio_cpu_read(gpio_t pin)
{
return (_SFR_MEM8(_pin_addr(pin)) & (1 << _pin_num(pin)));
}

void gpio_set(gpio_t pin)
void gpio_cpu_set(gpio_t pin)
{
_SFR_MEM8(_port_addr(pin)) |= (1 << _pin_num(pin));
}

void gpio_clear(gpio_t pin)
void gpio_cpu_clear(gpio_t pin)
{
_SFR_MEM8(_port_addr(pin)) &= ~(1 << _pin_num(pin));
}

void gpio_toggle(gpio_t pin)
void gpio_cpu_toggle(gpio_t pin)
{
if (gpio_read(pin)) {
gpio_clear(pin);
if (gpio_cpu_read(pin)) {
gpio_cpu_clear(pin);
}
else {
gpio_set(pin);
gpio_cpu_set(pin);
}
}
Comment thread
maribu marked this conversation as resolved.

void gpio_write(gpio_t pin, int value)
void gpio_cpu_write(gpio_t pin, int value)
{
if (value) {
gpio_set(pin);
gpio_cpu_set(pin);
}
else {
gpio_clear(pin);
gpio_cpu_clear(pin);
}
}

Expand All @@ -292,8 +292,8 @@ static inline int8_t _int_num(gpio_t pin)
return -1;
}

int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
int gpio_cpu_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
{
int8_t int_num = _int_num(pin);

Expand Down Expand Up @@ -332,7 +332,7 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
pcint_config[offset].cb = cb;

/* init gpio */
gpio_init(pin, mode);
gpio_cpu_init(pin, mode);
/* configure pcint */
cli();
switch (bank) {
Expand Down Expand Up @@ -385,7 +385,7 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
return -1;
}

gpio_init(pin, mode);
gpio_cpu_init(pin, mode);

/* clear global interrupt flag */
cli();
Expand Down Expand Up @@ -416,13 +416,13 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
return 0;
}

void gpio_irq_enable(gpio_t pin)
void gpio_cpu_irq_enable(gpio_t pin)
{
EIFR |= (1 << _int_num(pin));
EIMSK |= (1 << _int_num(pin));
}

void gpio_irq_disable(gpio_t pin)
void gpio_cpu_irq_disable(gpio_t pin)
{
EIMSK &= ~(1 << _int_num(pin));
}
Expand Down
22 changes: 11 additions & 11 deletions cpu/cc2538/periph/gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static inline uint8_t _pp_num(gpio_t pin)
return (uint8_t)((_port_num(pin) * GPIO_BITS_PER_PORT) + _pin_num(pin));
}

int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_cpu_init(gpio_t pin, gpio_mode_t mode)
{
/* check if mode is valid */
if (mode == MODE_NOTSUP) {
Expand All @@ -120,27 +120,27 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
return 0;
}

int gpio_read(gpio_t pin)
int gpio_cpu_read(gpio_t pin)
{
return (int)(gpio(pin)->DATA & _pin_mask(pin));
}

void gpio_set(gpio_t pin)
void gpio_cpu_set(gpio_t pin)
{
gpio(pin)->DATA |= _pin_mask(pin);
}

void gpio_clear(gpio_t pin)
void gpio_cpu_clear(gpio_t pin)
{
gpio(pin)->DATA &= ~_pin_mask(pin);
}

void gpio_toggle(gpio_t pin)
void gpio_cpu_toggle(gpio_t pin)
{
gpio(pin)->DATA ^= _pin_mask(pin);
}

void gpio_write(gpio_t pin, int value)
void gpio_cpu_write(gpio_t pin, int value)
{
if (value) {
gpio(pin)->DATA |= _pin_mask(pin);
Expand All @@ -151,10 +151,10 @@ void gpio_write(gpio_t pin, int value)
}

#ifdef MODULE_PERIPH_GPIO_IRQ
int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
int gpio_cpu_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
{
if (gpio_init(pin, mode) != 0) {
if (gpio_cpu_init(pin, mode) != 0) {
return -1;
}

Expand Down Expand Up @@ -196,12 +196,12 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
return 0;
}

void gpio_irq_enable(gpio_t pin)
void gpio_cpu_irq_enable(gpio_t pin)
{
gpio(pin)->IE |= _pin_mask(pin);
}

void gpio_irq_disable(gpio_t pin)
void gpio_cpu_irq_disable(gpio_t pin)
{
gpio(pin)->IE &= ~_pin_mask(pin);
}
Expand Down
28 changes: 14 additions & 14 deletions cpu/cc26x0/periph/gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
static gpio_isr_ctx_t gpio_chan[GPIO_ISR_CHAN_NUMOF];
#endif /* MODULE_PERIPH_GPIO_IRQ */

int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_cpu_init(gpio_t pin, gpio_mode_t mode)
{
if ((unsigned int)pin > 31)
return -1;
Expand All @@ -53,7 +53,7 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
return 0;
}

int gpio_read(gpio_t pin)
int gpio_cpu_read(gpio_t pin)
{
if (GPIO->DOE & (1 << pin)) {
return (GPIO->DOUT >> pin) & 1;
Expand All @@ -63,22 +63,22 @@ int gpio_read(gpio_t pin)
}
}

void gpio_set(gpio_t pin)
void gpio_cpu_set(gpio_t pin)
{
GPIO->DOUTSET = (1 << pin);
}

void gpio_clear(gpio_t pin)
void gpio_cpu_clear(gpio_t pin)
{
GPIO->DOUTCLR = (1 << pin);
}

void gpio_toggle(gpio_t pin)
void gpio_cpu_toggle(gpio_t pin)
{
GPIO->DOUTTGL = (1 << pin);
}

void gpio_write(gpio_t pin, int value)
void gpio_cpu_write(gpio_t pin, int value)
{
if (value) {
GPIO->DOUTSET = (1 << pin);
Expand All @@ -88,10 +88,10 @@ void gpio_write(gpio_t pin, int value)
}

#ifdef MODULE_PERIPH_GPIO_IRQ
int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
int gpio_cpu_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
{
int init = gpio_init(pin, mode);
int init = gpio_cpu_init(pin, mode);
if (init != 0)
return init;

Expand All @@ -105,17 +105,17 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
/* clears the interrupt flag */
GPIO->EVFLAGS |= (1 << pin);

gpio_irq_enable(pin);
gpio_cpu_irq_enable(pin);

return 0;
}

void gpio_irq_enable(gpio_t pin)
void gpio_cpu_irq_enable(gpio_t pin)
{
IOC->CFG[pin] |= IOCFG_EDGEIRQ_ENABLE;
}

void gpio_irq_disable(gpio_t pin)
void gpio_cpu_irq_disable(gpio_t pin)
{
IOC->CFG[pin] &= ~IOCFG_EDGEIRQ_ENABLE;
}
Expand All @@ -124,8 +124,8 @@ void isr_edge(void)
{
for (unsigned pin = 0; pin < GPIO_ISR_CHAN_NUMOF; pin++) {
/* doc claims EVFLAGS will only be set for pins that have edge detection enabled */
if (GPIO->EVFLAGS & (1 << pin)) {
GPIO->EVFLAGS |= (1 << pin);
if (GPIO->EVFLAGS & (1U << pin)) {
GPIO->EVFLAGS |= (1U << pin);
gpio_chan[pin].cb(gpio_chan[pin].arg);
}
}
Expand Down
22 changes: 11 additions & 11 deletions cpu/efm32/periph/gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ static inline uint32_t _pin_num(gpio_t pin)
return (pin & 0x0f);
}

int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_cpu_init(gpio_t pin, gpio_mode_t mode)
{
/* check for valid pin */
if (pin == GPIO_UNDEF) {
Expand All @@ -69,27 +69,27 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
return 0;
}

int gpio_read(gpio_t pin)
int gpio_cpu_read(gpio_t pin)
{
return GPIO_PinInGet(_port_num(pin), _pin_num(pin));
}

void gpio_set(gpio_t pin)
void gpio_cpu_set(gpio_t pin)
{
GPIO_PinOutSet(_port_num(pin), _pin_num(pin));
}

void gpio_clear(gpio_t pin)
void gpio_cpu_clear(gpio_t pin)
{
GPIO_PinOutClear(_port_num(pin), _pin_num(pin));
}

void gpio_toggle(gpio_t pin)
void gpio_cpu_toggle(gpio_t pin)
{
GPIO_PinOutToggle(_port_num(pin), _pin_num(pin));
}

void gpio_write(gpio_t pin, int value)
void gpio_cpu_write(gpio_t pin, int value)
{
if (value) {
GPIO_PinOutSet(_port_num(pin), _pin_num(pin));
Expand All @@ -105,10 +105,10 @@ static inline uint32_t _pin_mask(gpio_t pin)
return (1 << _pin_num(pin));
}

int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
int gpio_cpu_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
gpio_cb_t cb, void *arg)
{
int result = gpio_init(pin, mode);
int result = gpio_cpu_init(pin, mode);

if (result != 0) {
return result;
Expand All @@ -134,12 +134,12 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
return 0;
}

void gpio_irq_enable(gpio_t pin)
void gpio_cpu_irq_enable(gpio_t pin)
{
GPIO_IntEnable(_pin_mask(pin));
}

void gpio_irq_disable(gpio_t pin)
void gpio_cpu_irq_disable(gpio_t pin)
{
GPIO_IntDisable(_pin_mask(pin));
}
Expand Down
Loading