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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race between hardirq and timer softirq leads to desynchronized key state
--

commit 9a729213fc1cfc21c866a352fad1a978bfa80359
Author: Joy Zou <joy.zou@nxp.com>

Input: snvs_pwrkey - report press event in interrupt handler

This commit modifies the snvs_pwrkey driver to directly report key press
events from the interrupt handler during system suspend. This bypasses the
debounce timer to prevent event loss during the no_irq resume phase.

> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 4a1d048984826..f212a6b26185d 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
[ ... ]
> @@ -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) {
> +				pdata->keystate = 1;
> +				input_report_key(input, pdata->keycode, 1);
> +				input_sync(input);
> +			}

[Severity: High]
Does this introduce a data race with the timer callback
imx_imx_snvs_check_for_events()?

The new hardirq logic writes to pdata->keystate locklessly here, but
imx_imx_snvs_check_for_events() also reads and writes it locklessly in
softirq context.

Could this sequence occur:

1. The timer callback reads state = 0 (key released) and evaluates
   (state ^ pdata->keystate) as true.
2. The timer sets pdata->keystate = 0.
3. Before the timer calls input_event(), a key press triggers the hardirq
   on the same CPU (preemption) or another CPU.
4. The hardirq sees pdata->suspended == true, sets pdata->keystate = 1,
   and reports a key press (1).
5. The timer resumes and reports the release (0) to the input core.

This would leave pdata->keystate as 1, while the input subsystem's last
reported state is 0. On the next physical key press, the timer would read
state = 1, evaluate 1 ^ 1 as 0, and ignore the event. This might cause the
power key to become logically stuck until physically released again.

>  			mod_timer(&pdata->check_timer,
>  			          jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-pwrkey-cleanup-v2-0-59fa0ddf0d7c@nxp.com?part=4