Skip to content

[TOPIC-GPIO] proposed workaround to distinguish interrupt disable from default no-change #19553

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

Merged
merged 2 commits into from
Oct 25, 2019
Merged

[TOPIC-GPIO] proposed workaround to distinguish interrupt disable from default no-change #19553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f1ac09
gpio: unify assert checks for pin and interrupt configuration
pabigot Oct 2, 2019
3f74359
gpio: avoid disabling interrupts when supporting legacy code
pabigot Oct 2, 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
32 changes: 21 additions & 11 deletions include/drivers/gpio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,19 @@ extern "C" {
*/

/** Disables GPIO pin interrupt. */
#define GPIO_INT_DISABLE (0U << 12)
#define GPIO_INT_DISABLE (1U << 12)

/** @cond INTERNAL_HIDDEN */

/* Enables GPIO pin interrupt. */
#define GPIO_INT_ENABLE (1U << 12)
#define GPIO_INT_ENABLE (1U << 13)

/* GPIO interrupt is sensitive to logical levels.
*
* This is a component flag that should be combined with other
* `GPIO_INT_*` flags to produce a meaningful configuration.
*/
#define GPIO_INT_LEVELS_LOGICAL (1U << 13)
#define GPIO_INT_LEVELS_LOGICAL (1U << 14)

/* GPIO interrupt is edge sensitive.
*
Expand All @@ -97,23 +97,23 @@ extern "C" {
* This is a component flag that should be combined with other
* `GPIO_INT_*` flags to produce a meaningful configuration.
*/
#define GPIO_INT_EDGE (1U << 14)
#define GPIO_INT_EDGE (1U << 15)

/* Trigger detection when input state is (or transitions to) physical low or
* logical 0 level.
*
* This is a component flag that should be combined with other
* `GPIO_INT_*` flags to produce a meaningful configuration.
*/
#define GPIO_INT_LOW_0 (1U << 15)
#define GPIO_INT_LOW_0 (1U << 16)

/* Trigger detection on input state is (or transitions to) physical high or
* logical 1 level.
*
* This is a component flag that should be combined with other
* `GPIO_INT_*` flags to produce a meaningful configuration.
*/
#define GPIO_INT_HIGH_1 (1U << 16)
#define GPIO_INT_HIGH_1 (1U << 17)

/** @endcond */

Expand Down Expand Up @@ -187,7 +187,7 @@ extern "C" {
* @note Drivers that do not support a debounce feature should ignore
* this flag rather than rejecting the configuration with -ENOTSUP.
*/
#define GPIO_INT_DEBOUNCE (1U << 17)
#define GPIO_INT_DEBOUNCE (1U << 18)

/**
* @name GPIO drive strength flags
Expand All @@ -212,7 +212,7 @@ extern "C" {
* @{
*/
/** @cond INTERNAL_HIDDEN */
#define GPIO_DS_LOW_POS 18
#define GPIO_DS_LOW_POS 19
#define GPIO_DS_LOW_MASK (0x3U << GPIO_DS_LOW_POS)
/** @endcond */

Expand All @@ -227,7 +227,7 @@ extern "C" {
#define GPIO_DS_ALT_LOW (0x1U << GPIO_DS_LOW_POS)

/** @cond INTERNAL_HIDDEN */
#define GPIO_DS_HIGH_POS 20
#define GPIO_DS_HIGH_POS 21
#define GPIO_DS_HIGH_MASK (0x3U << GPIO_DS_HIGH_POS)
/** @endcond */

Expand Down Expand Up @@ -611,6 +611,13 @@ static inline int z_impl_gpio_pin_interrupt_configure(struct device *port,

__ASSERT_NO_MSG((flags & GPIO_INT_DEBOUNCE) == 0);

__ASSERT((flags & (GPIO_INT_DISABLE | GPIO_INT_ENABLE))
!= (GPIO_INT_DISABLE | GPIO_INT_ENABLE),
"Cannot both enable and disable interrupts");

__ASSERT((flags & (GPIO_INT_DISABLE | GPIO_INT_ENABLE)) != 0U,
"Must either enable or disable interrupts");

__ASSERT(((flags & GPIO_INT_ENABLE) == 0) ||
((flags & GPIO_INT_EDGE) != 0) ||
((flags & (GPIO_INT_LOW_0 | GPIO_INT_HIGH_1)) !=
Expand All @@ -630,7 +637,7 @@ static inline int z_impl_gpio_pin_interrupt_configure(struct device *port,
}

trig = (enum gpio_int_trig)(flags & (GPIO_INT_LOW_0 | GPIO_INT_HIGH_1));
mode = (enum gpio_int_mode)(flags & (GPIO_INT_EDGE | GPIO_INT_ENABLE));
mode = (enum gpio_int_mode)(flags & (GPIO_INT_EDGE | GPIO_INT_DISABLE | GPIO_INT_ENABLE));

return api->pin_interrupt_configure(port, pin, mode, trig);
}
Expand Down Expand Up @@ -660,6 +667,8 @@ static inline int gpio_pin_configure(struct device *port, u32_t pin,
(struct gpio_driver_data *)port->driver_data;
int ret;

__ASSERT(pin < GPIO_MAX_PINS_PER_PORT, "Invalid pin number");

__ASSERT((flags & (GPIO_PULL_UP | GPIO_PULL_DOWN)) !=
(GPIO_PULL_UP | GPIO_PULL_DOWN),
"Pull Up and Pull Down should not be enabled simultaneously");
Expand All @@ -685,7 +694,8 @@ static inline int gpio_pin_configure(struct device *port, u32_t pin,
} else {
data->invert &= ~BIT(pin);
}
if (api->pin_interrupt_configure) {
if (((flags & (GPIO_INT_DISABLE | GPIO_INT_ENABLE)) != 0U)
&& (api->pin_interrupt_configure != NULL)) {
flags &= ~GPIO_INT_DEBOUNCE;
ret = z_impl_gpio_pin_interrupt_configure(port, pin, flags);
}
Expand Down
4 changes: 2 additions & 2 deletions include/dt-bindings/gpio/gpio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@
#define GPIO_DIR_OUT (1 << 9) /* GPIO_OUTPUT */
#define GPIO_PUD_PULL_UP GPIO_PULL_UP
#define GPIO_PUD_PULL_DOWN GPIO_PULL_DOWN
#define GPIO_INT_ACTIVE_LOW (1 << 15) /* GPIO_INT_LOW_0 */
#define GPIO_INT_ACTIVE_HIGH (1 << 16) /* GPIO_INT_HIGH_1 */
#define GPIO_INT_ACTIVE_LOW (1 << 16) /* GPIO_INT_LOW_0 */
#define GPIO_INT_ACTIVE_HIGH (1 << 17) /* GPIO_INT_HIGH_1 */
/** @endcond */

/**
Expand Down