2023-11-02 12:55:13

by Jonathan Denose

[permalink] [raw]
Subject: [PATCH] Input: psmouse - add resync_on_resume dmi check

Some elantech touchpads consistently fail after resuming from
suspend at sanity_check in elantech_packet_check_v4. This means
the touchpad is completely unusable after suspend resume.

With different permutations of i8042 nomux, nopnp, reset, and noloop
kernel options enabled, and with crc_enabled the touchpad fails in
the same way.

Resyncing the touchpad after receiving the
PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
function correctly on resume. The touchpad fails to reconnect with
the serio reconnect no matter how many times it retries, so this
change skips over that retry sequence and goes directly to resync.

Signed-off-by: Jonathan Denose <[email protected]>
---

drivers/input/mouse/psmouse-base.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index a0aac76b1e41d..3c6eefcb9582f 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -12,6 +12,7 @@

#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
@@ -105,6 +106,16 @@ static struct attribute *psmouse_dev_attrs[] = {

ATTRIBUTE_GROUPS(psmouse_dev);

+static const struct dmi_system_id resync_on_resume[] = {
+ {
+ .ident = "Lenovo N24",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo N24"),
+ },
+ }
+};
+
/*
* psmouse_mutex protects all operations changing state of mouse
* (connecting, disconnecting, changing rate or resolution via
@@ -285,6 +296,12 @@ static int psmouse_handle_byte(struct psmouse *psmouse)
"%s at %s lost sync at byte %d\n",
psmouse->name, psmouse->phys,
psmouse->pktcnt);
+ if (dmi_check_system(resync_on_resume)) {
+ psmouse_notice(psmouse, "issuing resync request");
+ __psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
+ psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
+ return -EIO;
+ }
if (++psmouse->out_of_sync_cnt == psmouse->resetafter) {
__psmouse_set_state(psmouse, PSMOUSE_IGNORE);
psmouse_notice(psmouse,
--
2.42.0.820.g83a721a137-goog


2024-02-06 22:04:39

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

Hi Jonathan,

On Thu, Nov 02, 2023 at 07:52:47AM -0500, Jonathan Denose wrote:
> Some elantech touchpads consistently fail after resuming from
> suspend at sanity_check in elantech_packet_check_v4. This means
> the touchpad is completely unusable after suspend resume.
>
> With different permutations of i8042 nomux, nopnp, reset, and noloop
> kernel options enabled, and with crc_enabled the touchpad fails in
> the same way.
>
> Resyncing the touchpad after receiving the
> PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
> function correctly on resume. The touchpad fails to reconnect with
> the serio reconnect no matter how many times it retries, so this
> change skips over that retry sequence and goes directly to resync.

Why can't we do this in elantech_reconnect()? I am sure we can make it
simpler and more robust than what the generic handler is trying to do
with polling and everything.

Thanks.

--
Dmitry

2024-02-07 16:44:45

by Jonathan Denose

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

Hi Dmitry,

Thanks for your reply.

On Tue, Feb 6, 2024 at 4:04 PM Dmitry Torokhov
<[email protected]> wrote:
>
> Hi Jonathan,
>
> On Thu, Nov 02, 2023 at 07:52:47AM -0500, Jonathan Denose wrote:
> > Some elantech touchpads consistently fail after resuming from
> > suspend at sanity_check in elantech_packet_check_v4. This means
> > the touchpad is completely unusable after suspend resume.
> >
> > With different permutations of i8042 nomux, nopnp, reset, and noloop
> > kernel options enabled, and with crc_enabled the touchpad fails in
> > the same way.
> >
> > Resyncing the touchpad after receiving the
> > PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
> > function correctly on resume. The touchpad fails to reconnect with
> > the serio reconnect no matter how many times it retries, so this
> > change skips over that retry sequence and goes directly to resync.
>
> Why can't we do this in elantech_reconnect()? I am sure we can make it
> simpler and more robust than what the generic handler is trying to do
> with polling and everything.
>
> Thanks.
>
> --
> Dmitry

I am fine with anything that would be simpler and more robust, though
I'm not sure how to implement what you are describing.

Are you suggesting that in this PSMOUSE_BAD_DATA case, instead of
using psmouse_set_state and psmouse_queue_work to call
psmouse->reconnect (which calls elantech_reconnect)?

Jonathan

2024-02-09 19:02:38

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

On Wed, Feb 07, 2024 at 10:39:03AM -0600, Jonathan Denose wrote:
> Hi Dmitry,
>
> Thanks for your reply.
>
> On Tue, Feb 6, 2024 at 4:04 PM Dmitry Torokhov
> <[email protected]> wrote:
> >
> > Hi Jonathan,
> >
> > On Thu, Nov 02, 2023 at 07:52:47AM -0500, Jonathan Denose wrote:
> > > Some elantech touchpads consistently fail after resuming from
> > > suspend at sanity_check in elantech_packet_check_v4. This means
> > > the touchpad is completely unusable after suspend resume.
> > >
> > > With different permutations of i8042 nomux, nopnp, reset, and noloop
> > > kernel options enabled, and with crc_enabled the touchpad fails in
> > > the same way.
> > >
> > > Resyncing the touchpad after receiving the
> > > PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
> > > function correctly on resume. The touchpad fails to reconnect with
> > > the serio reconnect no matter how many times it retries, so this
> > > change skips over that retry sequence and goes directly to resync.
> >
> > Why can't we do this in elantech_reconnect()? I am sure we can make it
> > simpler and more robust than what the generic handler is trying to do
> > with polling and everything.
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> I am fine with anything that would be simpler and more robust, though
> I'm not sure how to implement what you are describing.
>
> Are you suggesting that in this PSMOUSE_BAD_DATA case, instead of
> using psmouse_set_state and psmouse_queue_work to call
> psmouse->reconnect (which calls elantech_reconnect)?

No. From the description it sounds like the device sends wrong/extra
data right after resume. I think you can handle it in
elantech_reconnect() method by draining the buffer or issuing poll
request or something similar.

Can you post the i8042 data stream that happens on suspend/resume?
Toggling i8042.debug option will cause the driver to dump the data to
dmesg.

Thanks.

--
Dmitry

2024-02-12 20:57:54

by Jonathan Denose

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

Sure thing!

Attached is the dmesg output with the i8042.debug option and without
the change in this patch.

On Fri, Feb 9, 2024 at 1:02 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Wed, Feb 07, 2024 at 10:39:03AM -0600, Jonathan Denose wrote:
> > Hi Dmitry,
> >
> > Thanks for your reply.
> >
> > On Tue, Feb 6, 2024 at 4:04 PM Dmitry Torokhov
> > <[email protected]> wrote:
> > >
> > > Hi Jonathan,
> > >
> > > On Thu, Nov 02, 2023 at 07:52:47AM -0500, Jonathan Denose wrote:
> > > > Some elantech touchpads consistently fail after resuming from
> > > > suspend at sanity_check in elantech_packet_check_v4. This means
> > > > the touchpad is completely unusable after suspend resume.
> > > >
> > > > With different permutations of i8042 nomux, nopnp, reset, and noloop
> > > > kernel options enabled, and with crc_enabled the touchpad fails in
> > > > the same way.
> > > >
> > > > Resyncing the touchpad after receiving the
> > > > PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
> > > > function correctly on resume. The touchpad fails to reconnect with
> > > > the serio reconnect no matter how many times it retries, so this
> > > > change skips over that retry sequence and goes directly to resync.
> > >
> > > Why can't we do this in elantech_reconnect()? I am sure we can make it
> > > simpler and more robust than what the generic handler is trying to do
> > > with polling and everything.
> > >
> > > Thanks.
> > >
> > > --
> > > Dmitry
> >
> > I am fine with anything that would be simpler and more robust, though
> > I'm not sure how to implement what you are describing.
> >
> > Are you suggesting that in this PSMOUSE_BAD_DATA case, instead of
> > using psmouse_set_state and psmouse_queue_work to call
> > psmouse->reconnect (which calls elantech_reconnect)?
>
> No. From the description it sounds like the device sends wrong/extra
> data right after resume. I think you can handle it in
> elantech_reconnect() method by draining the buffer or issuing poll
> request or something similar.
>
> Can you post the i8042 data stream that happens on suspend/resume?
> Toggling i8042.debug option will cause the driver to dump the data to
> dmesg.
>
> Thanks.
>
> --
> Dmitry


Attachments:
i8042_suspend_resume_dmesg.txt (254.91 kB)

2024-02-29 18:24:01

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

On Mon, Feb 12, 2024 at 02:57:08PM -0600, Jonathan Denose wrote:
..
> [ 50.241235] ideapad_acpi VPC2004:00: PM: calling acpi_subsys_resume+0x0/0x5d @ 4492, parent: PNP0C09:00
> [ 50.242055] snd_hda_intel 0000:00:0e.0: PM: pci_pm_resume+0x0/0xed returned 0 after 13511 usecs
> [ 50.242120] snd_hda_codec_realtek hdaudioC0D0: PM: calling hda_codec_pm_resume+0x0/0x19 [snd_hda_codec] @ 4518, parent: 0000:00:0e.0
> [ 50.247406] i8042: [49434] a8 -> i8042 (command)
> [ 50.247468] ideapad_acpi VPC2004:00: PM: acpi_subsys_resume+0x0/0x5d returned 0 after 6220 usecs
..
> [ 50.247883] i8042 kbd 00:01: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> [ 50.247894] i8042 kbd 00:01: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
> [ 50.247906] i8042 aux 00:02: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> [ 50.247916] i8042 aux 00:02: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
..
> [ 50.248301] i8042 i8042: PM: calling platform_pm_resume+0x0/0x41 @ 4492, parent: platform
> [ 50.248377] i8042: [49434] 55 <- i8042 (flush, kbd)
> [ 50.248407] i8042: [49435] aa -> i8042 (command)
> [ 50.248601] i8042: [49435] 00 <- i8042 (return)
> [ 50.248604] i8042: [49435] i8042 controller selftest: 0x0 != 0x55

So here I see the ideapad-laptop driver trying to access i8042 before it
even starts resuming. I wonder, does it help if you disable
(temporarily) the ideapad driver?

Thanks.

--
Dmitry

2024-03-04 17:18:19

by Jonathan Denose

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

I disabled the ideapad driver by rebuilding the kernel without the
ideapad_laptop module. That does fix the suspend/resume issue!

Attached are the logs. Is there a way to make this permanent?

On Thu, Feb 29, 2024 at 12:23 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Mon, Feb 12, 2024 at 02:57:08PM -0600, Jonathan Denose wrote:
> ...
> > [ 50.241235] ideapad_acpi VPC2004:00: PM: calling acpi_subsys_resume+0x0/0x5d @ 4492, parent: PNP0C09:00
> > [ 50.242055] snd_hda_intel 0000:00:0e.0: PM: pci_pm_resume+0x0/0xed returned 0 after 13511 usecs
> > [ 50.242120] snd_hda_codec_realtek hdaudioC0D0: PM: calling hda_codec_pm_resume+0x0/0x19 [snd_hda_codec] @ 4518, parent: 0000:00:0e.0
> > [ 50.247406] i8042: [49434] a8 -> i8042 (command)
> > [ 50.247468] ideapad_acpi VPC2004:00: PM: acpi_subsys_resume+0x0/0x5d returned 0 after 6220 usecs
> ...
> > [ 50.247883] i8042 kbd 00:01: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> > [ 50.247894] i8042 kbd 00:01: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
> > [ 50.247906] i8042 aux 00:02: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> > [ 50.247916] i8042 aux 00:02: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
> ...
> > [ 50.248301] i8042 i8042: PM: calling platform_pm_resume+0x0/0x41 @ 4492, parent: platform
> > [ 50.248377] i8042: [49434] 55 <- i8042 (flush, kbd)
> > [ 50.248407] i8042: [49435] aa -> i8042 (command)
> > [ 50.248601] i8042: [49435] 00 <- i8042 (return)
> > [ 50.248604] i8042: [49435] i8042 controller selftest: 0x0 != 0x55
>
> So here I see the ideapad-laptop driver trying to access i8042 before it
> even starts resuming. I wonder, does it help if you disable
> (temporarily) the ideapad driver?
>
> Thanks.
>
> --
> Dmitry


Attachments:
dmesg_no_ideapad_laptop.txt (250.83 kB)

2024-03-05 18:47:40

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

On Mon, Mar 04, 2024 at 11:17:31AM -0600, Jonathan Denose wrote:
> I disabled the ideapad driver by rebuilding the kernel without the
> ideapad_laptop module. That does fix the suspend/resume issue!
>
> Attached are the logs. Is there a way to make this permanent?

Could you please try the patch below? Thanks!

---

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 9fbb8d31575a..2f0c143c3137 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -127,6 +127,7 @@ MODULE_PARM_DESC(unmask_kbd_data, "Unconditional enable (may reveal sensitive da
#endif

static bool i8042_present;
+static bool i8042_ready;
static bool i8042_bypass_aux_irq_test;
static char i8042_kbd_firmware_id[128];
static char i8042_aux_firmware_id[128];
@@ -190,6 +191,26 @@ void i8042_unlock_chip(void)
}
EXPORT_SYMBOL(i8042_unlock_chip);

+int i8042_device_link_add(struct device *consumer)
+{
+ if (!i8042_present)
+ return -ENODEV;
+
+ if (!i8042_platform_device || !i8042_ready)
+ return -EPROBE_DEFER;
+
+ return device_link_add(consumer, &i8042_platform_device->dev,
+ DL_FLAG_STATELESS) != NULL;
+}
+EXPORT_SYMBOL(i8042_device_link_add);
+
+void i8042_device_link_remove(struct device *consumer)
+{
+ if (i8042_platform_device)
+ device_link_remove(consumer, &i8042_platform_device->dev);
+}
+EXPORT_SYMBOL(i8042_device_link_remove);
+
int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
struct serio *serio))
{
@@ -1574,6 +1595,7 @@ static int i8042_probe(struct platform_device *dev)
*/
i8042_register_ports();

+ i8042_ready = true;
return 0;

out_fail:
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index ac037540acfc..d4d1bccbe882 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1842,6 +1842,13 @@ static int ideapad_acpi_add(struct platform_device *pdev)
ideapad_register_rfkill(priv, i);

ideapad_sync_rfk_state(priv);
+
+ err = i8042_device_link_add(&pdev->dev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to link with 8042 controller :%d", err);
+ goto i8042_link_failed;
+ }
+
ideapad_sync_touchpad_state(priv, false);

err = ideapad_dytc_profile_init(priv);
@@ -1882,7 +1889,9 @@ static int ideapad_acpi_add(struct platform_device *pdev)

backlight_failed:
ideapad_dytc_profile_exit(priv);
+ i8042_device_link_remove(&pdev->dev);

+i8042_link_failed:
for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
ideapad_unregister_rfkill(priv, i);

@@ -1909,6 +1918,7 @@ static void ideapad_acpi_remove(struct platform_device *pdev)

ideapad_backlight_exit(priv);
ideapad_dytc_profile_exit(priv);
+ i8042_device_link_remove(&pdev->dev);

for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
ideapad_unregister_rfkill(priv, i);
diff --git a/include/linux/i8042.h b/include/linux/i8042.h
index 95b07f8b77fe..ce45569f5246 100644
--- a/include/linux/i8042.h
+++ b/include/linux/i8042.h
@@ -52,12 +52,15 @@
#define I8042_CTR_AUXDIS 0x20
#define I8042_CTR_XLATE 0x40

+struct device;
struct serio;

#if defined(CONFIG_SERIO_I8042) || defined(CONFIG_SERIO_I8042_MODULE)

void i8042_lock_chip(void);
void i8042_unlock_chip(void);
+int i8042_device_link_add(struct device *dev);
+void i8042_device_link_remove(struct device *dev);
int i8042_command(unsigned char *param, int command);
int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
struct serio *serio));
@@ -74,6 +77,15 @@ static inline void i8042_unlock_chip(void)
{
}

+int i8042_device_link_add(struct device *dev)
+{
+ return -ENODEV;
+}
+
+void i8042_device_link_remove(struct device *dev)
+{
+}
+
static inline int i8042_command(unsigned char *param, int command)
{
return -ENODEV;



Thanks.

--
Dmitry

2024-03-05 21:49:19

by Jonathan Denose

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

Thanks for this.

I tried the patch and unfortunately the issue still occurs. Attached
are the dmesg logs.

On Tue, Mar 5, 2024 at 12:47 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Mon, Mar 04, 2024 at 11:17:31AM -0600, Jonathan Denose wrote:
> > I disabled the ideapad driver by rebuilding the kernel without the
> > ideapad_laptop module. That does fix the suspend/resume issue!
> >
> > Attached are the logs. Is there a way to make this permanent?
>
> Could you please try the patch below? Thanks!
>
> ---
>
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 9fbb8d31575a..2f0c143c3137 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -127,6 +127,7 @@ MODULE_PARM_DESC(unmask_kbd_data, "Unconditional enable (may reveal sensitive da
> #endif
>
> static bool i8042_present;
> +static bool i8042_ready;
> static bool i8042_bypass_aux_irq_test;
> static char i8042_kbd_firmware_id[128];
> static char i8042_aux_firmware_id[128];
> @@ -190,6 +191,26 @@ void i8042_unlock_chip(void)
> }
> EXPORT_SYMBOL(i8042_unlock_chip);
>
> +int i8042_device_link_add(struct device *consumer)
> +{
> + if (!i8042_present)
> + return -ENODEV;
> +
> + if (!i8042_platform_device || !i8042_ready)
> + return -EPROBE_DEFER;
> +
> + return device_link_add(consumer, &i8042_platform_device->dev,
> + DL_FLAG_STATELESS) != NULL;
> +}
> +EXPORT_SYMBOL(i8042_device_link_add);
> +
> +void i8042_device_link_remove(struct device *consumer)
> +{
> + if (i8042_platform_device)
> + device_link_remove(consumer, &i8042_platform_device->dev);
> +}
> +EXPORT_SYMBOL(i8042_device_link_remove);
> +
> int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
> struct serio *serio))
> {
> @@ -1574,6 +1595,7 @@ static int i8042_probe(struct platform_device *dev)
> */
> i8042_register_ports();
>
> + i8042_ready = true;
> return 0;
>
> out_fail:
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index ac037540acfc..d4d1bccbe882 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -1842,6 +1842,13 @@ static int ideapad_acpi_add(struct platform_device *pdev)
> ideapad_register_rfkill(priv, i);
>
> ideapad_sync_rfk_state(priv);
> +
> + err = i8042_device_link_add(&pdev->dev);
> + if (err) {
> + dev_err(&pdev->dev, "failed to link with 8042 controller :%d", err);
> + goto i8042_link_failed;
> + }
> +
> ideapad_sync_touchpad_state(priv, false);
>
> err = ideapad_dytc_profile_init(priv);
> @@ -1882,7 +1889,9 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>
> backlight_failed:
> ideapad_dytc_profile_exit(priv);
> + i8042_device_link_remove(&pdev->dev);
>
> +i8042_link_failed:
> for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
> ideapad_unregister_rfkill(priv, i);
>
> @@ -1909,6 +1918,7 @@ static void ideapad_acpi_remove(struct platform_device *pdev)
>
> ideapad_backlight_exit(priv);
> ideapad_dytc_profile_exit(priv);
> + i8042_device_link_remove(&pdev->dev);
>
> for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
> ideapad_unregister_rfkill(priv, i);
> diff --git a/include/linux/i8042.h b/include/linux/i8042.h
> index 95b07f8b77fe..ce45569f5246 100644
> --- a/include/linux/i8042.h
> +++ b/include/linux/i8042.h
> @@ -52,12 +52,15 @@
> #define I8042_CTR_AUXDIS 0x20
> #define I8042_CTR_XLATE 0x40
>
> +struct device;
> struct serio;
>
> #if defined(CONFIG_SERIO_I8042) || defined(CONFIG_SERIO_I8042_MODULE)
>
> void i8042_lock_chip(void);
> void i8042_unlock_chip(void);
> +int i8042_device_link_add(struct device *dev);
> +void i8042_device_link_remove(struct device *dev);
> int i8042_command(unsigned char *param, int command);
> int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
> struct serio *serio));
> @@ -74,6 +77,15 @@ static inline void i8042_unlock_chip(void)
> {
> }
>
> +int i8042_device_link_add(struct device *dev)
> +{
> + return -ENODEV;
> +}
> +
> +void i8042_device_link_remove(struct device *dev)
> +{
> +}
> +
> static inline int i8042_command(unsigned char *param, int command)
> {
> return -ENODEV;
>
>
>
> Thanks.
>
> --
> Dmitry


Attachments:
ideapad_laptoppatch_dmesg.txt (252.25 kB)

2024-03-05 23:18:27

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

On Tue, Mar 05, 2024 at 03:48:29PM -0600, Jonathan Denose wrote:
> Thanks for this.
>
> I tried the patch and unfortunately the issue still occurs. Attached
> are the dmesg logs.

So this is without going through suspend/resume, but straight up boot?
Could you please post the whole dmesg, not only data from 8042?

Thanks.

--
Dmitry

2024-03-06 16:44:28

by Jonathan Denose

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

Sorry about that, I didn't realize logs were getting cut off.

I think what's attached should have everything. This is through boot
and with one suspend/resume cycle wherein the touchpad fails after
resume.

On Tue, Mar 5, 2024 at 5:17 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Tue, Mar 05, 2024 at 03:48:29PM -0600, Jonathan Denose wrote:
> > Thanks for this.
> >
> > I tried the patch and unfortunately the issue still occurs. Attached
> > are the dmesg logs.
>
> So this is without going through suspend/resume, but straight up boot?
> Could you please post the whole dmesg, not only data from 8042?
>
> Thanks.
>
> --
> Dmitry


Attachments:
ideapad_patch_complete.txt (306.63 kB)

2024-03-07 18:29:17

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: psmouse - add resync_on_resume dmi check

On Mon, Mar 04, 2024 at 11:17:31AM -0600, Jonathan Denose wrote:
> I disabled the ideapad driver by rebuilding the kernel without the
> ideapad_laptop module. That does fix the suspend/resume issue!
>
> Attached are the logs. Is there a way to make this permanent?
>
> On Thu, Feb 29, 2024 at 12:23 PM Dmitry Torokhov
> <[email protected]> wrote:
> >
> > On Mon, Feb 12, 2024 at 02:57:08PM -0600, Jonathan Denose wrote:
> > ...
> > > [ 50.241235] ideapad_acpi VPC2004:00: PM: calling acpi_subsys_resume+0x0/0x5d @ 4492, parent: PNP0C09:00
> > > [ 50.242055] snd_hda_intel 0000:00:0e.0: PM: pci_pm_resume+0x0/0xed returned 0 after 13511 usecs
> > > [ 50.242120] snd_hda_codec_realtek hdaudioC0D0: PM: calling hda_codec_pm_resume+0x0/0x19 [snd_hda_codec] @ 4518, parent: 0000:00:0e.0
> > > [ 50.247406] i8042: [49434] a8 -> i8042 (command)
> > > [ 50.247468] ideapad_acpi VPC2004:00: PM: acpi_subsys_resume+0x0/0x5d returned 0 after 6220 usecs
> > ...
> > > [ 50.247883] i8042 kbd 00:01: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> > > [ 50.247894] i8042 kbd 00:01: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
> > > [ 50.247906] i8042 aux 00:02: PM: calling pnp_bus_resume+0x0/0x9d @ 4492, parent: pnp0
> > > [ 50.247916] i8042 aux 00:02: PM: pnp_bus_resume+0x0/0x9d returned 0 after 0 usecs
> > ...
> > > [ 50.248301] i8042 i8042: PM: calling platform_pm_resume+0x0/0x41 @ 4492, parent: platform
> > > [ 50.248377] i8042: [49434] 55 <- i8042 (flush, kbd)
> > > [ 50.248407] i8042: [49435] aa -> i8042 (command)
> > > [ 50.248601] i8042: [49435] 00 <- i8042 (return)
> > > [ 50.248604] i8042: [49435] i8042 controller selftest: 0x0 != 0x55
> >
> > So here I see the ideapad-laptop driver trying to access i8042 before it
> > even starts resuming. I wonder, does it help if you disable
> > (temporarily) the ideapad driver?

OK, so I tried to cook up a patch that would allow ideapad-laptop driver
to establish device link with i8042 so that the resume will be processed
after i8042 resumes, but the longer I think about it, the more I think
that ideapad driver should not be messing with the touchpad state
directly. The disable event may come up in a middle of the touchpad
resume transition, or when we decide to change touchpad mode for one
reason or another. It also does not respect inhibit/uninhibit controls
for input devices. I think that the proper way for ideapad driver to
handle this is to only send KEY_TOUCHPAD_OFF/KEY_TOUCHPAD_ON to
userspace and let userspace deal with toggling touchpad input (via
inhibit or by other means).

CC-ing ideapad maintainers for their thoughts.

Thanks.

--
Dmitry