Re: [PATCH v3 05/10] Input: samsung-keypad - remove support for platform data

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Premature IRQ enablement leads to unclocked hardware access and kernel panic.
--

--- Patch [5]: [PATCH v3 05/10] Input: samsung-keypad - remove support for platform data ---
commit 66287716c4c767b7655e9d4647ac5fd73fc2b713
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: samsung-keypad - remove support for platform data

This commit removes platform data support from the samsung-keypad driver,
as there are no remaining users in the kernel. The driver now relies entirely
on generic device properties for configuration.

> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index cc34f0146161e..e54b3b9f2cf2e 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c

[ ... ]

> @@ -424,7 +388,11 @@ static int samsung_keypad_probe(struct platform_device *pdev)
>  		return error;
>  	}
>  
> -	device_init_wakeup(&pdev->dev, pdata->wakeup);
> +	wakeup = device_property_read_bool(dev, "wakeup-source") ||
> +		 /* legacy name */
> +		 device_property_read_bool(dev, "linux,input-wakeup");
> +	device_init_wakeup(&pdev->dev, wakeup);
> +
>  	platform_set_drvdata(pdev, keypad);
>  
>  	error = devm_pm_runtime_enable(&pdev->dev);

[Severity: High]
This is a pre-existing issue, but I noticed that the IRQ is requested
and unmasked before runtime PM is enabled.

In samsung_keypad_probe(), devm_request_threaded_irq() is called just before
this point without IRQF_NO_AUTOEN. If a pending interrupt is left by the
bootloader or a spurious interrupt occurs, it will fire immediately.

When samsung_keypad_irq() executes, it calls pm_runtime_get_sync() (which
skips enabling the clock because keypad->stopped is true at probe time) and
then accesses hardware registers:

drivers/input/keyboard/samsung-keypad.c:samsung_keypad_irq() {
    ...
    do {
        readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
    ...
}

Can this unconditional access to unclocked peripheral registers trigger a
synchronous external abort and result in a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-samsung-kp-v3-0-b2fcaba77aff@gmail.com?part=5