Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 30-07-15 16:32, Pali Rohár wrote:
On Thursday 30 July 2015 16:28:39 Pali Rohár wrote:
On Thursday 30 July 2015 16:18:47 Hans de Goede wrote:
Hi,

On 30-07-15 16:11, Pali Rohár wrote:
On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
Hi Chandler,

On 29-07-15 22:45, cpaul@xxxxxxxxxx wrote:
From: Stephen Chandler Paul <cpaul@xxxxxxxxxx>

The data concerning which buttons on the touchpad are held down or not
are in the fourth packet we receive from the mouse, not the first.

Signed-off-by: Stephen Chandler Paul <cpaul@xxxxxxxxxx>
---
  drivers/input/mouse/alps.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 113d6f1..e2f9b25 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
  	/* Non interleaved V2 dualpoint has separate stick button bits */
  	if (priv->proto_version == ALPS_PROTO_V2 &&
  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
-		left |= packet[0] & 1;
-		right |= packet[0] & 2;
-		middle |= packet[0] & 4;
+		left |= packet[3] & 1;
+		right |= packet[3] & 2;
+		middle |= packet[3] & 4;
  	}

  	alps_report_buttons(dev, dev2, left, right, middle);

Thanks for taking a look at the recordings, but the above patch is wrong,
if you look slightly higher in the lps_process_packet_v1_v2() function there
is this:

if (priv->proto_version == ALPS_PROTO_V1) {
...
} else {
	left = packet[3] & 1;
	right = packet[3] & 2;
	middle = packet[3] & 4;
}

So with your patch for the devices in question the entire code flow
becomes:

	left = packet[3] & 1;
	right = packet[3] & 2;
	middle = packet[3] & 4;
	left |= packet[3] & 1;
	right |= packet[3] & 2;
	middle |= packet[3] & 4;

Which is not really helpful for the devices for which I added
commit 92bac83dd:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb

and will cause these devices to regress.

Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
the same problem and tested my patch on a Dell Latitude D630,
it seems the use of the low bits of packet[0] to report the
trackpoint buttons separately when the touchpad is active is
a Dell specific thing, so I believe that a patch to only
activate this code block on Dell's is the right solution for
the regression Douglas is seeing.

I'll write such a patch and post it shortly.

Regards,

Hans




Hans, can you check ec and e7 registers if are same or if they differs?

As Benjamin already pointed out Douglas' touchpad matches this
line in alps.c :

{ { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */

This is a full match

No, this is not full match. It just check e7, not ec. This is reason why
I asked for both ec and e7 registers of all affected machines.


Douglas already sent e7 and ec regs:
[    6.617471] psmouse serio1: alps: E7 report: 22 02 14
[    6.656974] psmouse serio1: alps: EC report: 10 00 64

Can you post ec regs from your tested Dell machine?

[    1.906031] psmouse serio1: alps: EC report: 10 00 64

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux