Re: [PATCH v2 1/3] Input: samsung-keypad - clean up wakeup configuration logic
From: sashiko-bot@kernel.org
Date: 2026-07-14 05:51:40
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] The threaded IRQ handler accesses memory-mapped registers without ensuring the peripheral clock is enabled if a wakeup interrupt fires while the device is closed, leading to a kernel crash. --
--- Patch [1]: [PATCH v2 1/3] Input: samsung-keypad - clean up wakeup configuration logic ---commit c263408953e10fb025a3702953499e150bde4016
Author: Dmitry Torokhov [off-list ref]
Input: samsung-keypad - clean up wakeup configuration logic
This commit refactors the wakeup configuration logic in the samsung-keypad
driver. It moves the device_may_wakeup check to the caller and adjusts the
ordering of register writes and interrupt configuration.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c index 17127269e3f05..a578f429d1009 100644 --- a/drivers/input/keyboard/samsung-keypad.c +++ b/drivers/input/keyboard/samsung-keypad.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -516,7 +515,8 @@ static int samsung_keypad_suspend(struct device *dev) if (input_device_enabled(input_dev)) samsung_keypad_stop(keypad); - samsung_keypad_toggle_wakeup(keypad, true); + if (device_may_wakeup(dev)) + samsung_keypad_toggle_wakeup(keypad, true); return 0; }
[Severity: High]
This is a pre-existing issue, but could a wakeup interrupt firing while the
device is closed cause an unclocked register access and crash the system?
If the device is closed by all users, samsung_keypad_stop() sets
keypad->stopped to true. When the system suspends, the code above enables
wakeup if device_may_wakeup() returns true.
If a key press triggers a wakeup interrupt, upon resume the primary IRQ
handler schedules the threaded handler samsung_keypad_irq():
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_irq() {
...
pm_runtime_get_sync(&keypad->pdev->dev);
do {
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
...
}
However, pm_runtime_get_sync() calls samsung_keypad_runtime_resume(), which
skips enabling the clock because keypad->stopped is true:
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_runtime_resume() {
...
if (keypad->stopped)
return 0;
clk_enable(keypad->clk);
...
}
Will the subsequent readl() in samsung_keypad_irq() trigger a synchronous
external abort since the peripheral clock was never enabled?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-samsung-kp-irq-v2-0-acc84b6daf9a@gmail.com?part=1