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

[PATCH 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 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.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/input/keyboard/st-keyscan.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index e53ef4c670e4..d86f6f16ae1b 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) {
@@ -192,10 +191,8 @@ static int keyscan_probe(struct platform_device *pdev)
 
 	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 2/2] Input: st-keyscan: Improve probe error handling

[sashiko-bot]

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

Pre-existing issues:
- [High] Deferred probe errors from `platform_get_irq()` are incorrectly masked and converted to `-EINVAL`.
- [High] The interrupt handler is registered and unmasked while the hardware clock is disabled, which could lead to a crash on an unhandled or pending interrupt.
--

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

Input: st-keyscan: Improve probe error handling

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.

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

[ ... ]

> @@ -192,10 +191,8 @@ static int keyscan_probe(struct platform_device *pdev)
>  
>  	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 since you are improving error handling in 
keyscan_probe(), could we also fix the error return for platform_get_irq() 
just prior to this?

Currently the code does this:

	keypad_data->irq = platform_get_irq(pdev, 0);
	if (keypad_data->irq < 0)
		return -EINVAL;

Does overwriting the real error code from platform_get_irq() with -EINVAL
break deferred probe if the interrupt controller is not yet ready?

[Severity: High]
This isn't a bug introduced by this patch, but I also noticed a potential 
issue here related to the IRQ registration.

Just before calling devm_request_irq(), keyscan_probe() calls 
keyscan_stop() which disables the clock.

If the IRQ line is asserted (e.g. from a pending bootloader state or
spurious event), keyscan_isr() will fire immediately once devm_request_irq()
is called.

The ISR then accesses peripheral registers:

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

Can accessing peripheral registers while the corresponding clock is disabled 
cause a synchronous bus fault or kernel panic on these systems? Should the 
devm_request_irq() call be moved after the clock is re-enabled?

> -	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/20260720085041.224896-1-phucduc.bui@gmail.com?part=2