[PATCH] Input: st-keyscan - disable interrupt while clock is gated

[PATCH] Input: st-keyscan - disable interrupt while clock is gated

[Dmitry Torokhov]

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: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/st-keyscan.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index ebeda020ffe1..6101bf53379a 100644
--- a/drivers/input/keyboard/st-keyscan.c
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -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;
 
-- 
2.55.0.897.gb25b4bd76c-goog


-- 
Dmitry

Re: [PATCH] Input: st-keyscan - disable interrupt while clock is gated

[sashiko-bot]

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 <dmitry.torokhov@gmail.com>

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.

> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> --- a/drivers/input/keyboard/st-keyscan.c
> +++ b/drivers/input/keyboard/st-keyscan.c
[ ... ]
> @@ -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.

[ ... ]
> @@ -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;

[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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/apK-PZ5c97mCxBSC@google.com?part=1

Re: [PATCH] Input: st-keyscan - disable interrupt while clock is gated

[Dmitry Torokhov]

On Sat, Aug 29, 2026 at 11:27:20AM +0000, sashiko-bot@kernel.org 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.

-- 
Dmitry