[PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling

[PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
deferred probe correctly.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Drop the redundant error message after devm_request_irq(), as the
helper already reports request failures internally.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e2..fa08ac561bf2 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -142,16 +142,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "Failed to get snvs clock (%pe)\n", clk);
-		return PTR_ERR(clk);
-	}
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+				     "Failed to get snvs clock\n");
 
 	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
 
 	pdata->irq = platform_get_irq(pdev, 0);
 	if (pdata->irq < 0)
-		return -EINVAL;
+		return pdata->irq;
 
 	error = of_property_read_u32(np, "power-off-time-sec", &val);
 	if (!error) {
@@ -209,10 +208,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	error = devm_request_irq(&pdev->dev, pdata->irq,
 			       imx_snvs_pwrkey_interrupt,
 			       0, pdev->name, pdev);
-	if (error) {
-		dev_err(&pdev->dev, "interrupt not available.\n");
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(input);
 	if (error < 0) {
-- 
2.43.0

[PATCH v2 2/2] Input: st-keyscan: Improve probe error handling

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Use dev_err_probe() for devm_clk_get() so deferred probe errors are
handled correctly.
Drop the redundant error message after devm_request_irq(), since the
helper already reports request failures internally.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Changes in v2 : 
 -Return the original error from platform_get_irq() instead of replacing
  it with -EINVAL.

 drivers/input/keyboard/st-keyscan.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index e53ef4c670e4..8e9bac339dc4 100644
--- a/drivers/input/keyboard/st-keyscan.c
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -173,10 +173,9 @@ static int keyscan_probe(struct platform_device *pdev)
 		return PTR_ERR(keypad_data->base);
 
 	keypad_data->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(keypad_data->clk)) {
-		dev_err(&pdev->dev, "cannot get clock\n");
-		return PTR_ERR(keypad_data->clk);
-	}
+	if (IS_ERR(keypad_data->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
+				     "cannot get clock\n");
 
 	error = clk_enable(keypad_data->clk);
 	if (error) {
@@ -188,14 +187,12 @@ static int keyscan_probe(struct platform_device *pdev)
 
 	keypad_data->irq = platform_get_irq(pdev, 0);
 	if (keypad_data->irq < 0)
-		return -EINVAL;
+		return keypad_data->irq;
 
 	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
 				 pdev->name, keypad_data);
-	if (error) {
-		dev_err(&pdev->dev, "failed to request IRQ\n");
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(input_dev);
 	if (error) {
-- 
2.43.0

Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver requests and leaves the interrupt enabled while the hardware clock is explicitly disabled, leading to a potential crash (synchronous external abort) upon a spurious or pending interrupt.
--

--- Patch [2]: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling ---
commit 036864a311a2de2b62ec154e1ff242ede73fb89b
Author: bui duc phuc <phucduc.bui@gmail.com>

Input: st-keyscan: Improve probe error handling

This commit improves error handling in the probe function by using
dev_err_probe() for clock fetching, removing redundant error logs for
devm_request_irq(), and returning the original error code from
platform_get_irq().

> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> index e53ef4c670e4b..8e9bac339dc47 100644
> --- a/drivers/input/keyboard/st-keyscan.c
> +++ b/drivers/input/keyboard/st-keyscan.c

[ ... ]

> @@ -188,14 +187,12 @@ static int keyscan_probe(struct platform_device *pdev)
>  
>  	keypad_data->irq = platform_get_irq(pdev, 0);
>  	if (keypad_data->irq < 0)
> -		return -EINVAL;
> +		return keypad_data->irq;
>  
>  	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
>  				 pdev->name, keypad_data);

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here while the
hardware clock is explicitly disabled risk a bus fault?

Earlier in keyscan_probe(), keyscan_stop() is called to disable the clock.
When devm_request_irq() is called here, it unmasks and enables the interrupt
while the clock remains gated.

If a spurious interrupt or a bootloader-pending interrupt occurs at this
point, keyscan_isr() will execute and attempt to read the hardware register:

drivers/input/keyboard/st-keyscan.c:keyscan_isr() {
    ...
    state = readl(keypad->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
    ...
}

Accessing the memory-mapped register while the IP block's clock is disabled
can trigger a synchronous external abort. The same condition appears to happen
during keyscan_close(), which disables the clock while the IRQ remains active.

Would it be safer to request the interrupt with IRQF_NO_AUTOEN or delay
requesting it until the device is actually opened and clocked?

> -	if (error) {
> -		dev_err(&pdev->dev, "failed to request IRQ\n");
> +	if (error)
>  		return error;
> -	}
>  
>  	error = input_register_device(input_dev);
>  	if (error) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720090851.230244-1-phucduc.bui@gmail.com?part=2

Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling

[Dmitry Torokhov]

On Mon, Jul 20, 2026 at 04:08:50PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Use dev_err_probe() for devm_clk_get() so deferred probe errors are
> handled correctly.

Same here: in what way the does existing code handle deferred probe
incorrectly?

Thanks.

-- 
Dmitry

Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling

[Bui Duc Phuc]

Hi Dmitry,

Thank you for your feedback.

> > Use dev_err_probe() for devm_clk_get() so deferred probe errors are
> > handled correctly.
>
> Same here: in what way the does existing code handle deferred probe
> incorrectly?
>
This is the same case as the previous patch. The current code uses dev_err(),
which logs an error message even when devm_clk_get() returns -EPROBE_DEFER.
Since the probe will be retried later, this leads to unnecessary log spam.
Link to previous discussion for reference:
https://lore.kernel.org/all/CAABR9nHHe1AzWmqhV0d5p87qfSsBk2O7bYj-Weowyr=99wtNNA@mail.gmail.com/

Would you like me to send a v3 updating the commit message to be clearer,
such as: "Use dev_err_probe() for devm_clk_get() to prevent log spam
when probe returns -EPROBE_DEFER.""
or keep the current patch as is?

Best regards,
Phuc

Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling

[Bui Duc Phuc]

Hi Dmitry,

Additionally, using dev_err_probe() here also simplifies the error
handling path
by combining logging and returning into a single statement,
making the code shorter and cleaner.

Best regards,
Phuc

Re: [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling

[Dmitry Torokhov]

On Mon, Jul 20, 2026 at 04:08:49PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
> deferred probe correctly.

In what way the does existing code handle deferred probe incorrectly?

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling

[Bui Duc Phuc]

Hi Dmitry,

Thank you for your feedback.

> > Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
> > deferred probe correctly.
>
> In what way the does existing code handle deferred probe incorrectly?
>

With the current implementation, dev_err() is still called even when
devm_clk_get_optional_enabled() returns -EPROBE_DEFER.
Since the probe will be retried later anyway, logging an error via dev_err()
causes unnecessary log spam.
Using dev_err_probe() fixes this because under the hood (in
__dev_probe_failed()),
it handles -EPROBE_DEFER using dev_dbg() instead of dev_err():

-----------------------
case -EPROBE_DEFER:
        device_set_deferred_probe_reason(dev, &vaf);
        dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
        break;
----------------------

This avoids log spam while maintaining deferred probe information.

Best regards,
Phuc