[PATCH AUTOSEL 6.19-5.15] char: tpm: cr50: Remove IRQF_ONESHOT
Sasha Levin
sashal at kernel.org
Wed Feb 11 17:09:30 PST 2026
From: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
[ Upstream commit 1affd29ffbd50125a5492c6be1dbb1f04be18d4f ]
Passing IRQF_ONESHOT ensures that the interrupt source is masked until
the secondary (threaded) handler is done. If only a primary handler is
used then the flag makes no sense because the interrupt can not fire
(again) while its handler is running.
The flag also prevents force-threading of the primary handler and the
irq-core will warn about this.
Remove IRQF_ONESHOT from irqflags.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
Signed-off-by: Thomas Gleixner <tglx at kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko at kernel.org>
Link: https://patch.msgid.link/20260128095540.863589-10-bigeasy@linutronix.de
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete understanding. Let me write up the comprehensive
analysis.
---
## Comprehensive Analysis
### 1. Commit Message Analysis
The commit subject is "char: tpm: cr50: Remove IRQF_ONESHOT" and the
message explains three points:
1. `IRQF_ONESHOT` is for masking the interrupt source until the
**threaded** (secondary) handler completes
2. These drivers only have primary handlers, so the flag is meaningless
3. The flag **prevents force-threading** of the primary handler, and the
irq-core will warn about this
The commit is authored by **Sebastian Andrzej Siewior** (bigeasy), the
PREEMPT_RT maintainer, and signed off by **Thomas Gleixner**, the IRQ
subsystem maintainer and co-maintainer of the Linux kernel. It was
reviewed by **Jarkko Sakkinen**, the TPM subsystem maintainer.
### 2. Code Change Analysis
The change is minimal and surgical:
- **`tpm_tis_i2c_cr50.c`**: Removes `IRQF_ONESHOT` from the flags in
`devm_request_irq()`, keeping `IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN`
- **`tpm_tis_spi_cr50.c`**: Removes `IRQF_ONESHOT` from the flags in
`devm_request_irq()`, keeping `IRQF_TRIGGER_RISING`
Both interrupt handlers (`tpm_cr50_i2c_int_handler` at line 74 and
`cr50_spi_irq_handler` at line 65) are trivially simple - they just call
`complete()` and return `IRQ_HANDLED`. There is no thread_fn.
`devm_request_irq()` is a wrapper that calls
`devm_request_threaded_irq()` with `thread_fn = NULL`.
### 3. The Real Bug
The companion commit **`aef30c8d569c`** ("genirq: Warn about using
IRQF_ONESHOT without a threaded handler") was merged on 2026-01-12 and
adds a `WARN_ON_ONCE()` in `__setup_irq()`:
```c
WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn);
```
This means that **without this cr50 fix**, every time the cr50 TPM
driver probes on a system with the updated IRQ core, it will emit a
`WARN_ON_ONCE` kernel warning at boot. This is a real runtime issue that
would affect all Chromebook and other systems using cr50/ti50 TPM chips.
More importantly, the core technical issue is that `IRQF_ONESHOT`
prevents force-threading of the primary handler. From
`irq_setup_forced_threading()` in `kernel/irq/manage.c`:
```c
if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))
return 0; // Skip force-threading!
```
On **PREEMPT_RT kernels** (where `force_irqthreads()` returns `true`),
this means the cr50 interrupt handler runs in hardirq context instead of
being force-threaded. While the handler itself (`complete()`) is safe in
hardirq context, this defeats the PREEMPT_RT design goal of having all
interrupt handlers run in thread context. On non-RT systems with
`threadirqs` boot parameter, the same issue occurs.
### 4. Classification
This is a **bug fix** that addresses:
1. **A spurious kernel warning** triggered by the new `WARN_ON_ONCE`
check added in `aef30c8d569c`
2. **Incorrect IRQ flags** - `IRQF_ONESHOT` has never been semantically
correct for these drivers since they only use primary handlers
3. **PREEMPT_RT compatibility** - the flag prevents force-threading,
which can be problematic
The fix is not a feature addition - it removes an incorrect flag that
was always wrong.
### 5. Scope and Risk Assessment
- **Lines changed**: 2 files, 2 lines total (removing `IRQF_ONESHOT`
from two flag constants)
- **Risk**: Extremely low. `IRQF_ONESHOT` had no functional effect on
these drivers since they use only primary handlers. The interrupt
cannot fire while its primary handler is already running (hardware
guarantee), so removing the flag changes nothing about interrupt
masking behavior.
- **Subsystem**: TPM drivers, well-understood, mature code
- **Testing**: Reviewed by TPM maintainer (Jarkko Sakkinen), authored by
PREEMPT_RT maintainer, signed off by IRQ core maintainer
### 6. User Impact
- **Who is affected**: All users with cr50/ti50 TPM chips (primarily
Chromebooks and Google-based embedded devices)
- **Severity without fix**: Kernel `WARN_ON_ONCE` at boot if the
`aef30c8d569c` warning commit is also backported; PREEMPT_RT
degradation
- **Severity of regression risk**: Essentially zero - the flag had no
functional purpose
### 7. Dependencies
This commit is **self-contained**. It does not require any other patches
to apply cleanly. However, the motivating warning comes from
`aef30c8d569c` - if that commit is backported to stable, this fix
becomes essential to avoid warnings. Even without the warning commit,
this fix is correct and beneficial (enables proper force-threading on RT
systems).
The i2c-spacemit driver already got a similar fix (`e351836a54e3`) with
`Cc: stable at vger.kernel.org` tag, establishing precedent that these
IRQF_ONESHOT removal patches are considered stable material.
### 8. Stability Indicators
- Author: Sebastian Andrzej Siewior (PREEMPT_RT maintainer) - domain
expert
- Signed-off-by: Thomas Gleixner (IRQ subsystem maintainer) - domain
expert
- Reviewed-by: Jarkko Sakkinen (TPM maintainer) - domain expert
- Part of a systematic cleanup pattern with prior art (i2c-exynos5,
i2c-hix5hd2, i2c-spacemit, drm/msm)
### Conclusion
This is a small, surgical, obviously correct bug fix. The `IRQF_ONESHOT`
flag was always semantically wrong for these primary-only handlers. The
fix removes a flag that had no functional benefit but actively prevented
force-threading on PREEMPT_RT systems and will trigger a `WARN_ON_ONCE`
with the companion IRQ core warning check. The risk is near-zero, the
change is trivial (2 lines across 2 files), it was reviewed by all three
relevant subsystem maintainers, and there is clear precedent for
identical fixes being marked as stable material (the i2c-spacemit fix
had `Cc: stable at vger.kernel.org`).
**YES**
drivers/char/tpm/tpm_tis_i2c_cr50.c | 3 +--
drivers/char/tpm/tpm_tis_spi_cr50.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index fc6891a0b6936..b48cacacc0664 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -749,8 +749,7 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
if (client->irq > 0) {
rc = devm_request_irq(dev, client->irq, tpm_cr50_i2c_int_handler,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
- IRQF_NO_AUTOEN,
+ IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN,
dev->driver->name, chip);
if (rc < 0) {
dev_err(dev, "Failed to probe IRQ %d\n", client->irq);
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index f4937280e9406..32920b4cecfb4 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -287,7 +287,7 @@ int cr50_spi_probe(struct spi_device *spi)
if (spi->irq > 0) {
ret = devm_request_irq(&spi->dev, spi->irq,
cr50_spi_irq_handler,
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ IRQF_TRIGGER_RISING,
"cr50_spi", cr50_phy);
if (ret < 0) {
if (ret == -EPROBE_DEFER)
--
2.51.0
More information about the linux-riscv
mailing list