linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH
@ 2020-06-22 21:45 Paul Cercueil
  2020-06-22 21:45 ` [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ Paul Cercueil
  2020-07-07 12:22 ` [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Linus Walleij
  0 siblings, 2 replies; 4+ messages in thread
From: Paul Cercueil @ 2020-06-22 21:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: od, linux-gpio, linux-kernel, Paul Cercueil, stable, João Henrique

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);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ
  2020-06-22 21:45 [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Paul Cercueil
@ 2020-06-22 21:45 ` Paul Cercueil
  2020-06-25 14:53   ` Sasha Levin
  2020-07-07 12:22 ` [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Linus Walleij
  1 sibling, 1 reply; 4+ messages in thread
From: Paul Cercueil @ 2020-06-22 21:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: od, linux-gpio, linux-kernel, Paul Cercueil, stable, João Henrique

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;
 	}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ
  2020-06-22 21:45 ` [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ Paul Cercueil
@ 2020-06-25 14:53   ` Sasha Levin
  0 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2020-06-25 14:53 UTC (permalink / raw)
  To: Sasha Levin, Paul Cercueil, Linus Walleij; +Cc: od, linux-gpio, stable, stable

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?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH
  2020-06-22 21:45 [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Paul Cercueil
  2020-06-22 21:45 ` [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ Paul Cercueil
@ 2020-07-07 12:22 ` Linus Walleij
  1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2020-07-07 12:22 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: od, open list:GPIO SUBSYSTEM, linux-kernel, stable, João Henrique

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-07 12:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 21:45 [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Paul Cercueil
2020-06-22 21:45 ` [PATCH 2/2] pinctrl: ingenic: Properly detect GPIO direction when configured for IRQ Paul Cercueil
2020-06-25 14:53   ` Sasha Levin
2020-07-07 12:22 ` [PATCH 1/2] pinctrl: ingenic: Enhance support for IRQ_TYPE_EDGE_BOTH Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).