Skip to content

[TOPIC-GPIO] gpio: Update mcux igpio driver to use new gpio api #19247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 15, 2019
Merged

[TOPIC-GPIO] gpio: Update mcux igpio driver to use new gpio api #19247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9e37cf
gpio: Update mcux igpio driver to use new gpio api
MaureenHelm Sep 17, 2019
7b8e549
boards: mimxrt1020_evk: Fix button label to match silkscreen
MaureenHelm Sep 18, 2019
16396f7
tests: gpio: Enable gpio_basic_api test on mimxrt1050_evk
MaureenHelm Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions boards/arm/mimxrt1015_evk/mimxrt1015_evk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
leds {
compatible = "gpio-leds";
green_led: led-1 {
gpios = <&gpio3 21 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio3 21 GPIO_ACTIVE_LOW>;
label = "User LD1";
};
};
Expand All @@ -42,7 +42,7 @@
compatible = "gpio-keys";
user_button: button-1 {
label = "User SW4";
gpios = <&gpio2 9 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio2 9 GPIO_ACTIVE_LOW>;
};
};
};
Expand Down
6 changes: 3 additions & 3 deletions boards/arm/mimxrt1020_evk/mimxrt1020_evk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,16 @@
leds {
compatible = "gpio-leds";
green_led: led-1 {
gpios = <&gpio1 5 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
label = "User LD1";
};
};

gpio_keys {
compatible = "gpio-keys";
user_button: button-1 {
label = "User SW8";
gpios = <&gpio5 0 GPIO_INT_ACTIVE_LOW>;
label = "User SW4";
gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
};
};
};
Expand Down
4 changes: 2 additions & 2 deletions boards/arm/mimxrt1050_evk/mimxrt1050_evk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
leds {
compatible = "gpio-leds";
green_led: led_0 {
gpios = <&gpio1 9 0>;
gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
label = "User LD1";
};
};
Expand All @@ -51,7 +51,7 @@
compatible = "gpio-keys";
user_button: button_0 {
label = "User SW8";
gpios = <&gpio5 0 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
};
};

Expand Down
4 changes: 2 additions & 2 deletions boards/arm/mimxrt1060_evk/mimxrt1060_evk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
leds {
compatible = "gpio-leds";
green_led: led-1 {
gpios = <&gpio1 9 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
label = "User LD1";
};
};
Expand All @@ -48,7 +48,7 @@
compatible = "gpio-keys";
user_button: button-1 {
label = "User SW8";
gpios = <&gpio5 0 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
};
};

Expand Down
4 changes: 2 additions & 2 deletions boards/arm/mimxrt1064_evk/mimxrt1064_evk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
leds {
compatible = "gpio-leds";
green_led: led-1 {
gpios = <&gpio1 9 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
label = "User LD1";
};
};
Expand All @@ -57,7 +57,7 @@
compatible = "gpio-keys";
user_button: button-1 {
label = "User SW8";
gpios = <&gpio5 0 GPIO_INT_ACTIVE_LOW>;
gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
};
};

Expand Down
174 changes: 129 additions & 45 deletions drivers/gpio/gpio_mcux_igpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ struct mcux_igpio_config {
};

struct mcux_igpio_data {
/* gpio_driver_data needs to be first */
struct gpio_driver_data general;
/* port ISR callback routine address */
sys_slist_t callbacks;
/* pin callback routine enable flags, by pin number */
Expand All @@ -28,49 +30,34 @@ static int mcux_igpio_configure(struct device *dev,
int access_op, u32_t pin, int flags)
{
const struct mcux_igpio_config *config = dev->config->config_info;
gpio_pin_config_t pin_config;
u32_t i;
GPIO_Type *base = config->base;

/* Check for an invalid pin configuration */
if ((flags & GPIO_INT) && (flags & GPIO_DIR_OUT)) {
return -EINVAL;
if (((flags & GPIO_INPUT) != 0) && ((flags & GPIO_OUTPUT) != 0)) {
return -ENOTSUP;
}

pin_config.direction = ((flags & GPIO_DIR_MASK) == GPIO_DIR_IN)
? kGPIO_DigitalInput : kGPIO_DigitalOutput;

pin_config.outputLogic = 0;

if (flags & GPIO_INT) {
if (flags & GPIO_INT_EDGE) {
if (flags & GPIO_INT_ACTIVE_HIGH) {
pin_config.interruptMode = kGPIO_IntRisingEdge;
} else if (flags & GPIO_INT_DOUBLE_EDGE) {
pin_config.interruptMode =
kGPIO_IntRisingOrFallingEdge;
} else {
pin_config.interruptMode =
kGPIO_IntFallingEdge;
}
} else { /* GPIO_INT_LEVEL */
if (flags & GPIO_INT_ACTIVE_HIGH) {
pin_config.interruptMode = kGPIO_IntHighLevel;
} else {
pin_config.interruptMode = kGPIO_IntLowLevel;
}
}
} else {
pin_config.interruptMode = kGPIO_NoIntmode;
if ((flags & GPIO_SINGLE_ENDED) != 0) {
return -ENOTSUP;
}

if (access_op == GPIO_ACCESS_BY_PIN) {
GPIO_PinInit(config->base, pin, &pin_config);
} else { /* GPIO_ACCESS_BY_PORT */
for (i = 0U; i < 32; i++) {
GPIO_PinInit(config->base, i, &pin_config);
}
if (((flags & GPIO_PULL_UP) != 0) || ((flags & GPIO_PULL_DOWN) != 0)) {
return -ENOTSUP;
}

if (access_op == GPIO_ACCESS_BY_PORT) {
return -ENOTSUP;
}

if (flags & GPIO_OUTPUT_INIT_HIGH) {
base->DR_SET = BIT(pin);
}

if (flags & GPIO_OUTPUT_INIT_LOW) {
base->DR_CLEAR = BIT(pin);
}

WRITE_BIT(base->GDIR, pin, flags & GPIO_OUTPUT);

return 0;
}

Expand Down Expand Up @@ -102,6 +89,103 @@ static int mcux_igpio_read(struct device *dev,
return 0;
}

static int mcux_igpio_port_get_raw(struct device *dev, u32_t *value)
{
const struct mcux_igpio_config *config = dev->config->config_info;
GPIO_Type *base = config->base;

*value = base->DR;

return 0;
}

static int mcux_igpio_port_set_masked_raw(struct device *dev, u32_t mask,
u32_t value)
{
const struct mcux_igpio_config *config = dev->config->config_info;
GPIO_Type *base = config->base;

base->DR = (base->DR & ~mask) | (mask & value);

return 0;
}

static int mcux_igpio_port_set_bits_raw(struct device *dev, u32_t mask)
{
const struct mcux_igpio_config *config = dev->config->config_info;
GPIO_Type *base = config->base;

base->DR_SET = mask;

return 0;
}

static int mcux_igpio_port_clear_bits_raw(struct device *dev, u32_t mask)
{
const struct mcux_igpio_config *config = dev->config->config_info;
GPIO_Type *base = config->base;

base->DR_CLEAR = mask;

return 0;
}

static int mcux_igpio_port_toggle_bits(struct device *dev, u32_t mask)
{
const struct mcux_igpio_config *config = dev->config->config_info;
GPIO_Type *base = config->base;

base->DR_TOGGLE = mask;

return 0;
}

static int mcux_igpio_pin_interrupt_configure(struct device *dev,
unsigned int pin, enum gpio_int_mode mode,
enum gpio_int_trig trig)
{
const struct mcux_igpio_config *config = dev->config->config_info;
struct mcux_igpio_data *data = dev->driver_data;
GPIO_Type *base = config->base;
unsigned int key;
u8_t icr;
int shift;

if ((mode == GPIO_INT_MODE_EDGE) && (trig == GPIO_INT_TRIG_LOW)) {
icr = 3;
} else if ((mode == GPIO_INT_MODE_EDGE) &&
(trig == GPIO_INT_TRIG_HIGH)) {
icr = 2;
} else if ((mode == GPIO_INT_MODE_LEVEL) &&
(trig == GPIO_INT_TRIG_HIGH)) {
icr = 1;
} else {
icr = 0;
}

if (pin < 16) {
shift = 2 * pin;
base->ICR1 = (base->ICR1 & ~(3 << shift)) | (icr << shift);
} else if (pin < 32) {
shift = 2 * (pin - 16);
base->ICR2 = (base->ICR2 & ~(3 << shift)) | (icr << shift);
} else {
Copy link
Member

@mnkp mnkp Sep 28, 2019

Choose a reason for hiding this comment

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

There is an assert in the API that guarantees that pin number is smaller than the maximum port width. Since the maximum port width in case of mcux driver is 32 we could skip the else.

return -EINVAL;
}

key = irq_lock();

WRITE_BIT(base->EDGE_SEL, pin, trig == GPIO_INT_TRIG_BOTH);
WRITE_BIT(base->ISR, pin, mode != GPIO_INT_MODE_DISABLED);
WRITE_BIT(base->IMR, pin, mode != GPIO_INT_MODE_DISABLED);
WRITE_BIT(data->pin_callback_enables, pin,
mode != GPIO_INT_MODE_DISABLED);

irq_unlock(key);

return 0;
}

static int mcux_igpio_manage_callback(struct device *dev,
struct gpio_callback *callback, bool set)
{
Expand All @@ -113,15 +197,12 @@ static int mcux_igpio_manage_callback(struct device *dev,
static int mcux_igpio_enable_callback(struct device *dev,
int access_op, u32_t pin)
{
const struct mcux_igpio_config *config = dev->config->config_info;
struct mcux_igpio_data *data = dev->driver_data;

if (access_op == GPIO_ACCESS_BY_PIN) {
data->pin_callback_enables |= BIT(pin);
GPIO_PortEnableInterrupts(config->base, BIT(pin));
} else {
data->pin_callback_enables = 0xFFFFFFFF;
GPIO_PortEnableInterrupts(config->base, 0xFFFFFFFF);
}

return 0;
Expand All @@ -130,14 +211,11 @@ static int mcux_igpio_enable_callback(struct device *dev,
static int mcux_igpio_disable_callback(struct device *dev,
int access_op, u32_t pin)
{
const struct mcux_igpio_config *config = dev->config->config_info;
struct mcux_igpio_data *data = dev->driver_data;

if (access_op == GPIO_ACCESS_BY_PIN) {
GPIO_PortDisableInterrupts(config->base, BIT(pin));
data->pin_callback_enables &= ~BIT(pin);
} else {
GPIO_PortDisableInterrupts(config->base, 0);
data->pin_callback_enables = 0U;
}

Expand All @@ -149,20 +227,26 @@ static void mcux_igpio_port_isr(void *arg)
struct device *dev = (struct device *)arg;
const struct mcux_igpio_config *config = dev->config->config_info;
struct mcux_igpio_data *data = dev->driver_data;
GPIO_Type *base = config->base;
u32_t enabled_int, int_flags;

int_flags = GPIO_PortGetInterruptFlags(config->base);
int_flags = base->ISR;
enabled_int = int_flags & data->pin_callback_enables;
base->ISR = enabled_int;

gpio_fire_callbacks(&data->callbacks, dev, enabled_int);

GPIO_ClearPinsInterruptFlags(config->base, enabled_int);
}

static const struct gpio_driver_api mcux_igpio_driver_api = {
.config = mcux_igpio_configure,
.write = mcux_igpio_write,
.read = mcux_igpio_read,
.port_get_raw = mcux_igpio_port_get_raw,
.port_set_masked_raw = mcux_igpio_port_set_masked_raw,
.port_set_bits_raw = mcux_igpio_port_set_bits_raw,
.port_clear_bits_raw = mcux_igpio_port_clear_bits_raw,
.port_toggle_bits = mcux_igpio_port_toggle_bits,
.pin_interrupt_configure = mcux_igpio_pin_interrupt_configure,
.manage_callback = mcux_igpio_manage_callback,
.enable_callback = mcux_igpio_enable_callback,
.disable_callback = mcux_igpio_disable_callback,
Expand Down
13 changes: 13 additions & 0 deletions tests/drivers/gpio/gpio_basic_api/boards/mimxrt1050_evk.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright (c) 2019 Nordic Semiconductor ASA
Copy link
Contributor

@agansari agansari Oct 7, 2019

Choose a reason for hiding this comment

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

change copyright

Copy link
Member Author

@MaureenHelm MaureenHelm Oct 14, 2019

Choose a reason for hiding this comment

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

I kept the Nordic copyright because I copied the file from another board

*
* SPDX-License-Identifier: Apache-2.0
*/

/ {
resources {
compatible = "test,gpio_basic_api";
out-gpios = <&gpio1 23 0>; /* Arduino D0 */
in-gpios = <&gpio1 22 0>; /* Arduino D1 */
};
};
20 changes: 20 additions & 0 deletions tests/drivers/gpio/gpio_basic_api/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
#include <fsl_port.h>
#endif

#ifdef CONFIG_BOARD_MIMXRT1050_EVK
#include <fsl_iomuxc.h>
#endif

static void board_setup(void)
{
#ifdef DT_INST_0_TEST_GPIO_BASIC_API
Expand All @@ -34,6 +38,22 @@ static void board_setup(void)
pinmux_pin_set(pmx, PIN_OUT, PORT_PCR_MUX(kPORT_MuxAsGpio));
pinmux_pin_set(pmx, PIN_IN, PORT_PCR_MUX(kPORT_MuxAsGpio));
#endif

#ifdef CONFIG_BOARD_MIMXRT1050_EVK
IOMUXC_SetPinMux(IOMUXC_GPIO_AD_B1_06_GPIO1_IO22, 0);
IOMUXC_SetPinMux(IOMUXC_GPIO_AD_B1_07_GPIO1_IO23, 0);

IOMUXC_SetPinConfig(IOMUXC_GPIO_AD_B1_06_GPIO1_IO22,
IOMUXC_SW_PAD_CTL_PAD_PKE_MASK |
IOMUXC_SW_PAD_CTL_PAD_HYS_MASK |
IOMUXC_SW_PAD_CTL_PAD_SPEED(2) |
IOMUXC_SW_PAD_CTL_PAD_DSE(6));

IOMUXC_SetPinConfig(IOMUXC_GPIO_AD_B1_07_GPIO1_IO23,
IOMUXC_SW_PAD_CTL_PAD_PKE_MASK |
IOMUXC_SW_PAD_CTL_PAD_SPEED(2) |
IOMUXC_SW_PAD_CTL_PAD_DSE(6));
#endif
Comment on lines +42 to +56
Copy link
Contributor

@agansari agansari Oct 7, 2019

Choose a reason for hiding this comment

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

i moved this to board's pinmux initialization

Copy link
Member Author

@MaureenHelm MaureenHelm Oct 14, 2019

Choose a reason for hiding this comment

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

Yes this is ugly here, but I also don't want to make it the default pinmux configuration for the board. I think it's more likely these pins will be used for UART.

}

void test_main(void)
Expand Down