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

[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 2/2] Input: st-keyscan: Improve probe error handling

[Dmitry Torokhov]

On Tue, Jul 21, 2026 at 09:20:30AM +0700, Bui Duc Phuc wrote:
> 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?

Yes, please, since the handling of deferrals was not broken. We just had
some harmless extra logging.

Thanks.

-- 
Dmitry

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

[Bui Duc Phuc]

Hi Dmitry,

Thank you for your feedback.

> >
> > 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?
>
> Yes, please, since the handling of deferrals was not broken. We just had
> some harmless extra logging.
>

I'll send v3 with the updated commit message shortly.

Best regards,
Phuc

[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() to prevent log spam when probe
returns -EPROBE_DEFER.
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>
---

Changes in v2:
 - Clarify the commit message to better describe the purpose of 
   using dev_err_probe()

 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() to prevent log spam when probe
returns -EPROBE_DEFER.
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:
 - Clarify the commit message to better describe the purpose of 
   using dev_err_probe()
 
 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] Hardware access with disabled clock in ISR causes bus faults (spurious or pending interrupts)
--

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

Input: st-keyscan: Improve probe error handling

This commit updates probe error handling by using dev_err_probe() for
devm_clk_get(), dropping a redundant error message after 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
> --- 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);
> -	if (error) {
> -		dev_err(&pdev->dev, "failed to request IRQ\n");
> +	if (error)
>  		return error;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing issue
with how the interrupt is registered here.

Since keyscan_stop() is called just before this in keyscan_probe(), the IP
block's clock is already gated. By requesting the interrupt without
IRQF_NO_AUTOEN, the interrupt is immediately unmasked at the interrupt
controller.

If a pending or spurious interrupt fires before the device is opened, could
the ISR attempt to access hardware registers while the clock is disabled?

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

Does this sequence risk a synchronous external abort (bus fault) causing a
kernel panic on this platform? 

Would it be safer to request the IRQ with IRQF_NO_AUTOEN so it remains masked
until keyscan_open() explicitly enables the clock? I noticed this same window
might exist in keyscan_close(), which disables the clock via keyscan_stop()
but leaves the IRQ unmasked.

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

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

[sashiko-bot]

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

Pre-existing issues:
- [Low] Direct use of `input_event()` instead of specific reporting helpers.
--

--- Patch [1]: [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling ---
commit 1bfa57ab7fcf3071071b3cf879e2dfcda7662e49
Author: bui duc phuc <phucduc.bui@gmail.com>

Input: snvs_pwrkey: Improve probe error handling

Use dev_err_probe() for devm_clk_get() to prevent log spam when probe
returns -EPROBE_DEFER. Return the original error from platform_get_irq()
instead of replacing it with -EINVAL. Drop the redundant error message
after devm_request_irq().

> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 954055aaf6e29..fa08ac561bf23 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");

[ ... ]

[Severity: Low]
This isn't a bug introduced by this patch, but since you are improving this
driver, should imx_imx_snvs_check_for_events() use input_report_key() rather
than calling input_event() directly?

drivers/input/keyboard/snvs_pwrkey.c:imx_imx_snvs_check_for_events() {
    ...
	if (state ^ pdata->keystate) {
		pdata->keystate = state;
		input_event(input, EV_KEY, pdata->keycode, state);
		input_sync(input);
		pm_relax(pdata->input->dev.parent);
	}
    ...
}

The subsystem guidelines suggest using the specific reporting helpers like
input_report_key() instead of generic input_event() when the event type
is known.

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