linux-kernel - Re: [PATCH v1] gpio: pca953x: handle short interrupt pulses on PCAL devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aVF2CIorE5ylV1b4@smile.fi.intel.com>
Date: Sun, 28 Dec 2025 20:25:12 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Ernest Van Hoecke <ernestvanhoecke@...il.com>
Cc: Linus Walleij <linusw@...nel.org>, Bartosz Golaszewski <brgl@...ev.pl>,
	Mark Tomlinson <mark.tomlinson@...iedtelesis.co.nz>,
	Ernest Van Hoecke <ernest.vanhoecke@...adex.com>,
	Emanuele Ghidoli <emanuele.ghidoli@...adex.com>,
	Francesco Dolcini <francesco.dolcini@...adex.com>,
	lakabd <lakabd.work@...il.com>, Yong Li <yong.b.li@...el.com>,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] gpio: pca953x: handle short interrupt pulses on PCAL
 devices

On Wed, Dec 17, 2025 at 04:30:25PM +0100, Ernest Van Hoecke wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@...adex.com>
> 
> GPIO drivers with latch input support may miss short pulses on input
> pins even when input latching is enabled. The generic interrupt logic in
> the pca953x driver reports interrupts by comparing the current input
> value against the previously sampled one and only signals an event when
> a level change is observed between two reads.
> 
> For short pulses, the first edge is captured when the input register is
> read, but if the signal returns to its previous level before the read,
> the second edge is not observed. As a result, successive pulses can
> produce identical input values at read time and no level change is
> detected, causing interrupts to be missed. Below timing diagram shows
> this situation where the top signal is the input pin level and the
> bottom signal indicates the latched value.
> 
> ─────┐     ┌──*───────────────┐     ┌──*─────────────────┐     ┌──*───
>      │     │  .               │     │  .                 │     │  .
>      │     │  │               │     │  │                 │     │  │
>      └──*──┘  │               └──*──┘  │                 └──*──┘  │
> Input   │     │                  │     │                    │     │
>         ▼     │                  ▼     │                    ▼     │
>        IRQ    │                 IRQ    │                   IRQ    │
>               .                        .                          .
> ─────┐        .┌──────────────┐        .┌────────────────┐        .┌──
>      │         │              │         │                │         │
>      │         │              │         │                │         │
>      └────────*┘              └────────*┘                └────────*┘
> Latched       │                        │                          │
>               ▼                        ▼                          ▼
>             READ 0                   READ 0                     READ 0
>                                    NO CHANGE                  NO CHANGE
> 
> PCAL variants provide an interrupt status register that records which
> pins triggered an interrupt, but the status and input registers cannot
> be read atomically. The interrupt status is only cleared when the input
> port is read, and the input value must also be read to determine the
> triggering edge. If another interrupt occurs on a different line after
> the status register has been read but before the input register is
> sampled, that event will not be reflected in the earlier status
> snapshot, so relying solely on the interrupt status register is also
> insufficient.
> 
> Support for input latching and interrupt status handling was previously
> added by [1], but the interrupt status-based logic was reverted by [2]
> due to these issues. This patch addresses the original problem by
> combining both sources of information. Events indicated by the interrupt
> status register are merged with events detected through the existing
> level-change logic. As a result:
> 
> * short pulses, whose second edges are invisible, are detected via the
>   interrupt status register, and
> * interrupts that occur between the status and input reads are still
>   caught by the generic level-change logic.
> 
> This significantly improves robustness on devices that signal interrupts
> as short pulses, while avoiding the issues that led to the earlier
> reversion. In practice, even if only the first edge of a pulse is
> observable, the interrupt is reliably detected.
> 
> This fixes missed interrupts from an Ilitek touch controller with its
> interrupt line connected to a PCAL6416A, where active-low pulses are
> approximately 200 us long.
> 
> [1] commit 44896beae605 ("gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2")
> [2] commit d6179f6c6204 ("gpio: pca953x: Improve interrupt support")

Wow, this is a very good wrap-up of the interrupt behaviour on those chips.
While I am fully with you on the fix (this patch), can you also add a
chapter/section about this to the documentation file (now we have it for these
chips!) Documentation/driver-api/gpio/pca953x.rst?

(As a separate patch.)

-- 
With Best Regards,
Andy Shevchenko