From 846feb9545fa6426cf577c779bd4235d055f78d1 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Tue, 17 Sep 2019 08:42:29 -0500 Subject: [PATCH 01/17] FIXUP gpio.h: put back deprecation comments The comments identifying replacement API were inadvertently removed along with the flag that triggers deprecation warnings. Signed-off-by: Peter Bigot --- include/drivers/gpio.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/drivers/gpio.h b/include/drivers/gpio.h index 778b92a1a5dc3..42864c1e27075 100644 --- a/include/drivers/gpio.h +++ b/include/drivers/gpio.h @@ -960,6 +960,8 @@ static inline int gpio_pin_toggle(struct device *port, unsigned int pin) * @param pin Pin number where the data is written. * @param value Value set on the pin. * @return 0 if successful, negative errno code on failure. + * + * @deprecated Replace with gpio_pin_set_raw(). */ static inline int gpio_pin_write(struct device *port, u32_t pin, u32_t value) @@ -976,6 +978,8 @@ static inline int gpio_pin_write(struct device *port, u32_t pin, * @param pin Pin number where data is read. * @param value Integer pointer to receive the data values from the pin. * @return 0 if successful, negative errno code on failure. + * + * @deprecated Replace with gpio_pin_get_raw(). */ static inline int gpio_pin_read(struct device *port, u32_t pin, u32_t *value) @@ -1098,6 +1102,9 @@ static inline int gpio_remove_callback(struct device *port, * Note: Depending on the driver implementation, this function will enable * the pin to trigger an interruption. So as a semantic detail, if no * callback is registered, of course none will be called. + * + * @deprecated Replace with ``gpio_pin_interrupt_configure()`` with + * ``GPIO_INT_ENABLE``. */ static inline int gpio_pin_enable_callback(struct device *port, u32_t pin) { @@ -1109,6 +1116,9 @@ static inline int gpio_pin_enable_callback(struct device *port, u32_t pin) * @param port Pointer to the device structure for the driver instance. * @param pin Pin number where the callback function is disabled. * @return 0 if successful, negative errno code on failure. + * + * @deprecated Replace with ``gpio_pin_interrupt_configure()`` with + * ``GPIO_INT_DISABLE``. */ static inline int gpio_pin_disable_callback(struct device *port, u32_t pin) { From 7c2cb7231561304d91ee265d3299e348002ba6b8 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Tue, 17 Sep 2019 09:09:41 -0500 Subject: [PATCH 02/17] FIXUP gpio.h: add error return when blocking might occur External GPIO drivers may not be supported from interrupt context because they involve blocking bus transactions. Describe the return value for this situation, and add the I/O error to operations where it was missing. Signed-off-by: Peter Bigot --- include/drivers/gpio.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/include/drivers/gpio.h b/include/drivers/gpio.h index 42864c1e27075..059f7e4f8acb0 100644 --- a/include/drivers/gpio.h +++ b/include/drivers/gpio.h @@ -554,6 +554,8 @@ static inline int z_impl_gpio_disable_callback(struct device *port, * @retval 0 If successful. * @retval -ENOTSUP if any of the configuration options is not supported. * @retval -EINVAL Invalid argument. + * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_configure(struct device *port, u32_t pin, unsigned int flags) @@ -584,6 +586,7 @@ static inline int gpio_pin_configure(struct device *port, u32_t pin, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_port_get_raw(struct device *port, gpio_port_value_t *value); @@ -612,6 +615,7 @@ static inline int z_impl_gpio_port_get_raw(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_get(struct device *port, gpio_port_value_t *value) { @@ -642,6 +646,7 @@ static inline int gpio_port_get(struct device *port, gpio_port_value_t *value) * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_port_set_masked_raw(struct device *port, gpio_port_pins_t mask, gpio_port_value_t value); @@ -673,6 +678,7 @@ static inline int z_impl_gpio_port_set_masked_raw(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_set_masked(struct device *port, gpio_port_pins_t mask, gpio_port_value_t value) @@ -693,6 +699,7 @@ static inline int gpio_port_set_masked(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_port_set_bits_raw(struct device *port, gpio_port_pins_t pins); @@ -714,6 +721,7 @@ static inline int z_impl_gpio_port_set_bits_raw(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_set_bits(struct device *port, gpio_port_pins_t pins) { @@ -728,6 +736,7 @@ static inline int gpio_port_set_bits(struct device *port, gpio_port_pins_t pins) * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_port_clear_bits_raw(struct device *port, gpio_port_pins_t pins); @@ -749,6 +758,7 @@ static inline int z_impl_gpio_port_clear_bits_raw(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_clear_bits(struct device *port, gpio_port_pins_t pins) @@ -764,6 +774,7 @@ static inline int gpio_port_clear_bits(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_port_toggle_bits(struct device *port, gpio_port_pins_t pins); @@ -785,6 +796,7 @@ static inline int z_impl_gpio_port_toggle_bits(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_set_clr_bits_raw(struct device *port, gpio_port_pins_t set_pins, gpio_port_pins_t clear_pins) @@ -803,6 +815,7 @@ static inline int gpio_port_set_clr_bits_raw(struct device *port, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_port_set_clr_bits(struct device *port, gpio_port_pins_t set_pins, gpio_port_pins_t clear_pins) @@ -825,6 +838,7 @@ static inline int gpio_port_set_clr_bits(struct device *port, * @retval 1 If pin physical level is high. * @retval 0 If pin physical level is low. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_get_raw(struct device *port, unsigned int pin) { @@ -858,6 +872,7 @@ static inline int gpio_pin_get_raw(struct device *port, unsigned int pin) * @retval 1 If pin logical value is 1 / active. * @retval 0 If pin logical value is 0 / inactive. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_get(struct device *port, unsigned int pin) { @@ -887,6 +902,7 @@ static inline int gpio_pin_get(struct device *port, unsigned int pin) * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_set_raw(struct device *port, unsigned int pin, int value) @@ -923,6 +939,7 @@ static inline int gpio_pin_set_raw(struct device *port, unsigned int pin, * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_set(struct device *port, unsigned int pin, int value) { @@ -946,6 +963,7 @@ static inline int gpio_pin_set(struct device *port, unsigned int pin, int value) * * @retval 0 If successful. * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ static inline int gpio_pin_toggle(struct device *port, unsigned int pin) { @@ -1003,6 +1021,8 @@ static inline int gpio_pin_read(struct device *port, u32_t pin, * @retval -EINVAL Invalid argument. * @retval -EBUSY Interrupt line required to configure pin interrupt is * already in use. + * @retval -EIO I/O error when accessing an external GPIO chip. + * @retval -EWOULDBLOCK if operation would block. */ __syscall int gpio_pin_interrupt_configure(struct device *port, unsigned int pin, unsigned int flags); From 0e458c6cd9f3316e37ec13c5591ee31898d07cb3 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Mon, 26 Aug 2019 17:53:34 -0500 Subject: [PATCH 03/17] gpio: document GPIO flag values in table format Add a comment that shows the hexadecimal representation of each flag, to help a user who needs to decode a packed set of flags. Signed-off-by: Peter Bigot --- include/drivers/gpio.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/include/drivers/gpio.h b/include/drivers/gpio.h index 059f7e4f8acb0..9bbf0ce6dc4ec 100644 --- a/include/drivers/gpio.h +++ b/include/drivers/gpio.h @@ -38,6 +38,34 @@ extern "C" { * @{ */ +/* + * Assigned pin values in hexadecimal form, to assist in decoding flag + * values: + * + * FieldMask Bit(s) Role + * 0x 0000 0001 0 ACTIVE_HIGH + * 0x 0000 0002 1 SINGLE_ENDED + * 0x 0000 0004 2 OPEN_DRAIN + * 0x 0000 0008 3 unused + * 0x 0000 0010 4 PULL_UP + * 0x 0000 0020 5 PULL_DOWN + * 0x 0000 0040 6 unused + * 0x 0000 0080 7 unused + * 0x 0000 0100 8 INPUT + * 0x 0000 0200 9 OUTPUT + * 0x 0000 0400 10 OUTPUT_INIT_LOW + * 0x 0000 0800 11 OUTPUT_INIT_HIGH + * 0x 0000 1000 12 INT_ENABLE + * 0x 0000 2000 13 INT_LEVELS_LOGICAL + * 0x 0000 4000 14 INT_EDGE + * 0x 0000 8000 15 INT_LOW_0 + * 0x 0001 0000 16 INT_HIGH_1 + * 0x 0002 0000 17 INT_DEBOUNCE + * 0x 000C 0000 18..19 DS_LOW + * 0x 0030 0000 20..21 DS_LOW + * 0x FFC0 0000 22..31 unused + */ + /** Enables pin as input. */ #define GPIO_INPUT (1U << 8) From 4d87440c530199e3013b221190e8d4d89af9370e Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:05:18 -0500 Subject: [PATCH 04/17] drivers: gpio_sx1509b: update to use new GPIO API Update driver code and board files to use new GPIO configuration flags such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver API. Tested on external SX1509B breakout board and Thingy:52. Signed-off-by: Peter Bigot --- boards/arm/nrf52_pca20020/nrf52_pca20020.dts | 6 +- drivers/gpio/gpio_sx1509b.c | 521 +++++++++++-------- 2 files changed, 318 insertions(+), 209 deletions(-) diff --git a/boards/arm/nrf52_pca20020/nrf52_pca20020.dts b/boards/arm/nrf52_pca20020/nrf52_pca20020.dts index 7d4d65f6fc65f..b63256736af1e 100644 --- a/boards/arm/nrf52_pca20020/nrf52_pca20020.dts +++ b/boards/arm/nrf52_pca20020/nrf52_pca20020.dts @@ -34,15 +34,15 @@ compatible = "gpio-leds"; /* Lightwell RGB */ led0: led_0 { - gpios = <&sx1509b 5 0>; + gpios = <&sx1509b 5 GPIO_ACTIVE_LOW>; label = "Green LED"; }; led1: led_1 { - gpios = <&sx1509b 6 0>; + gpios = <&sx1509b 6 GPIO_ACTIVE_LOW>; label = "Blue LED"; }; led2: led_2 { - gpios = <&sx1509b 7 0>; + gpios = <&sx1509b 7 GPIO_ACTIVE_LOW>; label = "Red LED"; }; }; diff --git a/drivers/gpio/gpio_sx1509b.c b/drivers/gpio/gpio_sx1509b.c index cabde4fb16469..ac1ffffd27e9d 100644 --- a/drivers/gpio/gpio_sx1509b.c +++ b/drivers/gpio/gpio_sx1509b.c @@ -1,5 +1,7 @@ /* * Copyright (c) 2018 Aapo Vienamo + * Copyright (c) 2018 Peter Bigot Consulting, LLC + * Copyright (c) 2019 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,37 +11,37 @@ #include #include #include -#include -#include -#include -#include - -#ifdef CONFIG_HAS_DTS_I2C -#define CONFIG_GPIO_SX1509B_DEV_NAME DT_INST_0_SEMTECH_SX1509B_LABEL -#define CONFIG_GPIO_SX1509B_I2C_ADDR DT_INST_0_SEMTECH_SX1509B_BASE_ADDRESS -#define CONFIG_GPIO_SX1509B_I2C_MASTER_DEV_NAME DT_INST_0_SEMTECH_SX1509B_BUS_NAME -#endif - -/** Cache of the output configuration and data of the pins */ -struct gpio_sx1509b_pin_state { - u16_t input_disable; - u16_t pull_up; - u16_t pull_down; - u16_t open_drain; - u16_t polarity; - u16_t dir; - u16_t data; +#include +#include +#include +#include + +#include +LOG_MODULE_REGISTER(sx1509b, CONFIG_GPIO_LOG_LEVEL); + +/** Cache of the output configuration and data of the pins. */ +struct sx1509b_pin_state { + u16_t input_disable; /* 0x00 */ + u16_t long_slew; /* 0x02 */ + u16_t low_drive; /* 0x04 */ + u16_t pull_up; /* 0x06 */ + u16_t pull_down; /* 0x08 */ + u16_t open_drain; /* 0x0A */ + u16_t polarity; /* 0x0C */ + u16_t dir; /* 0x0E */ + u16_t data; /* 0x10 */ }; /** Runtime driver data */ -struct gpio_sx1509b_drv_data { +struct sx1509b_drv_data { + struct gpio_driver_data common; struct device *i2c_master; - struct gpio_sx1509b_pin_state pin_state; + struct sx1509b_pin_state pin_state; struct k_sem lock; }; /** Configuration data */ -struct gpio_sx1509b_config { +struct sx1509b_config { const char *i2c_master_dev_name; u16_t i2c_slave_addr; }; @@ -47,32 +49,32 @@ struct gpio_sx1509b_config { /* General configuration register addresses */ enum { /* TODO: Add rest of the regs */ - SX1509B_REG_CLOCK = 0x1e, - SX1509B_REG_RESET = 0x7d, + SX1509B_REG_CLOCK = 0x1e, + SX1509B_REG_RESET = 0x7d, }; /* Magic values for softreset */ enum { - SX1509B_REG_RESET_MAGIC0 = 0x12, - SX1509B_REG_RESET_MAGIC1 = 0x34, + SX1509B_REG_RESET_MAGIC0 = 0x12, + SX1509B_REG_RESET_MAGIC1 = 0x34, }; /* Register bits for SX1509B_REG_CLOCK */ enum { - SX1509B_REG_CLOCK_FOSC_OFF = 0 << 5, - SX1509B_REG_CLOCK_FOSC_EXT = 1 << 5, - SX1509B_REG_CLOCK_FOSC_INT_2MHZ = 2 << 5, + SX1509B_REG_CLOCK_FOSC_OFF = 0 << 5, + SX1509B_REG_CLOCK_FOSC_EXT = 1 << 5, + SX1509B_REG_CLOCK_FOSC_INT_2MHZ = 2 << 5, }; /* Pin configuration register addresses */ enum { - SX1509B_REG_INPUT_DISABLE = 0x00, - SX1509B_REG_PULL_UP = 0x06, - SX1509B_REG_PULL_DOWN = 0x08, - SX1509B_REG_OPEN_DRAIN = 0x0a, - SX1509B_REG_DIR = 0x0e, - SX1509B_REG_DATA = 0x10, - SX1509B_REG_LED_DRIVER_ENABLE = 0x20, + SX1509B_REG_INPUT_DISABLE = 0x00, + SX1509B_REG_PULL_UP = 0x06, + SX1509B_REG_PULL_DOWN = 0x08, + SX1509B_REG_OPEN_DRAIN = 0x0a, + SX1509B_REG_DIR = 0x0e, + SX1509B_REG_DATA = 0x10, + SX1509B_REG_LED_DRIVER_ENABLE = 0x20, }; /** @@ -94,153 +96,259 @@ static inline int i2c_reg_write_word_be(struct device *dev, u16_t dev_addr, return i2c_write(dev, tx_buf, 3, dev_addr); } -/** - * @brief Configure pin or port - * - * @param dev Device struct of the SX1509B - * @param access_op Access operation (pin or port) - * @param pin The pin number - * @param flags Flags of pin or port - * - * @return 0 if successful, failed otherwise - */ -static int gpio_sx1509b_config(struct device *dev, int access_op, u32_t pin, - int flags) +static int config(struct device *dev, + int access_op, /* unused */ + u32_t pin, + int flags) { - const struct gpio_sx1509b_config *cfg = dev->config->config_info; - struct gpio_sx1509b_drv_data *drv_data = dev->driver_data; - struct gpio_sx1509b_pin_state *pins = &drv_data->pin_state; - int ret = 0; + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; + struct sx1509b_pin_state *pins = &drv_data->pin_state; + + struct { + u8_t reg; + struct sx1509b_pin_state pins; + } __packed outbuf; + int rc = 0; + u16_t bit = BIT(pin); + bool data_first = false; + + /* Can't do I2C bus operations from an ISR */ + if (k_is_in_isr()) { + return -EWOULDBLOCK; + } - if (flags & GPIO_INT) { + /* Zephyr currently defines drive strength support based on + * the behavior and capabilities of the Nordic GPIO + * peripheral: strength defaults to low but can be set high, + * and is controlled independently for output levels. + * + * SX150x defaults to high strength, and does not support + * different strengths for different levels. + * + * Until something more general is available reject any + * attempt to set a non-default drive strength. + * + * This code is fragile because it has to assume the flags + * we're checking are the non-zero valued ones. Switching to + * mask-and-compare would be more robust, but requires adding + * more defines. + */ + if ((flags & (GPIO_DS_ALT_LOW | GPIO_DS_ALT_HIGH)) != 0) { return -ENOTSUP; } k_sem_take(&drv_data->lock, K_FOREVER); - switch (access_op) { - case GPIO_ACCESS_BY_PIN: - if ((flags & GPIO_DIR_MASK) == GPIO_DIR_IN) { - pins->dir |= BIT(pin); - pins->input_disable &= ~BIT(pin); - } else { - pins->dir &= ~BIT(pin); - pins->input_disable |= BIT(pin); - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_UP) { - pins->pull_up |= BIT(pin); - } else { - pins->pull_up &= ~BIT(pin); - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_DOWN) { - pins->pull_down |= BIT(pin); - } else { - pins->pull_down &= ~BIT(pin); - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_NORMAL) { - pins->pull_up &= ~BIT(pin); - pins->pull_down &= ~BIT(pin); - } - if (flags & GPIO_DS_DISCONNECT_HIGH) { - pins->open_drain |= BIT(pin); - } else { - pins->open_drain &= ~BIT(pin); - } - if (flags & GPIO_POL_INV) { - pins->polarity |= BIT(pin); - } else { - pins->polarity &= ~BIT(pin); - } - break; - case GPIO_ACCESS_BY_PORT: - if ((flags & GPIO_DIR_MASK) == GPIO_DIR_IN) { - pins->dir = 0xffff; - pins->input_disable = 0x0000; - } else { - pins->dir = 0x0000; - pins->input_disable = 0xffff; - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_UP) { - pins->pull_up = 0xffff; - } else { - pins->pull_up = 0x0000; - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_DOWN) { - pins->pull_down = 0xffff; - } else { - pins->pull_down = 0x0000; - } - if ((flags & GPIO_PUD_MASK) == GPIO_PUD_NORMAL) { - pins->pull_up = 0x0000; - pins->pull_down = 0x0000; - } - if (flags & GPIO_DS_DISCONNECT_HIGH) { - pins->open_drain = 0xffff; - } else { - pins->open_drain = 0x0000; - } - if (flags & GPIO_POL_INV) { - pins->polarity = 0xffff; + pins->open_drain &= ~bit; + if ((flags & GPIO_SINGLE_ENDED) != 0) { + if ((flags & GPIO_LINE_OPEN_DRAIN) != 0) { + pins->open_drain |= bit; } else { - pins->polarity = 0x0000; + /* Open source not supported */ + rc = -ENOTSUP; + goto out; } - break; + } + + switch (flags & (GPIO_PULL_UP | GPIO_PULL_DOWN)) { default: - ret = -ENOTSUP; + rc = -EINVAL; goto out; + case 0: + pins->pull_up &= ~bit; + pins->pull_down &= ~bit; + break; + case GPIO_PULL_UP: + pins->pull_up |= bit; + pins->pull_down &= ~bit; + break; + case GPIO_PULL_DOWN: + pins->pull_up &= ~bit; + pins->pull_down |= bit; + break; } - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_DIR, pins->dir); - if (ret) { - goto out; + if ((flags & GPIO_INPUT) != 0) { + pins->input_disable &= ~bit; + } else { + pins->input_disable |= bit; } - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_INPUT_DISABLE, - pins->input_disable); - if (ret) { - goto out; + if ((flags & GPIO_OUTPUT) != 0) { + pins->dir &= ~bit; + if ((flags & GPIO_OUTPUT_INIT_LOW) != 0) { + pins->data &= ~bit; + } else if ((flags & GPIO_OUTPUT_INIT_HIGH) != 0) { + pins->data |= bit; + } + } else { + pins->dir |= bit; } - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_PULL_UP, - pins->pull_up); - if (ret) { - goto out; + WRITE_BIT(drv_data->common.invert, pin, (flags & GPIO_ACTIVE_LOW) != 0); + + outbuf.reg = SX1509B_REG_INPUT_DISABLE; + outbuf.pins.input_disable = sys_cpu_to_be16(pins->input_disable); + outbuf.pins.long_slew = sys_cpu_to_be16(pins->long_slew); + outbuf.pins.low_drive = sys_cpu_to_be16(pins->low_drive); + outbuf.pins.pull_up = sys_cpu_to_be16(pins->pull_up); + outbuf.pins.pull_down = sys_cpu_to_be16(pins->pull_down); + outbuf.pins.open_drain = sys_cpu_to_be16(pins->open_drain); + outbuf.pins.polarity = sys_cpu_to_be16(pins->polarity); + outbuf.pins.dir = sys_cpu_to_be16(pins->dir); + outbuf.pins.data = sys_cpu_to_be16(pins->data); + + LOG_DBG("CFG %u %x : ID %04x ; PU %04x ; PD %04x ; DIR %04x ; DAT %04x", + pin, flags, + pins->input_disable, pins->pull_up, pins->pull_down, + pins->dir, pins->data); + if (data_first) { + rc = i2c_reg_write_word_be(drv_data->i2c_master, + cfg->i2c_slave_addr, + SX1509B_REG_DATA, pins->data); + if (rc == 0) { + rc = i2c_write(drv_data->i2c_master, + &outbuf.reg, + sizeof(outbuf) - sizeof(pins->data), + cfg->i2c_slave_addr); + } + } else { + rc = i2c_write(drv_data->i2c_master, + &outbuf.reg, + sizeof(outbuf), + cfg->i2c_slave_addr); } - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_PULL_DOWN, - pins->pull_down); - if (ret) { +out: + k_sem_give(&drv_data->lock); + return rc; +} + +static int port_get(struct device *dev, + gpio_port_value_t *value) +{ + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; + u16_t pin_data; + int rc = 0; + + /* Can't do I2C bus operations from an ISR */ + if (k_is_in_isr()) { + return -EWOULDBLOCK; + } + + k_sem_take(&drv_data->lock, K_FOREVER); + + u8_t cmd = SX1509B_REG_DATA; + + rc = i2c_write_read(drv_data->i2c_master, cfg->i2c_slave_addr, + &cmd, sizeof(cmd), + &pin_data, sizeof(pin_data)); + LOG_DBG("read %04x got %d", sys_be16_to_cpu(pin_data), rc); + if (rc != 0) { goto out; } - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_OPEN_DRAIN, - pins->open_drain); + *value = sys_be16_to_cpu(pin_data); out: k_sem_give(&drv_data->lock); - return ret; + return rc; } -/** - * @brief Set the pin or port output - * - * @param dev Device struct of the SX1509B - * @param access_op Access operation (pin or port) - * @param pin The pin number - * @param value Value to set (0 or 1) - * - * @return 0 if successful, failed otherwise - */ -static int gpio_sx1509b_write(struct device *dev, int access_op, u32_t pin, - u32_t value) +static int port_write_locked(struct device *dev, + gpio_port_pins_t mask, + gpio_port_value_t value) +{ + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; + u16_t *outp = &drv_data->pin_state.data; + u16_t out = *outp; + int rc = 0; + + out = (out & ~mask) | (u16_t)(value & mask); + rc = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, + SX1509B_REG_DATA, out); + LOG_DBG("write %04x msk %04x val %04x => %04x: %d", *outp, mask, value, out, rc); + if (rc == 0) { + *outp = out; + } + + return rc; +} + +static int port_set_masked(struct device *dev, + gpio_port_pins_t mask, + gpio_port_value_t value) +{ + struct sx1509b_drv_data *drv_data = dev->driver_data; + + /* Can't do I2C bus operations from an ISR */ + if (k_is_in_isr()) { + return -EWOULDBLOCK; + } + + if (mask & ~(gpio_port_pins_t)0xFFFF) { + return -EINVAL; + } + + k_sem_take(&drv_data->lock, K_FOREVER); + + int rc = port_write_locked(dev, mask, value); + + k_sem_give(&drv_data->lock); + return rc; +} + +static int port_set_bits(struct device *dev, + gpio_port_pins_t pins) { - const struct gpio_sx1509b_config *cfg = dev->config->config_info; - struct gpio_sx1509b_drv_data *drv_data = dev->driver_data; + return port_set_masked(dev, pins, pins); +} + +static int port_clear_bits(struct device *dev, + gpio_port_pins_t pins) +{ + return port_set_masked(dev, pins, 0); +} + +static int port_toggle_bits(struct device *dev, + gpio_port_pins_t pins) +{ + struct sx1509b_drv_data *drv_data = dev->driver_data; + + /* Can't do I2C bus operations from an ISR */ + if (k_is_in_isr()) { + return -EWOULDBLOCK; + } + + if (pins & ~(gpio_port_pins_t)0xFFFF) { + return -EINVAL; + } + + k_sem_take(&drv_data->lock, K_FOREVER); + + int rc = port_write_locked(dev, pins, + drv_data->pin_state.data ^ pins); + + k_sem_give(&drv_data->lock); + + return rc; +} + +static int pin_interrupt_configure(struct device *dev, + unsigned int pin, + unsigned int flags) +{ + return -ENOTSUP; +} + +static int sx1509b_write(struct device *dev, int access_op, u32_t pin, + u32_t value) +{ + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; u16_t *pin_data = &drv_data->pin_state.data; int ret = 0; @@ -269,21 +377,11 @@ static int gpio_sx1509b_write(struct device *dev, int access_op, u32_t pin, return ret; } -/** - * @brief Read the pin or port data - * - * @param dev Device struct of the SX1509B - * @param access_op Access operation (pin or port) - * @param pin The pin number - * @param value Value of input pin(s) - * - * @return 0 if successful, failed otherwise - */ -static int gpio_sx1509b_read(struct device *dev, int access_op, u32_t pin, - u32_t *value) +static int sx1509b_read(struct device *dev, int access_op, u32_t pin, + u32_t *value) { - const struct gpio_sx1509b_config *cfg = dev->config->config_info; - struct gpio_sx1509b_drv_data *drv_data = dev->driver_data; + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; u16_t pin_data; int ret; @@ -320,68 +418,79 @@ static int gpio_sx1509b_read(struct device *dev, int access_op, u32_t pin, * @param dev Device struct * @return 0 if successful, failed otherwise. */ -static int gpio_sx1509b_init(struct device *dev) +static int sx1509b_init(struct device *dev) { - const struct gpio_sx1509b_config *cfg = dev->config->config_info; - struct gpio_sx1509b_drv_data *drv_data = dev->driver_data; - int ret; + const struct sx1509b_config *cfg = dev->config->config_info; + struct sx1509b_drv_data *drv_data = dev->driver_data; + int rc; drv_data->i2c_master = device_get_binding(cfg->i2c_master_dev_name); if (!drv_data->i2c_master) { - ret = -EINVAL; + LOG_ERR("%s: no bus %s", dev->config->name, + cfg->i2c_master_dev_name); + rc = -EINVAL; goto out; } /* Reset state */ - drv_data->pin_state = (struct gpio_sx1509b_pin_state) { - .input_disable = 0x0000, - .pull_up = 0x0000, - .pull_down = 0x0000, - .open_drain = 0x0000, - .dir = 0xffff, - .data = 0xffff, + drv_data->pin_state = (struct sx1509b_pin_state) { + .dir = (u16_t)-1, + .data = (u16_t)-1, }; - ret = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_RESET, SX1509B_REG_RESET_MAGIC0); - if (ret) { + rc = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, + SX1509B_REG_RESET, SX1509B_REG_RESET_MAGIC0); + if (rc != 0) { + LOG_ERR("%s: reset m0 failed: %d\n", dev->config->name, rc); goto out; } - - ret = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_RESET, SX1509B_REG_RESET_MAGIC1); - if (ret) { + rc = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, + SX1509B_REG_RESET, SX1509B_REG_RESET_MAGIC1); + if (rc != 0) { goto out; } - ret = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_CLOCK, - SX1509B_REG_CLOCK_FOSC_INT_2MHZ); - if (ret) { + k_sleep(K_MSEC(3)); + + rc = i2c_reg_write_byte(drv_data->i2c_master, cfg->i2c_slave_addr, + SX1509B_REG_CLOCK, + SX1509B_REG_CLOCK_FOSC_INT_2MHZ); + if (rc != 0) { goto out; } out: + if (rc != 0) { + LOG_ERR("%s init failed: %d", dev->config->name, rc); + } else { + LOG_INF("%s init ok", dev->config->name); + } k_sem_give(&drv_data->lock); - return ret; + return rc; } -static const struct gpio_sx1509b_config gpio_sx1509b_cfg = { - .i2c_master_dev_name = CONFIG_GPIO_SX1509B_I2C_MASTER_DEV_NAME, - .i2c_slave_addr = CONFIG_GPIO_SX1509B_I2C_ADDR, +static const struct gpio_driver_api api_table = { + .config = config, + .write = sx1509b_write, + .read = sx1509b_read, + .port_get_raw = port_get, + .port_set_masked_raw = port_set_masked, + .port_set_bits_raw = port_set_bits, + .port_clear_bits_raw = port_clear_bits, + .port_toggle_bits = port_toggle_bits, + .pin_interrupt_configure = pin_interrupt_configure, }; -static struct gpio_sx1509b_drv_data gpio_sx1509b_drvdata = { - .lock = Z_SEM_INITIALIZER(gpio_sx1509b_drvdata.lock, 1, 1), +static const struct sx1509b_config sx1509b_cfg = { + .i2c_master_dev_name = DT_INST_0_SEMTECH_SX1509B_BUS_NAME, + .i2c_slave_addr = DT_INST_0_SEMTECH_SX1509B_BASE_ADDRESS, }; -static const struct gpio_driver_api gpio_sx1509b_drv_api_funcs = { - .config = gpio_sx1509b_config, - .write = gpio_sx1509b_write, - .read = gpio_sx1509b_read, +static struct sx1509b_drv_data sx1509b_drvdata = { + .lock = Z_SEM_INITIALIZER(sx1509b_drvdata.lock, 1, 1), }; -DEVICE_AND_API_INIT(gpio_sx1509b, CONFIG_GPIO_SX1509B_DEV_NAME, - gpio_sx1509b_init, &gpio_sx1509b_drvdata, &gpio_sx1509b_cfg, +DEVICE_AND_API_INIT(sx1509b, DT_INST_0_SEMTECH_SX1509B_LABEL, + sx1509b_init, &sx1509b_drvdata, &sx1509b_cfg, POST_KERNEL, CONFIG_GPIO_SX1509B_INIT_PRIORITY, - &gpio_sx1509b_drv_api_funcs); + &api_table); From 50f9dced97018db75abb49523381bb5f85a3b9b7 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Mon, 26 Aug 2019 14:57:59 -0500 Subject: [PATCH 05/17] test/drivers/gpio: add tests for new API Test that the new port API functions all behave as expected, including physical vs logical level for input and output as well as masked and set-based output operations. Also tests the new pin API functions. Signed-off-by: Peter Bigot --- CODEOWNERS | 1 + tests/drivers/gpio/gpio_basic_api/src/main.c | 1 + .../gpio/gpio_basic_api/src/test_gpio.h | 2 + .../gpio/gpio_basic_api/src/test_gpio_port.c | 483 ++++++++++++++++++ 4 files changed, 487 insertions(+) create mode 100644 tests/drivers/gpio/gpio_basic_api/src/test_gpio_port.c diff --git a/CODEOWNERS b/CODEOWNERS index b51287f409c05..1ea3c7f1cd4be 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -375,6 +375,7 @@ /tests/crypto/mbedtls/ @nashif @ceolin /tests/drivers/can/ @alexanderwachter /tests/drivers/flash_simulator/ @nvlsianpu +/tests/drivers/gpio/ @pabigot @mnkp /tests/drivers/hwinfo/ @alexanderwachter /tests/drivers/spi/ @tbursztyka /tests/drivers/uart/uart_async_api/ @Mierunski diff --git a/tests/drivers/gpio/gpio_basic_api/src/main.c b/tests/drivers/gpio/gpio_basic_api/src/main.c index 12c251b5b5aa8..3c7771df3c786 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/main.c +++ b/tests/drivers/gpio/gpio_basic_api/src/main.c @@ -10,6 +10,7 @@ void test_main(void) { ztest_test_suite(gpio_basic_test, + ztest_unit_test(test_gpio_port), ztest_unit_test(test_gpio_pin_read_write), ztest_unit_test(test_gpio_callback_edge_high), ztest_unit_test(test_gpio_callback_edge_low), diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h index 61b566fe12c4e..9a7fa1a345e26 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h @@ -43,4 +43,6 @@ void test_gpio_callback_add_remove(void); void test_gpio_callback_self_remove(void); void test_gpio_callback_enable_disable(void); +void test_gpio_port(void); + #endif /* __TEST_GPIO_H__ */ diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio_port.c b/tests/drivers/gpio/gpio_basic_api/src/test_gpio_port.c new file mode 100644 index 0000000000000..f7ed8360fbf4d --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio_port.c @@ -0,0 +1,483 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @addtogroup t_gpio_basic_api + * @{ + * @defgroup t_gpio_basic_read_write test_gpio_basic_read_write + * @brief TestPurpose: verify zephyr gpio read and write correctly + * @} + */ + +#include "test_gpio.h" + +#define ALL_BITS ((gpio_port_value_t)-1) + +static struct device *dev; + +/* Short-hand for a checked read of PIN_IN raw state */ +static bool raw_in(void) +{ + gpio_port_value_t v; + int rc = gpio_port_get_raw(dev, &v); + + zassert_equal(rc, 0, + "raw_in failed"); + return (v & BIT(PIN_IN)) ? true : false; +} + +/* Short-hand for a checked read of PIN_IN logical state */ +static bool logic_in(void) +{ + gpio_port_value_t v; + int rc = gpio_port_get(dev, &v); + + zassert_equal(rc, 0, + "raw_in failed"); + return (v & BIT(PIN_IN)) ? true : false; +} + +/* Short-hand for a checked write of PIN_OUT raw state */ +static void raw_out(bool set) +{ + int rc; + + if (set) { + rc = gpio_port_set_bits_raw(dev, BIT(PIN_OUT)); + } else { + rc = gpio_port_clear_bits_raw(dev, BIT(PIN_OUT)); + } + zassert_equal(rc, 0, + "raw_out failed"); +} + +/* Verify device, configure for physical in and out, verify + * connection, verify raw_in(). + */ +static int setup(void) +{ + int rc; + gpio_port_value_t v1; + + TC_PRINT("Validate device %s\n", DEV_NAME); + dev = device_get_binding(DEV_NAME); + zassert_not_equal(dev, NULL, + "Device not found"); + + TC_PRINT("Check %s output %d connected to input %d\n", DEV_NAME, PIN_OUT, PIN_IN); + rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_LOW); + zassert_equal(rc, 0, + "pin config output low failed"); + + rc = gpio_pin_configure(dev, PIN_IN, GPIO_INPUT); + zassert_equal(rc, 0, + "pin config input failed"); + + rc = gpio_port_get_raw(dev, &v1); + zassert_equal(rc, 0, + "get raw low failed"); + zassert_equal(raw_in(), false, + "raw_in() low failed"); + + zassert_equal(v1 & BIT(PIN_IN), 0, + "out low does not read low"); + + rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_HIGH); + zassert_equal(rc, 0, + "pin config output high failed"); + + rc = gpio_port_get_raw(dev, &v1); + zassert_equal(rc, 0, + "get raw high failed"); + zassert_equal(raw_in(), true, + "raw_in() high failed"); + + zassert_not_equal(v1 & BIT(PIN_IN), 0, + "out high does not read low"); + + TC_PRINT("OUT %d to IN %d linkage works\n", PIN_OUT, PIN_IN); + return TC_PASS; +} + +/* gpio_port_set_bits_raw() + * gpio_port_clear_bits_raw() + * gpio_port_set_masked_raw() + * gpio_port_toggle_bits() + */ +static int bits_physical(void) +{ + int rc; + + TC_PRINT("- %s\n", __func__); + + /* port_set_bits_raw */ + rc = gpio_port_set_bits_raw(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port set raw out failed"); + zassert_equal(raw_in(), true, + "raw set mismatch"); + + /* port_clear_bits_raw */ + rc = gpio_port_clear_bits_raw(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port clear raw out failed"); + zassert_equal(raw_in(), false, + "raw clear mismatch"); + + /* set after clear changes */ + rc = gpio_port_set_bits_raw(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port set raw out failed"); + zassert_equal(raw_in(), true, + "raw set mismatch"); + + /* raw_out() after set works */ + raw_out(false); + zassert_equal(raw_in(), false, + "raw_out() false mismatch"); + + /* raw_out() set after raw_out() clear works */ + raw_out(true); + zassert_equal(raw_in(), true, + "raw_out() true mismatch"); + + rc = gpio_port_set_masked_raw(dev, BIT(PIN_OUT), 0); + zassert_equal(rc, 0, + "set_masked_raw low failed"); + zassert_equal(raw_in(), false, + "set_masked_raw low mismatch"); + + rc = gpio_port_set_masked_raw(dev, BIT(PIN_OUT), ALL_BITS); + zassert_equal(rc, 0, + "set_masked_raw high failed"); + zassert_equal(raw_in(), true, + "set_masked_raw high mismatch"); + + rc = gpio_port_set_clr_bits_raw(dev, BIT(PIN_IN), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "set in clear out failed"); + zassert_equal(raw_in(), false, + "set in clear out mismatch"); + + rc = gpio_port_set_clr_bits_raw(dev, BIT(PIN_OUT), BIT(PIN_IN)); + zassert_equal(rc, 0, + "set out clear in failed"); + zassert_equal(raw_in(), true, + "set out clear in mismatch"); + + /* Conditionally verify that behavior with __ASSERT disabled + * is to set the bit. */ + if (false) { + // preserve set + rc = gpio_port_set_clr_bits_raw(dev, BIT(PIN_OUT), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "s/c dup set failed"); + zassert_equal(raw_in(), true, + "s/c dup set mismatch"); + + // do set + raw_out(false); + rc = gpio_port_set_clr_bits_raw(dev, BIT(PIN_OUT), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "s/c dup2 set failed"); + zassert_equal(raw_in(), true, + "s/c dup2 set mismatch"); + } + + rc = gpio_port_toggle_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "toggle_bits high-to-low failed"); + zassert_equal(raw_in(), false, + "toggle_bits high-to-low mismatch"); + + rc = gpio_port_toggle_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "toggle_bits low-to-high failed"); + zassert_equal(raw_in(), true, + "toggle_bits low-to-high mismatch"); + + return TC_PASS; +} + +/* gpio_pin_get_raw() + * gpio_pin_set_raw() + */ +static int pin_physical(void) +{ + int rc; + + TC_PRINT("- %s\n", __func__); + + raw_out(true); + zassert_equal(gpio_pin_get_raw(dev, PIN_IN), raw_in(), + "pin_get_raw high failed"); + + raw_out(false); + zassert_equal(gpio_pin_get_raw(dev, PIN_IN), raw_in(), + "pin_get_raw low failed"); + + rc = gpio_pin_set_raw(dev, PIN_OUT, 32); + zassert_equal(rc, 0, + "pin_set_raw high failed"); + zassert_equal(raw_in(), true, + "pin_set_raw high failed"); + + rc = gpio_pin_set_raw(dev, PIN_OUT, 0); + zassert_equal(rc, 0, + "pin_set_raw low failed"); + zassert_equal(raw_in(), false, + "pin_set_raw low failed"); + + rc = gpio_pin_toggle(dev, PIN_OUT); + zassert_equal(rc, 0, + "pin_toggle low-to-high failed"); + zassert_equal(raw_in(), true, + "pin_toggle low-to-high mismatch"); + + rc = gpio_pin_toggle(dev, PIN_OUT); + zassert_equal(rc, 0, + "pin_toggle high-to-low failed"); + zassert_equal(raw_in(), false, + "pin_toggle high-to-low mismatch"); + + return TC_PASS; +} + +/* Verify configure output level is independent of active level, and + * raw output is independent of active level. + */ +static int check_raw_output_levels(void) +{ + int rc; + + TC_PRINT("- %s\n", __func__); + + rc = gpio_pin_configure(dev, PIN_OUT, + GPIO_ACTIVE_HIGH | GPIO_OUTPUT_LOW); + zassert_equal(rc, 0, + "active high output low failed"); + zassert_equal(raw_in(), false, + "active high output low raw mismatch"); + raw_out(true); + zassert_equal(raw_in(), true, + "set high mismatch"); + + rc = gpio_pin_configure(dev, PIN_OUT, + GPIO_ACTIVE_HIGH | GPIO_OUTPUT_HIGH); + zassert_equal(rc, 0, + "active high output high failed"); + zassert_equal(raw_in(), true, + "active high output high raw mismatch"); + raw_out(false); + zassert_equal(raw_in(), false, + "set low mismatch"); + + rc = gpio_pin_configure(dev, PIN_OUT, + GPIO_ACTIVE_LOW | GPIO_OUTPUT_LOW); + zassert_equal(rc, 0, + "active low output low failed"); + zassert_equal(raw_in(), false, + "active low output low raw mismatch"); + raw_out(true); + zassert_equal(raw_in(), true, + "set high mismatch"); + + rc = gpio_pin_configure(dev, PIN_OUT, + GPIO_ACTIVE_LOW | GPIO_OUTPUT_HIGH); + zassert_equal(rc, 0, + "active low output high failed"); + zassert_equal(raw_in(), true, + "active low output high raw mismatch"); + raw_out(false); + zassert_equal(raw_in(), false, + "set low mismatch"); + + return TC_PASS; +} + +/* Verify active-high input matches physical level, and active-low + * input inverts physical level. + */ +static int check_input_levels(void) +{ + int rc; + + TC_PRINT("- %s\n", __func__); + + rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT); + zassert_equal(rc, 0, + "output configure failed"); + + rc = gpio_pin_configure(dev, PIN_IN, GPIO_INPUT); + zassert_equal(rc, 0, + "active high failed"); + raw_out(true); + zassert_equal(raw_in(), true, + "raw high mismatch"); + zassert_equal(logic_in(), true, + "logic high mismatch"); + + raw_out(false); + zassert_equal(raw_in(), false, + "raw low mismatch"); + zassert_equal(logic_in(), false, + "logic low mismatch"); + + rc = gpio_pin_configure(dev, PIN_IN, GPIO_INPUT | GPIO_ACTIVE_LOW); + zassert_equal(rc, 0, + "active low failed"); + + raw_out(true); + zassert_equal(raw_in(), true, + "raw high mismatch"); + zassert_equal(logic_in(), false, + "logic inactive mismatch"); + + raw_out(false); + zassert_equal(raw_in(), false, + "raw low mismatch"); + zassert_equal(logic_in(), true, + "logic active mismatch"); + + return TC_PASS; +} + +/* gpio_port_set_bits() + * gpio_port_clear_bits() + * gpio_port_set_masked() + * gpio_port_toggle_bits() + */ +static int bits_logical(void) +{ + int rc; + + TC_PRINT("- %s\n", __func__); + + rc = gpio_pin_configure(dev, PIN_OUT, + GPIO_OUTPUT_HIGH | GPIO_ACTIVE_LOW); + zassert_equal(rc, 0, + "output configure failed"); + zassert_equal(raw_in(), true, + "raw out high mismatch"); + zassert_equal(logic_in(), !raw_in(), + "logic in active mismatch"); + + /* port_set_bits */ + rc = gpio_port_set_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port set raw out failed"); + zassert_equal(raw_in(), false, + "raw low set mismatch"); + zassert_equal(logic_in(), !raw_in(), + "logic in inactive mismatch"); + + /* port_clear_bits */ + rc = gpio_port_clear_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port clear raw out failed"); + zassert_equal(logic_in(), false, + "low clear mismatch"); + + /* set after clear changes */ + rc = gpio_port_set_bits_raw(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "port set raw out failed"); + zassert_equal(logic_in(), false, + "raw set mismatch"); + + /* pin_set false */ + rc = gpio_pin_set(dev, PIN_OUT, 0); + zassert_equal(rc, 0, + "pin clear failed"); + zassert_equal(logic_in(), false, + "pin clear mismatch"); + + /* pin_set true */ + rc = gpio_pin_set(dev, PIN_OUT, 32); + zassert_equal(rc, 0, + "pin set failed"); + zassert_equal(logic_in(), true, + "pin set mismatch"); + + rc = gpio_port_set_masked(dev, BIT(PIN_OUT), 0); + zassert_equal(rc, 0, + "set_masked low failed"); + zassert_equal(logic_in(), false, + "set_masked low mismatch"); + + rc = gpio_port_set_masked(dev, BIT(PIN_OUT), ALL_BITS); + zassert_equal(rc, 0, + "set_masked high failed"); + zassert_equal(logic_in(), true, + "set_masked high mismatch"); + + rc = gpio_port_set_clr_bits(dev, BIT(PIN_IN), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "set in clear out failed"); + zassert_equal(logic_in(), false, + "set in clear out mismatch"); + + rc = gpio_port_set_clr_bits(dev, BIT(PIN_OUT), BIT(PIN_IN)); + zassert_equal(rc, 0, + "set out clear in failed"); + zassert_equal(logic_in(), true, + "set out clear in mismatch"); + + /* Conditionally verify that behavior with __ASSERT disabled + * is to set the bit. */ + if (false) { + // preserve set + rc = gpio_port_set_clr_bits(dev, BIT(PIN_OUT), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "s/c set toggle failed"); + zassert_equal(logic_in(), false, + "s/c set toggle mismatch"); + + // force set + raw_out(true); + rc = gpio_port_set_clr_bits(dev, BIT(PIN_OUT), BIT(PIN_OUT)); + zassert_equal(rc, 0, + "s/c dup set failed"); + zassert_equal(logic_in(), false, + "s/c dup set mismatch"); + } + + rc = gpio_port_toggle_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "toggle_bits active-to-inactive failed"); + zassert_equal(logic_in(), false, + "toggle_bits active-to-inactive mismatch"); + + rc = gpio_port_toggle_bits(dev, BIT(PIN_OUT)); + zassert_equal(rc, 0, + "toggle_bits inactive-to-active failed"); + zassert_equal(logic_in(), true, + "toggle_bits inactive-to-active mismatch"); + + rc = gpio_pin_toggle(dev, PIN_OUT); + zassert_equal(rc, 0, + "pin_toggle low-to-high failed"); + zassert_equal(logic_in(), false, + "pin_toggle low-to-high mismatch"); + + return TC_PASS; +} + +void test_gpio_port(void) +{ + zassert_equal(setup(), TC_PASS, + "device setup failed"); + zassert_equal(bits_physical(), TC_PASS, + "bits_physical failed"); + zassert_equal(pin_physical(), TC_PASS, + "pin_physical failed"); + zassert_equal(check_raw_output_levels(), TC_PASS, + "check_raw_output_levels failed"); + zassert_equal(check_input_levels(), TC_PASS, + "check_input_levels failed"); + zassert_equal(bits_logical(), TC_PASS, + "bits_logical failed"); +} From 6c9de371a9d47180eed1966ace216e1cda6c3b95 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:12:22 -0500 Subject: [PATCH 06/17] gpio: reimplement legacy API using new API Use the recommended replacement as the implementation of the deprecated pin read and write functions, rather than calling the old implementation. Retain the existing test, but silence the complaints related to using deprecated functions. Signed-off-by: Peter Bigot --- include/drivers/gpio.h | 10 +++++++-- .../gpio/gpio_basic_api/src/test_pin_rw.c | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/include/drivers/gpio.h b/include/drivers/gpio.h index 9bbf0ce6dc4ec..306fa375754b8 100644 --- a/include/drivers/gpio.h +++ b/include/drivers/gpio.h @@ -1012,7 +1012,7 @@ static inline int gpio_pin_toggle(struct device *port, unsigned int pin) static inline int gpio_pin_write(struct device *port, u32_t pin, u32_t value) { - return gpio_write(port, GPIO_ACCESS_BY_PIN, pin, value); + return gpio_pin_set_raw(port, pin, value); } /** @@ -1030,7 +1030,13 @@ static inline int gpio_pin_write(struct device *port, u32_t pin, static inline int gpio_pin_read(struct device *port, u32_t pin, u32_t *value) { - return gpio_read(port, GPIO_ACCESS_BY_PIN, pin, value); + int rc = gpio_pin_get_raw(port, pin); + + if (rc >= 0) { + *value = rc; + rc = 0; + } + return rc; } /** diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_pin_rw.c b/tests/drivers/gpio/gpio_basic_api/src/test_pin_rw.c index f6b2b0ee91c75..b780a75d82cbd 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_pin_rw.c +++ b/tests/drivers/gpio/gpio_basic_api/src/test_pin_rw.c @@ -16,20 +16,31 @@ #include "test_gpio.h" +#undef __DEPRECATED_MACRO +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void test_gpio_pin_read_write(void) { struct device *dev = device_get_binding(DEV_NAME); /* set PIN_OUT as writer */ - TC_PRINT("device=%s, pin1=%d, pin2=%d\n", DEV_NAME, PIN_OUT, PIN_IN); - gpio_pin_configure(dev, PIN_OUT, GPIO_DIR_OUT); + TC_PRINT("device=%s, pin1=%d, pin2=%d: %p\n", DEV_NAME, PIN_OUT, PIN_IN, dev); + gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT); /* set PIN_IN as reader */ - gpio_pin_configure(dev, PIN_IN, GPIO_DIR_IN); - gpio_pin_disable_callback(dev, PIN_IN); + gpio_pin_configure(dev, PIN_IN, GPIO_INPUT); + gpio_pin_interrupt_configure(dev, PIN_IN, GPIO_INT_DISABLE); u32_t val_write, val_read = 0U; int i = 0; + zassert_equal(gpio_pin_write(dev, PIN_OUT, 1), 0, + "write data fail"); + zassert_equal(gpio_pin_read(dev, PIN_IN, &val_read), 0, + "read data fail"); + zassert_equal(val_read, 1, + "read data mismatch"); + while (i++ < 32) { val_write = sys_rand32_get() / 3U % 2; zassert_true(gpio_pin_write(dev, PIN_OUT, val_write) == 0, @@ -45,3 +56,5 @@ void test_gpio_pin_read_write(void) "Inconsistent GPIO read/write value"); } } + +#pragma GCC diagnostic pop From 42d21a10bcd9590bb91b0c8f1d036a4eef702409 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Mon, 26 Aug 2019 18:00:51 -0500 Subject: [PATCH 07/17] tests/drivers/gpio: replace legacy API in callback manage tests Switch to gpio_pin_interrupt_configure() and the new interrupt flags. Use logical level pin set operations. Signed-off-by: Peter Bigot --- tests/drivers/gpio/gpio_basic_api/src/main.c | 6 +- .../gpio_basic_api/src/test_callback_manage.c | 92 +++++++++++++------ 2 files changed, 65 insertions(+), 33 deletions(-) diff --git a/tests/drivers/gpio/gpio_basic_api/src/main.c b/tests/drivers/gpio/gpio_basic_api/src/main.c index 3c7771df3c786..f17d195b92eba 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/main.c +++ b/tests/drivers/gpio/gpio_basic_api/src/main.c @@ -12,12 +12,12 @@ void test_main(void) ztest_test_suite(gpio_basic_test, ztest_unit_test(test_gpio_port), ztest_unit_test(test_gpio_pin_read_write), - ztest_unit_test(test_gpio_callback_edge_high), - ztest_unit_test(test_gpio_callback_edge_low), - ztest_unit_test(test_gpio_callback_level_high), ztest_unit_test(test_gpio_callback_add_remove), ztest_unit_test(test_gpio_callback_self_remove), ztest_unit_test(test_gpio_callback_enable_disable), + ztest_unit_test(test_gpio_callback_edge_high), + ztest_unit_test(test_gpio_callback_edge_low), + ztest_unit_test(test_gpio_callback_level_high), ztest_unit_test(test_gpio_callback_level_low)); ztest_run_test_suite(gpio_basic_test); } diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c b/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c index f7ecdc59aaa9f..c599cfa49760c 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c +++ b/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c @@ -39,43 +39,54 @@ static void callback_remove_self(struct device *dev, dd->aux = gpio_remove_callback(dev, gpio_cb); } -static void init_callback(struct device *dev, - gpio_callback_handler_t handler_1, - gpio_callback_handler_t handler_2) +static int init_callback(struct device *dev, + gpio_callback_handler_t handler_1, + gpio_callback_handler_t handler_2) { - gpio_pin_disable_callback(dev, PIN_IN); - gpio_pin_disable_callback(dev, PIN_OUT); + int rc = gpio_pin_interrupt_configure(dev, PIN_IN, GPIO_INT_DISABLE); - /* 1. set PIN_OUT */ - gpio_pin_configure(dev, PIN_OUT, GPIO_DIR_OUT); - gpio_pin_write(dev, PIN_OUT, 0); + if (rc == 0) { + rc = gpio_pin_interrupt_configure(dev, PIN_OUT, GPIO_INT_DISABLE); + } + if (rc == 0) { + /* 1. set PIN_OUT */ + rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_LOW); + } + + if (rc == 0) { + /* 2. configure PIN_IN callback, but don't enable */ + rc = gpio_pin_configure(dev, PIN_IN, GPIO_INPUT); + } - /* 2. configure PIN_IN callback and trigger condition */ - gpio_pin_configure(dev, PIN_IN, - GPIO_DIR_IN | GPIO_INT | GPIO_INT_EDGE | \ - GPIO_INT_ACTIVE_HIGH | GPIO_INT_DEBOUNCE); + if (rc == 0) { + gpio_init_callback(&cb_data[0].gpio_cb, handler_1, BIT(PIN_IN)); + rc = gpio_add_callback(dev, &cb_data[0].gpio_cb); + } - gpio_init_callback(&cb_data[0].gpio_cb, handler_1, BIT(PIN_IN)); - gpio_add_callback(dev, &cb_data[0].gpio_cb); + if (rc == 0) { + gpio_init_callback(&cb_data[1].gpio_cb, handler_2, BIT(PIN_IN)); + rc = gpio_add_callback(dev, &cb_data[1].gpio_cb); + } - gpio_init_callback(&cb_data[1].gpio_cb, handler_2, BIT(PIN_IN)); - gpio_add_callback(dev, &cb_data[1].gpio_cb); + return rc; } static void trigger_callback(struct device *dev, int enable_cb) { - gpio_pin_write(dev, PIN_OUT, 0); + gpio_pin_set(dev, PIN_OUT, 0); k_sleep(100); cb_cnt[0] = 0; cb_cnt[1] = 0; if (enable_cb == 1) { - gpio_pin_enable_callback(dev, PIN_IN); + gpio_pin_interrupt_configure(dev, PIN_IN, + GPIO_INT_EDGE_RISING + | GPIO_INT_DEBOUNCE); } else { - gpio_pin_disable_callback(dev, PIN_IN); + gpio_pin_interrupt_configure(dev, PIN_IN, GPIO_INT_DISABLE); } k_sleep(100); - gpio_pin_write(dev, PIN_OUT, 1); + gpio_pin_set(dev, PIN_OUT, 1); k_sleep(1000); } @@ -84,7 +95,14 @@ static int test_callback_add_remove(void) struct device *dev = device_get_binding(DEV_NAME); /* SetUp: initialize environment */ - init_callback(dev, callback_1, callback_2); + int rc = init_callback(dev, callback_1, callback_2); + + if (rc == -ENOTSUP) { + TC_PRINT("%s not supported\n", __func__); + return TC_PASS; + } + zassert_equal(rc, 0, + "init_callback failed"); /* 3. enable callback, trigger PIN_IN interrupt by operate PIN_OUT */ trigger_callback(dev, 1); @@ -126,9 +144,16 @@ static int test_callback_self_remove(void) struct device *dev = device_get_binding(DEV_NAME); /* SetUp: initialize environment */ - init_callback(dev, callback_1, callback_remove_self); + int rc = init_callback(dev, callback_1, callback_remove_self); + + if (rc == -ENOTSUP) { + TC_PRINT("%s not supported\n", __func__); + return TC_PASS; + } + zassert_equal(rc, 0, + "init_callback failed"); - gpio_pin_write(dev, PIN_OUT, 0); + gpio_pin_set(dev, PIN_OUT, 0); k_sleep(100); cb_data[0].aux = INT_MAX; @@ -171,7 +196,14 @@ static int test_callback_enable_disable(void) struct device *dev = device_get_binding(DEV_NAME); /* SetUp: initialize environment */ - init_callback(dev, callback_1, callback_2); + int rc = init_callback(dev, callback_1, callback_2); + + if (rc == -ENOTSUP) { + TC_PRINT("%s not supported\n", __func__); + return TC_PASS; + } + zassert_equal(rc, 0, + "init_callback failed"); /* 3. enable callback, trigger PIN_IN interrupt by operate PIN_OUT */ trigger_callback(dev, 1); @@ -208,18 +240,18 @@ static int test_callback_enable_disable(void) void test_gpio_callback_add_remove(void) { - zassert_true( - test_callback_add_remove() == TC_PASS, NULL); + zassert_equal(test_callback_add_remove(), TC_PASS, + NULL); } void test_gpio_callback_self_remove(void) { - zassert_true( - test_callback_self_remove() == TC_PASS, NULL); + zassert_equal(test_callback_self_remove(), TC_PASS, + NULL); } void test_gpio_callback_enable_disable(void) { - zassert_true( - test_callback_enable_disable() == TC_PASS, NULL); + zassert_equal(test_callback_enable_disable(), TC_PASS, + NULL); } From b31a8183a9a577b3808fac9998170ad9cdea3adb Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Mon, 26 Aug 2019 18:19:21 -0500 Subject: [PATCH 08/17] tests/drivers/gpio: replace legacy API in callback tests Switch to gpio_pin_interrupt_configure() and the new interrupt flags. Use logical level pin set operations. Test all standard interrupt configurations including double edge. Signed-off-by: Peter Bigot --- tests/drivers/gpio/gpio_basic_api/src/main.c | 5 +- .../src/test_callback_trigger.c | 115 ++++++++++-------- .../gpio/gpio_basic_api/src/test_gpio.h | 5 +- 3 files changed, 69 insertions(+), 56 deletions(-) diff --git a/tests/drivers/gpio/gpio_basic_api/src/main.c b/tests/drivers/gpio/gpio_basic_api/src/main.c index f17d195b92eba..96478f7957e6c 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/main.c +++ b/tests/drivers/gpio/gpio_basic_api/src/main.c @@ -15,9 +15,6 @@ void test_main(void) ztest_unit_test(test_gpio_callback_add_remove), ztest_unit_test(test_gpio_callback_self_remove), ztest_unit_test(test_gpio_callback_enable_disable), - ztest_unit_test(test_gpio_callback_edge_high), - ztest_unit_test(test_gpio_callback_edge_low), - ztest_unit_test(test_gpio_callback_level_high), - ztest_unit_test(test_gpio_callback_level_low)); + ztest_unit_test(test_gpio_callback_variants)); ztest_run_test_suite(gpio_basic_test); } diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_callback_trigger.c b/tests/drivers/gpio/gpio_basic_api/src/test_callback_trigger.c index 28c01d941d850..ab4a895819ed1 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_callback_trigger.c +++ b/tests/drivers/gpio/gpio_basic_api/src/test_callback_trigger.c @@ -31,15 +31,20 @@ static int pin_num(u32_t pins) static void callback(struct device *dev, struct gpio_callback *gpio_cb, u32_t pins) { + const struct drv_data *dd = CONTAINER_OF(gpio_cb, + struct drv_data, gpio_cb); + /*= checkpoint: pins should be marked with correct pin number bit =*/ zassert_true(pin_num(pins) == PIN_IN, NULL); - TC_PRINT("callback triggered: %d\n", ++cb_cnt); + ++cb_cnt; + TC_PRINT("callback triggered: %d\n", cb_cnt); + if ((cb_cnt == 1) + && (dd->mode == GPIO_INT_EDGE_BOTH)) { + gpio_pin_toggle(dev, PIN_OUT); + } if (cb_cnt >= MAX_INT_CNT) { - struct drv_data *drv_data = CONTAINER_OF(gpio_cb, - struct drv_data, gpio_cb); - gpio_pin_write(dev, PIN_OUT, - (drv_data->mode & GPIO_INT_ACTIVE_HIGH) ? 0 : 1); - gpio_pin_configure(dev, PIN_IN, GPIO_DIR_IN); + gpio_pin_set(dev, PIN_OUT, 0); + gpio_pin_interrupt_configure(dev, PIN_OUT, GPIO_INT_DISABLE); } } @@ -48,43 +53,63 @@ static int test_callback(int mode) struct device *dev = device_get_binding(DEV_NAME); struct drv_data *drv_data = &data; - gpio_pin_disable_callback(dev, PIN_IN); - gpio_pin_disable_callback(dev, PIN_OUT); + gpio_pin_interrupt_configure(dev, PIN_IN, GPIO_INT_DISABLE); + gpio_pin_interrupt_configure(dev, PIN_OUT, GPIO_INT_DISABLE); - /* 1. set PIN_OUT to initial state */ - if (gpio_pin_configure(dev, PIN_OUT, GPIO_DIR_OUT) != 0) { - TC_ERROR("PIN_OUT config fail\n"); - return TC_FAIL; + /* 1. set PIN_OUT to logical initial state inactive */ + u32_t out_flags = GPIO_OUTPUT_LOW; + + if ((mode & GPIO_INT_LOW_0) != 0) { + out_flags = GPIO_OUTPUT_HIGH | GPIO_ACTIVE_LOW; } - if (gpio_pin_write(dev, PIN_OUT, - (mode & GPIO_INT_ACTIVE_HIGH) ? 0 : 1) != 0) { - TC_ERROR("set PIN_OUT init voltage fail\n"); + + int rc = gpio_pin_configure(dev, PIN_OUT, out_flags); + if (rc != 0) { + TC_ERROR("PIN_OUT config fail: %d", rc); return TC_FAIL; } /* 2. configure PIN_IN callback and trigger condition */ - if (gpio_pin_configure(dev, PIN_IN, - GPIO_DIR_IN | GPIO_INT | mode | GPIO_INT_DEBOUNCE) != 0) { - TC_ERROR("config PIN_IN fail"); + rc = gpio_pin_configure(dev, PIN_IN, GPIO_INPUT); + if (rc != 0) { + TC_ERROR("config PIN_IN fail: %d\n", rc); goto err_exit; } drv_data->mode = mode; gpio_init_callback(&drv_data->gpio_cb, callback, BIT(PIN_IN)); - if (gpio_add_callback(dev, &drv_data->gpio_cb) != 0) { - TC_ERROR("set PIN_IN callback fail\n"); + rc = gpio_add_callback(dev, &drv_data->gpio_cb); + if (rc == -ENOTSUP) { + TC_PRINT("interrupts not supported\n"); + return TC_PASS; + } else if (rc != 0) { + TC_ERROR("set PIN_IN callback fail: %d\n", rc); return TC_FAIL; } /* 3. enable callback, trigger PIN_IN interrupt by operate PIN_OUT */ cb_cnt = 0; - gpio_pin_enable_callback(dev, PIN_IN); + rc = gpio_pin_interrupt_configure(dev, PIN_IN, mode | GPIO_INT_DEBOUNCE); + if (rc == -ENOTSUP) { + TC_PRINT("Mode %x not supported\n", mode); + goto pass_exit; + } else if (rc != 0) { + TC_ERROR("config PIN_IN interrupt fail: %d\n", rc); + goto err_exit; + } k_sleep(100); - gpio_pin_write(dev, PIN_OUT, (mode & GPIO_INT_ACTIVE_HIGH) ? 1 : 0); + gpio_pin_set(dev, PIN_OUT, 1); k_sleep(1000); /*= checkpoint: check callback is triggered =*/ - TC_PRINT("check enabled callback\n"); + TC_PRINT("OUT init %x, IN cfg %x, cnt %d\n", out_flags, mode, cb_cnt); + if (mode == GPIO_INT_EDGE_BOTH) { + if (cb_cnt != 2) { + TC_ERROR("double edge not detected\n"); + goto err_exit; + } + goto pass_exit; + } if ((mode & GPIO_INT_EDGE) == GPIO_INT_EDGE) { if (cb_cnt != 1) { TC_ERROR("not trigger callback correctly\n"); @@ -110,30 +135,24 @@ static int test_callback(int mode) } /* export test cases */ -void test_gpio_callback_edge_high(void) -{ - zassert_true( - test_callback(GPIO_INT_EDGE | GPIO_INT_ACTIVE_HIGH) == TC_PASS, - NULL); -} - -void test_gpio_callback_edge_low(void) -{ - zassert_true( - test_callback(GPIO_INT_EDGE | GPIO_INT_ACTIVE_LOW) == TC_PASS, - NULL); -} - -void test_gpio_callback_level_high(void) -{ - zassert_true( - test_callback(GPIO_INT_LEVEL | GPIO_INT_ACTIVE_HIGH) == TC_PASS, - NULL); -} - -void test_gpio_callback_level_low(void) +void test_gpio_callback_variants(void) { - zassert_true( - test_callback(GPIO_INT_LEVEL | GPIO_INT_ACTIVE_LOW) == TC_PASS, - NULL); + zassert_equal(test_callback(GPIO_INT_EDGE_FALLING), TC_PASS, + "falling edge failed"); + zassert_equal(test_callback(GPIO_INT_EDGE_RISING), TC_PASS, + "rising edge failed"); + zassert_equal(test_callback(GPIO_INT_EDGE_TO_ACTIVE), TC_PASS, + "edge active failed"); + zassert_equal(test_callback(GPIO_INT_EDGE_TO_INACTIVE), TC_PASS, + "edge inactive failed"); + zassert_equal(test_callback(GPIO_INT_LEVEL_HIGH), TC_PASS, + "level high failed"); + zassert_equal(test_callback(GPIO_INT_LEVEL_LOW), TC_PASS, + "level low failed"); + zassert_equal(test_callback(GPIO_INT_LEVEL_ACTIVE), TC_PASS, + "level active failed"); + zassert_equal(test_callback(GPIO_INT_LEVEL_INACTIVE), TC_PASS, + "level inactive failed"); + zassert_equal(test_callback(GPIO_INT_EDGE_BOTH), TC_PASS, + "edge both failed"); } diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h index 9a7fa1a345e26..0846ef3edc089 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h @@ -35,13 +35,10 @@ struct drv_data { }; void test_gpio_pin_read_write(void); -void test_gpio_callback_edge_high(void); -void test_gpio_callback_edge_low(void); -void test_gpio_callback_level_high(void); -void test_gpio_callback_level_low(void); void test_gpio_callback_add_remove(void); void test_gpio_callback_self_remove(void); void test_gpio_callback_enable_disable(void); +void test_gpio_callback_variants(void); void test_gpio_port(void); From e7f2c9cef8fb38187313fade46a51ca5c279fff4 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:41:16 -0500 Subject: [PATCH 09/17] boards/particle_*: update for revised GPIO API Set the devicetree nodes to correctly reflect active levels for buttons, leds, and antenna switch. Update the board initialization code to use the non-deprecated GPIO API for antenna switching. Signed-off-by: Peter Bigot --- boards/arm/particle_argon/board.c | 12 ++++++------ boards/arm/particle_argon/dts/mesh_feather.dtsi | 12 ++++++------ boards/arm/particle_argon/particle_argon.dts | 4 ++-- boards/arm/particle_boron/board.c | 6 +++--- boards/arm/particle_boron/dts/mesh_feather.dtsi | 12 ++++++------ boards/arm/particle_boron/particle_boron.dts | 2 +- boards/arm/particle_xenon/board.c | 12 ++++++------ boards/arm/particle_xenon/dts/mesh_feather.dtsi | 12 ++++++------ boards/arm/particle_xenon/particle_xenon.dts | 4 ++-- 9 files changed, 38 insertions(+), 38 deletions(-) diff --git a/boards/arm/particle_argon/board.c b/boards/arm/particle_argon/board.c index 2bdd65c9c0732..ed481fff9f285 100644 --- a/boards/arm/particle_argon/board.c +++ b/boards/arm/particle_argon/board.c @@ -24,16 +24,16 @@ static inline void external_antenna(bool on) } gpio_pin_configure(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, - GPIO_DIR_OUT | SKY_UFLn_GPIO_FLAGS); + GPIO_OUTPUT | SKY_UFLn_GPIO_FLAGS); gpio_pin_configure(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, - GPIO_DIR_OUT | SKY_PCBn_GPIO_FLAGS); + GPIO_OUTPUT | SKY_PCBn_GPIO_FLAGS); if (on) { - gpio_pin_write(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 1); - gpio_pin_write(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 0); + gpio_pin_set(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 1); + gpio_pin_set(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 0); } else { - gpio_pin_write(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 0); - gpio_pin_write(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 1); + gpio_pin_set(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 0); + gpio_pin_set(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 1); } } diff --git a/boards/arm/particle_argon/dts/mesh_feather.dtsi b/boards/arm/particle_argon/dts/mesh_feather.dtsi index 5288cc9320127..84f5bb0c15655 100644 --- a/boards/arm/particle_argon/dts/mesh_feather.dtsi +++ b/boards/arm/particle_argon/dts/mesh_feather.dtsi @@ -32,19 +32,19 @@ leds { compatible = "gpio-leds"; user_led: led_0 { - gpios = <&gpio1 12 0>; + gpios = <&gpio1 12 GPIO_ACTIVE_HIGH>; label = "User LED"; }; status_red: led_1 { - gpios = <&gpio0 13 0>; + gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; label = "Red LED"; }; status_green: led_2 { - gpios = <&gpio0 14 0>; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; label = "Green LED"; }; status_blue: led_3 { - gpios = <&gpio0 15 0>; + gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; label = "Blue LED"; }; }; @@ -52,12 +52,12 @@ gpio_keys { compatible = "gpio-keys"; mode_button: button_0 { - gpios = <&gpio0 11 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 11 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Mode Button"; }; reset_button: button_1 { - gpios = <&gpio0 18 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 18 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Reset Button"; }; }; diff --git a/boards/arm/particle_argon/particle_argon.dts b/boards/arm/particle_argon/particle_argon.dts index 481db5f527d63..fd50c7fce3184 100644 --- a/boards/arm/particle_argon/particle_argon.dts +++ b/boards/arm/particle_argon/particle_argon.dts @@ -16,8 +16,8 @@ sky13351 { compatible = "skyworks,sky13351"; - vctl1-gpios = <&gpio0 25 0>; - vctl2-gpios = <&gpio0 2 0>; + vctl1-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>; + vctl2-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>; }; }; diff --git a/boards/arm/particle_boron/board.c b/boards/arm/particle_boron/board.c index 9b0c57ac79faf..dfb69898f3757 100644 --- a/boards/arm/particle_boron/board.c +++ b/boards/arm/particle_boron/board.c @@ -24,12 +24,12 @@ static inline void external_antenna(bool on) } gpio_pin_configure(ant_sel_gpio_dev, ANT_SEL_GPIO_PIN, - GPIO_DIR_OUT | ANT_SEL_GPIO_FLAGS); + GPIO_OUTPUT | ANT_SEL_GPIO_FLAGS); if (on) { - gpio_pin_write(ant_sel_gpio_dev, ANT_SEL_GPIO_PIN, 1); + gpio_pin_set(ant_sel_gpio_dev, ANT_SEL_GPIO_PIN, 1); } else { - gpio_pin_write(ant_sel_gpio_dev, ANT_SEL_GPIO_PIN, 0); + gpio_pin_set(ant_sel_gpio_dev, ANT_SEL_GPIO_PIN, 0); } } diff --git a/boards/arm/particle_boron/dts/mesh_feather.dtsi b/boards/arm/particle_boron/dts/mesh_feather.dtsi index 5288cc9320127..84f5bb0c15655 100644 --- a/boards/arm/particle_boron/dts/mesh_feather.dtsi +++ b/boards/arm/particle_boron/dts/mesh_feather.dtsi @@ -32,19 +32,19 @@ leds { compatible = "gpio-leds"; user_led: led_0 { - gpios = <&gpio1 12 0>; + gpios = <&gpio1 12 GPIO_ACTIVE_HIGH>; label = "User LED"; }; status_red: led_1 { - gpios = <&gpio0 13 0>; + gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; label = "Red LED"; }; status_green: led_2 { - gpios = <&gpio0 14 0>; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; label = "Green LED"; }; status_blue: led_3 { - gpios = <&gpio0 15 0>; + gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; label = "Blue LED"; }; }; @@ -52,12 +52,12 @@ gpio_keys { compatible = "gpio-keys"; mode_button: button_0 { - gpios = <&gpio0 11 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 11 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Mode Button"; }; reset_button: button_1 { - gpios = <&gpio0 18 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 18 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Reset Button"; }; }; diff --git a/boards/arm/particle_boron/particle_boron.dts b/boards/arm/particle_boron/particle_boron.dts index b0b29d9f58dc8..55c2ed8fdea8e 100644 --- a/boards/arm/particle_boron/particle_boron.dts +++ b/boards/arm/particle_boron/particle_boron.dts @@ -16,7 +16,7 @@ sky13351 { compatible = "skyworks,sky13351"; - vctl1-gpios = <&gpio0 7 0>; + vctl1-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>; /* on Boron VCTL2 is inverted VCTL1 signal via SN74LVC1G04 * single inverter gate -- requires a definition below, * but is not used in board.c */ diff --git a/boards/arm/particle_xenon/board.c b/boards/arm/particle_xenon/board.c index c395c4c76669f..43e444d86d2e9 100644 --- a/boards/arm/particle_xenon/board.c +++ b/boards/arm/particle_xenon/board.c @@ -24,16 +24,16 @@ static inline void external_antenna(bool on) } gpio_pin_configure(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, - GPIO_DIR_OUT | SKY_UFLn_GPIO_FLAGS); + GPIO_OUTPUT | SKY_UFLn_GPIO_FLAGS); gpio_pin_configure(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, - GPIO_DIR_OUT | SKY_PCBn_GPIO_FLAGS); + GPIO_OUTPUT | SKY_PCBn_GPIO_FLAGS); if (on) { - gpio_pin_write(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 1); - gpio_pin_write(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 0); + gpio_pin_set(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 1); + gpio_pin_set(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 0); } else { - gpio_pin_write(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 0); - gpio_pin_write(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 1); + gpio_pin_set(ufl_gpio_dev, SKY_UFLn_GPIO_PIN, 0); + gpio_pin_set(pcb_gpio_dev, SKY_PCBn_GPIO_PIN, 1); } } diff --git a/boards/arm/particle_xenon/dts/mesh_feather.dtsi b/boards/arm/particle_xenon/dts/mesh_feather.dtsi index 5288cc9320127..84f5bb0c15655 100644 --- a/boards/arm/particle_xenon/dts/mesh_feather.dtsi +++ b/boards/arm/particle_xenon/dts/mesh_feather.dtsi @@ -32,19 +32,19 @@ leds { compatible = "gpio-leds"; user_led: led_0 { - gpios = <&gpio1 12 0>; + gpios = <&gpio1 12 GPIO_ACTIVE_HIGH>; label = "User LED"; }; status_red: led_1 { - gpios = <&gpio0 13 0>; + gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; label = "Red LED"; }; status_green: led_2 { - gpios = <&gpio0 14 0>; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; label = "Green LED"; }; status_blue: led_3 { - gpios = <&gpio0 15 0>; + gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; label = "Blue LED"; }; }; @@ -52,12 +52,12 @@ gpio_keys { compatible = "gpio-keys"; mode_button: button_0 { - gpios = <&gpio0 11 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 11 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Mode Button"; }; reset_button: button_1 { - gpios = <&gpio0 18 GPIO_PUD_PULL_UP>; + gpios = <&gpio0 18 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; label = "Reset Button"; }; }; diff --git a/boards/arm/particle_xenon/particle_xenon.dts b/boards/arm/particle_xenon/particle_xenon.dts index ec241a0f95ea1..a3c50227ed780 100644 --- a/boards/arm/particle_xenon/particle_xenon.dts +++ b/boards/arm/particle_xenon/particle_xenon.dts @@ -16,7 +16,7 @@ sky13351 { compatible = "skyworks,sky13351"; - vctl1-gpios = <&gpio0 24 0>; - vctl2-gpios = <&gpio0 25 0>; + vctl1-gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>; + vctl2-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>; }; }; From e33530b3ae7caf899d1ff21a816d420d11294252 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:13:28 -0500 Subject: [PATCH 10/17] gpio: remove GPIO legacy API functions Remove the old read and write implementation functions from the driver API table, along with their system call implementations. Signed-off-by: Peter Bigot --- include/drivers/gpio.h | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/include/drivers/gpio.h b/include/drivers/gpio.h index 306fa375754b8..f00edfb51c6cb 100644 --- a/include/drivers/gpio.h +++ b/include/drivers/gpio.h @@ -480,10 +480,6 @@ struct gpio_callback { */ struct gpio_driver_api { int (*config)(struct device *port, int access_op, u32_t pin, int flags); - int (*write)(struct device *port, int access_op, u32_t pin, - u32_t value); - int (*read)(struct device *port, int access_op, u32_t pin, - u32_t *value); int (*port_get_raw)(struct device *port, gpio_port_value_t *value); int (*port_set_masked_raw)(struct device *port, gpio_port_pins_t mask, gpio_port_value_t value); @@ -511,30 +507,6 @@ static inline int z_impl_gpio_config(struct device *port, int access_op, return api->config(port, access_op, pin, flags); } -__syscall int gpio_write(struct device *port, int access_op, u32_t pin, - u32_t value); - -static inline int z_impl_gpio_write(struct device *port, int access_op, - u32_t pin, u32_t value) -{ - const struct gpio_driver_api *api = - (const struct gpio_driver_api *)port->driver_api; - - return api->write(port, access_op, pin, value); -} - -__syscall int gpio_read(struct device *port, int access_op, u32_t pin, - u32_t *value); - -static inline int z_impl_gpio_read(struct device *port, int access_op, - u32_t pin, u32_t *value) -{ - const struct gpio_driver_api *api = - (const struct gpio_driver_api *)port->driver_api; - - return api->read(port, access_op, pin, value); -} - __syscall int gpio_enable_callback(struct device *port, int access_op, u32_t pin); From de5a3279fe99385d1f506e36d5c0590729bf77ff Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Thu, 29 Aug 2019 11:34:55 -0500 Subject: [PATCH 11/17] drivers: gpio_nrfx: remove outdated driver functions The access-oriented GPIO read and write system calls have been replaced with new API. Remove the old implementation. Signed-off-by: Peter Bigot --- drivers/gpio/gpio_nrfx.c | 41 ---------------------------------------- 1 file changed, 41 deletions(-) diff --git a/drivers/gpio/gpio_nrfx.c b/drivers/gpio/gpio_nrfx.c index 36216eeb1146a..934a212404d18 100644 --- a/drivers/gpio/gpio_nrfx.c +++ b/drivers/gpio/gpio_nrfx.c @@ -221,45 +221,6 @@ static int gpio_nrfx_config(struct device *port, int access_op, return 0; } -static int gpio_nrfx_write(struct device *port, int access_op, - u32_t pin, u32_t value) -{ - NRF_GPIO_Type *reg = get_port_cfg(port)->port; - struct gpio_nrfx_data *data = get_port_data(port); - - if (access_op == GPIO_ACCESS_BY_PORT) { - nrf_gpio_port_out_write(reg, value ^ data->general.invert); - } else { - if ((value > 0) ^ ((BIT(pin) & data->general.invert) != 0)) { - nrf_gpio_port_out_set(reg, BIT(pin)); - } else { - nrf_gpio_port_out_clear(reg, BIT(pin)); - } - } - - return 0; -} - -static int gpio_nrfx_read(struct device *port, int access_op, - u32_t pin, u32_t *value) -{ - NRF_GPIO_Type *reg = get_port_cfg(port)->port; - struct gpio_nrfx_data *data = get_port_data(port); - - u32_t dir = nrf_gpio_port_dir_read(reg); - u32_t port_in = nrf_gpio_port_in_read(reg) & ~dir; - u32_t port_out = nrf_gpio_port_out_read(reg) & dir; - u32_t port_val = (port_in | port_out) ^ data->general.invert; - - if (access_op == GPIO_ACCESS_BY_PORT) { - *value = port_val; - } else { - *value = (port_val & BIT(pin)) ? 1 : 0; - } - - return 0; -} - static int gpio_nrfx_port_get_raw(struct device *port, u32_t *value) { NRF_GPIO_Type *reg = get_port_cfg(port)->port; @@ -399,8 +360,6 @@ static inline int gpio_nrfx_pin_disable_callback(struct device *port, static const struct gpio_driver_api gpio_nrfx_drv_api_funcs = { .config = gpio_nrfx_config, - .write = gpio_nrfx_write, - .read = gpio_nrfx_read, .port_get_raw = gpio_nrfx_port_get_raw, .port_set_masked_raw = gpio_nrfx_port_set_masked_raw, .port_set_bits_raw = gpio_nrfx_port_set_bits_raw, From 0fc9da6202b50b4e1a0c46acb962597911fe45fa Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 08:14:59 -0500 Subject: [PATCH 12/17] drivers: gpio_mcux: remove outdated driver functions The access-oriented GPIO read and write system calls have been replaced with new API. Remove the old implementation. Signed-off-by: Peter Bigot --- drivers/gpio/gpio_mcux.c | 47 ---------------------------------------- 1 file changed, 47 deletions(-) diff --git a/drivers/gpio/gpio_mcux.c b/drivers/gpio/gpio_mcux.c index 8d6ab367b755b..9724592ea5b7d 100644 --- a/drivers/gpio/gpio_mcux.c +++ b/drivers/gpio/gpio_mcux.c @@ -179,51 +179,6 @@ static int gpio_mcux_configure(struct device *dev, return 0; } -static int gpio_mcux_write(struct device *dev, - int access_op, u32_t pin, u32_t value) -{ - const struct gpio_mcux_config *config = dev->config->config_info; - GPIO_Type *gpio_base = config->gpio_base; - - if (access_op == GPIO_ACCESS_BY_PIN) { - if (value) { - /* Set the data output for the corresponding pin. - * Writing zeros to the other bits leaves the data - * output unchanged for the other pins. - */ - gpio_base->PSOR = BIT(pin); - } else { - /* Clear the data output for the corresponding pin. - * Writing zeros to the other bits leaves the data - * output unchanged for the other pins. - */ - gpio_base->PCOR = BIT(pin); - } - } else { /* GPIO_ACCESS_BY_PORT */ - /* Write the data output for all the pins */ - gpio_base->PDOR = value; - } - - return 0; -} - -static int gpio_mcux_read(struct device *dev, - int access_op, u32_t pin, u32_t *value) -{ - const struct gpio_mcux_config *config = dev->config->config_info; - GPIO_Type *gpio_base = config->gpio_base; - - *value = gpio_base->PDIR; - - if (access_op == GPIO_ACCESS_BY_PIN) { - *value = (*value & BIT(pin)) >> pin; - } - - /* nothing more to do for GPIO_ACCESS_BY_PORT */ - - return 0; -} - static int gpio_mcux_port_get_raw(struct device *dev, u32_t *value) { const struct gpio_mcux_config *config = dev->config->config_info; @@ -357,8 +312,6 @@ static void gpio_mcux_port_isr(void *arg) static const struct gpio_driver_api gpio_mcux_driver_api = { .config = gpio_mcux_configure, - .write = gpio_mcux_write, - .read = gpio_mcux_read, .port_get_raw = gpio_mcux_port_get_raw, .port_set_masked_raw = gpio_mcux_port_set_masked_raw, .port_set_bits_raw = gpio_mcux_port_set_bits_raw, From a73ddb5113e9dc3cb9e2c20b2893f2b72ec876bf Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 08:18:28 -0500 Subject: [PATCH 13/17] drivers: gpio_gecko: remove outdated driver functions The access-oriented GPIO read and write system calls have been replaced with new API. Remove the old implementation. Signed-off-by: Peter Bigot --- drivers/gpio/gpio_gecko.c | 47 --------------------------------------- 1 file changed, 47 deletions(-) diff --git a/drivers/gpio/gpio_gecko.c b/drivers/gpio/gpio_gecko.c index 8da6b758378b6..232fa05053aa6 100644 --- a/drivers/gpio/gpio_gecko.c +++ b/drivers/gpio/gpio_gecko.c @@ -160,51 +160,6 @@ static int gpio_gecko_configure(struct device *dev, return 0; } -static int gpio_gecko_write(struct device *dev, - int access_op, u32_t pin, u32_t value) -{ - const struct gpio_gecko_config *config = dev->config->config_info; - GPIO_P_TypeDef *gpio_base = config->gpio_base; - - if (access_op == GPIO_ACCESS_BY_PIN) { - if (value) { - /* Set the data output for the corresponding pin. - * Writing zeros to the other bits leaves the data - * output unchanged for the other pins. - */ - GPIO_PinOutSet(config->gpio_index, pin); - } else { - /* Clear the data output for the corresponding pin. - * Writing zeros to the other bits leaves the data - * output unchanged for the other pins. - */ - GPIO_PinOutClear(config->gpio_index, pin); - } - } else { /* GPIO_ACCESS_BY_PORT */ - /* Write the data output for all the pins */ - gpio_base->DOUT = value; - } - - return 0; -} - -static int gpio_gecko_read(struct device *dev, - int access_op, u32_t pin, u32_t *value) -{ - const struct gpio_gecko_config *config = dev->config->config_info; - GPIO_P_TypeDef *gpio_base = config->gpio_base; - - *value = gpio_base->DIN; - - if (access_op == GPIO_ACCESS_BY_PIN) { - *value = (*value & BIT(pin)) >> pin; - } - - /* nothing more to do for GPIO_ACCESS_BY_PORT */ - - return 0; -} - static int gpio_gecko_port_get_raw(struct device *dev, u32_t *value) { const struct gpio_gecko_config *config = dev->config->config_info; @@ -373,8 +328,6 @@ static void gpio_gecko_common_isr(void *arg) static const struct gpio_driver_api gpio_gecko_driver_api = { .config = gpio_gecko_configure, - .write = gpio_gecko_write, - .read = gpio_gecko_read, .port_get_raw = gpio_gecko_port_get_raw, .port_set_masked_raw = gpio_gecko_port_set_masked_raw, .port_set_bits_raw = gpio_gecko_port_set_bits_raw, From e487fa7020737d88326c4029685805e2a810b03b Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:17:21 -0500 Subject: [PATCH 14/17] drivers: gpio_sx1509b: remove outdated driver functions The access-oriented GPIO read and write system calls have been replaced with new API. Remove the old implementation. Signed-off-by: Peter Bigot --- drivers/gpio/gpio_sx1509b.c | 70 ------------------------------------- 1 file changed, 70 deletions(-) diff --git a/drivers/gpio/gpio_sx1509b.c b/drivers/gpio/gpio_sx1509b.c index ac1ffffd27e9d..277a37fec2ee6 100644 --- a/drivers/gpio/gpio_sx1509b.c +++ b/drivers/gpio/gpio_sx1509b.c @@ -344,74 +344,6 @@ static int pin_interrupt_configure(struct device *dev, return -ENOTSUP; } -static int sx1509b_write(struct device *dev, int access_op, u32_t pin, - u32_t value) -{ - const struct sx1509b_config *cfg = dev->config->config_info; - struct sx1509b_drv_data *drv_data = dev->driver_data; - u16_t *pin_data = &drv_data->pin_state.data; - int ret = 0; - - k_sem_take(&drv_data->lock, K_FOREVER); - - switch (access_op) { - case GPIO_ACCESS_BY_PIN: - if (value) { - *pin_data |= BIT(pin); - } else { - *pin_data &= ~BIT(pin); - } - break; - case GPIO_ACCESS_BY_PORT: - *pin_data = value; - break; - default: - ret = -ENOTSUP; - goto out; - } - - ret = i2c_reg_write_word_be(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_DATA, *pin_data); -out: - k_sem_give(&drv_data->lock); - return ret; -} - -static int sx1509b_read(struct device *dev, int access_op, u32_t pin, - u32_t *value) -{ - const struct sx1509b_config *cfg = dev->config->config_info; - struct sx1509b_drv_data *drv_data = dev->driver_data; - u16_t pin_data; - int ret; - - k_sem_take(&drv_data->lock, K_FOREVER); - - ret = i2c_burst_read(drv_data->i2c_master, cfg->i2c_slave_addr, - SX1509B_REG_DATA, (u8_t *)&pin_data, - sizeof(pin_data)); - if (ret) { - goto out; - } - - pin_data = sys_be16_to_cpu(pin_data); - - switch (access_op) { - case GPIO_ACCESS_BY_PIN: - *value = !!(pin_data & (BIT(pin))); - break; - case GPIO_ACCESS_BY_PORT: - *value = pin_data; - break; - default: - ret = -ENOTSUP; - } - -out: - k_sem_give(&drv_data->lock); - return ret; -} - /** * @brief Initialization function of SX1509B * @@ -471,8 +403,6 @@ static int sx1509b_init(struct device *dev) static const struct gpio_driver_api api_table = { .config = config, - .write = sx1509b_write, - .read = sx1509b_read, .port_get_raw = port_get, .port_set_masked_raw = port_set_masked, .port_set_bits_raw = port_set_bits, From 2db9c668f6d1d3a2f239c8312d1d35db29816e26 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Tue, 27 Aug 2019 10:42:28 -0500 Subject: [PATCH 15/17] WIP tests/drivers/gpio: generalize gpio specification solution Provide board-specific overlays that specify which pins to use for the test, rather than use aliases in the board device tree. This uses a binding that's specifically documented for use in GPIO applications. Signed-off-by: Peter Bigot --- dts/bindings/misc/test,scope-pins.yaml | 29 +++++++++++++++++++ .../gpio_basic_api/efr32mg_sltb004a.overlay | 12 ++++++++ .../gpio/gpio_basic_api/frdm_k64f.overlay | 18 ++++++++++++ .../gpio_basic_api/nrf52840_pca10056.overlay | 17 +++++++++++ tests/drivers/gpio/gpio_basic_api/prj.conf | 1 + tests/drivers/gpio/gpio_basic_api/src/main.c | 21 ++++++++++++++ .../gpio/gpio_basic_api/src/test_gpio.h | 9 ++++-- 7 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 dts/bindings/misc/test,scope-pins.yaml create mode 100644 tests/drivers/gpio/gpio_basic_api/efr32mg_sltb004a.overlay create mode 100644 tests/drivers/gpio/gpio_basic_api/frdm_k64f.overlay create mode 100644 tests/drivers/gpio/gpio_basic_api/nrf52840_pca10056.overlay diff --git a/dts/bindings/misc/test,scope-pins.yaml b/dts/bindings/misc/test,scope-pins.yaml new file mode 100644 index 0000000000000..998490bb715fa --- /dev/null +++ b/dts/bindings/misc/test,scope-pins.yaml @@ -0,0 +1,29 @@ +# +# Copyright (c) 2019 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: Apache-2.0 +# + +title: GPIO pin definitions for tests + +description: > + Identify accessible board pins that can be used for GPIO signals. + + Most development boards expose several GPIO pins on a header. These + may be used as signals to a logic analyzer, or to simulate an + external device. This binding provides a board-independent + description of pins that can be made available for this purpose. + +compatible: "test,scope-pins" + +properties: + gpios: + type: compound + required: true + description: > + Accessible pins used for GPIO-based instrumentation and + testing. + + Any number of pins may be provided, on multiple GPIO devices. + However at least two entries must be present, and the first + two must be on the same GPIO device. diff --git a/tests/drivers/gpio/gpio_basic_api/efr32mg_sltb004a.overlay b/tests/drivers/gpio/gpio_basic_api/efr32mg_sltb004a.overlay new file mode 100644 index 0000000000000..de8381e5d0aee --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/efr32mg_sltb004a.overlay @@ -0,0 +1,12 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/ { + scope-pins { + compatible = "test,scope-pins"; + gpios = <&gpiof 6 0>, <&gpiof 5 0>; + }; +}; diff --git a/tests/drivers/gpio/gpio_basic_api/frdm_k64f.overlay b/tests/drivers/gpio/gpio_basic_api/frdm_k64f.overlay new file mode 100644 index 0000000000000..538ae4deb163c --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/frdm_k64f.overlay @@ -0,0 +1,18 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/ { + scope-pins { + compatible = "test,scope-pins"; + gpios = + <&gpioc 16 0>, <&gpioc 17 0>, + <&gpioc 2 0>, <&gpioc 3 0>, + <&gpioc 1 0>, <&gpioc 8 0>, + <&gpioc 9 0>, <&gpioc 0 0>, + <&gpioc 7 0>, <&gpioc 5 0>; + + }; +}; diff --git a/tests/drivers/gpio/gpio_basic_api/nrf52840_pca10056.overlay b/tests/drivers/gpio/gpio_basic_api/nrf52840_pca10056.overlay new file mode 100644 index 0000000000000..c361039af3e8b --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/nrf52840_pca10056.overlay @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/ { + scope-pins { + compatible = "test,scope-pins"; + /* Use pins on P3 (Arduino D0) header. */ + gpios = + <&gpio1 1 0>, <&gpio1 2 0>, + <&gpio1 3 0>, <&gpio1 4 0>, + <&gpio1 5 0>, <&gpio1 6 0>, + <&gpio1 7 0>, <&gpio1 8 0>; + }; +}; diff --git a/tests/drivers/gpio/gpio_basic_api/prj.conf b/tests/drivers/gpio/gpio_basic_api/prj.conf index 505646e546992..b6b799238234d 100644 --- a/tests/drivers/gpio/gpio_basic_api/prj.conf +++ b/tests/drivers/gpio/gpio_basic_api/prj.conf @@ -2,3 +2,4 @@ CONFIG_GPIO=y CONFIG_ZTEST=y CONFIG_ENTROPY_GENERATOR=y CONFIG_TEST_RANDOM_GENERATOR=y +#CONFIG_TEST_USERSPACE=y diff --git a/tests/drivers/gpio/gpio_basic_api/src/main.c b/tests/drivers/gpio/gpio_basic_api/src/main.c index 96478f7957e6c..48fa8be944748 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/main.c +++ b/tests/drivers/gpio/gpio_basic_api/src/main.c @@ -7,8 +7,29 @@ #include "test_gpio.h" +/* Grotesque hack for pinmux boards */ +#ifdef CONFIG_BOARD_FRDM_K64F +#include +#include +#endif + +static void board_setup(void) +{ +#ifdef CONFIG_BOARD_FRDM_K64F + printk("Setting up pinmux for K64F\n"); + const char *pmx_name = "portc"; + struct device *pmx = device_get_binding(pmx_name); + printk("pmx %s for %s: %p\n", pmx_name, DEV_NAME, pmx); + int rc = pinmux_pin_set(pmx, PIN_OUT, PORT_PCR_MUX(kPORT_MuxAsGpio)); + printk("pmx pin %u: %d\n", PIN_OUT, rc); + rc = pinmux_pin_set(pmx, PIN_IN, PORT_PCR_MUX(kPORT_MuxAsGpio)); + printk("pmx pin %u: %d\n", PIN_IN, rc); +#endif +} + void test_main(void) { + board_setup(); ztest_test_suite(gpio_basic_test, ztest_unit_test(test_gpio_port), ztest_unit_test(test_gpio_pin_read_write), diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h index 0846ef3edc089..bf8c5c2fb931a 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h @@ -12,7 +12,11 @@ #include #include -#if defined(DT_ALIAS_GPIO_0_LABEL) +#ifdef DT_INST_0_TEST_SCOPE_PINS +#define DEV_NAME DT_INST_0_TEST_SCOPE_PINS_GPIOS_CONTROLLER_0 +#define PIN_OUT DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_0 +#define PIN_IN DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_1 +#elif defined(DT_ALIAS_GPIO_0_LABEL) #define DEV_NAME DT_ALIAS_GPIO_0_LABEL #elif defined(DT_ALIAS_GPIO_1_LABEL) #define DEV_NAME DT_ALIAS_GPIO_1_LABEL @@ -22,9 +26,10 @@ #error Unsupported board #endif +#ifndef PIN_OUT #define PIN_OUT 2 #define PIN_IN 3 - +#endif /* PIN_OUT */ #define MAX_INT_CNT 3 struct drv_data { From 33a28b4ea441167054bbf9f8298b18d1487d2055 Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Sun, 1 Sep 2019 11:05:53 -0500 Subject: [PATCH 16/17] DNM tests/drivers/gpio: sx1509B test infrastructure --- .../gpio_basic_api/particle_xenon.overlay | 23 +++++++++++++++++++ tests/drivers/gpio/gpio_basic_api/prj.conf | 2 ++ 2 files changed, 25 insertions(+) create mode 100644 tests/drivers/gpio/gpio_basic_api/particle_xenon.overlay diff --git a/tests/drivers/gpio/gpio_basic_api/particle_xenon.overlay b/tests/drivers/gpio/gpio_basic_api/particle_xenon.overlay new file mode 100644 index 0000000000000..5d85e29b1fb5d --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/particle_xenon.overlay @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +&i2c0 { + sx1509b: sx1509b@3e { + compatible = "semtech,sx1509b"; + reg = <0x3e>; + label = "IOX0"; + #gpio-cells = <2>; + gpio-controller; + }; +}; + +/ { + scope-pins { + compatible = "test,scope-pins"; + gpios = + <&sx1509b 0 0>, <&sx1509b 1 0>; + }; +}; diff --git a/tests/drivers/gpio/gpio_basic_api/prj.conf b/tests/drivers/gpio/gpio_basic_api/prj.conf index b6b799238234d..fde8d3f3f39c4 100644 --- a/tests/drivers/gpio/gpio_basic_api/prj.conf +++ b/tests/drivers/gpio/gpio_basic_api/prj.conf @@ -3,3 +3,5 @@ CONFIG_ZTEST=y CONFIG_ENTROPY_GENERATOR=y CONFIG_TEST_RANDOM_GENERATOR=y #CONFIG_TEST_USERSPACE=y +#CONFIG_I2C=y +#CONFIG_GPIO_SX1509B=y From ab0d822ad58e0eb1290dee1b5b797a7100acbace Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Thu, 29 Aug 2019 11:14:36 -0500 Subject: [PATCH 17/17] WIP tests/drivers/gpio: add performance tests Time execution of a sequence of operations. s1 test takes 7.02 us on pca10056. --- tests/drivers/gpio/gpio_basic_api/src/main.c | 1 + .../gpio/gpio_basic_api/src/test_gpio.h | 2 +- .../gpio/gpio_basic_api/src/test_gpio_perf.c | 158 ++++++++++++++++++ 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 tests/drivers/gpio/gpio_basic_api/src/test_gpio_perf.c diff --git a/tests/drivers/gpio/gpio_basic_api/src/main.c b/tests/drivers/gpio/gpio_basic_api/src/main.c index 48fa8be944748..87f4aa82c6b41 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/main.c +++ b/tests/drivers/gpio/gpio_basic_api/src/main.c @@ -31,6 +31,7 @@ void test_main(void) { board_setup(); ztest_test_suite(gpio_basic_test, + ztest_unit_test(test_gpio_perf), ztest_unit_test(test_gpio_port), ztest_unit_test(test_gpio_pin_read_write), ztest_unit_test(test_gpio_callback_add_remove), diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h index bf8c5c2fb931a..04a70f6ef7ee3 100644 --- a/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio.h @@ -44,7 +44,7 @@ void test_gpio_callback_add_remove(void); void test_gpio_callback_self_remove(void); void test_gpio_callback_enable_disable(void); void test_gpio_callback_variants(void); - +void test_gpio_perf(void); void test_gpio_port(void); #endif /* __TEST_GPIO_H__ */ diff --git a/tests/drivers/gpio/gpio_basic_api/src/test_gpio_perf.c b/tests/drivers/gpio/gpio_basic_api/src/test_gpio_perf.c new file mode 100644 index 0000000000000..8b5815543b385 --- /dev/null +++ b/tests/drivers/gpio/gpio_basic_api/src/test_gpio_perf.c @@ -0,0 +1,158 @@ +/* + * Copyright (c) 2019 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "test_gpio.h" + +static struct device *dev; +static gpio_port_pins_t out_pins; +static gpio_port_pins_t in_pins; +static gpio_port_pins_t os1_pins; /* active high pins */ +static gpio_port_pins_t os2_pins; /* active low pins */ + +static void measure(const char* tag, + void (*op)(void)) +{ + u32_t ts = k_cycle_get_32(); + u32_t const dur_cyc = sys_clock_hw_cycles_per_sec() / 2; + u32_t te; + unsigned int count = 0; + + do { + op(); + te = k_cycle_get_32(); + ++count; + } while ((te - ts) < dur_cyc); + + + gpio_port_clear_bits_raw(dev, out_pins); + + u64_t nom = (te - ts) * (u64_t)NSEC_PER_SEC; + u64_t div = count * (u64_t)sys_clock_hw_cycles_per_sec(); + u32_t op_us = ceiling_fraction(nom, div); + + TC_PRINT("- %s : %u iterations in %u cycles : %u ns / op\n", + tag, count, te - ts, op_us); +} + +static void iter_nop(void) +{ +} + +static void iter_s1(void) +{ + /* All pins high */ + gpio_port_set_bits_raw(dev, out_pins); + + /* All pins inactive: s1 goes low, s2 stays high */ + gpio_port_clear_bits(dev, out_pins); + + /* s1 goes high, s2 stays high */ + gpio_port_set_bits(dev, os1_pins); + + /* s1 stays high, s2 goes low */ + gpio_port_set_bits(dev, os2_pins); + + /* toggle: s1 low, s2 high */ + gpio_port_toggle_bits(dev, out_pins); + + /* OUT high, rest unchanged */ + gpio_pin_set(dev, PIN_OUT, 1); + + /* All pins low */ + gpio_port_clear_bits_raw(dev, out_pins); +} + +static int test_perf(void) +{ + static const struct scope_type { + const char* devname; + u8_t pin; + } scope_pins[] = { +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_2 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_2, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_2 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_3 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_3, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_3 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_4 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_4, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_4 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_5 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_5, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_5 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_6 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_6, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_6 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_7 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_7, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_7 }, +#endif +#ifdef DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_8 + { DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_CONTROLLER_8, + DT_TEST_SCOPE_PINS_SCOPE_PINS_GPIOS_PIN_8 }, +#endif + }; + const struct scope_type *sp = scope_pins; + const struct scope_type *const spe = sp + sizeof(scope_pins) / sizeof(*scope_pins); + size_t nout = 1; + + out_pins = BIT(PIN_OUT); + dev = device_get_binding(DEV_NAME); + zassert_not_equal(dev, NULL, + "Device not found"); + + zassert_equal(gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_LOW), 0, + "PIN_OUT configure failed"); + zassert_equal(gpio_pin_configure(dev, PIN_IN, GPIO_INPUT), 0, + "PIN_OUT configure failed"); + + while (sp < spe) { + printk("P%u : %s %u\n", (u32_t)(sp - scope_pins), sp->devname, sp->pin); + if (strcmp(sp->devname, DEV_NAME) == 0) { + u32_t flags = GPIO_OUTPUT_LOW; + out_pins |= BIT(sp->pin); + if ((nout & 1) == 1) { + flags |= GPIO_ACTIVE_LOW; + os2_pins |= BIT(sp->pin); + } + zassert_equal(gpio_pin_configure(dev, sp->pin, flags), 0, + "Scope pin configure failed"); + ++nout; + } + ++sp; + } + + os1_pins = out_pins ^ os2_pins; + + out_pins |= BIT(PIN_OUT); + in_pins = BIT(PIN_IN); + + TC_PRINT("%s : on %s os1 %x os2 %x in %x\n", __func__, + DEV_NAME, os1_pins, os2_pins, in_pins); + + k_usleep(1); + + measure("nop", iter_nop); + measure("s1", iter_s1); + + return TC_PASS; +} + + +void test_gpio_perf(void) +{ + if (!IS_ENABLED(DT_INST_0_TEST_SCOPE_PINS)) { + TC_PRINT("Performance test not supported\n"); + return; + } + zassert_equal(test_perf(), TC_PASS, + "performance test completed"); +}