Skip to content

driver: gpio: update gpio_mcux.c driver #89020

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.

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

Already on GitHub? Sign in to your account

Closed
Changes from all commits
Commits
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
21 changes: 9 additions & 12 deletions drivers/gpio/gpio_mcux.c
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ static uint32_t get_port_pcr_irqc_value_from_flags(const struct device *dev, uin
#endif /* !defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT */
#endif /* !(defined(CONFIG_PINCTRL_NXP_IOCON)) */

#if (defined(FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT) && \
FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT)
#if (defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT) || \
(!defined(FSL_FEATURE_SOC_PORT_COUNT))
Copy link
Collaborator

@hakehuang hakehuang Apr 24, 2025

Choose a reason for hiding this comment

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

the logic seems not correct. use to be FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT but now FSL_FEATURE_PORT_HAS_NO_INTERRUPT, this could impact other platform

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi hake, as my comment in this PR, cannot use FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT to judge if ICR register exist. This feature only use to judge if bitfield IRQS exist in ICR register. So suggest to use feature FSL_FEATURE_PORT_HAS_NO_INTERRUPT to do this judgment. This judgment is also used in our sdk, it can work fine. the comment you mentioned that this may impact other platform need to be considered, I tested the boards(frdmmcxn947,frdmmcxn236,frdmmcxa156,frdmmcxa153) I have, all can work fine. Could you help do some tests on other platform?

Copy link
Collaborator

Choose a reason for hiding this comment

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

please take a look at #88979. this avoid such potential issue


#define GPIO_MCUX_INTERRUPT_DISABLED 0
#define GPIO_MCUX_INTERRUPT_LOGIC_0 0x8
Expand Down Expand Up @@ -335,7 +335,7 @@ static uint32_t get_gpio_icr_irqc_value_from_flags(const struct device *dev, uin

return GPIO_ICR_IRQC(gpio_interrupt);
}
#endif /* (defined(FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT) */
#endif /* (defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) */

static int gpio_mcux_pin_interrupt_configure(const struct device *dev, gpio_pin_t pin,
enum gpio_int_mode mode, enum gpio_int_trig trig)
Expand All @@ -361,12 +361,12 @@ static int gpio_mcux_pin_interrupt_configure(const struct device *dev, gpio_pin_
return -ENOTSUP;
}

#if (defined(FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT) && \
FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT)
#if (defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT) || \
(!defined(FSL_FEATURE_SOC_PORT_COUNT))
uint32_t icr = get_gpio_icr_irqc_value_from_flags(dev, pin, mode, trig);

gpio_base->ICR[pin] = (gpio_base->ICR[pin] & ~GPIO_ICR_IRQC_MASK) | icr;
#elif !(defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT)
#else
uint32_t pcr = get_port_pcr_irqc_value_from_flags(dev, pin, mode, trig);

port_base->PCR[pin] = (port_base->PCR[pin] & ~PORT_PCR_IRQC_MASK) | pcr;
Expand All @@ -389,20 +389,17 @@ static void gpio_mcux_port_isr(const struct device *dev)
struct gpio_mcux_data *data = dev->data;
uint32_t int_status;

#if (defined(FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT) && \
FSL_FEATURE_GPIO_HAS_INTERRUPT_CHANNEL_SELECT)
#if (defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT) || \
(!defined(FSL_FEATURE_SOC_PORT_COUNT))
int_status = config->gpio_base->ISFR[0];

/* Clear the gpio interrupts */
config->gpio_base->ISFR[0] = int_status;
#elif !(defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) && FSL_FEATURE_PORT_HAS_NO_INTERRUPT)
#else
int_status = config->port_base->ISFR;

/* Clear the port interrupts */
config->port_base->ISFR = int_status;
#else
int_status = 0U;
ARG_UNUSED(config);
#endif /* !(defined(FSL_FEATURE_PORT_HAS_NO_INTERRUPT) */

gpio_fire_callbacks(&data->callbacks, dev, int_status);
Expand Down