Re: [PATCH 0/6] Fixes for ALPS trackstick

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 6844 bytes --]

On Monday 02 February 2015 11:49:58 Pali Rohár wrote:
> On Monday 02 February 2015 06:49:31 Dmitry Torokhov wrote:
> > On Sun, Jan 18, 2015 at 10:47:06AM +0100, Pali Rohár wrote:
> > > On Sunday 18 January 2015 08:22:45 Dmitry Torokhov wrote:
> > > > On Sat, Jan 17, 2015 at 11:01:56AM +0100, Pali Rohár wrote:
> > > > > On Thursday 15 January 2015 22:02:16 Dmitry Torokhov
> 
> wrote:
> > > > > > On Thu, Jan 15, 2015 at 09:28:41PM +0100, Pali Rohár
> 
> wrote:
> > > > > > > On Thursday 15 January 2015 20:38:18 Dmitry
> > > > > > > Torokhov
> > > 
> > > wrote:
> > > > > > > > On Thu, Jan 15, 2015 at 08:19:59PM +0100, Pali
> > > > > > > > Rohár
> > > 
> > > wrote:
> > > > > > > > > On Thursday 15 January 2015 19:18:20 Dmitry
> > > > > > > > > Torokhov
> > > > > 
> > > > > wrote:
> > > > > > > > > > On Thu, Jan 15, 2015 at 11:49:32AM +0100,
> > > > > > > > > > Pali Rohár
> > > > > 
> > > > > wrote:
> > > > > > > > > > > On Wednesday 14 January 2015 23:55:48
> > > > > > > > > > > Dmitry Torokhov
> > > > > > > 
> > > > > > > wrote:
> > > > > > > > > > > > Hi Pali,
> > > > > > > > > > > > 
> > > > > > > > > > > > This series try to address the issue you
> > > > > > > > > > > > brought regarding trackstick
> > > > > > > > > > > > initialization on Dell Latitudes in a
> > > > > > > > > > > > different way than the patches you
> > > > > > > > > > > > proposed. Basically in this series we
> > > > > > > > > > > > move resetting and all detection in
> > > > > > > > > > > > alps_detect() and make sure we keep the
> > > > > > > > > > > > state so alps_init() can reuse it and
> > > > > > > > > > > > not perform the detection all over
> > > > > > > > > > > > again. Doing this allows us to set up
> > > > > > > > > > > > device characteristics (name, version,
> > > > > > > > > > > > etc) properly from the get go while
> > > > > > > > > > > > still performing reset only once.
> > > > > > > > > > > > 
> > > > > > > > > > > > This is untested as I do not have any
> > > > > > > > > > > > ALPS devices anymore so I'd appreciate
> > > > > > > > > > > > you giving it a spin.
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks!
> > > > > > > > > > > 
> > > > > > > > > > > Hi Dmitry,
> > > > > > > > > > > 
> > > > > > > > > > > on top of which branch/repository should I
> > > > > > > > > > > apply your patches?
> > > > > > > > > > 
> > > > > > > > > > Should be applicable to my 'next' branch
> > > > > > > > > > (which I just upreved to 3.19-rc4).
> > > > > > > > > > 
> > > > > > > > > > Thanks.
> > > > > > > > > 
> > > > > > > > > Not working at top of next (0c3e994).
> > > > > > > > > 
> > > > > > > > > Applying: Input: ALPS - renumber protocol
> > > > > > > > > numbers Applying: Input: ALPS - make Rushmore
> > > > > > > > > a separate protocol error: patch failed:
> > > > > > > > > drivers/input/mouse/alps.c:1275 error:
> > > > > > > > > drivers/input/mouse/alps.c: patch does not
> > > > > > > > > apply Patch failed at 0002 Input: ALPS - make
> > > > > > > > > Rushmore a separate protocol
> > > > > > > > 
> > > > > > > > Hmm.. I created a new alps branch (based on
> > > > > > > > 3.19-rc4), can you try it?
> > > > > > > > 
> > > > > > > > Thanks.
> > > > > > > 
> > > > > > > Compiled from your new alps branch (with "if
> > > > > > > (!priv)" fix) and modprobing psmouse.ko caused
> > > > > > > laptop freeze :-( Even sysrq not responded. So
> > > > > > > something is not working...
> > > > > > 
> > > > > > Hmm, is it on text console or in X? Any chance you
> > > > > > could go through pathes - there are only 8 of them
> > > > > > including 2 of yours that should be unmodified.
> > > > > > 
> > > > > > Thanks.
> > > > > 
> > > > > Hi, now I tested patch by patch and kernel crash is
> > > > > caused only by last patch 6/6 and only after I touch
> > > > > touchpad or trackstick.
> > > > > 
> > > > > In text console it prints lot of panic messages and
> > > > > because it prints lot of messages I cannot read (or
> > > > > record) more then last.
> > > > > 
> > > > > In last call trace I see that
> > > > > alps_register_bare_ps2_mouse() was called and it
> > > > > generated page_fault.
> > > > 
> > > > That happens because while you added priv->psmouse
> > > > pointer it looks like you forgot to initialize it and I
> > > > missed that too...
> > > 
> > > Right your patch 6/6 does not initialize priv->psmouse. I
> > > looked into my original patch and it initialize it, so
> > > there was some copy-paste error.
> > > 
> > > Look at other emails... can you fix problems and send new
> > > version of your patches for testing?
> > 
> > I think I did. Can you please take another look at my alps
> > branch?
> > 
> > Thanks!
> 
> Now I tried it. It does not crash anymore which is good. But
> it does not working. Driver alsp.c receive only bare PS/2
> packets, not 6 bites ALPS packets. So ALPS touchpad is not
> properly initialized and so is in "legacy" mode when it act
> as genetic mouse. And events are sent via dev3 device (that
> PS/2 mouse).
> 
> So your patches do not initialize my ALPS touchpad correctly.

Hello Dmitry, I found where is problem.

This chunk is needed:

@@ -135,7 +135,7 @@ static const struct alps_protocol_info alps_v3_protocol_data = {
 };
 
 static const struct alps_protocol_info alps_v3_rushmore_data = {
-	ALPS_PROTO_V3, 0x8f, 0x8f, ALPS_DUALPOINT
+	ALPS_PROTO_V3_RUSHMORE, 0x8f, 0x8f, ALPS_DUALPOINT
 };
 
 static const struct alps_protocol_info alps_v5_protocol_data = {

because rushmore is now separate protocol.

And maybe also these two chunks are needed too:

@@ -663,7 +696,8 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
 		 */
 		if (f->is_mp) {
 			fingers = f->fingers;
-			if (priv->proto_version == ALPS_PROTO_V3) {
+			if (priv->proto_version == ALPS_PROTO_V3 ||
+			    priv->proto_version == ALPS_PROTO_V3_RUSHMORE) {
 				if (alps_process_bitmap(priv, f) == 0)
 					fingers = 0; /* Use st data */
 
@@ -1365,8 +1399,9 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 			    psmouse->pktcnt - 1,
 			    psmouse->packet[psmouse->pktcnt - 1]);
 
-		if (priv->proto_version == ALPS_PROTO_V3_RUSHMORE &&
-		    psmouse->pktcnt == psmouse->pktsize) {
+		if ((priv->proto_version == ALPS_PROTO_V3 ||
+		     priv->proto_version == ALPS_PROTO_V3_RUSHMORE
+		    ) && psmouse->pktcnt == psmouse->pktsize) {
 			/*
 			 * Some Dell boxes, such as Latitude E6440 or E7440
 			 * with closed lid, quite often smash last byte of


With all these tree parts, initialization of my ALPS touchpad is OK and working as expected. I 
compared PS/2 init commands and are same as on 3.13 kernel.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]