Input: st-keyscan - disable interrupt while clock is gated

PATCH In review Dmitry Torokhov 2 participants 3 messages Aug 29

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2026-08-29 11:12 UTC #0 lore raw

PATCH 1/1 Input: st-keyscan - disable interrupt while clock is gated

The driver gates off its peripheral clock when the input device is not open and during system suspend without wakeup. However, the interrupt is requested without IRQF_NO_AUTOEN and remains unmasked when the device is stopped. If a pending or spurious interrupt fires while the clock is disabled, keyscan_isr() will attempt to read the matrix state register with the clock gated off, triggering a bus fault (synchronous external abort) and panicking the system.

Request the interrupt with IRQF_NO_AUTOEN, enable it in keyscan_start(), and disable it in keyscan_stop() prior to gating the clock. Drop the pointless clock enable/stop sequence in keyscan_probe(), leaving the clock gated and hardware untouched until the device is opened.

Fixes 062589b13991 ("Input: add st-keyscan driver")

Reported-by [email protected]

Assisted-by LLM

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/keyboard/st-keyscan.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

drivers/input/keyboard/st-keyscan.c +6 ~~-10~~

index ebeda020ffe1..6101bf53379a 100644
@@ -75,6 +75,8 @@ static int keyscan_start(struct st_keyscan *keypad)
 
 	writel(KEYSCAN_CONFIG_ENABLE, keypad->base + KEYSCAN_CONFIG_OFF);
 
+	enable_irq(keypad->irq);
+
 	return 0;
 }

@@ -82,6 +84,8 @@ static void keyscan_stop(struct st_keyscan *keypad)
 {
 	writel(0, keypad->base + KEYSCAN_CONFIG_OFF);
 
+	disable_irq(keypad->irq);
+
 	clk_disable(keypad->clk);
 }

@@ -177,20 +181,12 @@ static int keyscan_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
 				     "cannot get clock\n");
 
-	error = clk_enable(keypad_data->clk);
-	if (error) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
-		return error;
-	}
-
-	keyscan_stop(keypad_data);
-
 	keypad_data->irq = platform_get_irq(pdev, 0);
 	if (keypad_data->irq < 0)
 		return keypad_data->irq;
 
-	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
-				 pdev->name, keypad_data);
+	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr,
+				 IRQF_NO_AUTOEN, pdev->name, keypad_data);
 	if (error)
 		return error;

Signature · 5 lines

2.55.0.897.gb25b4bd76c-goog


-- 
Dmitry

[email protected]

AI review

2026-08-29 11:27 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Removing the hardware stop sequence in probe() fails to clear the hardware state left by the bootloader. If the bootloader left the keypad active and asserting an interrupt, this state persists, leading to immediate spurious wakeups that permanently prevent system sleep when the device is configured as a wakeup source.

Pre-existing issues:
- [High] If keyscan_start() fails during system resume (e.g. clock enable fails), the device remains marked as enabled. A subsequent close of the device will invoke keyscan_stop(). This triggers a bus fault (synchronous external abort) because writel() is called while the clock is gated, and causes unbalanced disable_irq() and clk_disable() calls. --

commit cfa9ab2fa10d41e20ca2124965465f938faf3789
Author: Dmitry Torokhov <[email protected]>

Input: st-keyscan - disable interrupt while clock is gated

This commit fixes a bus fault during system suspend by requesting the interrupt with IRQF_NO_AUTOEN. It also drops the clock enable and stop sequence in probe, leaving the hardware untouched until opened.

Quoted 3 lines

drivers/input/keyboard/st-keyscan.c +0 -0

[ ... ]

drivers/input/keyboard/st-keyscan.c static void keyscan_stop(struct st_keyscan *keypad) · line 90

Quoted 8 lines

drivers/input/keyboard/st-keyscan.c +2 -0

@@ -82,6 +84,8 @@ static void keyscan_stop(struct st_keyscan *keypad)
 {
 	writel(0, keypad->base + KEYSCAN_CONFIG_OFF);
 
+	disable_irq(keypad->irq);
+
 	clk_disable(keypad->clk);
 }

[Severity: High]
This isn't a bug introduced by this patch, but does this unconditional teardown risk a bus fault during close if resume fails?

If keyscan_start() fails during keyscan_resume() (for instance, if clk_enable() fails), keyscan_start() returns an error and skips enable_irq(). The PM core aborts or logs the resume error, but the input device remains marked as enabled and open.

When userspace subsequently closes the device, keyscan_stop() will execute writel() with the clock still gated, which triggers a synchronous external abort. Additionally, it will call disable_irq() and clk_disable(), causing an unbalanced state since they were never enabled in resume.

The patch adds the unbalanced IRQ disable to the existing bus fault.

[ ... ]

Quoted 5 lines

drivers/input/keyboard/st-keyscan.c +0 -1

@@ -177,20 +181,12 @@ static int keyscan_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
 				     "cannot get clock\n");
 
-	error = clk_enable(keypad_data->clk);

drivers/input/keyboard/st-keyscan.c static int keyscan_probe(struct platform_device *pdev) · line 186

Quoted 10 lines

drivers/input/keyboard/st-keyscan.c +0 -7

-	if (error) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
-		return error;
-	}
-
-	keyscan_stop(keypad_data);
-
 	keypad_data->irq = platform_get_irq(pdev, 0);
 	if (keypad_data->irq < 0)
 		return keypad_data->irq;

[Severity: Medium]
Could removing the hardware stop sequence in keyscan_probe() cause spurious wakeups if the bootloader leaves the hardware active?

Previously, this code ensured the hardware was quiescent and its interrupt line deasserted before registering the device. If the bootloader hands off the hardware in an active, interrupt-asserting state, that state now persists.

When the system suspends and enable_irq_wake() is called, could a pending interrupt from the bootloader's hardware state immediately wake the system, permanently preventing sleep when the device is configured as a wakeup source?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Dmitry Torokhov

author maintainer

2026-08-29 13:05 UTC #2 lore raw

Quoted 4 lines · On Sat, Aug 29, 2026 at 11:27:20AM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Removing the hardware stop sequence in probe() fails to clear the hardware state left by the bootloader. If the bootloader left the keypad active and asserting an interrupt, this state persists, leading to immediate spurious wakeups that permanently prevent system sleep when the device is configured as a wakeup source.

We should expect the bootloader behave reasonably. The default stat upon reset is disabled.

Thanks.

Signature · 1 line

Dmitry