Skip to content

api: gpio: Clean up GPIO dt-bindings flags #12651

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

api: gpio: Clean up GPIO dt-bindings flags #12651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
24a785b
drivers: gpio: fix misuse of u8_t where bool is intended
pabigot Dec 5, 2018
05344a0
api: gpio: Clean up GPIO dt-bindings flags
mnkp Jan 22, 2019
d026c6e
boards: update .dts files to use new gpio.h bindings
mnkp Jan 28, 2019
70fbbe9
samples: blinky: Add support for DT GPIO flags
mnkp Jan 28, 2019
5877649
drivers: gpio_gecko: update to use new GPIO flags
mnkp Jan 28, 2019
c19c884
drivers: gpio_sam: update to use new GPIO flags
mnkp Jan 29, 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
8 changes: 4 additions & 4 deletions boards/arm/efr32_slwstk6061a/efr32_slwstk6061a.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@
leds {
compatible = "gpio-leds";
led0: led_0 {
gpios = <&gpiof 4 0>;
gpios = <&gpiof 4 GPIO_ACTIVE_HIGH>;
label = "LED 0";
};
led1: led_1 {
gpios = <&gpiof 5 0>;
gpios = <&gpiof 5 GPIO_ACTIVE_HIGH>;
label = "LED 1";
};
};
Expand All @@ -42,12 +42,12 @@
compatible = "gpio-keys";
button0: button_0 {
/* gpio flags need validation */
gpios = <&gpiof 6 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpiof 6 GPIO_INT_LOW>;
Copy link
Member

@anangl anangl Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Wasn't the GPIO_ACTIVE_LOW flag supposed to be used also to specify active state of inputs? One may not want to use interrupts to handle buttons. Sometimes just reading the current state of the button could be desired.

Copy link
Member Author

@mnkp mnkp Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed. I will change this line to

	gpios = <&gpiof 6 (GPIO_ACTIVE_LOW | GPIO_INT_LOW)>;

label = "User Push Button 0";
};
button1: button_1 {
/* gpio flags need validation */
gpios = <&gpiof 7 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpiof 7 GPIO_INT_LOW>;
label = "User Push Button 1";
};
};
Expand Down
5 changes: 2 additions & 3 deletions boards/arm/sam_e70_xplained/sam_e70_xplained.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
leds {
compatible = "gpio-leds";
green_led: led_0 {
gpios = <&portc 8 0>;
gpios = <&portc 8 GPIO_ACTIVE_LOW>;
label = "User LED";
};
};
Expand All @@ -45,8 +45,7 @@
*/
sw0_user_button: button_1 {
label = "User Button";
gpios = <&porta 11 (GPIO_PUD_PULL_UP |
GPIO_INT_ACTIVE_LOW)>;
gpios = <&porta 11 (GPIO_PUD_PULL_UP | GPIO_INT_LOW)>;
};
};
};
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpio/gpio_dw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static inline void dw_interrupt_config(struct device *port, int access_op,
struct gpio_dw_runtime *context = port->driver_data;
const struct gpio_dw_config *config = port->config->config_info;
u32_t base_addr = dw_base_to_block_base(context->base_addr);
u8_t flag_is_set;
bool flag_is_set;

ARG_UNUSED(access_op);

Expand Down
50 changes: 38 additions & 12 deletions drivers/gpio/gpio_gecko.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ struct gpio_gecko_data {
sys_slist_t callbacks;
/* pin callback routine enable flags, by pin number */
u32_t pin_callback_enables;
u32_t invert;
};

static inline void gpio_gecko_add_port(struct gpio_gecko_common_data *data,
Expand All @@ -73,25 +74,38 @@ static int gpio_gecko_configure(struct device *dev,
const struct gpio_gecko_config *config = dev->config->config_info;
GPIO_P_TypeDef *gpio_base = config->gpio_base;
GPIO_Port_TypeDef gpio_index = config->gpio_index;
struct gpio_gecko_data *data = dev->driver_data;
GPIO_Mode_TypeDef mode;
unsigned int out = 0U;

/* Check for an invalid pin configuration */
if ((flags & GPIO_INT) && (flags & GPIO_DIR_OUT)) {
if ((flags & GPIO_INT_ENABLE) && (flags & GPIO_OUTPUT_ENABLE)) {
return -EINVAL;
}

/* Interrupt on static level is not supported by the hardware */
if ((flags & GPIO_INT) && !(flags & GPIO_INT_EDGE)) {
if ((flags & GPIO_INT_ENABLE) && !(flags & GPIO_INT_EDGE)) {
return -ENOTSUP;
}

/* Setting interrupt flags for a complete port is not implemented */
if ((flags & GPIO_INT) && (access_op == GPIO_ACCESS_BY_PORT)) {
if ((flags & GPIO_INT_ENABLE) && (access_op == GPIO_ACCESS_BY_PORT)) {
return -ENOTSUP;
}

if ((flags & GPIO_DIR_MASK) == GPIO_DIR_IN) {
if (flags & GPIO_OUTPUT_ENABLE) {
/* Following modes enable both output and input */
if (flags & GPIO_SINGLE_ENDED) {
if (flags & GPIO_LINE_OPEN_DRAIN) {
mode = gpioModeWiredAnd;
} else {
mode = gpioModeWiredOr;
}
} else {
mode = gpioModePushPull;
}
out = (flags & GPIO_OUTPUT_INIT_HIGH) ? 1U : 0U;
} else if (flags & GPIO_INPUT_ENABLE) {
if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_UP) {
mode = gpioModeInputPull;
out = 1U; /* pull-up*/
Expand All @@ -101,8 +115,9 @@ static int gpio_gecko_configure(struct device *dev,
} else {
mode = gpioModeInput;
}
} else { /* GPIO_DIR_OUT */
mode = gpioModePushPull;
} else {
/* Neither input nor output mode is selected */
mode = gpioModeDisabled;
}
/* The flags contain options that require touching registers in the
* GPIO module and the corresponding PORT module.
Expand All @@ -113,6 +128,12 @@ static int gpio_gecko_configure(struct device *dev,

if (access_op == GPIO_ACCESS_BY_PIN) {
GPIO_PinModeSet(gpio_index, pin, mode, out);
if (flags & GPIO_ACTIVE_LOW) {
data->invert |= BIT(pin);
} else {
data->invert &= ~BIT(pin);
}

} else { /* GPIO_ACCESS_BY_PORT */
gpio_base->MODEL = GECKO_GPIO_MODEL(7, mode)
| GECKO_GPIO_MODEL(6, mode) | GECKO_GPIO_MODEL(5, mode)
Expand All @@ -128,15 +149,14 @@ static int gpio_gecko_configure(struct device *dev,
| GECKO_GPIO_MODEH(9, mode)
| GECKO_GPIO_MODEH(8, mode);
gpio_base->DOUT = (out ? 0xFFFF : 0x0000);
data->invert = (flags & GPIO_ACTIVE_LOW ? 0xFFFF : 0x0000);
}

if (access_op == GPIO_ACCESS_BY_PIN) {
GPIO_IntConfig(gpio_index, pin,
(flags & GPIO_INT_ACTIVE_HIGH)
|| (flags & GPIO_INT_DOUBLE_EDGE),
!(flags & GPIO_INT_ACTIVE_HIGH)
|| (flags & GPIO_INT_DOUBLE_EDGE),
!!(flags & GPIO_INT));
flags & GPIO_INT_HIGH,
flags & GPIO_INT_LOW,
flags & GPIO_INT_ENABLE);
}

return 0;
Expand All @@ -147,8 +167,12 @@ static int gpio_gecko_write(struct device *dev,
{
const struct gpio_gecko_config *config = dev->config->config_info;
GPIO_P_TypeDef *gpio_base = config->gpio_base;
struct gpio_gecko_data *data = dev->driver_data;

if (access_op == GPIO_ACCESS_BY_PIN) {
if (data->invert & BIT(pin)) {
value = !value;
}
if (value) {
/* Set the data output for the corresponding pin.
* Writing zeros to the other bits leaves the data
Expand All @@ -164,7 +188,7 @@ static int gpio_gecko_write(struct device *dev,
}
} else { /* GPIO_ACCESS_BY_PORT */
/* Write the data output for all the pins */
gpio_base->DOUT = value;
gpio_base->DOUT = value ^ data->invert;
}

return 0;
Expand All @@ -175,8 +199,10 @@ static int gpio_gecko_read(struct device *dev,
{
const struct gpio_gecko_config *config = dev->config->config_info;
GPIO_P_TypeDef *gpio_base = config->gpio_base;
struct gpio_gecko_data *data = dev->driver_data;

*value = gpio_base->DIN;
*value ^= data->invert;

if (access_op == GPIO_ACCESS_BY_PIN) {
*value = (*value & BIT(pin)) >> pin;
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpio/gpio_qmsi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static void gpio_qmsi_callback(void *data, u32_t status)
}
}

static void qmsi_write_bit(u32_t *target, u8_t bit, u8_t value)
static void qmsi_write_bit(u32_t *target, u8_t bit, bool value)
{
if (value) {
sys_set_bit((uintptr_t) target, bit);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpio/gpio_qmsi_ss.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ static void ss_gpio_qmsi_callback(void *data, uint32_t status)
}
}

static void ss_qmsi_write_bit(u32_t *target, u8_t bit, u8_t value)
static void ss_qmsi_write_bit(u32_t *target, u8_t bit, bool value)
{
if (value) {
sys_set_bit((uintptr_t) target, bit);
Expand Down
59 changes: 51 additions & 8 deletions drivers/gpio/gpio_sam.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,61 @@ struct gpio_sam_config {
};

struct gpio_sam_runtime {
u32_t invert;
sys_slist_t cb;
};

#define DEV_CFG(dev) \
((const struct gpio_sam_config *const)(dev)->config->config_info)
#define DEV_DATA(dev) \
((struct gpio_sam_runtime *const)(dev)->driver_data)

static int gpio_sam_config_pin(Pio * const pio, u32_t mask, int flags)
static int gpio_sam_config_pin(struct device *dev, Pio * const pio, u32_t mask,
int flags)
{
struct gpio_sam_runtime *const dev_data = DEV_DATA(dev);

if (!(flags & (GPIO_OUTPUT_ENABLE | GPIO_INPUT_ENABLE))) {
/* Neither input nor output mode is selected */

/* Disable the interrupt. */
pio->PIO_IDR = mask;
/* Disable pull-up. */
pio->PIO_PUDR = mask;
/* Disable pull-down. */
pio->PIO_PPDDR = mask;
/* Let the PIO control the pin (instead of a peripheral). */
pio->PIO_PER = mask;

return 0;
}

/* Setup the pin direcion. */
if ((flags & GPIO_DIR_MASK) == GPIO_DIR_OUT) {
if (flags & GPIO_OUTPUT_ENABLE) {
if (flags & GPIO_OUTPUT_INIT_HIGH) {
/* Set the pin. */
pio->PIO_SODR = mask;
}
if (flags & GPIO_OUTPUT_INIT_LOW) {
/* Clear the pin. */
pio->PIO_CODR = mask;
}
pio->PIO_OER = mask;
} else {
pio->PIO_ODR = mask;
}

/* Note: Input is always enabled. */

if (flags & GPIO_ACTIVE_LOW) {
dev_data->invert |= mask;
} else {
dev_data->invert &= ~mask;
}

/* Setup interrupt configuration. */
if (flags & GPIO_INT) {
if (flags & GPIO_INT_DOUBLE_EDGE) {
if (flags & GPIO_INT_ENABLE) {
if ((flags & GPIO_INT_LOW) && (flags & GPIO_INT_HIGH)) {
return -ENOTSUP;
}

Expand All @@ -55,7 +92,7 @@ static int gpio_sam_config_pin(Pio * const pio, u32_t mask, int flags)
pio->PIO_LSR = mask;
}

if (flags & GPIO_INT_ACTIVE_HIGH) {
if (flags & GPIO_INT_HIGH) {
/* Set to high-level or rising edge. */
pio->PIO_REHLSR = mask;
} else {
Expand Down Expand Up @@ -116,11 +153,11 @@ static int gpio_sam_config(struct device *dev, int access_op, u32_t pin,

switch (access_op) {
case GPIO_ACCESS_BY_PIN:
gpio_sam_config_pin(pio, BIT(pin), flags);
gpio_sam_config_pin(dev, pio, BIT(pin), flags);
break;
case GPIO_ACCESS_BY_PORT:
for (i = 0; i < 32; i++) {
result = gpio_sam_config_pin(pio, BIT(i), flags);
result = gpio_sam_config_pin(dev, pio, BIT(i), flags);
if (result < 0) {
return result;
}
Expand All @@ -137,11 +174,15 @@ static int gpio_sam_write(struct device *dev, int access_op, u32_t pin,
u32_t value)
{
const struct gpio_sam_config * const cfg = DEV_CFG(dev);
struct gpio_sam_runtime *const dev_data = DEV_DATA(dev);
Pio *const pio = cfg->regs;
u32_t mask = 1 << pin;

switch (access_op) {
case GPIO_ACCESS_BY_PIN:
if (dev_data->invert & mask) {
value = !value;
}
if (value) {
/* Set the pin. */
pio->PIO_SODR = mask;
Expand All @@ -152,7 +193,7 @@ static int gpio_sam_write(struct device *dev, int access_op, u32_t pin,
break;
case GPIO_ACCESS_BY_PORT:
pio->PIO_OWER = pio->PIO_OSR; /* Write those out pin only */
pio->PIO_ODSR = value;
pio->PIO_ODSR = value ^ dev_data->invert;
pio->PIO_OWDR = 0xffffffff; /* Disable write ODSR */
break;
default:
Expand All @@ -166,9 +207,11 @@ static int gpio_sam_read(struct device *dev, int access_op, u32_t pin,
u32_t *value)
{
const struct gpio_sam_config * const cfg = DEV_CFG(dev);
struct gpio_sam_runtime *const dev_data = DEV_DATA(dev);
Pio *const pio = cfg->regs;

*value = pio->PIO_PDSR;
*value ^= dev_data->invert;

switch (access_op) {
case GPIO_ACCESS_BY_PIN:
Expand Down
Loading