Re: [PATCH v2 4/4] Input: snvs_pwrkey - report press event in interrupt handler

[Bough Chen]

On Thu, Jun 04, 2026 at 02:56:24PM +0800, Joy Zou wrote:
> The driver implements debounce protection using a timer-based mechanism:
> when a key interrupt occurs, a timer is scheduled to verify the key state
> after DEBOUNCE_TIME before reporting the event. This works well during
> normal operation.
> 
> However, key press events can be lost during system resume on platforms
> like i.MX8MQ-EVK because:
> 1. During the no_irq resume phase, PCIe driver restoration can take up to
> 200ms with IRQs disabled.
> 2. The power key interrupt remains pending during the no_irq phase.
> 3. If the key is released before IRQs are re-enabled, the timer eventually
> runs but sees the key as released and skips reporting the event.
> 
> Report key press events directly in interrupt handler to prevent event
> loss during system suspend. This is safe because:
> 
> 1. Only one event is reported per suspend cycle.
> 2. Normal operation retains the existing timer-based debounce mechanism.
> 
> Signed-off-by: Joy Zou <joy.zou@xxxxxxx>
> ---
> Changes for v2:
> 1. Add a boolean variable suspended and PM callback functions to replace
>    the use of the is_suspended field per AI review comments.
> 2. Move event report handle to else branch in suspended state, since the
>    pdata->minor_rev == 0 branch has no debounce detection per AI review
>    comments.
> 3. Modify the commit message.
> ---
>  drivers/input/keyboard/snvs_pwrkey.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 4a1d04898482669894e9978014b62e4e9774b4e4..f212a6b26185d13e1af62728e7b2add5010adc5a 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -39,6 +39,7 @@ struct pwrkey_drv_data {
>  	int keycode;
>  	int keystate;  /* 1:pressed */
>  	int wakeup;
> +	bool suspended; /* Track suspend state */
>  	struct timer_list check_timer;
>  	struct input_dev *input;
>  	u8 minor_rev;
> @@ -92,6 +93,15 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
>  			input_sync(input);
>  			pm_relax(input->dev.parent);
>  		} else {
> +			/*
> +			 * Report key press events directly in interrupt handler to prevent event
> +			 * loss during system suspend.
> +			 */
> +			if (pdata->suspended) {

Here, pdata->suspended should not be safe for SMP system, maybe to use atomic_read() to make the code more stronger.

> +				pdata->keystate = 1;
> +				input_report_key(input, pdata->keycode, 1);
> +				input_sync(input);
> +			}
>  			mod_timer(&pdata->check_timer,
>  			          jiffies + msecs_to_jiffies(DEBOUNCE_TIME));

Here already report the key when pdata->suspend == true, seems do not need to trigger the timer again.
Is it more reasonable to change like this?
if (atomic_read(&pdata->suspended)) {
	...
} else {
	mode_timer()
}

>  		}
> @@ -219,6 +229,30 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static int __maybe_unused imx_snvs_pwrkey_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> +
> +	pdata->suspended = true;
> +

atomic_set(&pdata->suspended, 1) 

> +	return 0;
> +}
> +
> +static int __maybe_unused imx_snvs_pwrkey_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> +
> +	pdata->suspended = false;
> +

atomic_set(&pdata->suspended, 0);

Regards
Haibo Chen
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx_snvs_pwrkey_pm_ops,
> +			 imx_snvs_pwrkey_suspend,
> +			 imx_snvs_pwrkey_resume);
> +
>  static const struct of_device_id imx_snvs_pwrkey_ids[] = {
>  	{ .compatible = "fsl,sec-v4.0-pwrkey" },
>  	{ /* sentinel */ }
> @@ -229,6 +263,7 @@ static struct platform_driver imx_snvs_pwrkey_driver = {
>  	.driver = {
>  		.name = "snvs_pwrkey",
>  		.of_match_table = imx_snvs_pwrkey_ids,
> +		.pm   = &imx_snvs_pwrkey_pm_ops,
>  	},
>  	.probe = imx_snvs_pwrkey_probe,
>  };
> 
> -- 
> 2.50.1
>