Ingenic SoCs don't natively support registering an interrupt for both rising and falling edges. This has to be emulated in software.
Until now, this was emulated by switching back and forth between IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING according to the level of the GPIO. While this worked most of the time, when used with GPIOs that need debouncing, some events would be lost. For instance, between the time a falling-edge interrupt happens and the interrupt handler configures the hardware for rising-edge, the level of the pin may have already risen, and the rising-edge event is lost.
To address that issue, instead of switching back and forth between IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING, we now switch back and forth between IRQ_TYPE_LEVEL_LOW and IRQ_TYPE_LEVEL_HIGH. Since we always switch in the interrupt handler, they actually permit to detect level changes. In the example above, if the pin level rises before switching the IRQ type from IRQ_TYPE_LEVEL_LOW to IRQ_TYPE_LEVEL_HIGH, a new interrupt will raise as soon as the handler exits, and the rising-edge event will be properly detected.
Cc: stable@vger.kernel.org Fixes: e72394e2ea19 ("pinctrl: ingenic: Merge GPIO functionality") Reported-by: João Henrique johnnyonflame@hotmail.com Tested-by: João Henrique johnnyonflame@hotmail.com Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/pinctrl/pinctrl-ingenic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c index fc0d10411aa9..241e563d5814 100644 --- a/drivers/pinctrl/pinctrl-ingenic.c +++ b/drivers/pinctrl/pinctrl-ingenic.c @@ -1813,9 +1813,9 @@ static void ingenic_gpio_irq_ack(struct irq_data *irqd) */ high = ingenic_gpio_get_value(jzgc, irq); if (high) - irq_set_type(jzgc, irq, IRQ_TYPE_EDGE_FALLING); + irq_set_type(jzgc, irq, IRQ_TYPE_LEVEL_LOW); else - irq_set_type(jzgc, irq, IRQ_TYPE_EDGE_RISING); + irq_set_type(jzgc, irq, IRQ_TYPE_LEVEL_HIGH); }
if (jzgc->jzpc->info->version >= ID_JZ4760) @@ -1851,7 +1851,7 @@ static int ingenic_gpio_irq_set_type(struct irq_data *irqd, unsigned int type) */ bool high = ingenic_gpio_get_value(jzgc, irqd->hwirq);
- type = high ? IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING; + type = high ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_LEVEL_HIGH; }
irq_set_type(jzgc, irqd->hwirq, type);
The PAT1 register contains information about the IRQ type (edge/level) for input GPIOs with IRQ enabled, and the direction for non-IRQ GPIOs. So it makes sense to read it only if the GPIO has no interrupt configured, otherwise input GPIOs configured for level IRQs are misdetected as output GPIOs.
Cc: stable@vger.kernel.org Fixes: ebd6651418b6 ("pinctrl: ingenic: Implement .get_direction for GPIO chips") Reported-by: João Henrique johnnyonflame@hotmail.com Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/pinctrl/pinctrl-ingenic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c index 241e563d5814..a8d1b53ec4c1 100644 --- a/drivers/pinctrl/pinctrl-ingenic.c +++ b/drivers/pinctrl/pinctrl-ingenic.c @@ -1958,7 +1958,8 @@ static int ingenic_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) unsigned int pin = gc->base + offset;
if (jzpc->info->version >= ID_JZ4760) { - if (ingenic_get_pin_config(jzpc, pin, JZ4760_GPIO_PAT1)) + if (ingenic_get_pin_config(jzpc, pin, JZ4760_GPIO_INT) || + ingenic_get_pin_config(jzpc, pin, JZ4760_GPIO_PAT1)) return GPIO_LINE_DIRECTION_IN; return GPIO_LINE_DIRECTION_OUT; }
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag fixing commit: ebd6651418b6 ("pinctrl: ingenic: Implement .get_direction for GPIO chips").
The bot has tested the following trees: v5.7.5, v5.4.48.
v5.7.5: Build OK! v5.4.48: Failed to apply! Possible dependencies: 1948d5c51dba4 ("pinctrl: Add pinmux & GPIO controller driver for a new SoC") 3c827873590c3 ("pinctrl: Use new GPIO_LINE_DIRECTION") 54787d7c14a4a ("pinctrl: rza1: remove unnecessary static inline function") 5ec008bfac7da ("pinctrl: ingenic: Remove platform ID table") 7009d046a6011 ("pinctrl: ingenic: Handle PIN_CONFIG_OUTPUT config") 9e65527ac3bab ("pinctrl: ingenic: Fixup PIN_CONFIG_OUTPUT config") baf15647387e8 ("pinctrl: ingenic: Put ingenic_chip_info pointer in match data") d5a362149c4db ("pinctrl: Modify Kconfig to fix linker error") d7da2a1e4e084 ("pinctrl: Ingenic: Add pinctrl driver for X1830.") f742e5ebdd633 ("pinctrl: Ingenic: Introduce reg_offset and use it instead hard code.")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
On Mon, Jun 22, 2020 at 11:46 PM Paul Cercueil paul@crapouillou.net wrote:
Ingenic SoCs don't natively support registering an interrupt for both rising and falling edges. This has to be emulated in software.
Until now, this was emulated by switching back and forth between IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING according to the level of the GPIO. While this worked most of the time, when used with GPIOs that need debouncing, some events would be lost. For instance, between the time a falling-edge interrupt happens and the interrupt handler configures the hardware for rising-edge, the level of the pin may have already risen, and the rising-edge event is lost.
To address that issue, instead of switching back and forth between IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING, we now switch back and forth between IRQ_TYPE_LEVEL_LOW and IRQ_TYPE_LEVEL_HIGH. Since we always switch in the interrupt handler, they actually permit to detect level changes. In the example above, if the pin level rises before switching the IRQ type from IRQ_TYPE_LEVEL_LOW to IRQ_TYPE_LEVEL_HIGH, a new interrupt will raise as soon as the handler exits, and the rising-edge event will be properly detected.
Cc: stable@vger.kernel.org Fixes: e72394e2ea19 ("pinctrl: ingenic: Merge GPIO functionality") Reported-by: João Henrique johnnyonflame@hotmail.com Tested-by: João Henrique johnnyonflame@hotmail.com Signed-off-by: Paul Cercueil paul@crapouillou.net
I have applied these two as non-urgent fixes for v5.9.
Are they urgent? Are they causing regressions? Tell me if they need to be merged to v5.8-rcs.
Yours, Linus Walleij
linux-stable-mirror@lists.linaro.org