pussuw commented on code in PR #12170:
URL: https://github.com/apache/nuttx/pull/12170#discussion_r1568927538
##########
arch/arm64/src/imx9/imx9_gpioirq.c:
##########
@@ -236,8 +236,8 @@ int imx9_gpioirq_enable(gpio_pinset_t pinset)
{
uint32_t port = (pinset & GPIO_PORT_MASK) >> GPIO_PORT_SHIFT;
uint32_t pin = (pinset & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT;
- uint32_t icr = (pinset & GPIO_INTCFG_MASK) >> GPIO_INTCFG_SHIFT;
uint32_t both = (pinset & GPIO_INTBOTHCFG_MASK) >> GPIO_INTBOTHCFG_SHIFT;
+ uint32_t icr = (pinset & GPIO_INTCFG_MASK);
Review Comment:
It is explained by the commit message.
There is a set of tests below, like:
```
if (both)
{
regval |= IMX9_GPIO_ICRN_BOTH;
}
else if (icr == GPIO_INT_LOWLEVEL)
{
regval |= IMX9_GPIO_ICRN_ZERO;
}
else if (icr == GPIO_INT_HIGHLEVEL)
{
regval |= IMX9_GPIO_ICRN_ONE;
}
else if (icr == GPIO_INT_RISINGEDGE)
{
regval |= IMX9_GPIO_ICRN_RISING;
}
else /* GPIO_INT_FALLINGEDGE */
{
regval |= IMX9_GPIO_ICRN_FALLING;
}
```
The values are already shifted left:
```
#define GPIO_INTCFG_SHIFT (9) /* Bits 9-10: Interrupt edge/level
configuration */
#define GPIO_INTCFG_MASK (0x3 << GPIO_INTCFG_SHIFT)
# define GPIO_INT_LOWLEVEL (0 << GPIO_INTCFG_SHIFT)
# define GPIO_INT_HIGHLEVEL (1 << GPIO_INTCFG_SHIFT)
# define GPIO_INT_RISINGEDGE (2 << GPIO_INTCFG_SHIFT)
# define GPIO_INT_FALLINGEDGE (3 << GPIO_INTCFG_SHIFT)
```
So if icr is shifted left `uint32_t icr = (pinset & GPIO_INTCFG_MASK) >>
GPIO_INTCFG_SHIFT;`, we are comparing the wrong bits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
