Re: [PATCH] mouse_cypress_ps2: Fix 6.11 regression on xps15z

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Aug 27, 2024 at 10:46:11PM -0400, Woody Suwalski wrote:
> Dmitry Torokhov wrote:
> > Hi Woody,
> > 
> > On Tue, Aug 27, 2024 at 07:44:12PM -0400, Woody Suwalski wrote:
> > > Kernel 6.11 rcN on Dell XPS 15Z:  touch pad has stopped working after the
> > > patch
> > > 
> > > commit 8bccf667f62a2351fd0b2a2fe5ba90806702c048
> > > Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Date:   Fri Jun 28 15:47:25 2024 -0700
> > > 
> > >      Input: cypress_ps2 - report timeouts when reading command status
> > > 
> > > It seems that the first communication is with an invalid packet of 3 NULLs,
> > > and that
> > > status failure used to be ignored. With the above patch it is now returning
> > > an error and
> > > that results in a dead touch pad.
> > > 
> > > The fix is to stop flagging an error for 3-byte null packets, just keep
> > > ignoring them as before.
> > > [    2.338016] [    T591] err: Command 0x00 response data (0x): 00 00 00
> > > [    2.338032] [    T591] ok: Command 0x00 response data (0x): 33 cc a2
> > > ...
> > > [    2.770029] [    T591] ok: Command 0x00 response data (0x): 33 cc a2
> > > [    2.998030] [    T591] ok: Command 0x11 response data (0x): 01 00 64
> > Could you please send me logs with i8042.debug=1 kernel command line
> > option please?  I wonder if we need to wake up the controller...
> > 
> > Thanks.
> > 
> Sure, the dmesg log is attached (for the failing scenario)

Thank you.

> [    0.000000] [      T0] Linux version 6.9.0-pingu+ (root@DellXPS15Z) (gcc (Debian 14.2.0-1) 14.2.0, GNU ld (GNU Binutils for Debian) 2.43.1) #24 SMP PREEMPT_DYNAMIC Tue Aug 27 22:33:33 EDT 2024

This is not 6.11, did you patch 6.9 with cypress patches from 6.11?

Anyway, the patch you posted does not make sense. You are doing the
following check:

+        if (!(pktsize == 3 && param[0] == 0 && param[1] == 0 )) {
+            rc = -ETIMEDOUT;
+            goto out;
+        }

trying to ignore "all zeroes" response from the device, however at this
point param array does not contain data from the device, it always
contains all zeroes because of memset() a few lines above. So in effect
you always skipping reporting timeout.

However I think cypress is busted in general as it looks like it times
out all the commands, because it tries to issue them outside of libp2s,
and so noone is actually wakes it up when we get enough response from
the device. To prove this could you please try applying this:

diff --git a/drivers/input/mouse/cypress_ps2.c b/drivers/input/mouse/cypress_ps2.c
index b3c34ebcc4ef..8c0c7100aa4d 100644
--- a/drivers/input/mouse/cypress_ps2.c
+++ b/drivers/input/mouse/cypress_ps2.c
@@ -115,8 +115,9 @@ static int cypress_ps2_read_cmd_status(struct psmouse *psmouse,
 	if (!wait_event_timeout(ps2dev->wait,
 				psmouse->pktcnt >= pktsize,
 				msecs_to_jiffies(CYTP_CMD_TIMEOUT))) {
-		rc = -ETIMEDOUT;
-		goto out;
+//		rc = -ETIMEDOUT;
+//		goto out;
+		pr_err("XXX looks like timeout\n");
 	}
 
 	memcpy(param, psmouse->packet, pktsize);

and let me know if you see "XXX looks like timeout" multiple times
during initialization (essentially for each extended command issued by
the cypress driver)?

Thanks!

-- 
Dmitry