diff mbox series

Record interrupt status when an IRQ is masked

Message ID 20190720113155.12276-1-tomasz.motyl@se.com
State New
Headers show
Series Record interrupt status when an IRQ is masked | expand

Commit Message

Tomasz Kazimierz Motyl July 20, 2019, 11:31 a.m. UTC 
When one changes the state of any input pins of a PCA9555 chip before
 setting up the IRQ mask through i.e. SysFS e.g. echo "both" >
 /sys/class/gpio/gpioXYZ/edge the epoll_wait shall not exit on the subsequent
 change of the GPIO state. The reason behind it is that the IRQ status is not
 being saved when the IRQ is masked.

---
 drivers/gpio/gpio-pca953x.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Linus Walleij July 20, 2019, 7:42 p.m. UTC | #1 
Looping in some experts on PCA953xx by top posting, sorry.

Linus Walleij

On Sat, Jul 20, 2019 at 1:32 PM Tomasz Kazimierz Motyl
<tomasz.motyl666@gmail.com> wrote:
>
>  When one changes the state of any input pins of a PCA9555 chip before
>  setting up the IRQ mask through i.e. SysFS e.g. echo "both" >
>  /sys/class/gpio/gpioXYZ/edge the epoll_wait shall not exit on the subsequent
>  change of the GPIO state. The reason behind it is that the IRQ status is not
>  being saved when the IRQ is masked.
>
> ---
>  drivers/gpio/gpio-pca953x.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 7e76830b3368..088bef902156 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -716,13 +716,16 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending)
>                 trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i];
>                 if (trigger[i])
>                         trigger_seen = true;
> +
> +    /* We want the current status recorded in the chip->irq stat regardless the
> +     * chip->irq_mask setting in order to have a change detected when the interrupt
> +     * mask gets changed i.e. echo "both" > /sys/class/gpioXYZ/edge */
> +    chip->irq_stat[i] = cur_stat[i];
>         }
>
>         if (!trigger_seen)
>                 return false;
>
> -       memcpy(chip->irq_stat, cur_stat, NBANK(chip));
> -
>         for (i = 0; i < NBANK(chip); i++) {
>                 pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) |
>                         (cur_stat[i] & chip->irq_trig_raise[i]);
> --
> 2.17.1
>
Andy Shevchenko Oct. 18, 2019, 9:18 a.m. UTC | #2 
On Sat, Jul 20, 2019 at 09:42:35PM +0200, Linus Walleij wrote:
> Looping in some experts on PCA953xx by top posting, sorry.

NP.

I would like to have my conversion patch be applied first. It will improve
reading of this change I suppose.

P.S. My patch still needs a bit of work, so, I'll send new version.
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 7e76830b3368..088bef902156 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -716,13 +716,16 @@  static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending)
 		trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i];
 		if (trigger[i])
 			trigger_seen = true;
+
+    /* We want the current status recorded in the chip->irq stat regardless the
+     * chip->irq_mask setting in order to have a change detected when the interrupt
+     * mask gets changed i.e. echo "both" > /sys/class/gpioXYZ/edge */
+    chip->irq_stat[i] = cur_stat[i];
 	}
 
 	if (!trigger_seen)
 		return false;
 
-	memcpy(chip->irq_stat, cur_stat, NBANK(chip));
-
 	for (i = 0; i < NBANK(chip); i++) {
 		pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) |
 			(cur_stat[i] & chip->irq_trig_raise[i]);