[PATCH 0/3] snvs_pwrkey - code improvements and add report event

[PATCH 0/3] snvs_pwrkey - code improvements and add report event

[Joy Zou]

This patch series improves the snvs_pwrkey driver with better code quality
and add report press event.

The main improvements include:
1. Clean up the code by using local device pointers and dev_err_probe() for
better readability and easier debugging.

2. Fix potential event loss during system suspend by reporting key press events
directly in the interrupt handler.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Joy Zou (3):
      Input: snvs_pwrkey - make use of dev_err_probe()
      Input: snvs_pwrkey - use local device pointer avoid reference platform_device pointer every time
      Input: snvs_pwrkey - report press event in interrupt handler

 drivers/input/keyboard/snvs_pwrkey.c | 75 ++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 37 deletions(-)
---
base-commit: 66ba480978ce390e631e870b740a3406e3eb6b01
change-id: 20260326-pwrkey-cleanup-99d3de61ed6d

Best regards,
-- 
Joy Zou <joy.zou@nxp.com>

[PATCH 1/3] Input: snvs_pwrkey - make use of dev_err_probe()

[Joy Zou]

Add dev_err_probe() at return path of probe() to support users to
identify issues easier.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 40 ++++++++++++++----------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e29527f22f8129fd47ca17722e2bc9..836ab94c160615f4b0f645d9b9f85d54638c2624 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -124,17 +124,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	/* Get SNVS register Page */
 	np = pdev->dev.of_node;
 	if (!np)
-		return -ENODEV;
+		return dev_err_probe(&pdev->dev, -ENODEV, "Device tree node not found\n");
 
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
 	pdata->snvs = syscon_regmap_lookup_by_phandle(np, "regmap");
-	if (IS_ERR(pdata->snvs)) {
-		dev_err(&pdev->dev, "Can't get snvs syscon\n");
-		return PTR_ERR(pdata->snvs);
-	}
+	if (IS_ERR(pdata->snvs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
 
 	if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
 		pdata->keycode = KEY_POWER;
@@ -142,16 +140,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 (%pe)\n", clk);
 
 	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
 
 	pdata->irq = platform_get_irq(pdev, 0);
 	if (pdata->irq < 0)
-		return -EINVAL;
+		return dev_err_probe(&pdev->dev, -EINVAL, "Failed to get interrupt\n");
 
 	error = of_property_read_u32(np, "power-off-time-sec", &val);
 	if (!error) {
@@ -165,9 +162,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 			bpt = (val / 5) - 1;
 			break;
 		default:
-			dev_err(&pdev->dev,
-				"power-off-time-sec %d out of range\n", val);
-			return -EINVAL;
+			return dev_err_probe(&pdev->dev, -EINVAL,
+					     "power-off-time-sec %d out of range\n", val);
 		}
 
 		regmap_update_bits(pdata->snvs, SNVS_LPCR_REG, SNVS_LPCR_BPT_MASK,
@@ -198,10 +194,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 
 	/* input customer action to cancel release timer */
 	error = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
-	if (error) {
-		dev_err(&pdev->dev, "failed to register remove action\n");
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error, "failed to register remove action\n");
 
 	pdata->input = input;
 	platform_set_drvdata(pdev, pdata);
@@ -209,16 +203,12 @@ 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");
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error, "interrupt not available.\n");
 
 	error = input_register_device(input);
-	if (error < 0) {
-		dev_err(&pdev->dev, "failed to register input device\n");
-		return error;
-	}
+	if (error < 0)
+		return dev_err_probe(&pdev->dev, error, "failed to register input device\n");
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);

-- 
2.37.1

Re: [PATCH 1/3] Input: snvs_pwrkey - make use of dev_err_probe()

[Frank Li]

On Thu, Mar 26, 2026 at 06:39:38PM +0800, Joy Zou wrote:
> Add dev_err_probe() at return path of probe() to support users to
> identify issues easier.
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/input/keyboard/snvs_pwrkey.c | 40 ++++++++++++++----------------------
>  1 file changed, 15 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 954055aaf6e29527f22f8129fd47ca17722e2bc9..836ab94c160615f4b0f645d9b9f85d54638c2624 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -124,17 +124,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	/* Get SNVS register Page */
>  	np = pdev->dev.of_node;
>  	if (!np)
> -		return -ENODEV;
> +		return dev_err_probe(&pdev->dev, -ENODEV, "Device tree node not found\n");
>
>  	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata)
>  		return -ENOMEM;
>
>  	pdata->snvs = syscon_regmap_lookup_by_phandle(np, "regmap");
> -	if (IS_ERR(pdata->snvs)) {
> -		dev_err(&pdev->dev, "Can't get snvs syscon\n");
> -		return PTR_ERR(pdata->snvs);
> -	}
> +	if (IS_ERR(pdata->snvs))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
>
>  	if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
>  		pdata->keycode = KEY_POWER;
> @@ -142,16 +140,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 (%pe)\n", clk);
>
>  	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
>
>  	pdata->irq = platform_get_irq(pdev, 0);
>  	if (pdata->irq < 0)
> -		return -EINVAL;
> +		return dev_err_probe(&pdev->dev, -EINVAL, "Failed to get interrupt\n");
>
>  	error = of_property_read_u32(np, "power-off-time-sec", &val);
>  	if (!error) {
> @@ -165,9 +162,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  			bpt = (val / 5) - 1;
>  			break;
>  		default:
> -			dev_err(&pdev->dev,
> -				"power-off-time-sec %d out of range\n", val);
> -			return -EINVAL;
> +			return dev_err_probe(&pdev->dev, -EINVAL,
> +					     "power-off-time-sec %d out of range\n", val);
>  		}
>
>  		regmap_update_bits(pdata->snvs, SNVS_LPCR_REG, SNVS_LPCR_BPT_MASK,
> @@ -198,10 +194,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>
>  	/* input customer action to cancel release timer */
>  	error = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
> -	if (error) {
> -		dev_err(&pdev->dev, "failed to register remove action\n");
> -		return error;
> -	}
> +	if (error)
> +		return dev_err_probe(&pdev->dev, error, "failed to register remove action\n");
>
>  	pdata->input = input;
>  	platform_set_drvdata(pdev, pdata);
> @@ -209,16 +203,12 @@ 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");
> -		return error;
> -	}
> +	if (error)
> +		return dev_err_probe(&pdev->dev, error, "interrupt not available.\n");
>
>  	error = input_register_device(input);
> -	if (error < 0) {
> -		dev_err(&pdev->dev, "failed to register input device\n");
> -		return error;
> -	}
> +	if (error < 0)
> +		return dev_err_probe(&pdev->dev, error, "failed to register input device\n");
>
>  	device_init_wakeup(&pdev->dev, pdata->wakeup);
>  	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);
>
> --
> 2.37.1
>

[PATCH 2/3] Input: snvs_pwrkey - use local device pointer avoid reference platform_device pointer every time

[Joy Zou]

Make use of local struct device pointer to not dereference the
platform_device pointer every time.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 41 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 836ab94c160615f4b0f645d9b9f85d54638c2624..bab3ab57fdac77256be75a080773ea99372ec9c7 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -112,6 +112,7 @@ static void imx_snvs_pwrkey_act(void *pdata)
 
 static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct pwrkey_drv_data *pdata;
 	struct input_dev *input;
 	struct device_node *np;
@@ -122,33 +123,33 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	u32 vid;
 
 	/* Get SNVS register Page */
-	np = pdev->dev.of_node;
+	np = dev->of_node;
 	if (!np)
-		return dev_err_probe(&pdev->dev, -ENODEV, "Device tree node not found\n");
+		return dev_err_probe(dev, -ENODEV, "Device tree node not found\n");
 
-	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
 	pdata->snvs = syscon_regmap_lookup_by_phandle(np, "regmap");
 	if (IS_ERR(pdata->snvs))
-		return dev_err_probe(&pdev->dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
+		return dev_err_probe(dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
 
 	if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
 		pdata->keycode = KEY_POWER;
-		dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
+		dev_warn(dev, "KEY_POWER without setting in dts\n");
 	}
 
-	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
+	clk = devm_clk_get_optional_enabled(dev, NULL);
 	if (IS_ERR(clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+		return dev_err_probe(dev, PTR_ERR(clk),
 				     "Failed to get snvs clock (%pe)\n", clk);
 
 	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
 
 	pdata->irq = platform_get_irq(pdev, 0);
 	if (pdata->irq < 0)
-		return dev_err_probe(&pdev->dev, -EINVAL, "Failed to get interrupt\n");
+		return dev_err_probe(dev, -EINVAL, "Failed to get interrupt\n");
 
 	error = of_property_read_u32(np, "power-off-time-sec", &val);
 	if (!error) {
@@ -162,7 +163,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 			bpt = (val / 5) - 1;
 			break;
 		default:
-			return dev_err_probe(&pdev->dev, -EINVAL,
+			return dev_err_probe(dev, -EINVAL,
 					     "power-off-time-sec %d out of range\n", val);
 		}
 
@@ -182,7 +183,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 
 	input = devm_input_allocate_device(&pdev->dev);
 	if (!input) {
-		dev_err(&pdev->dev, "failed to allocate the input device\n");
+		dev_err(dev, "failed to allocate the input device\n");
 		return -ENOMEM;
 	}
 
@@ -193,27 +194,27 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	input_set_capability(input, EV_KEY, pdata->keycode);
 
 	/* input customer action to cancel release timer */
-	error = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
+	error = devm_add_action(dev, imx_snvs_pwrkey_act, pdata);
 	if (error)
-		return dev_err_probe(&pdev->dev, error, "failed to register remove action\n");
+		return dev_err_probe(dev, error, "failed to register remove action\n");
 
 	pdata->input = input;
 	platform_set_drvdata(pdev, pdata);
 
-	error = devm_request_irq(&pdev->dev, pdata->irq,
-			       imx_snvs_pwrkey_interrupt,
-			       0, pdev->name, pdev);
+	error = devm_request_irq(dev, pdata->irq,
+				 imx_snvs_pwrkey_interrupt,
+				 0, pdev->name, pdev);
 	if (error)
-		return dev_err_probe(&pdev->dev, error, "interrupt not available.\n");
+		return dev_err_probe(dev, error, "interrupt not available.\n");
 
 	error = input_register_device(input);
 	if (error < 0)
-		return dev_err_probe(&pdev->dev, error, "failed to register input device\n");
+		return dev_err_probe(dev, error, "failed to register input device\n");
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup);
-	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);
+	device_init_wakeup(dev, pdata->wakeup);
+	error = dev_pm_set_wake_irq(dev, pdata->irq);
 	if (error)
-		dev_err(&pdev->dev, "irq wake enable failed.\n");
+		dev_err(dev, "irq wake enable failed.\n");
 
 	return 0;
 }

-- 
2.37.1

Re: [PATCH 2/3] Input: snvs_pwrkey - use local device pointer avoid reference platform_device pointer every time

[Frank Li]

On Thu, Mar 26, 2026 at 06:39:39PM +0800, Joy Zou wrote:
> Make use of local struct device pointer to not dereference the
> platform_device pointer every time.


Input: snvs_pwrkey - use local device pointer to simple code

Use local struct device pointer to avoid reference the platform_device
pointer every time.

No functional change.

Frank

>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
>  drivers/input/keyboard/snvs_pwrkey.c | 41 ++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 836ab94c160615f4b0f645d9b9f85d54638c2624..bab3ab57fdac77256be75a080773ea99372ec9c7 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -112,6 +112,7 @@ static void imx_snvs_pwrkey_act(void *pdata)
>
>  static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  {
> +	struct device *dev = &pdev->dev;
>  	struct pwrkey_drv_data *pdata;
>  	struct input_dev *input;
>  	struct device_node *np;
> @@ -122,33 +123,33 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	u32 vid;
>
>  	/* Get SNVS register Page */
> -	np = pdev->dev.of_node;
> +	np = dev->of_node;
>  	if (!np)
> -		return dev_err_probe(&pdev->dev, -ENODEV, "Device tree node not found\n");
> +		return dev_err_probe(dev, -ENODEV, "Device tree node not found\n");
>
> -	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata)
>  		return -ENOMEM;
>
>  	pdata->snvs = syscon_regmap_lookup_by_phandle(np, "regmap");
>  	if (IS_ERR(pdata->snvs))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
> +		return dev_err_probe(dev, PTR_ERR(pdata->snvs), "Can't get snvs syscon\n");
>
>  	if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
>  		pdata->keycode = KEY_POWER;
> -		dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
> +		dev_warn(dev, "KEY_POWER without setting in dts\n");
>  	}
>
> -	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
> +	clk = devm_clk_get_optional_enabled(dev, NULL);
>  	if (IS_ERR(clk))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> +		return dev_err_probe(dev, PTR_ERR(clk),
>  				     "Failed to get snvs clock (%pe)\n", clk);
>
>  	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
>
>  	pdata->irq = platform_get_irq(pdev, 0);
>  	if (pdata->irq < 0)
> -		return dev_err_probe(&pdev->dev, -EINVAL, "Failed to get interrupt\n");
> +		return dev_err_probe(dev, -EINVAL, "Failed to get interrupt\n");
>
>  	error = of_property_read_u32(np, "power-off-time-sec", &val);
>  	if (!error) {
> @@ -162,7 +163,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  			bpt = (val / 5) - 1;
>  			break;
>  		default:
> -			return dev_err_probe(&pdev->dev, -EINVAL,
> +			return dev_err_probe(dev, -EINVAL,
>  					     "power-off-time-sec %d out of range\n", val);
>  		}
>
> @@ -182,7 +183,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>
>  	input = devm_input_allocate_device(&pdev->dev);
>  	if (!input) {
> -		dev_err(&pdev->dev, "failed to allocate the input device\n");
> +		dev_err(dev, "failed to allocate the input device\n");
>  		return -ENOMEM;
>  	}
>
> @@ -193,27 +194,27 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	input_set_capability(input, EV_KEY, pdata->keycode);
>
>  	/* input customer action to cancel release timer */
> -	error = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
> +	error = devm_add_action(dev, imx_snvs_pwrkey_act, pdata);
>  	if (error)
> -		return dev_err_probe(&pdev->dev, error, "failed to register remove action\n");
> +		return dev_err_probe(dev, error, "failed to register remove action\n");
>
>  	pdata->input = input;
>  	platform_set_drvdata(pdev, pdata);
>
> -	error = devm_request_irq(&pdev->dev, pdata->irq,
> -			       imx_snvs_pwrkey_interrupt,
> -			       0, pdev->name, pdev);
> +	error = devm_request_irq(dev, pdata->irq,
> +				 imx_snvs_pwrkey_interrupt,
> +				 0, pdev->name, pdev);
>  	if (error)
> -		return dev_err_probe(&pdev->dev, error, "interrupt not available.\n");
> +		return dev_err_probe(dev, error, "interrupt not available.\n");
>
>  	error = input_register_device(input);
>  	if (error < 0)
> -		return dev_err_probe(&pdev->dev, error, "failed to register input device\n");
> +		return dev_err_probe(dev, error, "failed to register input device\n");
>
> -	device_init_wakeup(&pdev->dev, pdata->wakeup);
> -	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);
> +	device_init_wakeup(dev, pdata->wakeup);
> +	error = dev_pm_set_wake_irq(dev, pdata->irq);
>  	if (error)
> -		dev_err(&pdev->dev, "irq wake enable failed.\n");
> +		dev_err(dev, "irq wake enable failed.\n");
>
>  	return 0;
>  }
>
> --
> 2.37.1
>

[PATCH 3/3] Input: snvs_pwrkey - report press event in interrupt handler

[Joy Zou]

On some boards such as i.MX8MQ-EVK, the PCIe driver may take up to
200ms to restore the PCIe link during the no_irq resume phase. This
causes key press events to be lost because the key may be released
before the timer starts running, as interrupts are disabled during
this 200ms window.

Report key press events directly in interrupt handler to prevent event
loss during system suspend.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index bab3ab57fdac77256be75a080773ea99372ec9c7..b557c1618d7369e872c6ce708a7b3017264ee385 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -78,6 +78,16 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
 
 	pm_wakeup_event(input->dev.parent, 0);
 
+	/*
+	 * Report key press events directly in interrupt handler to prevent event
+	 * loss during system suspend.
+	 */
+	if (pdev->dev.power.is_suspended) {
+		pdata->keystate = 1;
+		input_report_key(input, pdata->keycode, 1);
+		input_sync(input);
+	}
+
 	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
 	if (lp_status & SNVS_LPSR_SPO) {
 		if (pdata->minor_rev == 0) {

-- 
2.37.1

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

[Frank Li]

On Thu, Mar 26, 2026 at 06:39:40PM +0800, Joy Zou wrote:
> On some boards such as i.MX8MQ-EVK, the PCIe driver may take up to
> 200ms to restore the PCIe link during the no_irq resume phase. This
> causes key press events to be lost because the key may be released
> before the timer starts running, as interrupts are disabled during
> this 200ms window.

if irq disable, how imx_snvs_pwrkey_interrupt get run?

Frank
>
> Report key press events directly in interrupt handler to prevent event
> loss during system suspend.
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
>  drivers/input/keyboard/snvs_pwrkey.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index bab3ab57fdac77256be75a080773ea99372ec9c7..b557c1618d7369e872c6ce708a7b3017264ee385 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -78,6 +78,16 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
>
>  	pm_wakeup_event(input->dev.parent, 0);
>
> +	/*
> +	 * Report key press events directly in interrupt handler to prevent event
> +	 * loss during system suspend.
> +	 */
> +	if (pdev->dev.power.is_suspended) {
> +		pdata->keystate = 1;
> +		input_report_key(input, pdata->keycode, 1);
> +		input_sync(input);
> +	}
> +
>  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
>  	if (lp_status & SNVS_LPSR_SPO) {
>  		if (pdata->minor_rev == 0) {
>
> --
> 2.37.1
>

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

[Joy Zou]

On Thu, Mar 26, 2026 at 03:57:27PM -0400, Frank Li wrote:
> On Thu, Mar 26, 2026 at 06:39:40PM +0800, Joy Zou wrote:
> > On some boards such as i.MX8MQ-EVK, the PCIe driver may take up to
> > 200ms to restore the PCIe link during the no_irq resume phase. This
> > causes key press events to be lost because the key may be released
> > before the timer starts running, as interrupts are disabled during
> > this 200ms window.
> 
> if irq disable, how imx_snvs_pwrkey_interrupt get run?
> 
Thank you for your comments. I might have missed some details in my commit
message. Could you please review the description below and let me know if
it's clear and comprehensive enough?

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.
BR
Joy Zou
> Frank
> >
> > Report key press events directly in interrupt handler to prevent event
> > loss during system suspend.
> >
> > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > ---
> >  drivers/input/keyboard/snvs_pwrkey.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> > index bab3ab57fdac77256be75a080773ea99372ec9c7..b557c1618d7369e872c6ce708a7b3017264ee385 100644
> > --- a/drivers/input/keyboard/snvs_pwrkey.c
> > +++ b/drivers/input/keyboard/snvs_pwrkey.c
> > @@ -78,6 +78,16 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
> >
> >  	pm_wakeup_event(input->dev.parent, 0);
> >
> > +	/*
> > +	 * Report key press events directly in interrupt handler to prevent event
> > +	 * loss during system suspend.
> > +	 */
> > +	if (pdev->dev.power.is_suspended) {
> > +		pdata->keystate = 1;
> > +		input_report_key(input, pdata->keycode, 1);
> > +		input_sync(input);
> > +	}
> > +
> >  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> >  	if (lp_status & SNVS_LPSR_SPO) {
> >  		if (pdata->minor_rev == 0) {
> >
> > --
> > 2.37.1
> >

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

[Frank Li]

On Tue, Mar 31, 2026 at 06:46:55PM +0800, Joy Zou wrote:
> On Thu, Mar 26, 2026 at 03:57:27PM -0400, Frank Li wrote:
> > On Thu, Mar 26, 2026 at 06:39:40PM +0800, Joy Zou wrote:
> > > On some boards such as i.MX8MQ-EVK, the PCIe driver may take up to
> > > 200ms to restore the PCIe link during the no_irq resume phase. This
> > > causes key press events to be lost because the key may be released
> > > before the timer starts running, as interrupts are disabled during
> > > this 200ms window.
> >
> > if irq disable, how imx_snvs_pwrkey_interrupt get run?
> >
> Thank you for your comments. I might have missed some details in my commit
> message. Could you please review the description below and let me know if
> it's clear and comprehensive enough?
>
> 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.

much better. Thanks

Frank

> BR
> Joy Zou
> > Frank
> > >
> > > Report key press events directly in interrupt handler to prevent event
> > > loss during system suspend.
> > >
> > > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > > ---
> > >  drivers/input/keyboard/snvs_pwrkey.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> > > index bab3ab57fdac77256be75a080773ea99372ec9c7..b557c1618d7369e872c6ce708a7b3017264ee385 100644
> > > --- a/drivers/input/keyboard/snvs_pwrkey.c
> > > +++ b/drivers/input/keyboard/snvs_pwrkey.c
> > > @@ -78,6 +78,16 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
> > >
> > >  	pm_wakeup_event(input->dev.parent, 0);
> > >
> > > +	/*
> > > +	 * Report key press events directly in interrupt handler to prevent event
> > > +	 * loss during system suspend.
> > > +	 */
> > > +	if (pdev->dev.power.is_suspended) {
> > > +		pdata->keystate = 1;
> > > +		input_report_key(input, pdata->keycode, 1);
> > > +		input_sync(input);
> > > +	}
> > > +
> > >  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> > >  	if (lp_status & SNVS_LPSR_SPO) {
> > >  		if (pdata->minor_rev == 0) {
> > >
> > > --
> > > 2.37.1
> > >

Re: [PATCH 0/3] snvs_pwrkey - code improvements and add report event

[Dmitry Torokhov]

On Thu, Mar 26, 2026 at 06:39:37PM +0800, Joy Zou wrote:
> This patch series improves the snvs_pwrkey driver with better code quality
> and add report press event.
> 
> The main improvements include:
> 1. Clean up the code by using local device pointers and dev_err_probe() for
> better readability and easier debugging.
> 
> 2. Fix potential event loss during system suspend by reporting key press events
> directly in the interrupt handler.
> 
> Signed-off-by: Joy Zou <joy.zou@nxp.com>

Please address comments form here:

https://sashiko.dev/#/patchset/20260326-pwrkey-cleanup-v1-0-d85d7c0bf275%40nxp.com

Majority of them seem valid.

Thanks.

-- 
Dmitry