* [PATCH 2/2] input: keyboard: snvs_pwrkey: Fix missed key press after suspend
2024-07-16 20:56 [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers Frank Li
@ 2024-07-16 20:56 ` Frank Li
2024-07-16 20:56 ` [PATCH v2 1/1] input: bbnsm_pwrkey: " Frank Li
2024-08-12 17:35 ` [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers Dmitry Torokhov
2 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2024-07-16 20:56 UTC (permalink / raw)
To: Dmitry Torokhov, Dong Aisheng, Joy Zou, Robin Gong, Shawn Guo,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list
Cc: imx
Report input event directly on wakeup to ensure no press event is missed
when resuming from suspend.
Fixes: d3dc6e232215 ("input: keyboard: imx: add snvs power key driver")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joy Zou <joy.zou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/input/keyboard/snvs_pwrkey.c | 37 ++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index b352148a0cfb2..a1c04fe8fe05c 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -37,6 +37,7 @@ struct pwrkey_drv_data {
int keycode;
int keystate; /* 1:pressed */
int wakeup;
+ bool suspended;
struct clk *clk;
struct timer_list check_timer;
struct input_dev *input;
@@ -79,6 +80,18 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
pm_wakeup_event(input->dev.parent, 0);
+ /*
+ * Directly report press event in interrupt handler after suspend
+ * to ensure no press event miss.
+ */
+ if (pdata->suspended) {
+ pdata->keystate = 1;
+ input_event(input, EV_KEY, pdata->keycode, 1);
+ input_sync(input);
+ /* Fire at most once per suspend/resume cycle */
+ pdata->suspended = false;
+ }
+
regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
if (lp_status & SNVS_LPSR_SPO) {
if (pdata->minor_rev == 0) {
@@ -236,6 +249,29 @@ 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;
+
+ 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;
+
+ 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 */ }
@@ -245,6 +281,7 @@ MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
static struct platform_driver imx_snvs_pwrkey_driver = {
.driver = {
.name = "snvs_pwrkey",
+ .pm = &imx_snvs_pwrkey_pm_ops,
.of_match_table = imx_snvs_pwrkey_ids,
},
.probe = imx_snvs_pwrkey_probe,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 1/1] input: bbnsm_pwrkey: Fix missed key press after suspend
2024-07-16 20:56 [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers Frank Li
2024-07-16 20:56 ` [PATCH 2/2] input: keyboard: snvs_pwrkey: Fix missed key press after suspend Frank Li
@ 2024-07-16 20:56 ` Frank Li
2024-07-16 21:00 ` Frank Li
2024-08-12 17:35 ` [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers Dmitry Torokhov
2 siblings, 1 reply; 5+ messages in thread
From: Frank Li @ 2024-07-16 20:56 UTC (permalink / raw)
To: Dmitry Torokhov, Jason Liu, Jacky Bai, Peng Fan,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list
Cc: imx
From: Jacky Bai <ping.bai@nxp.com>
Report input event directly on wakeup to ensure no press event is missed
when resuming from suspend.
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Acked-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v1 to v2
- move before mod_time
- clean suspend flag to make sure only fire once
---
drivers/input/misc/nxp-bbnsm-pwrkey.c | 38 +++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/input/misc/nxp-bbnsm-pwrkey.c b/drivers/input/misc/nxp-bbnsm-pwrkey.c
index 1d99206dd3a8b..eb4173f9c8204 100644
--- a/drivers/input/misc/nxp-bbnsm-pwrkey.c
+++ b/drivers/input/misc/nxp-bbnsm-pwrkey.c
@@ -38,6 +38,7 @@ struct bbnsm_pwrkey {
int irq;
int keycode;
int keystate; /* 1:pressed */
+ bool suspended;
struct timer_list check_timer;
struct input_dev *input;
};
@@ -70,6 +71,7 @@ static irqreturn_t bbnsm_pwrkey_interrupt(int irq, void *dev_id)
{
struct platform_device *pdev = dev_id;
struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
+ struct input_dev *input = bbnsm->input;
u32 event;
regmap_read(bbnsm->regmap, BBNSM_EVENTS, &event);
@@ -78,6 +80,18 @@ static irqreturn_t bbnsm_pwrkey_interrupt(int irq, void *dev_id)
pm_wakeup_event(bbnsm->input->dev.parent, 0);
+ /*
+ * Directly report key event after resume to make sure key press
+ * event is never missed.
+ */
+ if (bbnsm->suspended) {
+ bbnsm->keystate = 1;
+ input_event(input, EV_KEY, bbnsm->keycode, 1);
+ input_sync(input);
+ /* Fire at most once per suspend/resume cycle */
+ bbnsm->suspended = false;
+ }
+
mod_timer(&bbnsm->check_timer,
jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
@@ -173,6 +187,29 @@ static int bbnsm_pwrkey_probe(struct platform_device *pdev)
return 0;
}
+static int __maybe_unused bbnsm_pwrkey_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
+
+ bbnsm->suspended = true;
+
+ return 0;
+}
+
+static int __maybe_unused bbnsm_pwrkey_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
+
+ bbnsm->suspended = false;
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(bbnsm_pwrkey_pm_ops, bbnsm_pwrkey_suspend,
+ bbnsm_pwrkey_resume);
+
static const struct of_device_id bbnsm_pwrkey_ids[] = {
{ .compatible = "nxp,imx93-bbnsm-pwrkey" },
{ /* sentinel */ }
@@ -182,6 +219,7 @@ MODULE_DEVICE_TABLE(of, bbnsm_pwrkey_ids);
static struct platform_driver bbnsm_pwrkey_driver = {
.driver = {
.name = "bbnsm_pwrkey",
+ .pm = &bbnsm_pwrkey_pm_ops,
.of_match_table = bbnsm_pwrkey_ids,
},
.probe = bbnsm_pwrkey_probe,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] input: bbnsm_pwrkey: Fix missed key press after suspend
2024-07-16 20:56 ` [PATCH v2 1/1] input: bbnsm_pwrkey: " Frank Li
@ 2024-07-16 21:00 ` Frank Li
0 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2024-07-16 21:00 UTC (permalink / raw)
To: Dmitry Torokhov, Jason Liu, Jacky Bai, Peng Fan,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list
Cc: imx
On Tue, Jul 16, 2024 at 04:56:11PM -0400, Frank Li wrote:
> From: Jacky Bai <ping.bai@nxp.com>
Sorry, Please forget this patch, this one accidently sent out with other
2 patches. And this one already applied.
Frank
>
> Report input event directly on wakeup to ensure no press event is missed
> when resuming from suspend.
>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Acked-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - move before mod_time
> - clean suspend flag to make sure only fire once
> ---
> drivers/input/misc/nxp-bbnsm-pwrkey.c | 38 +++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/input/misc/nxp-bbnsm-pwrkey.c b/drivers/input/misc/nxp-bbnsm-pwrkey.c
> index 1d99206dd3a8b..eb4173f9c8204 100644
> --- a/drivers/input/misc/nxp-bbnsm-pwrkey.c
> +++ b/drivers/input/misc/nxp-bbnsm-pwrkey.c
> @@ -38,6 +38,7 @@ struct bbnsm_pwrkey {
> int irq;
> int keycode;
> int keystate; /* 1:pressed */
> + bool suspended;
> struct timer_list check_timer;
> struct input_dev *input;
> };
> @@ -70,6 +71,7 @@ static irqreturn_t bbnsm_pwrkey_interrupt(int irq, void *dev_id)
> {
> struct platform_device *pdev = dev_id;
> struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
> + struct input_dev *input = bbnsm->input;
> u32 event;
>
> regmap_read(bbnsm->regmap, BBNSM_EVENTS, &event);
> @@ -78,6 +80,18 @@ static irqreturn_t bbnsm_pwrkey_interrupt(int irq, void *dev_id)
>
> pm_wakeup_event(bbnsm->input->dev.parent, 0);
>
> + /*
> + * Directly report key event after resume to make sure key press
> + * event is never missed.
> + */
> + if (bbnsm->suspended) {
> + bbnsm->keystate = 1;
> + input_event(input, EV_KEY, bbnsm->keycode, 1);
> + input_sync(input);
> + /* Fire at most once per suspend/resume cycle */
> + bbnsm->suspended = false;
> + }
> +
> mod_timer(&bbnsm->check_timer,
> jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
>
> @@ -173,6 +187,29 @@ static int bbnsm_pwrkey_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused bbnsm_pwrkey_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
> +
> + bbnsm->suspended = true;
> +
> + return 0;
> +}
> +
> +static int __maybe_unused bbnsm_pwrkey_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct bbnsm_pwrkey *bbnsm = platform_get_drvdata(pdev);
> +
> + bbnsm->suspended = false;
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(bbnsm_pwrkey_pm_ops, bbnsm_pwrkey_suspend,
> + bbnsm_pwrkey_resume);
> +
> static const struct of_device_id bbnsm_pwrkey_ids[] = {
> { .compatible = "nxp,imx93-bbnsm-pwrkey" },
> { /* sentinel */ }
> @@ -182,6 +219,7 @@ MODULE_DEVICE_TABLE(of, bbnsm_pwrkey_ids);
> static struct platform_driver bbnsm_pwrkey_driver = {
> .driver = {
> .name = "bbnsm_pwrkey",
> + .pm = &bbnsm_pwrkey_pm_ops,
> .of_match_table = bbnsm_pwrkey_ids,
> },
> .probe = bbnsm_pwrkey_probe,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers
2024-07-16 20:56 [PATCH 1/2] input: keyboard: snvs_pwrkey: Enable clock when accessing HP* registers Frank Li
2024-07-16 20:56 ` [PATCH 2/2] input: keyboard: snvs_pwrkey: Fix missed key press after suspend Frank Li
2024-07-16 20:56 ` [PATCH v2 1/1] input: bbnsm_pwrkey: " Frank Li
@ 2024-08-12 17:35 ` Dmitry Torokhov
2 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2024-08-12 17:35 UTC (permalink / raw)
To: Frank Li
Cc: Joy Zou, Dong Aisheng, Robin Gong,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list, imx
Hi Frank,
On Tue, Jul 16, 2024 at 04:56:09PM -0400, Frank Li wrote:
> From: Robin Gong <yibin.gong@nxp.com>
>
> SNVS requires two clock sources:
> - LP (32k always on): All LP* registers need this clock. No management is
> needed as it is always on.
> - HP: All HP* registers require this clock to be enabled before accessing
> these registers. Some platforms (e.g., i.MX6SX/i.MX6UL) do not have a
> separate HP root clock and it is always on.
>
> Add an optional "snvs-pwrkey" clock for the HP clock and enable it only
> when accessing HP* registers.
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> - clock name snvs-pwrkey already documented at
> Documentation/devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml
> ---
> drivers/input/keyboard/snvs_pwrkey.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index ad8660be0127c..b352148a0cfb2 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -37,6 +37,7 @@ struct pwrkey_drv_data {
> int keycode;
> int keystate; /* 1:pressed */
> int wakeup;
> + struct clk *clk;
> struct timer_list check_timer;
> struct input_dev *input;
> u8 minor_rev;
> @@ -48,7 +49,10 @@ static void imx_imx_snvs_check_for_events(struct timer_list *t)
> struct input_dev *input = pdata->input;
> u32 state;
>
> + clk_prepare_enable(pdata->clk);
We are in timer context here, "prepare" is not allowed here. Can we
prepare the clock once and enable it as needed. Does it even need to be
disabled? Can it be also always on?
If you want enable/disable then error handling is needed.
> regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
> + clk_disable_unprepare(pdata->clk);
> +
> state = state & SNVS_HPSR_BTN ? 1 : 0;
>
> /* only report new event if status changed */
> @@ -169,7 +173,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> if (pdata->irq < 0)
> return -EINVAL;
>
> + pdata->clk = devm_clk_get_optional(&pdev->dev, "snvs-pwrkey");
> + if (IS_ERR(pdata->clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(pdata->clk),
> + "Could not get the snvs-pwrkey clock");
> +
> + clk_prepare_enable(pdata->clk);
> regmap_read(pdata->snvs, SNVS_HPVIDR1_REG, &vid);
> + clk_disable_unprepare(pdata->clk);
> +
> pdata->minor_rev = vid & 0xff;
>
> regmap_update_bits(pdata->snvs, SNVS_LPCR_REG, SNVS_LPCR_DEP_EN, SNVS_LPCR_DEP_EN);
> --
> 2.34.1
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread