Hi,
On 1/4/24 19:31, Werner Sembach wrote:
> On s3 resume the i8042 driver tries to restore the controller to a known
> state by reinitializing things, however this can confuse the controller
> with different effects. Mostly occasionally unresponsive keyboards after
> resume.
>
> These issues do not rise on s0ix resume as here the controller is assumed
> to preserved its state from before suspend.
>
> This patch adds a quirk for devices where the reinitialization on s3 resume
> is not needed and might be harmful as described above. It does this by
> using the s0ix resume code path at selected locations.
>
> This new quirk goes beyond what the preexisting reset=never quirk does,
> which only skips some reinitialization steps.
>
> Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> ---
> drivers/input/serio/i8042-acpipnpio.h | 10 +++++++---
> drivers/input/serio/i8042.c | 10 +++++++---
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
> index b585b1dab870e..10ec4534e5e14 100644
> --- a/drivers/input/serio/i8042-acpipnpio.h
> +++ b/drivers/input/serio/i8042-acpipnpio.h
> @@ -83,6 +83,7 @@ static inline void i8042_write_command(int val)
> #define SERIO_QUIRK_KBDRESET BIT(12)
> #define SERIO_QUIRK_DRITEK BIT(13)
> #define SERIO_QUIRK_NOPNP BIT(14)
> +#define SERIO_QUIRK_FORCENORESTORE BIT(15)
>
> /* Quirk table for different mainboards. Options similar or identical to i8042
> * module parameters.
> @@ -1657,6 +1658,8 @@ static void __init i8042_check_quirks(void)
> if (quirks & SERIO_QUIRK_NOPNP)
> i8042_nopnp = true;
> #endif
> + if (quirks & SERIO_QUIRK_FORCENORESTORE)
> + i8042_forcenorestore = true;
> }
> #else
> static inline void i8042_check_quirks(void) {}
> @@ -1690,7 +1693,7 @@ static int __init i8042_platform_init(void)
>
> i8042_check_quirks();
>
> - pr_debug("Active quirks (empty means none):%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
> + pr_debug("Active quirks (empty means none):%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
> i8042_nokbd ? " nokbd" : "",
> i8042_noaux ? " noaux" : "",
> i8042_nomux ? " nomux" : "",
> @@ -1710,10 +1713,11 @@ static int __init i8042_platform_init(void)
> "",
> #endif
> #ifdef CONFIG_PNP
> - i8042_nopnp ? " nopnp" : "");
> + i8042_nopnp ? " nopnp" : "",
> #else
> - "");
> + "",
> #endif
> + i8042_forcenorestore ? " forcenorestore" : "");
>
> retval = i8042_pnp_init();
> if (retval)
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 9fbb8d31575ae..2233d93f90e81 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -115,6 +115,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
> MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller settings");
> #endif
>
> +static bool i8042_forcenorestore;
> +module_param_named(forcenorestore, i8042_forcenorestore, bool, 0);
> +MODULE_PARM_DESC(forcenorestore, "Force no restore on s3 resume, copying s2idle behaviour");
> +
> #define DEBUG
> #ifdef DEBUG
> static bool i8042_debug;
> @@ -1232,7 +1236,7 @@ static int i8042_pm_suspend(struct device *dev)
> {
> int i;
>
> - if (pm_suspend_via_firmware())
> + if (!i8042_forcenorestore && pm_suspend_via_firmware())
> i8042_controller_reset(true);
>
> /* Set up serio interrupts for system wakeup. */
> @@ -1248,7 +1252,7 @@ static int i8042_pm_suspend(struct device *dev)
>
> static int i8042_pm_resume_noirq(struct device *dev)
> {
> - if (!pm_resume_via_firmware())
> + if (i8042_forcenorestore || !pm_resume_via_firmware())
> i8042_interrupt(0, NULL);
>
> return 0;
> @@ -1271,7 +1275,7 @@ static int i8042_pm_resume(struct device *dev)
> * not restore the controller state to whatever it had been at boot
> * time, so we do not need to do anything.
> */
> - if (!pm_suspend_via_firmware())
> + if (i8042_forcenorestore || !pm_suspend_via_firmware())
> return 0;
>
> /*
