All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Fixes for ALPS trackstick
@ 2015-01-14 22:55 Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 1/6] Input: ALPS - renumber protocol numbers Dmitry Torokhov
                   ` (6 more replies)
  0 siblings, 7 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

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!

-- 
Dmitry

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/6] Input: ALPS - renumber protocol numbers
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 2/6] Input: ALPS - make Rushmore a separate protocol Dmitry Torokhov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

In order to accommodate new protocol number for Rushmore touchpads
let's shift protocol numbers by 8 bits (i.e. 1 -> 0x100) - this way
we keep protocol version reported in input device id the same as it
was, but add some holes in numbering.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c |  2 +-
 drivers/input/mouse/alps.h | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index f719f28..d995523 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2526,7 +2526,7 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
 		psmouse->vendor = "ALPS";
 		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
 				"DualPoint TouchPad" : "GlidePoint";
-		psmouse->model = dummy.proto_version << 8;
+		psmouse->model = dummy.proto_version;
 	}
 	return 0;
 }
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 66240b4..94645bf 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -14,13 +14,13 @@
 
 #include <linux/input/mt.h>
 
-#define ALPS_PROTO_V1	1
-#define ALPS_PROTO_V2	2
-#define ALPS_PROTO_V3	3
-#define ALPS_PROTO_V4	4
-#define ALPS_PROTO_V5	5
-#define ALPS_PROTO_V6	6
-#define ALPS_PROTO_V7	7	/* t3btl t4s */
+#define ALPS_PROTO_V1		0x100
+#define ALPS_PROTO_V2		0x200
+#define ALPS_PROTO_V3		0x300
+#define ALPS_PROTO_V4		0x400
+#define ALPS_PROTO_V5		0x500
+#define ALPS_PROTO_V6		0x600
+#define ALPS_PROTO_V7		0x700	/* t3btl t4s */
 
 #define MAX_TOUCHES	2
 
@@ -64,11 +64,11 @@ enum V7_PACKET_ID {
  * lists a number of such touchpads.
  */
 struct alps_model_info {
-	unsigned char signature[3];
-	unsigned char command_mode_resp;
-	unsigned char proto_version;
-	unsigned char byte0, mask0;
-	int flags;
+	u8 signature[3];
+	u8 command_mode_resp;
+	u16 proto_version;
+	u8 byte0, mask0;
+	unsigned int flags;
 };
 
 /**
@@ -166,9 +166,9 @@ struct alps_data {
 	/* these are autodetected when the device is identified */
 	const struct alps_nibble_commands *nibble_commands;
 	int addr_command;
-	unsigned char proto_version;
-	unsigned char byte0, mask0;
-	unsigned char fw_ver[3];
+	u16 proto_version;
+	u8 byte0, mask0;
+	u8 fw_ver[3];
 	int flags;
 	int x_max;
 	int y_max;
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/6] Input: ALPS - make Rushmore a separate protocol
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 1/6] Input: ALPS - renumber protocol numbers Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 3/6] Input: ALPS - split protocol data from model info Dmitry Torokhov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

Even though Rushmore is very close to V3 protocol it is sufficiently
different to warrant it's own protocol name.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c | 26 ++++++++++++++++----------
 drivers/input/mouse/alps.h |  1 +
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index d995523..80aa6f8 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -99,7 +99,6 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
 #define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
 					   6-byte ALPS packet */
-#define ALPS_IS_RUSHMORE	0x100	/* device is a rushmore */
 #define ALPS_BUTTONPAD		0x200	/* device is a clickpad */
 
 static const struct alps_model_info alps_model_data[] = {
@@ -412,7 +411,7 @@ static int alps_process_bitmap(struct alps_data *priv,
 		(2 * (priv->y_bits - 1));
 
 	/* y-bitmap order is reversed, except on rushmore */
-	if (!(priv->flags & ALPS_IS_RUSHMORE)) {
+	if (priv->proto_version != ALPS_PROTO_V3_RUSHMORE) {
 		fields->mt[0].y = priv->y_max - fields->mt[0].y;
 		fields->mt[1].y = priv->y_max - fields->mt[1].y;
 	}
@@ -1275,7 +1274,7 @@ 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 &&
+		if (priv->proto_version == ALPS_PROTO_V3_RUSHMORE &&
 		    psmouse->pktcnt == psmouse->pktsize) {
 			/*
 			 * Some Dell boxes, such as Latitude E6440 or E7440
@@ -2182,6 +2181,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->x_max = 1023;
 		priv->y_max = 767;
 		break;
+
 	case ALPS_PROTO_V3:
 		priv->hw_init = alps_hw_init_v3;
 		priv->process_packet = alps_process_packet_v3;
@@ -2190,6 +2190,18 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->nibble_commands = alps_v3_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
 		break;
+
+	case ALPS_PROTO_V3_RUSHMORE:
+		priv->hw_init = alps_hw_init_rushmore_v3;
+		priv->process_packet = alps_process_packet_v3;
+		priv->set_abs_params = alps_set_abs_params_mt;
+		priv->decode_fields = alps_decode_rushmore;
+		priv->nibble_commands = alps_v3_nibble_commands;
+		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
+		priv->x_bits = 16;
+		priv->y_bits = 12;
+		break;
+
 	case ALPS_PROTO_V4:
 		priv->hw_init = alps_hw_init_v4;
 		priv->process_packet = alps_process_packet_v4;
@@ -2313,15 +2325,9 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 
 		return 0;
 	} else if (ec[0] == 0x88 && ec[1] == 0x08) {
-		priv->proto_version = ALPS_PROTO_V3;
+		priv->proto_version = ALPS_PROTO_V3_RUSHMORE;
 		alps_set_defaults(priv);
 
-		priv->hw_init = alps_hw_init_rushmore_v3;
-		priv->decode_fields = alps_decode_rushmore;
-		priv->x_bits = 16;
-		priv->y_bits = 12;
-		priv->flags |= ALPS_IS_RUSHMORE;
-
 		/* hack to make addr_command, nibble_command available */
 		psmouse->private = priv;
 
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 94645bf..f19908a 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -17,6 +17,7 @@
 #define ALPS_PROTO_V1		0x100
 #define ALPS_PROTO_V2		0x200
 #define ALPS_PROTO_V3		0x300
+#define ALPS_PROTO_V3_RUSHMORE	0x310
 #define ALPS_PROTO_V4		0x400
 #define ALPS_PROTO_V5		0x500
 #define ALPS_PROTO_V6		0x600
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/6] Input: ALPS - split protocol data from model info
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 1/6] Input: ALPS - renumber protocol numbers Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 2/6] Input: ALPS - make Rushmore a separate protocol Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 4/6] Input: ALPS - consolidate setting protocol parameters Dmitry Torokhov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

In preparation of reworking the way we set protocol parameters let's
split certain protocol items from alps_model_info into a separate
structure.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c | 58 +++++++++++++++++++++++-----------------------
 drivers/input/mouse/alps.h | 26 ++++++++++++++-------
 2 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 80aa6f8..1de0345 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -102,32 +102,32 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
 #define ALPS_BUTTONPAD		0x200	/* device is a clickpad */
 
 static const struct alps_model_info alps_model_data[] = {
-	{ { 0x32, 0x02, 0x14 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },	/* Toshiba Salellite Pro M10 */
-	{ { 0x33, 0x02, 0x0a },	0x00, ALPS_PROTO_V1, 0x88, 0xf8, 0 },				/* UMAX-530T */
-	{ { 0x53, 0x02, 0x0a },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x53, 0x02, 0x14 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x60, 0x03, 0xc8 }, 0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },				/* HP ze1115 */
-	{ { 0x63, 0x02, 0x0a },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x63, 0x02, 0x14 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x63, 0x02, 0x28 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 },		/* Fujitsu Siemens S6010 */
-	{ { 0x63, 0x02, 0x3c },	0x00, ALPS_PROTO_V2, 0x8f, 0x8f, ALPS_WHEEL },			/* Toshiba Satellite S2400-103 */
-	{ { 0x63, 0x02, 0x50 },	0x00, ALPS_PROTO_V2, 0xef, 0xef, ALPS_FW_BK_1 },		/* NEC Versa L320 */
-	{ { 0x63, 0x02, 0x64 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x63, 0x03, 0xc8 }, 0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },	/* Dell Latitude D800 */
-	{ { 0x73, 0x00, 0x0a },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_DUALPOINT },		/* ThinkPad R61 8918-5QG */
-	{ { 0x73, 0x02, 0x0a },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, 0 },
-	{ { 0x73, 0x02, 0x14 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 },		/* Ahtec Laptop */
-	{ { 0x20, 0x02, 0x0e },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },	/* XXX */
-	{ { 0x22, 0x02, 0x0a },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
-	{ { 0x22, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT },	/* Dell Latitude D600 */
+	{ { 0x32, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },	/* Toshiba Salellite Pro M10 */
+	{ { 0x33, 0x02, 0x0a }, 0x00, { ALPS_PROTO_V1, 0x88, 0xf8, 0 } },				/* UMAX-530T */
+	{ { 0x53, 0x02, 0x0a }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x53, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x60, 0x03, 0xc8 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },				/* HP ze1115 */
+	{ { 0x63, 0x02, 0x0a }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x63, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x63, 0x02, 0x28 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },		/* Fujitsu Siemens S6010 */
+	{ { 0x63, 0x02, 0x3c }, 0x00, { ALPS_PROTO_V2, 0x8f, 0x8f, ALPS_WHEEL } },		/* Toshiba Satellite S2400-103 */
+	{ { 0x63, 0x02, 0x50 }, 0x00, { ALPS_PROTO_V2, 0xef, 0xef, ALPS_FW_BK_1 } },		/* NEC Versa L320 */
+	{ { 0x63, 0x02, 0x64 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x63, 0x03, 0xc8 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },	/* Dell Latitude D800 */
+	{ { 0x73, 0x00, 0x0a }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_DUALPOINT } },		/* ThinkPad R61 8918-5QG */
+	{ { 0x73, 0x02, 0x0a }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
+	{ { 0x73, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },		/* Ahtec Laptop */
+	{ { 0x20, 0x02, 0x0e }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },	/* XXX */
+	{ { 0x22, 0x02, 0x0a }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },
+	{ { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },	/* Dell Latitude D600 */
 	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
-	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
-		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
-	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V6, 0xff, 0xff, ALPS_DUALPOINT },		/* Dell XT2 */
-	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
-	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
-		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
-	{ { 0x73, 0x02, 0x64 },	0x8a, ALPS_PROTO_V4, 0x8f, 0x8f, 0 },
+	{ { 0x62, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED } },
+	{ { 0x73, 0x00, 0x14 }, 0x00, { ALPS_PROTO_V6, 0xff, 0xff, ALPS_DUALPOINT } },		/* Dell XT2 */
+	{ { 0x73, 0x02, 0x50 }, 0x00, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } },	/* Dell Vostro 1400 */
+	{ { 0x52, 0x01, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED } },				/* Toshiba Tecra A11-11L */
+	{ { 0x73, 0x02, 0x64 }, 0x8a, { ALPS_PROTO_V4, 0x8f, 0x8f, 0 } },
 };
 
 static void alps_set_abs_params_st(struct alps_data *priv,
@@ -2263,12 +2263,12 @@ static int alps_match_table(struct psmouse *psmouse, struct alps_data *priv,
 		    (!model->command_mode_resp ||
 		     model->command_mode_resp == ec[2])) {
 
-			priv->proto_version = model->proto_version;
+			priv->proto_version = model->protocol_info.version;
 			alps_set_defaults(priv);
 
-			priv->flags = model->flags;
-			priv->byte0 = model->byte0;
-			priv->mask0 = model->mask0;
+			priv->flags = model->protocol_info.flags;
+			priv->byte0 = model->protocol_info.byte0;
+			priv->mask0 = model->protocol_info.mask0;
 
 			return 0;
 		}
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index f19908a..c795235 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -47,18 +47,28 @@ enum V7_PACKET_ID {
 };
 
 /**
+ * struct alps_protocol_info - information about protocol used by a device
+ * @version: Indicates V1/V2/V3/...
+ * @byte0: Helps figure out whether a position report packet matches the
+ *   known format for this model.  The first byte of the report, ANDed with
+ *   mask0, should match byte0.
+ * @mask0: The mask used to check the first byte of the report.
+ * @flags: Additional device capabilities (passthrough port, trackstick, etc.).
+ */
+struct alps_protocol_info {
+	u16 version;
+	u8 byte0, mask0;
+	unsigned int flags;
+};
+
+/**
  * struct alps_model_info - touchpad ID table
  * @signature: E7 response string to match.
  * @command_mode_resp: For V3/V4 touchpads, the final byte of the EC response
  *   (aka command mode response) identifies the firmware minor version.  This
  *   can be used to distinguish different hardware models which are not
  *   uniquely identifiable through their E7 responses.
- * @proto_version: Indicates V1/V2/V3/...
- * @byte0: Helps figure out whether a position report packet matches the
- *   known format for this model.  The first byte of the report, ANDed with
- *   mask0, should match byte0.
- * @mask0: The mask used to check the first byte of the report.
- * @flags: Additional device capabilities (passthrough port, trackstick, etc.).
+ * @protocol_info: information about protcol used by the device.
  *
  * Many (but not all) ALPS touchpads can be identified by looking at the
  * values returned in the "E7 report" and/or the "EC report."  This table
@@ -67,9 +77,7 @@ enum V7_PACKET_ID {
 struct alps_model_info {
 	u8 signature[3];
 	u8 command_mode_resp;
-	u16 proto_version;
-	u8 byte0, mask0;
-	unsigned int flags;
+	struct alps_protocol_info protocol_info;
 };
 
 /**
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 4/6] Input: ALPS - consolidate setting protocol parameters
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2015-01-14 22:55 ` [PATCH 3/6] Input: ALPS - split protocol data from model info Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-14 22:55 ` [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes Dmitry Torokhov
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

Move setting of all protocol properties into alps_set_protocol (former
alps_set_defaults) instead of having it split between several functions.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c | 131 +++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 64 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 1de0345..cabb267 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -130,6 +130,22 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x73, 0x02, 0x64 }, 0x8a, { ALPS_PROTO_V4, 0x8f, 0x8f, 0 } },
 };
 
+static const struct alps_protocol_info alps_v3_protocol_data = {
+	ALPS_PROTO_V3, 0x8f, 0x8f, ALPS_DUALPOINT
+};
+
+static const struct alps_protocol_info alps_v3_rushmore_data = {
+	ALPS_PROTO_V3, 0x8f, 0x8f, ALPS_DUALPOINT
+};
+
+static const struct alps_protocol_info alps_v5_protocol_data = {
+	ALPS_PROTO_V5, 0xc8, 0xd8, 0
+};
+
+static const struct alps_protocol_info alps_v7_protocol_data = {
+	ALPS_PROTO_V7, 0x48, 0x48, ALPS_DUALPOINT
+};
+
 static void alps_set_abs_params_st(struct alps_data *priv,
 				   struct input_dev *dev1);
 static void alps_set_abs_params_mt(struct alps_data *priv,
@@ -2161,11 +2177,18 @@ error:
 	return ret;
 }
 
-static void alps_set_defaults(struct alps_data *priv)
+static int alps_set_protocol(struct psmouse *psmouse,
+			     struct alps_data *priv,
+			     const struct alps_protocol_info *protocol)
 {
-	priv->byte0 = 0x8f;
-	priv->mask0 = 0x8f;
-	priv->flags = ALPS_DUALPOINT;
+	psmouse->private = priv;
+
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
+	priv->proto_version = protocol->version;
+	priv->byte0 = protocol->byte0;
+	priv->mask0 = protocol->mask0;
+	priv->flags = protocol->flags;
 
 	priv->x_max = 2000;
 	priv->y_max = 1400;
@@ -2200,6 +2223,11 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
 		priv->x_bits = 16;
 		priv->y_bits = 12;
+
+		if (alps_probe_trackstick_v3(psmouse,
+					     ALPS_REG_BASE_RUSHMORE) < 0)
+			priv->flags &= ~ALPS_DUALPOINT;
+
 		break;
 
 	case ALPS_PROTO_V4:
@@ -2209,6 +2237,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->nibble_commands = alps_v4_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_DISABLE;
 		break;
+
 	case ALPS_PROTO_V5:
 		priv->hw_init = alps_hw_init_dolphin_v1;
 		priv->process_packet = alps_process_touchpad_packet_v3_v5;
@@ -2216,14 +2245,12 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->set_abs_params = alps_set_abs_params_mt;
 		priv->nibble_commands = alps_v3_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
-		priv->byte0 = 0xc8;
-		priv->mask0 = 0xd8;
-		priv->flags = 0;
 		priv->x_max = 1360;
 		priv->y_max = 660;
 		priv->x_bits = 23;
 		priv->y_bits = 12;
 		break;
+
 	case ALPS_PROTO_V6:
 		priv->hw_init = alps_hw_init_v6;
 		priv->process_packet = alps_process_packet_v6;
@@ -2232,6 +2259,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->x_max = 2047;
 		priv->y_max = 1535;
 		break;
+
 	case ALPS_PROTO_V7:
 		priv->hw_init = alps_hw_init_v7;
 		priv->process_packet = alps_process_packet_v7;
@@ -2239,19 +2267,21 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->set_abs_params = alps_set_abs_params_mt;
 		priv->nibble_commands = alps_v3_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
-		priv->x_max = 0xfff;
-		priv->y_max = 0x7ff;
-		priv->byte0 = 0x48;
-		priv->mask0 = 0x48;
+
+		if (alps_dolphin_get_device_area(psmouse, priv))
+			return -EIO;
 
 		if (priv->fw_ver[1] != 0xba)
 			priv->flags |= ALPS_BUTTONPAD;
+
 		break;
 	}
+
+	return 0;
 }
 
-static int alps_match_table(struct psmouse *psmouse, struct alps_data *priv,
-			    unsigned char *e7, unsigned char *ec)
+static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
+							 unsigned char *ec)
 {
 	const struct alps_model_info *model;
 	int i;
@@ -2263,22 +2293,16 @@ static int alps_match_table(struct psmouse *psmouse, struct alps_data *priv,
 		    (!model->command_mode_resp ||
 		     model->command_mode_resp == ec[2])) {
 
-			priv->proto_version = model->protocol_info.version;
-			alps_set_defaults(priv);
-
-			priv->flags = model->protocol_info.flags;
-			priv->byte0 = model->protocol_info.byte0;
-			priv->mask0 = model->protocol_info.mask0;
-
-			return 0;
+			return &model->protocol_info;
 		}
 	}
 
-	return -EINVAL;
+	return NULL;
 }
 
 static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 {
+	const struct alps_protocol_info *protocol;
 	unsigned char e6[4], e7[4], ec[4];
 
 	/*
@@ -2305,48 +2329,30 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 	    alps_exit_command_mode(psmouse))
 		return -EIO;
 
-	/* Save the Firmware version */
-	memcpy(priv->fw_ver, ec, 3);
-
-	if (alps_match_table(psmouse, priv, e7, ec) == 0) {
-		return 0;
-	} else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0x50 &&
-		   ec[0] == 0x73 && (ec[1] == 0x01 || ec[1] == 0x02)) {
-		priv->proto_version = ALPS_PROTO_V5;
-		alps_set_defaults(priv);
-		if (alps_dolphin_get_device_area(psmouse, priv))
-			return -EIO;
-		else
-			return 0;
-	} else if (ec[0] == 0x88 &&
-		   ((ec[1] & 0xf0) == 0xb0 || (ec[1] & 0xf0) == 0xc0)) {
-		priv->proto_version = ALPS_PROTO_V7;
-		alps_set_defaults(priv);
-
-		return 0;
-	} else if (ec[0] == 0x88 && ec[1] == 0x08) {
-		priv->proto_version = ALPS_PROTO_V3_RUSHMORE;
-		alps_set_defaults(priv);
-
-		/* hack to make addr_command, nibble_command available */
-		psmouse->private = priv;
-
-		if (alps_probe_trackstick_v3(psmouse, ALPS_REG_BASE_RUSHMORE))
-			priv->flags &= ~ALPS_DUALPOINT;
-
-		return 0;
-	} else if (ec[0] == 0x88 && ec[1] == 0x07 &&
-		   ec[2] >= 0x90 && ec[2] <= 0x9d) {
-		priv->proto_version = ALPS_PROTO_V3;
-		alps_set_defaults(priv);
-
-		return 0;
+	protocol = alps_match_table(e7, ec);
+	if (!protocol) {
+		if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0x50 &&
+			   ec[0] == 0x73 && (ec[1] == 0x01 || ec[1] == 0x02)) {
+			protocol = &alps_v5_protocol_data;
+		} else if (ec[0] == 0x88 &&
+			   ((ec[1] & 0xf0) == 0xb0 || (ec[1] & 0xf0) == 0xc0)) {
+			protocol = &alps_v7_protocol_data;
+		} else if (ec[0] == 0x88 && ec[1] == 0x08) {
+			protocol = &alps_v3_rushmore_data;
+		} else if (ec[0] == 0x88 && ec[1] == 0x07 &&
+			   ec[2] >= 0x90 && ec[2] <= 0x9d) {
+			protocol = &alps_v3_protocol_data;
+		} else {
+			psmouse_dbg(psmouse,
+				    "Likely not an ALPS touchpad: E7=%3ph, EC=%3ph\n", e7, ec);
+			return -EINVAL;
+		}
 	}
 
-	psmouse_dbg(psmouse,
-		    "Likely not an ALPS touchpad: E7=%3ph, EC=%3ph\n", e7, ec);
+	/* Save the Firmware version */
+	memcpy(priv->fw_ver, ec, 3);
 
-	return -EINVAL;
+	return alps_set_protocol(psmouse, priv, protocol);
 }
 
 static int alps_reconnect(struct psmouse *psmouse)
@@ -2409,9 +2415,6 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
-	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
-
-	psmouse->private = priv;
 
 	psmouse_reset(psmouse);
 
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2015-01-14 22:55 ` [PATCH 4/6] Input: ALPS - consolidate setting protocol parameters Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-15 20:21   ` Pali Rohár
  2015-01-17 10:26   ` Pali Rohár
  2015-01-14 22:55 ` [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data Dmitry Torokhov
  2015-01-15 10:49 ` [PATCH 0/6] Fixes for ALPS trackstick Pali Rohár
  6 siblings, 2 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

On some Dell Latitudes we fail to identify presence of trackstick unless we
reset the device. The issue is quite benign as we do perform reset in
alps_init(), so the trackstick ends up working, but mouse name reported to
userspace is not accurate.

In order to fix the issue while avoiding the additional lengthy reset we
move the resrt to alps_detect() and keep the discovered state to be used
later in alps_init().

Reported-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c | 74 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index cabb267..fd3303d 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2304,6 +2304,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 {
 	const struct alps_protocol_info *protocol;
 	unsigned char e6[4], e7[4], ec[4];
+	int error;
 
 	/*
 	 * First try "E6 report".
@@ -2349,10 +2350,15 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 		}
 	}
 
-	/* Save the Firmware version */
-	memcpy(priv->fw_ver, ec, 3);
+	if (priv) {
+		/* Save the Firmware version */
+		memcpy(priv->fw_ver, ec, 3);
+		error = alps_set_protocol(psmouse, priv, protocol);
+		if (error)
+			return error;
+	}
 
-	return alps_set_protocol(psmouse, priv, protocol);
+	return 0;
 }
 
 static int alps_reconnect(struct psmouse *psmouse)
@@ -2406,22 +2412,20 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
 
 int alps_init(struct psmouse *psmouse)
 {
-	struct alps_data *priv;
+	struct alps_data *priv = psmouse->private;
 	struct input_dev *dev1 = psmouse->dev, *dev2;
+	int error;
 
-	priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL);
 	dev2 = input_allocate_device();
-	if (!priv || !dev2)
+	if (!dev2) {
+		error = -ENOMEM;
 		goto init_fail;
+	}
 
 	priv->dev2 = dev2;
 
-	psmouse_reset(psmouse);
-
-	if (alps_identify(psmouse, priv) < 0)
-		goto init_fail;
-
-	if (priv->hw_init(psmouse))
+	error = priv->hw_init(psmouse);
+	if (error)
 		goto init_fail;
 
 	/*
@@ -2519,24 +2523,56 @@ int alps_init(struct psmouse *psmouse)
 init_fail:
 	psmouse_reset(psmouse);
 	input_free_device(dev2);
-	kfree(priv);
+	/*
+	 * Even though we did not allocate psmouse->private we do free
+	 * it here.
+	 */
+	kfree(psmouse->private);
 	psmouse->private = NULL;
-	return -1;
+	return error;
 }
 
 int alps_detect(struct psmouse *psmouse, bool set_properties)
 {
-	struct alps_data dummy;
+	struct alps_data *priv;
+	int error;
 
-	if (alps_identify(psmouse, &dummy) < 0)
-		return -1;
+	error = alps_identify(psmouse, NULL);
+	if (error)
+		return error;
+
+	/*
+	 * Reset the device to make sure it is fully operational:
+	 * on some laptops, like certain Dell Latitudes, we may
+	 * fail to properly detect presence of trackstick if device
+	 * has not been reset.
+	 */
+	psmouse_reset(psmouse);
+
+	priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL);
+	if (priv)
+		return -ENOMEM;
+
+	error = alps_identify(psmouse, priv);
+	if (error)
+		return error;
 
 	if (set_properties) {
 		psmouse->vendor = "ALPS";
-		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
+		psmouse->name = priv->flags & ALPS_DUALPOINT ?
 				"DualPoint TouchPad" : "GlidePoint";
-		psmouse->model = dummy.proto_version;
+		psmouse->model = priv->proto_version;
+	} else {
+		/*
+		 * Destroy alps_data structure we allocated earlier since
+		 * this was just a "trial run". Otherwise we'll keep it
+		 * to be used by alps_init() which has to be called if
+		 * we succeed and set_properties is true.
+		 */
+		kfree(priv);
+		psmouse->private = NULL;
 	}
+
 	return 0;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2015-01-14 22:55 ` [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes Dmitry Torokhov
@ 2015-01-14 22:55 ` Dmitry Torokhov
  2015-01-15 20:34   ` Pali Rohár
  2015-01-18  9:45   ` Pali Rohár
  2015-01-15 10:49 ` [PATCH 0/6] Fixes for ALPS trackstick Pali Rohár
  6 siblings, 2 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-14 22:55 UTC (permalink / raw)
  To: linux-input, Pali Rohár; +Cc: Yunkang Tang, Hans de Goede

From: Pali Rohár <pali.rohar@gmail.com>

Previously dev2 device was used for both external PS/2 mouse and internal
trackstick device (if available). This change introduces dev3 device which
is used for external PS/2 mouse data and dev2 is now used only for
trackstick.

In case that trackstick is not present dev2 is not created, so userspace
does not see non existent device in system.

Because laptops with ALPS devices often do not use i8042 active
multiplexing all data (from touchpad, trackstick and external PS/2 mouse)
come to one port.  So it is not possible to know if external PS/2 mouse is
connected or not. In most cases external PS/2 mouse is not connected so
driver will create dev3 input device after first bare PS/2 packet will be
received. So there will not be "ghost" input device.

This change also helps in identifying possible problems in future if driver
decides to report 6-bytes trackstick packets as 3-bytes bare PS/2 (data
will be reported to dev3 instead dev2).

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c | 188 +++++++++++++++++++++++++++++++++------------
 drivers/input/mouse/alps.h |  14 +++-
 2 files changed, 149 insertions(+), 53 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index fd3303d..48770cf 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -165,8 +165,7 @@ static bool alps_is_valid_first_byte(struct alps_data *priv,
 	return (data & priv->mask0) == priv->byte0;
 }
 
-static void alps_report_buttons(struct psmouse *psmouse,
-				struct input_dev *dev1, struct input_dev *dev2,
+static void alps_report_buttons(struct input_dev *dev1, struct input_dev *dev2,
 				int left, int right, int middle)
 {
 	struct input_dev *dev;
@@ -176,20 +175,21 @@ static void alps_report_buttons(struct psmouse *psmouse,
 	 * other device (dev2) then this event should be also
 	 * sent through that device.
 	 */
-	dev = test_bit(BTN_LEFT, dev2->key) ? dev2 : dev1;
+	dev = (dev2 && test_bit(BTN_LEFT, dev2->key)) ? dev2 : dev1;
 	input_report_key(dev, BTN_LEFT, left);
 
-	dev = test_bit(BTN_RIGHT, dev2->key) ? dev2 : dev1;
+	dev = (dev2 && test_bit(BTN_RIGHT, dev2->key)) ? dev2 : dev1;
 	input_report_key(dev, BTN_RIGHT, right);
 
-	dev = test_bit(BTN_MIDDLE, dev2->key) ? dev2 : dev1;
+	dev = (dev2 && test_bit(BTN_MIDDLE, dev2->key)) ? dev2 : dev1;
 	input_report_key(dev, BTN_MIDDLE, middle);
 
 	/*
 	 * Sync the _other_ device now, we'll do the first
 	 * device later once we report the rest of the events.
 	 */
-	input_sync(dev2);
+	if (dev2)
+		input_sync(dev2);
 }
 
 static void alps_process_packet_v1_v2(struct psmouse *psmouse)
@@ -236,13 +236,13 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		alps_report_buttons(psmouse, dev2, dev, left, right, middle);
+		alps_report_buttons(dev2, dev, left, right, middle);
 
 		input_sync(dev2);
 		return;
 	}
 
-	alps_report_buttons(psmouse, dev, dev2, left, right, middle);
+	alps_report_buttons(dev, dev2, left, right, middle);
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -1122,23 +1122,89 @@ static void alps_process_packet_v7(struct psmouse *psmouse)
 		alps_process_touchpad_packet_v7(psmouse);
 }
 
-static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
+static DEFINE_MUTEX(alps_mutex);
+
+static void alps_register_bare_ps2_mouse(struct work_struct *work)
+{
+	struct alps_data *priv =
+		container_of(work, struct alps_data, dev3_register_work.work);
+	struct psmouse *psmouse = priv->psmouse;
+	struct input_dev *dev3;
+	int error = 0;
+
+	mutex_lock(&alps_mutex);
+
+	if (priv->dev3)
+		goto out;
+
+	dev3 = input_allocate_device();
+	if (!dev3) {
+		psmouse_err(psmouse, "failed to alolocate secondary device\n");
+		error = -ENOMEM;
+		goto out;
+	}
+
+	snprintf(priv->phys3, sizeof(priv->phys3), "%s/%s",
+		 psmouse->ps2dev.serio->phys,
+		 (priv->dev2 ? "input2" : "input1"));
+	dev3->phys = priv->phys3;
+
+	/*
+	 * format of input device name is: "protocol vendor name"
+	 * see function psmouse_switch_protocol() in psmouse-base.c
+	 */
+	dev3->name = "PS/2 ALPS Mouse";
+
+	dev3->id.bustype = BUS_I8042;
+	dev3->id.vendor  = 0x0002;
+	dev3->id.product = PSMOUSE_PS2;
+	dev3->id.version = 0x0000;
+	dev3->dev.parent = &psmouse->ps2dev.serio->dev;
+
+	input_set_capability(dev3, EV_REL, REL_X);
+	input_set_capability(dev3, EV_REL, REL_Y);
+	input_set_capability(dev3, EV_KEY, BTN_LEFT);
+	input_set_capability(dev3, EV_KEY, BTN_RIGHT);
+	input_set_capability(dev3, EV_KEY, BTN_MIDDLE);
+
+	__set_bit(INPUT_PROP_POINTER, dev3->propbit);
+
+	error = input_register_device(dev3);
+	if (error) {
+		psmouse_err(psmouse,
+			    "failed to register secondary device: %d\n",
+			    error);
+		input_free_device(dev3);
+		goto out;
+	}
+
+	priv->dev3 = dev3;
+
+out:
+	/*
+	 * Save the error code so that we can detect that we
+	 * already tried to create the device.
+	 */
+	if (error)
+		priv->dev3 = ERR_PTR(error);
+
+	mutex_unlock(&alps_mutex);
+}
+
+static void alps_report_bare_ps2_packet(struct input_dev *dev,
 					unsigned char packet[],
 					bool report_buttons)
 {
-	struct alps_data *priv = psmouse->private;
-	struct input_dev *dev2 = priv->dev2;
-
 	if (report_buttons)
-		alps_report_buttons(psmouse, dev2, psmouse->dev,
+		alps_report_buttons(dev, NULL,
 				packet[0] & 1, packet[0] & 2, packet[0] & 4);
 
-	input_report_rel(dev2, REL_X,
+	input_report_rel(dev, REL_X,
 		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-	input_report_rel(dev2, REL_Y,
+	input_report_rel(dev, REL_Y,
 		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
 
-	input_sync(dev2);
+	input_sync(dev);
 }
 
 static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
@@ -1203,8 +1269,8 @@ static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
 		 * de-synchronization.
 		 */
 
-		alps_report_bare_ps2_packet(psmouse, &psmouse->packet[3],
-					    false);
+		alps_report_bare_ps2_packet(priv->dev2,
+					    &psmouse->packet[3], false);
 
 		/*
 		 * Continue with the standard ALPS protocol handling,
@@ -1260,9 +1326,18 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 	 * properly we only do this if the device is fully synchronized.
 	 */
 	if (!psmouse->out_of_sync_cnt && (psmouse->packet[0] & 0xc8) == 0x08) {
+
+		/* Register dev3 mouse if we received PS/2 packet first time */
+		if (unlikely(!priv->dev3))
+			psmouse_queue_work(psmouse,
+					   &priv->dev3_register_work, 0);
+
 		if (psmouse->pktcnt == 3) {
-			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
-						    true);
+			/* Once dev3 mouse device is registered report data */
+			if (likely(!IS_ERR_OR_NULL(priv->dev3)))
+				alps_report_bare_ps2_packet(priv->dev3,
+							    psmouse->packet,
+							    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
@@ -2379,7 +2454,10 @@ static void alps_disconnect(struct psmouse *psmouse)
 
 	psmouse_reset(psmouse);
 	del_timer_sync(&priv->timer);
-	input_unregister_device(priv->dev2);
+	if (priv->dev2)
+		input_unregister_device(priv->dev2);
+	if (!IS_ERR_OR_NULL(priv->dev3))
+		input_unregister_device(priv->dev3);
 	kfree(priv);
 }
 
@@ -2413,17 +2491,9 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
 int alps_init(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
-	struct input_dev *dev1 = psmouse->dev, *dev2;
+	struct input_dev *dev1 = psmouse->dev;
 	int error;
 
-	dev2 = input_allocate_device();
-	if (!dev2) {
-		error = -ENOMEM;
-		goto init_fail;
-	}
-
-	priv->dev2 = dev2;
-
 	error = priv->hw_init(psmouse);
 	if (error)
 		goto init_fail;
@@ -2475,36 +2545,55 @@ int alps_init(struct psmouse *psmouse)
 	}
 
 	if (priv->flags & ALPS_DUALPOINT) {
+		struct input_dev *dev2;
+
+		dev2 = input_allocate_device();
+		if (!dev2) {
+			psmouse_err(psmouse,
+				    "failed to allocate  trackstick device\n");
+			error = -ENOMEM;
+			goto init_fail;
+		}
+
+		snprintf(priv->phys2, sizeof(priv->phys2), "%s/input1",
+			 psmouse->ps2dev.serio->phys);
+		dev2->phys = priv->phys2;
+
 		/*
 		 * format of input device name is: "protocol vendor name"
 		 * see function psmouse_switch_protocol() in psmouse-base.c
 		 */
 		dev2->name = "AlpsPS/2 ALPS DualPoint Stick";
+
+		dev2->id.bustype = BUS_I8042;
+		dev2->id.vendor  = 0x0002;
 		dev2->id.product = PSMOUSE_ALPS;
 		dev2->id.version = priv->proto_version;
-	} else {
-		dev2->name = "PS/2 ALPS Mouse";
-		dev2->id.product = PSMOUSE_PS2;
-		dev2->id.version = 0x0000;
-	}
-
-	snprintf(priv->phys, sizeof(priv->phys), "%s/input1", psmouse->ps2dev.serio->phys);
-	dev2->phys = priv->phys;
-	dev2->id.bustype = BUS_I8042;
-	dev2->id.vendor  = 0x0002;
-	dev2->dev.parent = &psmouse->ps2dev.serio->dev;
+		dev2->dev.parent = &psmouse->ps2dev.serio->dev;
 
-	dev2->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
-	dev2->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
-	dev2->keybit[BIT_WORD(BTN_LEFT)] =
-		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
+		input_set_capability(dev2, EV_REL, REL_X);
+		input_set_capability(dev2, EV_REL, REL_Y);
+		input_set_capability(dev2, EV_KEY, BTN_LEFT);
+		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
+		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);
 
-	__set_bit(INPUT_PROP_POINTER, dev2->propbit);
-	if (priv->flags & ALPS_DUALPOINT)
+		__set_bit(INPUT_PROP_POINTER, dev2->propbit);
 		__set_bit(INPUT_PROP_POINTING_STICK, dev2->propbit);
 
-	if (input_register_device(priv->dev2))
-		goto init_fail;
+		error = input_register_device(dev2);
+		if (error) {
+			psmouse_err(psmouse,
+				    "failed to register trackstick device: %d\n",
+				    error);
+			input_free_device(dev2);
+			goto init_fail;
+		}
+
+		priv->dev2 = dev2;
+	}
+
+	INIT_DELAYED_WORK(&priv->dev3_register_work,
+			  alps_register_bare_ps2_mouse);
 
 	psmouse->protocol_handler = alps_process_byte;
 	psmouse->poll = alps_poll;
@@ -2522,7 +2611,6 @@ int alps_init(struct psmouse *psmouse)
 
 init_fail:
 	psmouse_reset(psmouse);
-	input_free_device(dev2);
 	/*
 	 * Even though we did not allocate psmouse->private we do free
 	 * it here.
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index c795235..02513c0 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -141,8 +141,12 @@ struct alps_fields {
 
 /**
  * struct alps_data - private data structure for the ALPS driver
- * @dev2: "Relative" device used to report trackstick or mouse activity.
- * @phys: Physical path for the relative device.
+ * @psmouse: Pointer to parent psmouse device
+ * @dev2: Trackstick device (can be NULL).
+ * @dev3: Generic PS/2 mouse (can be NULL, delayed registering).
+ * @phys2: Physical path for the trackstick device.
+ * @phys3: Physical path for the generic PS/2 mouse.
+ * @dev3_register_work: Delayed work for registering PS/2 mouse.
  * @nibble_commands: Command mapping used for touchpad register accesses.
  * @addr_command: Command used to tell the touchpad that a register address
  *   follows.
@@ -169,8 +173,12 @@ struct alps_fields {
  * @timer: Timer for flushing out the final report packet in the stream.
  */
 struct alps_data {
+	struct psmouse *psmouse;
 	struct input_dev *dev2;
-	char phys[32];
+	struct input_dev *dev3;
+	char phys2[32];
+	char phys3[32];
+	struct delayed_work dev3_register_work;
 
 	/* these are autodetected when the device is identified */
 	const struct alps_nibble_commands *nibble_commands;
-- 
2.2.0.rc0.207.ga3a616c

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
                   ` (5 preceding siblings ...)
  2015-01-14 22:55 ` [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data Dmitry Torokhov
@ 2015-01-15 10:49 ` Pali Rohár
  2015-01-15 18:18   ` Dmitry Torokhov
  6 siblings, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-15 10:49 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, Yunkang Tang, Hans de Goede

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

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?

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 10:49 ` [PATCH 0/6] Fixes for ALPS trackstick Pali Rohár
@ 2015-01-15 18:18   ` Dmitry Torokhov
  2015-01-15 19:19     ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 18:18 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-input, Yunkang Tang, Hans de Goede

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.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 18:18   ` Dmitry Torokhov
@ 2015-01-15 19:19     ` Pali Rohár
  2015-01-15 19:38       ` Dmitry Torokhov
  0 siblings, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-15 19:19 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input

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

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

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 19:19     ` Pali Rohár
@ 2015-01-15 19:38       ` Dmitry Torokhov
  2015-01-15 20:28         ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 19:38 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input

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.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-01-14 22:55 ` [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes Dmitry Torokhov
@ 2015-01-15 20:21   ` Pali Rohár
  2015-01-15 21:00     ` Dmitry Torokhov
  2015-01-17 10:26   ` Pali Rohár
  1 sibling, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-15 20:21 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input

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

On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote:
> +	/*
> +	 * Reset the device to make sure it is fully operational:
> +	 * on some laptops, like certain Dell Latitudes, we may
> +	 * fail to properly detect presence of trackstick if device
> +	 * has not been reset.
> +	 */
> +	psmouse_reset(psmouse);
> +
> +	priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL);
> +	if (priv)

if (!priv)

> +		return -ENOMEM;
> +

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 19:38       ` Dmitry Torokhov
@ 2015-01-15 20:28         ` Pali Rohár
  2015-01-15 21:02           ` Dmitry Torokhov
  0 siblings, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-15 20:28 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input

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

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...

(My original patch series worked fine on my laptop, so when you 
reused my code it should work)

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data
  2015-01-14 22:55 ` [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data Dmitry Torokhov
@ 2015-01-15 20:34   ` Pali Rohár
  2015-01-15 21:00     ` Dmitry Torokhov
  2015-01-18  9:45   ` Pali Rohár
  1 sibling, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-15 20:34 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, Yunkang Tang, Hans de Goede

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

On Wednesday 14 January 2015 23:55:54 Dmitry Torokhov wrote:
> From: Pali Rohár <pali.rohar@gmail.com>
> 
> Previously dev2 device was used for both external PS/2 mouse
> and internal trackstick device (if available). This change
> introduces dev3 device which is used for external PS/2 mouse
> data and dev2 is now used only for trackstick.
> 
> In case that trackstick is not present dev2 is not created, so
> userspace does not see non existent device in system.
> 
> Because laptops with ALPS devices often do not use i8042
> active multiplexing all data (from touchpad, trackstick and
> external PS/2 mouse) come to one port.  So it is not possible
> to know if external PS/2 mouse is connected or not. In most
> cases external PS/2 mouse is not connected so driver will
> create dev3 input device after first bare PS/2 packet will be
> received. So there will not be "ghost" input device.
> 
> This change also helps in identifying possible problems in
> future if driver decides to report 6-bytes trackstick packets
> as 3-bytes bare PS/2 (data will be reported to dev3 instead
> dev2).
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Subject should be: "do *not* mix" or "prevent mix"

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data
  2015-01-15 20:34   ` Pali Rohár
@ 2015-01-15 21:00     ` Dmitry Torokhov
  0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:00 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-input, Yunkang Tang, Hans de Goede

On Thu, Jan 15, 2015 at 09:34:51PM +0100, Pali Rohár wrote:
> On Wednesday 14 January 2015 23:55:54 Dmitry Torokhov wrote:
> > From: Pali Rohár <pali.rohar@gmail.com>
> > 
> > Previously dev2 device was used for both external PS/2 mouse
> > and internal trackstick device (if available). This change
> > introduces dev3 device which is used for external PS/2 mouse
> > data and dev2 is now used only for trackstick.
> > 
> > In case that trackstick is not present dev2 is not created, so
> > userspace does not see non existent device in system.
> > 
> > Because laptops with ALPS devices often do not use i8042
> > active multiplexing all data (from touchpad, trackstick and
> > external PS/2 mouse) come to one port.  So it is not possible
> > to know if external PS/2 mouse is connected or not. In most
> > cases external PS/2 mouse is not connected so driver will
> > create dev3 input device after first bare PS/2 packet will be
> > received. So there will not be "ghost" input device.
> > 
> > This change also helps in identifying possible problems in
> > future if driver decides to report 6-bytes trackstick packets
> > as 3-bytes bare PS/2 (data will be reported to dev3 instead
> > dev2).
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Subject should be: "do *not* mix" or "prevent mix"

Right, I'll adjust it.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-01-15 20:21   ` Pali Rohár
@ 2015-01-15 21:00     ` Dmitry Torokhov
  0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:00 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input

On Thu, Jan 15, 2015 at 09:21:34PM +0100, Pali Rohár wrote:
> On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote:
> > +	/*
> > +	 * Reset the device to make sure it is fully operational:
> > +	 * on some laptops, like certain Dell Latitudes, we may
> > +	 * fail to properly detect presence of trackstick if device
> > +	 * has not been reset.
> > +	 */
> > +	psmouse_reset(psmouse);
> > +
> > +	priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL);
> > +	if (priv)
> 
> if (!priv)

Yep, thanks.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 20:28         ` Pali Rohár
@ 2015-01-15 21:02           ` Dmitry Torokhov
  2015-01-17 10:01             ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:02 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input

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.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-15 21:02           ` Dmitry Torokhov
@ 2015-01-17 10:01             ` Pali Rohár
  2015-01-18  7:22               ` Dmitry Torokhov
  0 siblings, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-17 10:01 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input

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

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. I do not understand why that 
function was ever called (I do not have connected any PS/2 mouse 
which can generate bare 3 bytes PS/2 packet) and also why that 
function caused page fault.

Last time when I tested my original patch series I did not see 
any of above problems... I also modified alps.c code to process 
all ALPS packets into dev3 (as 3 bytes PS/2). It registered dev3 
from alps_register_bare_ps2_mouse() without problem and reported 
some data to userspace (of course data was incorrect, because 
kernel processed 6 bytes ALPS packets as two 3 bytes bare PS/2 
but there was no other problem...).

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-01-14 22:55 ` [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes Dmitry Torokhov
  2015-01-15 20:21   ` Pali Rohár
@ 2015-01-17 10:26   ` Pali Rohár
  2015-02-02  5:34     ` Dmitry Torokhov
  1 sibling, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-01-17 10:26 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input

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

On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote:
> On some Dell Latitudes we fail to identify presence of
> trackstick unless we reset the device. The issue is quite
> benign as we do perform reset in alps_init(), so the
> trackstick ends up working, but mouse name reported to
> userspace is not accurate.
> 
> In order to fix the issue while avoiding the additional
> lengthy reset we move the resrt to alps_detect() and keep the
> discovered state to be used later in alps_init().
> 
> Reported-by: Pali Rohár <pali.rohar@gmail.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This patch is not enough. ALPS_DUALPOINT flag can be removed also 
in function alps_hw_init_rushmore_v3() which is called from 
alps_init() but not from alps_detect(). So this patch does not 
have to set correct name in alps_detect() based on ALPS_DUALPOINT 
flag. My original patch set name in alps_init() after hw_init() 
which handled also this problem...

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-17 10:01             ` Pali Rohár
@ 2015-01-18  7:22               ` Dmitry Torokhov
  2015-01-18  9:47                 ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-01-18  7:22 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input

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...

> I do not understand why that 
> function was ever called (I do not have connected any PS/2 mouse 
> which can generate bare 3 bytes PS/2 packet) and also why that 
> function caused page fault.

Hmm... my memory might be hazy but I seem to recall that
trackstick data can come as either dedicated packets or bare PS/2
format, I am not sure if we can actually split the data streams into 2
devices.

I do not have boxes with ALPS devices though so I am unable to
experiment.

Thanks.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data
  2015-01-14 22:55 ` [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data Dmitry Torokhov
  2015-01-15 20:34   ` Pali Rohár
@ 2015-01-18  9:45   ` Pali Rohár
  1 sibling, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-01-18  9:45 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input

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

On Wednesday 14 January 2015 23:55:54 Dmitry Torokhov wrote:
> +       dev3 = input_allocate_device();
> +       if (!dev3) {
> +               psmouse_err(psmouse, "failed to alolocate secondary device\n");

                                                  allocate

> +               error = -ENOMEM;
> +               goto out;
> +       }

...

>         if (priv->flags & ALPS_DUALPOINT) {
> +               struct input_dev *dev2;
> +
> +               dev2 = input_allocate_device();
> +               if (!dev2) {
> +                       psmouse_err(psmouse,
> +                                   "failed to allocate  trackstick device\n");

                                                         ^^ 2x space

> +                       error = -ENOMEM;
> +                       goto init_fail;
> +               }

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-18  7:22               ` Dmitry Torokhov
@ 2015-01-18  9:47                 ` Pali Rohár
  2015-01-24 22:20                   ` Pali Rohár
  2015-02-02  5:49                   ` Dmitry Torokhov
  0 siblings, 2 replies; 34+ messages in thread
From: Pali Rohár @ 2015-01-18  9:47 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

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 do not understand why that
> > function was ever called (I do not have connected any PS/2
> > mouse which can generate bare 3 bytes PS/2 packet) and also
> > why that function caused page fault.
> 
> Hmm... my memory might be hazy but I seem to recall that
> trackstick data can come as either dedicated packets or bare
> PS/2 format, I am not sure if we can actually split the data
> streams into 2 devices.
> 
> I do not have boxes with ALPS devices though so I am unable to
> experiment.
> 
> Thanks.

From documentation and also source code it looks like trackstick 
data are reported in ALPS 6bytes packets and not in bare 3bytes.

3 months ago Vadim Klishko (CCed) wrote me:

"I don't think there is an Alps device that would mix 3-byte and 
6-byte packets."

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-18  9:47                 ` Pali Rohár
@ 2015-01-24 22:20                   ` Pali Rohár
  2015-02-02  5:49                   ` Dmitry Torokhov
  1 sibling, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-01-24 22:20 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

On Sunday 18 January 2015 10:47:06 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 do not understand why that
> > > function was ever called (I do not have connected any PS/2
> > > mouse which can generate bare 3 bytes PS/2 packet) and
> > > also why that function caused page fault.
> > 
> > Hmm... my memory might be hazy but I seem to recall that
> > trackstick data can come as either dedicated packets or bare
> > PS/2 format, I am not sure if we can actually split the data
> > streams into 2 devices.
> > 
> > I do not have boxes with ALPS devices though so I am unable
> > to experiment.
> > 
> > Thanks.
> 
> From documentation and also source code it looks like
> trackstick data are reported in ALPS 6bytes packets and not
> in bare 3bytes.
> 
> 3 months ago Vadim Klishko (CCed) wrote me:
> 
> "I don't think there is an Alps device that would mix 3-byte
> and 6-byte packets."

Dmitry ping. Will you fix patches and send new version?

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-01-17 10:26   ` Pali Rohár
@ 2015-02-02  5:34     ` Dmitry Torokhov
  2015-02-02 10:51       ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-02-02  5:34 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input

On Sat, Jan 17, 2015 at 11:26:50AM +0100, Pali Rohár wrote:
> On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote:
> > On some Dell Latitudes we fail to identify presence of
> > trackstick unless we reset the device. The issue is quite
> > benign as we do perform reset in alps_init(), so the
> > trackstick ends up working, but mouse name reported to
> > userspace is not accurate.
> > 
> > In order to fix the issue while avoiding the additional
> > lengthy reset we move the resrt to alps_detect() and keep the
> > discovered state to be used later in alps_init().
> > 
> > Reported-by: Pali Rohár <pali.rohar@gmail.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> This patch is not enough. ALPS_DUALPOINT flag can be removed also 
> in function alps_hw_init_rushmore_v3() which is called from 
> alps_init() but not from alps_detect(). So this patch does not 
> have to set correct name in alps_detect() based on ALPS_DUALPOINT 
> flag. My original patch set name in alps_init() after hw_init() 
> which handled also this problem...

Hmm, I think if we are still seeing these after somewhat recent addition
of full reset in detect procedure we need to fix our detection instead
of tweaking capabilities after initialization phase fails. So I will
just remove that bit from alps_hw_init_rushmore_v3(). FWIW I did a quick
search on Google and do not seem to find dmesgs with message
"trackstick E7 report failed".

Thanks.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-01-18  9:47                 ` Pali Rohár
  2015-01-24 22:20                   ` Pali Rohár
@ 2015-02-02  5:49                   ` Dmitry Torokhov
  2015-02-02 10:49                     ` Pali Rohár
  2015-02-05 11:41                     ` Pali Rohár
  1 sibling, 2 replies; 34+ messages in thread
From: Dmitry Torokhov @ 2015-02-02  5:49 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input, Vadim Klishko

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!

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-02  5:49                   ` Dmitry Torokhov
@ 2015-02-02 10:49                     ` Pali Rohár
  2015-02-02 14:27                       ` Pali Rohár
  2015-02-05 11:41                     ` Pali Rohár
  1 sibling, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-02-02 10:49 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

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.

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes
  2015-02-02  5:34     ` Dmitry Torokhov
@ 2015-02-02 10:51       ` Pali Rohár
  0 siblings, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-02-02 10:51 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input

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

On Monday 02 February 2015 06:34:08 Dmitry Torokhov wrote:
> On Sat, Jan 17, 2015 at 11:26:50AM +0100, Pali Rohár wrote:
> > On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote:
> > > On some Dell Latitudes we fail to identify presence of
> > > trackstick unless we reset the device. The issue is quite
> > > benign as we do perform reset in alps_init(), so the
> > > trackstick ends up working, but mouse name reported to
> > > userspace is not accurate.
> > > 
> > > In order to fix the issue while avoiding the additional
> > > lengthy reset we move the resrt to alps_detect() and keep
> > > the discovered state to be used later in alps_init().
> > > 
> > > Reported-by: Pali Rohár <pali.rohar@gmail.com>
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > This patch is not enough. ALPS_DUALPOINT flag can be removed
> > also in function alps_hw_init_rushmore_v3() which is called
> > from alps_init() but not from alps_detect(). So this patch
> > does not have to set correct name in alps_detect() based on
> > ALPS_DUALPOINT flag. My original patch set name in
> > alps_init() after hw_init() which handled also this
> > problem...
> 
> Hmm, I think if we are still seeing these after somewhat
> recent addition of full reset in detect procedure we need to
> fix our detection instead of tweaking capabilities after
> initialization phase fails. So I will just remove that bit
> from alps_hw_init_rushmore_v3(). FWIW I did a quick search on
> Google and do not seem to find dmesgs with message
> "trackstick E7 report failed".
> 
> Thanks.

Ok, in this case kernel just register redundant input device 
which does not send any events to userspace.

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-02 10:49                     ` Pali Rohár
@ 2015-02-02 14:27                       ` Pali Rohár
  2015-02-08 12:26                         ` Pali Rohár
  0 siblings, 1 reply; 34+ messages in thread
From: Pali Rohár @ 2015-02-02 14:27 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-02  5:49                   ` Dmitry Torokhov
  2015-02-02 10:49                     ` Pali Rohár
@ 2015-02-05 11:41                     ` Pali Rohár
  1 sibling, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-02-05 11:41 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

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!

You did not fixed commit message which add dev3 device. It should 
be "do not mix trackstick ...".

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-02 14:27                       ` Pali Rohár
@ 2015-02-08 12:26                         ` Pali Rohár
  2015-02-10  6:32                           ` Dmitry Torokhov
  2015-02-12  7:52                           ` Dmitry Torokhov
  0 siblings, 2 replies; 34+ messages in thread
From: Pali Rohár @ 2015-02-08 12:26 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> 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.
 
Dmitry, will you prepare v2 series of patches?

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-08 12:26                         ` Pali Rohár
@ 2015-02-10  6:32                           ` Dmitry Torokhov
  2015-02-11 18:13                             ` Pali Rohár
  2015-02-12  7:52                           ` Dmitry Torokhov
  1 sibling, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-02-10  6:32 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input, Vadim Klishko

On Sun, Feb 08, 2015 at 01:26:32PM +0100, Pali Rohár wrote:
> On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> > 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.
>  
> Dmitry, will you prepare v2 series of patches?

Pali, thanks for tracking this down. I'll try update the series
tomorrow.

Thanks.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-10  6:32                           ` Dmitry Torokhov
@ 2015-02-11 18:13                             ` Pali Rohár
  0 siblings, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-02-11 18:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

On Tuesday 10 February 2015 07:32:55 Dmitry Torokhov wrote:
> On Sun, Feb 08, 2015 at 01:26:32PM +0100, Pali Rohár wrote:
> > On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> > > 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.
> > 
> > Dmitry, will you prepare v2 series of patches?
> 
> Pali, thanks for tracking this down. I'll try update the
> series tomorrow.
> 
> Thanks.

Hello Dmitry, have you updated this alps patch series?

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-08 12:26                         ` Pali Rohár
  2015-02-10  6:32                           ` Dmitry Torokhov
@ 2015-02-12  7:52                           ` Dmitry Torokhov
  2015-02-12 20:25                             ` Pali Rohár
  1 sibling, 1 reply; 34+ messages in thread
From: Dmitry Torokhov @ 2015-02-12  7:52 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Hans de Goede, linux-input, Vadim Klishko

On Sun, Feb 08, 2015 at 01:26:32PM +0100, Pali Rohár wrote:
> On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> > 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.

Right you are.

> > 
> > 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) {

Yep.

> >  				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) {

No, I think this particular case we want to leave rushmore-only, since
that's what we have in those Latitudes.

> >  			/*
> >  			 * 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.
>  
> Dmitry, will you prepare v2 series of patches?

Yes, I rebased my alps branch on top of 3.19 and fixed up the patches.
Can you please try it?

Thanks.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/6] Fixes for ALPS trackstick
  2015-02-12  7:52                           ` Dmitry Torokhov
@ 2015-02-12 20:25                             ` Pali Rohár
  0 siblings, 0 replies; 34+ messages in thread
From: Pali Rohár @ 2015-02-12 20:25 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Vadim Klishko

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

On Thursday 12 February 2015 08:52:35 Dmitry Torokhov wrote:
> On Sun, Feb 08, 2015 at 01:26:32PM +0100, Pali Rohár wrote:
> > On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> > > 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.
> 
> Right you are.
> 
> > > 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) {
> 
> Yep.
> 
> > >  				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) {
> 
> No, I think this particular case we want to leave
> rushmore-only, since that's what we have in those Latitudes.
> 
> > >  			/*
> > >  			
> > >  			 * 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.
> > 
> > Dmitry, will you prepare v2 series of patches?
> 
> Yes, I rebased my alps branch on top of 3.19 and fixed up the
> patches. Can you please try it?
> 
> Thanks.

Tested version from commit 48aecc2 and my touchpad and trackstisk 
is working fine.

You can add my tested-by line to all patches:

Tested-by: Pali Rohár <pali.rohar@gmail.com>

Just one small cosmetic/format note:

In alps.c code is this comment:

 * XXX - this entry is suspicious. First byte has zero ...

It belongs to line with struct alps_model_info alps_model_data 
where is that /* XXX */ comment.

But your patches added another structures between these two 
comments and so it is not easy to understand what that XXX means.

Can you move this comment at correct place (maybe immediately 
after struct alps_model_info)?

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2015-02-12 20:25 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 22:55 [PATCH 0/6] Fixes for ALPS trackstick Dmitry Torokhov
2015-01-14 22:55 ` [PATCH 1/6] Input: ALPS - renumber protocol numbers Dmitry Torokhov
2015-01-14 22:55 ` [PATCH 2/6] Input: ALPS - make Rushmore a separate protocol Dmitry Torokhov
2015-01-14 22:55 ` [PATCH 3/6] Input: ALPS - split protocol data from model info Dmitry Torokhov
2015-01-14 22:55 ` [PATCH 4/6] Input: ALPS - consolidate setting protocol parameters Dmitry Torokhov
2015-01-14 22:55 ` [PATCH 5/6] Input: ALPS - fix trackstick detection on some Dell Latitudes Dmitry Torokhov
2015-01-15 20:21   ` Pali Rohár
2015-01-15 21:00     ` Dmitry Torokhov
2015-01-17 10:26   ` Pali Rohár
2015-02-02  5:34     ` Dmitry Torokhov
2015-02-02 10:51       ` Pali Rohár
2015-01-14 22:55 ` [PATCH 6/6] Input: ALPS - mix trackstick and external PS/2 mouse data Dmitry Torokhov
2015-01-15 20:34   ` Pali Rohár
2015-01-15 21:00     ` Dmitry Torokhov
2015-01-18  9:45   ` Pali Rohár
2015-01-15 10:49 ` [PATCH 0/6] Fixes for ALPS trackstick Pali Rohár
2015-01-15 18:18   ` Dmitry Torokhov
2015-01-15 19:19     ` Pali Rohár
2015-01-15 19:38       ` Dmitry Torokhov
2015-01-15 20:28         ` Pali Rohár
2015-01-15 21:02           ` Dmitry Torokhov
2015-01-17 10:01             ` Pali Rohár
2015-01-18  7:22               ` Dmitry Torokhov
2015-01-18  9:47                 ` Pali Rohár
2015-01-24 22:20                   ` Pali Rohár
2015-02-02  5:49                   ` Dmitry Torokhov
2015-02-02 10:49                     ` Pali Rohár
2015-02-02 14:27                       ` Pali Rohár
2015-02-08 12:26                         ` Pali Rohár
2015-02-10  6:32                           ` Dmitry Torokhov
2015-02-11 18:13                             ` Pali Rohár
2015-02-12  7:52                           ` Dmitry Torokhov
2015-02-12 20:25                             ` Pali Rohár
2015-02-05 11:41                     ` Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.