[PATCH 0/7] Fixes for ALPS trackstick

[PATCH 0/7] Fixes for ALPS trackstick

[Pali Rohár]

This patch series fix detection and identifying trackstick on machines with
ALPS devices. Last patch split trackstick and bare PS/2 mouse packets between
dev2 and dev3 input devices which make sure that driver will send only trackstick
data to trackstick input device.

Pali Rohár (7):
  input: alps: Set correct name of psmouse device in alps_init()
  input: alps: Move trackstick detection to alps_hw_init_*
  input: alps: Move alps_dolphin_get_device_area into
    alps_hw_init_dolphin_v1
  input: alps: Use NULL instead dummy argument for alps_identify
  input: alps: Fix name, product and version of dev2 input device
  input: alps: Add sanity checks for non DualPoint devices
  input: alps: Do not report both trackstick and external PS/2 mouse
    data to one input device

 drivers/input/mouse/alps.c |  361 +++++++++++++++++++++++++++++++-------------
 drivers/input/mouse/alps.h |   14 +-
 2 files changed, 264 insertions(+), 111 deletions(-)

-- 
1.7.9.5

[PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Pali Rohár]

On some laptops after starting them from off state (not after reboot), function
alps_probe_trackstick_v3() (called from function alps_identify()) does not
detect trackstick. To fix this problem we need to reset device. But function
alps_identify() is called also from alps_detect() and we do not want to reset
device in detect function because it will slow down initialization of all other
non alps devices.

This patch moves code for setting correct device name & protocol from function
alps_detect() to alps_init() which already doing full device reset.

So this patch removes need to do trackstick detection in alps_detect() function.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 8d85c79..9ffa98d 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2392,6 +2392,10 @@ int alps_init(struct psmouse *psmouse)
 	if (input_register_device(priv->dev2))
 		goto init_fail;
 
+	if (!(priv->flags & ALPS_DUALPOINT))
+		psmouse->name = "GlidePoint TouchPad";
+	psmouse->model = priv->proto_version;
+
 	psmouse->protocol_handler = alps_process_byte;
 	psmouse->poll = alps_poll;
 	psmouse->disconnect = alps_disconnect;
@@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
 		return -1;
 
 	if (set_properties) {
+		/*
+		 * NOTE: To detect model and trackstick presence we need to do
+		 *       full device reset. To speed up detection and prevent
+		 *       calling duplicate initialization sequence (both in
+		 *       alps_detect() and alps_init()) we set model/protocol
+		 *       version and correct name in alps_init() (which will
+		 *       do full device reset). For now set name to DualPoint.
+		 */
 		psmouse->vendor = "ALPS";
-		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
-				"DualPoint TouchPad" : "GlidePoint";
-		psmouse->model = dummy.proto_version << 8;
+		psmouse->name = "DualPoint TouchPad";
 	}
+
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 2/7] input: alps: Move trackstick detection to alps_hw_init_*

[Pali Rohár]

Now when alps_identify() does not need do trackstick detection it is safe to
move code init hw_init functions.

This patch also fix trackstick detection in function alps_hw_init_v3() which
remove ALPS_DUALPOINT flag when trackstick is not detected.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |   30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 9ffa98d..cbfdcfc 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1732,6 +1732,7 @@ error:
 
 static int alps_hw_init_v3(struct psmouse *psmouse)
 {
+	struct alps_data *priv = psmouse->private;
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
 	int reg_val;
 	unsigned char param[4];
@@ -1740,9 +1741,15 @@ static int alps_hw_init_v3(struct psmouse *psmouse)
 	if (reg_val == -EIO)
 		goto error;
 
-	if (reg_val == 0 &&
-	    alps_setup_trackstick_v3(psmouse, ALPS_REG_BASE_PINNACLE) == -EIO)
-		goto error;
+	if (reg_val == 0) {
+		reg_val = alps_setup_trackstick_v3(psmouse,
+						   ALPS_REG_BASE_PINNACLE);
+		if (reg_val == -EIO)
+			goto error;
+	}
+
+	if (reg_val == -ENODEV)
+		priv->flags &= ~ALPS_DUALPOINT;
 
 	if (alps_enter_command_mode(psmouse) ||
 	    alps_absolute_mode_v3(psmouse)) {
@@ -1849,15 +1856,20 @@ static int alps_hw_init_rushmore_v3(struct psmouse *psmouse)
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
 	int reg_val, ret = -1;
 
-	if (priv->flags & ALPS_DUALPOINT) {
+	reg_val = alps_probe_trackstick_v3(psmouse, ALPS_REG_BASE_RUSHMORE);
+	if (reg_val == -EIO)
+		goto error;
+
+	if (reg_val == 0) {
 		reg_val = alps_setup_trackstick_v3(psmouse,
 						   ALPS_REG_BASE_RUSHMORE);
 		if (reg_val == -EIO)
 			goto error;
-		if (reg_val == -ENODEV)
-			priv->flags &= ~ALPS_DUALPOINT;
 	}
 
+	if (reg_val == -ENODEV)
+		priv->flags &= ~ALPS_DUALPOINT;
+
 	if (alps_enter_command_mode(psmouse) ||
 	    alps_command_mode_read_reg(psmouse, 0xc2d9) == -1 ||
 	    alps_command_mode_write_reg(psmouse, 0xc2cb, 0x00))
@@ -2231,12 +2243,6 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 		priv->y_bits = 12;
 		priv->flags |= ALPS_IS_RUSHMORE;
 
-		/* 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) {
-- 
1.7.9.5

[PATCH 3/7] input: alps: Move alps_dolphin_get_device_area into alps_hw_init_dolphin_v1

[Pali Rohár]

This patch moves function call alps_dolphin_get_device_area() from function
alps_identify() to alps_hw_init_dolphin_v1() so alps_identify() will not call
any other psmouse commands.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index cbfdcfc..5603870 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1991,9 +1991,9 @@ error:
 	return -1;
 }
 
-static int alps_dolphin_get_device_area(struct psmouse *psmouse,
-					struct alps_data *priv)
+static int alps_dolphin_get_device_area(struct psmouse *psmouse)
 {
+	struct alps_data *priv = psmouse->private;
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
 	unsigned char param[4] = {0};
 	int num_x_electrode, num_y_electrode;
@@ -2042,6 +2042,9 @@ static int alps_hw_init_dolphin_v1(struct psmouse *psmouse)
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
 	unsigned char param[2];
 
+	if (alps_dolphin_get_device_area(psmouse))
+		return -EIO;
+
 	/* This is dolphin "v1" as empirically defined by florin9doi */
 	param[0] = 0x64;
 	param[1] = 0x28;
@@ -2223,10 +2226,8 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 		   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;
+
+		return 0;
 	} else if (ec[0] == 0x88 &&
 		   ((ec[1] & 0xf0) == 0xb0 || (ec[1] & 0xf0) == 0xc0)) {
 		priv->proto_version = ALPS_PROTO_V7;
-- 
1.7.9.5

[PATCH 4/7] input: alps: Use NULL instead dummy argument for alps_identify

[Pali Rohár]

This patch change alps_identify() function code so it can be called also without
priv parameter which is useful for alps_detect(). Instead passing dummy value
now alps_identify() accept also NULL value and skip filling priv data. It is
useless for alps_detect() and it will speed up detection of alps devices.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |  114 +++++++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 43 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 5603870..0176425 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2162,8 +2162,7 @@ static void alps_set_defaults(struct alps_data *priv)
 	}
 }
 
-static int alps_match_table(struct psmouse *psmouse, struct alps_data *priv,
-			    unsigned char *e7, unsigned char *ec)
+static const struct alps_model_info *alps_match_table(unsigned char *e7, unsigned char *ec)
 {
 	const struct alps_model_info *model;
 	int i;
@@ -2174,24 +2173,49 @@ static int alps_match_table(struct psmouse *psmouse, struct alps_data *priv,
 		if (!memcmp(e7, model->signature, sizeof(model->signature)) &&
 		    (!model->command_mode_resp ||
 		     model->command_mode_resp == ec[2])) {
+			return model;
+		}
+	}
 
-			priv->proto_version = model->proto_version;
-			alps_set_defaults(priv);
+	return NULL;
+}
 
-			priv->flags = model->flags;
-			priv->byte0 = model->byte0;
-			priv->mask0 = model->mask0;
+static inline bool alps_match_v5(unsigned char *e7, unsigned char *ec)
+{
+	return (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0x50 &&
+		ec[0] == 0x73 && (ec[1] == 0x01 || ec[1] == 0x02));
+}
 
-			return 0;
-		}
-	}
+static inline bool alps_match_v7(unsigned char *e7, unsigned char *ec)
+{
+	return (ec[0] == 0x88 &&
+		((ec[1] & 0xf0) == 0xb0 || (ec[1] & 0xf0) == 0xc0));
+}
 
-	return -EINVAL;
+static inline bool alps_match_rushmore_v3(unsigned char *e7, unsigned char *ec)
+{
+	return (ec[0] == 0x88 && ec[1] == 0x08);
+}
+
+static inline bool alps_match_v3(unsigned char *e7, unsigned char *ec)
+{
+	return (ec[0] == 0x88 && ec[1] == 0x07 &&
+		ec[2] >= 0x90 && ec[2] <= 0x9d);
+}
+
+static inline bool alps_match_any(unsigned char *e7, unsigned char *ec)
+{
+	return (alps_match_table(e7, ec) ||
+		alps_match_v5(e7, ec) ||
+		alps_match_v7(e7, ec) ||
+		alps_match_rushmore_v3(e7, ec) ||
+		alps_match_v3(e7, ec));
 }
 
 static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 {
 	unsigned char e6[4], e7[4], ec[4];
+	const struct alps_model_info *model;
 
 	/*
 	 * First try "E6 report".
@@ -2217,40 +2241,46 @@ 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 (priv) {
 
-	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);
+		/* Save the Firmware version */
+		memcpy(priv->fw_ver, ec, 3);
 
-		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;
-		alps_set_defaults(priv);
+		if ((model = alps_match_table(e7, ec))) {
+			priv->proto_version = model->proto_version;
+			alps_set_defaults(priv);
+			priv->flags = model->flags;
+			priv->byte0 = model->byte0;
+			priv->mask0 = model->mask0;
+			return 0;
+		} else if (alps_match_v5(e7, ec)) {
+			priv->proto_version = ALPS_PROTO_V5;
+			alps_set_defaults(priv);
+			return 0;
+		} else if (alps_match_v7(e7, ec)) {
+			priv->proto_version = ALPS_PROTO_V7;
+			alps_set_defaults(priv);
+			return 0;
+		} else if (alps_match_rushmore_v3(e7, ec)) {
+			priv->proto_version = ALPS_PROTO_V3;
+			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;
+			return 0;
+		} else if (alps_match_v3(e7, ec)) {
+			priv->proto_version = ALPS_PROTO_V3;
+			alps_set_defaults(priv);
+			return 0;
+		}
 
-		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;
+	} else {
 
-		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);
+		if (alps_match_any(e7, ec))
+			return 0;
 
-		return 0;
 	}
 
 	psmouse_dbg(psmouse,
@@ -2427,9 +2457,7 @@ init_fail:
 
 int alps_detect(struct psmouse *psmouse, bool set_properties)
 {
-	struct alps_data dummy;
-
-	if (alps_identify(psmouse, &dummy) < 0)
+	if (alps_identify(psmouse, NULL) < 0)
 		return -1;
 
 	if (set_properties) {
-- 
1.7.9.5

[PATCH 5/7] input: alps: Fix name, product and version of dev2 input device

[Pali Rohár]

This patch fix name, product and version of dev2 input device based on format
used in function psmouse_switch_protocol() in file psmouse-base.c.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 0176425..2b7b74d 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2407,14 +2407,24 @@ int alps_init(struct psmouse *psmouse)
 		dev1->keybit[BIT_WORD(BTN_MIDDLE)] |= BIT_MASK(BTN_MIDDLE);
 	}
 
+	if (priv->flags & ALPS_DUALPOINT) {
+		/*
+		 * 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.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->name = (priv->flags & ALPS_DUALPOINT) ?
-		     "DualPoint Stick" : "ALPS PS/2 Device";
 	dev2->id.bustype = BUS_I8042;
 	dev2->id.vendor  = 0x0002;
-	dev2->id.product = PSMOUSE_ALPS;
-	dev2->id.version = 0x0000;
 	dev2->dev.parent = &psmouse->ps2dev.serio->dev;
 
 	dev2->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
-- 
1.7.9.5

[PATCH 6/7] input: alps: Add sanity checks for non DualPoint devices

[Pali Rohár]

This patch adds sanity checks and reject trackstick packets from ALPS devices
which do not have trackstick present (those without ALPS_DUALPOINT flag).

Make sure that driver does not process some bogus packets as trackstick data
when there is no trackstick packet. Patch also write warning do dmesg so
possible problems with driver (e.g received invalid data) will be visible
for debugging.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/mouse/alps.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 2b7b74d..770bec5 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -475,6 +475,13 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
 	struct input_dev *dev = priv->dev2;
 	int x, y, z, left, right, middle;
 
+	/* It should be a DualPoint when received trackstick packet */
+	if (!(priv->flags & ALPS_DUALPOINT)) {
+		psmouse_warn(psmouse,
+			"Rejected trackstick packet from non DualPoint device");
+		return;
+	}
+
 	/* Sanity check packet */
 	if (!(packet[0] & 0x40)) {
 		psmouse_dbg(psmouse, "Bad trackstick packet, discarding\n");
@@ -699,7 +706,8 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
 
 	alps_report_semi_mt_data(psmouse, fingers);
 
-	if (!(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
+	if ((priv->flags & ALPS_DUALPOINT) &&
+	    !(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
 		input_report_key(dev2, BTN_LEFT, f->ts_left);
 		input_report_key(dev2, BTN_RIGHT, f->ts_right);
 		input_report_key(dev2, BTN_MIDDLE, f->ts_middle);
@@ -743,8 +751,11 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 	 */
 	if (packet[5] == 0x7F) {
 		/* It should be a DualPoint when received Trackpoint packet */
-		if (!(priv->flags & ALPS_DUALPOINT))
+		if (!(priv->flags & ALPS_DUALPOINT)) {
+			psmouse_warn(psmouse,
+			"Rejected trackstick packet from non DualPoint device");
 			return;
+		}
 
 		/* Trackpoint packet */
 		x = packet[1] | ((packet[3] & 0x20) << 2);
@@ -962,6 +973,13 @@ static void alps_process_trackstick_packet_v7(struct psmouse *psmouse)
 	struct input_dev *dev2 = priv->dev2;
 	int x, y, z, left, right, middle;
 
+	/* It should be a DualPoint when received trackstick packet */
+	if (!(priv->flags & ALPS_DUALPOINT)) {
+		psmouse_warn(psmouse,
+			"Rejected trackstick packet from non DualPoint device");
+		return;
+	}
+
 	/*
 	 *        b7 b6 b5 b4 b3 b2 b1 b0
 	 * Byte0   0  1  0  0  1  0  0  0
-- 
1.7.9.5

[PATCH 7/7] input: alps: Do not report both trackstick and external PS/2 mouse data to one input device

[Pali Rohár]

Before this patch dev2 device was used for both external PS/2 mouse and internal
trackstick device (if available).

This patch introduce dev3 device which is used for external PS/2 mouse data and
dev2 is not 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 patch also helps identify possible problems in future if driver decide
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>
---
 drivers/input/mouse/alps.c |  163 +++++++++++++++++++++++++++++++-------------
 drivers/input/mouse/alps.h |   14 +++-
 2 files changed, 128 insertions(+), 49 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 770bec5..6b69f4b 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -150,8 +150,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;
@@ -161,20 +160,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)
@@ -221,13 +221,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)
@@ -1043,23 +1043,73 @@ 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;
+
+	mutex_lock(&alps_mutex);
+
+	if (priv->dev3)
+		goto out;
+
+	dev3 = input_allocate_device();
+	if (!dev3)
+		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;
+
+	dev3->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+	dev3->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
+	dev3->keybit[BIT_WORD(BTN_LEFT)] =
+		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
+
+	__set_bit(INPUT_PROP_POINTER, dev3->propbit);
+
+	if (input_register_device(dev3)) {
+		input_free_device(dev3);
+		goto out;
+	}
+
+	priv->dev3 = dev3;
+
+out:
+	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)
@@ -1124,8 +1174,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,
@@ -1174,12 +1224,24 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
 
-	/* FIXME: Could we receive bare PS/2 packets from DualPoint devices?? */
+	/* NOTE: Some ALPS devices do not have active multiplexing controller
+	 *       so everything comes mixed into single data stream. Which means
+	 *       also external devices plugged into PS/2 ports. If ALPS device
+	 *       is not out of sync check for bare PS/2 packet which could come
+	 *       from external PS/2 mouse.
+	 */
 	if (!psmouse->out_of_sync_cnt &&
 	    (psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
+		/* Register dev3 mouse if we received PS/2 packet first time */
+		if (!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 (priv->dev3)
+				alps_report_bare_ps2_packet(priv->dev3,
+							    psmouse->packet,
+							    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
@@ -2325,7 +2387,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 (priv->dev3)
+		input_unregister_device(priv->dev3);
 	kfree(priv);
 }
 
@@ -2359,17 +2424,17 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
 int alps_init(struct psmouse *psmouse)
 {
 	struct alps_data *priv;
-	struct input_dev *dev1 = psmouse->dev, *dev2;
+	struct input_dev *dev1 = psmouse->dev;
+	struct input_dev *dev2 = NULL;
 
 	priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL);
-	dev2 = input_allocate_device();
-	if (!priv || !dev2)
+	if (!priv)
 		goto init_fail;
 
-	priv->dev2 = dev2;
 	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
 
 	psmouse->private = priv;
+	priv->psmouse = psmouse;
 
 	psmouse_reset(psmouse);
 
@@ -2426,36 +2491,41 @@ int alps_init(struct psmouse *psmouse)
 	}
 
 	if (priv->flags & ALPS_DUALPOINT) {
+		dev2 = input_allocate_device();
+		if (!priv || !dev2)
+			goto init_fail;
+
+		priv->dev2 = dev2;
+
+		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;
-	}
+		dev2->dev.parent = &psmouse->ps2dev.serio->dev;
 
-	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->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);
 
-	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);
-
-	__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;
+		if (input_register_device(priv->dev2))
+			goto init_fail;
+	}
+
+	INIT_DELAYED_WORK(&priv->dev3_register_work, alps_register_bare_ps2_mouse);
 
 	if (!(priv->flags & ALPS_DUALPOINT))
 		psmouse->name = "GlidePoint TouchPad";
@@ -2477,7 +2547,8 @@ int alps_init(struct psmouse *psmouse)
 
 init_fail:
 	psmouse_reset(psmouse);
-	input_free_device(dev2);
+	if (dev2)
+		input_free_device(dev2);
 	kfree(priv);
 	psmouse->private = NULL;
 	return -1;
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 66240b4..8d4de04 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -132,8 +132,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.
@@ -160,8 +164,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;
-- 
1.7.9.5

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

[Dmitry Torokhov]

Hi Pali,

On Friday, November 14, 2014 08:38:19 PM Pali Rohár wrote:
> This patch series fix detection and identifying trackstick on machines with
> ALPS devices. Last patch split trackstick and bare PS/2 mouse packets
> between dev2 and dev3 input devices which make sure that driver will send
> only trackstick data to trackstick input device.
> 

Thank you for splitting the change, unfortunately it is now quite big
to apply to 3.18. Any chance you could try implementing what I suggested
in http://www.spinics.net/lists/linux-input/msg34029.html
and then we can do the more comprehensive solution in 3.19.

Thanks.

-- 
Dmitry

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

[Pali Rohár]

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

On Friday 14 November 2014 21:59:31 Dmitry Torokhov wrote:
> Hi Pali,
> 
> On Friday, November 14, 2014 08:38:19 PM Pali Rohár wrote:
> > This patch series fix detection and identifying trackstick
> > on machines with ALPS devices. Last patch split trackstick
> > and bare PS/2 mouse packets between dev2 and dev3 input
> > devices which make sure that driver will send only
> > trackstick data to trackstick input device.
> 
> Thank you for splitting the change, unfortunately it is now
> quite big to apply to 3.18. Any chance you could try
> implementing what I suggested in
> http://www.spinics.net/lists/linux-input/msg34029.html and
> then we can do the more comprehensive solution in 3.19.
> 
> Thanks.

Hello, I think that patches 1/7 and 5/7 could do that job. I did 
not tested them alone (without other patches), but if you think 
that two patches are ok for 3.18 & stable I can test them...

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

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

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

[Pali Rohár]

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

On Monday 17 November 2014 08:39:14 Pali Rohár wrote:
> On Friday 14 November 2014 21:59:31 Dmitry Torokhov wrote:
> > Hi Pali,
> > 
> > On Friday, November 14, 2014 08:38:19 PM Pali Rohár wrote:
> > > This patch series fix detection and identifying trackstick
> > > on machines with ALPS devices. Last patch split trackstick
> > > and bare PS/2 mouse packets between dev2 and dev3 input
> > > devices which make sure that driver will send only
> > > trackstick data to trackstick input device.
> > 
> > Thank you for splitting the change, unfortunately it is now
> > quite big to apply to 3.18. Any chance you could try
> > implementing what I suggested in
> > http://www.spinics.net/lists/linux-input/msg34029.html and
> > then we can do the more comprehensive solution in 3.19.
> > 
> > Thanks.
> 
> Hello, I think that patches 1/7 and 5/7 could do that job. I
> did not tested them alone (without other patches), but if you
> think that two patches are ok for 3.18 & stable I can test
> them...

Dmitry, ping.

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

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

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

[Pali Rohár]

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

On Thursday 20 November 2014 00:29:49 Pali Rohár wrote:
> On Monday 17 November 2014 08:39:14 Pali Rohár wrote:
> > On Friday 14 November 2014 21:59:31 Dmitry Torokhov wrote:
> > > Hi Pali,
> > > 
> > > On Friday, November 14, 2014 08:38:19 PM Pali Rohár wrote:
> > > > This patch series fix detection and identifying
> > > > trackstick on machines with ALPS devices. Last patch
> > > > split trackstick and bare PS/2 mouse packets between
> > > > dev2 and dev3 input devices which make sure that driver
> > > > will send only trackstick data to trackstick input
> > > > device.
> > > 
> > > Thank you for splitting the change, unfortunately it is
> > > now quite big to apply to 3.18. Any chance you could try
> > > implementing what I suggested in
> > > http://www.spinics.net/lists/linux-input/msg34029.html and
> > > then we can do the more comprehensive solution in 3.19.
> > > 
> > > Thanks.
> > 
> > Hello, I think that patches 1/7 and 5/7 could do that job. I
> > did not tested them alone (without other patches), but if
> > you think that two patches are ok for 3.18 & stable I can
> > test them...
> 
> Dmitry, ping.

Dmitry: ping again.

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

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

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

[Dmitry Torokhov]

On November 25, 2014 3:08:31 AM PST, "Pali Rohár" <pali.rohar@gmail.com> wrote:
>On Thursday 20 November 2014 00:29:49 Pali Rohár wrote:
>> On Monday 17 November 2014 08:39:14 Pali Rohár wrote:
>> > On Friday 14 November 2014 21:59:31 Dmitry Torokhov wrote:
>> > > Hi Pali,
>> > > 
>> > > On Friday, November 14, 2014 08:38:19 PM Pali Rohár wrote:
>> > > > This patch series fix detection and identifying
>> > > > trackstick on machines with ALPS devices. Last patch
>> > > > split trackstick and bare PS/2 mouse packets between
>> > > > dev2 and dev3 input devices which make sure that driver
>> > > > will send only trackstick data to trackstick input
>> > > > device.
>> > > 
>> > > Thank you for splitting the change, unfortunately it is
>> > > now quite big to apply to 3.18. Any chance you could try
>> > > implementing what I suggested in
>> > > http://www.spinics.net/lists/linux-input/msg34029.html and
>> > > then we can do the more comprehensive solution in 3.19.
>> > > 
>> > > Thanks.
>> > 
>> > Hello, I think that patches 1/7 and 5/7 could do that job. I
>> > did not tested them alone (without other patches), but if
>> > you think that two patches are ok for 3.18 & stable I can
>> > test them...
>> 
>> Dmitry, ping.
>
>Dmitry: ping again.

Hi Pali,

I'm on vacation and unfortunately connection here is horrendous so I likely won't be able to do anything until after 12/03.

Sorry about that.


-- 
Dmitry

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

[Pali Rohár]

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

On Thursday 27 November 2014 19:08:04 Dmitry Torokhov wrote:
> On November 25, 2014 3:08:31 AM PST, "Pali Rohár" 
<pali.rohar@gmail.com> wrote:
> >On Thursday 20 November 2014 00:29:49 Pali Rohár wrote:
> >> On Monday 17 November 2014 08:39:14 Pali Rohár wrote:
> >> > On Friday 14 November 2014 21:59:31 Dmitry Torokhov wrote:
> >> > > Hi Pali,
> >> > > 
> >> > > On Friday, November 14, 2014 08:38:19 PM Pali Rohár 
wrote:
> >> > > > This patch series fix detection and identifying
> >> > > > trackstick on machines with ALPS devices. Last patch
> >> > > > split trackstick and bare PS/2 mouse packets between
> >> > > > dev2 and dev3 input devices which make sure that
> >> > > > driver will send only trackstick data to trackstick
> >> > > > input device.
> >> > > 
> >> > > Thank you for splitting the change, unfortunately it is
> >> > > now quite big to apply to 3.18. Any chance you could
> >> > > try implementing what I suggested in
> >> > > http://www.spinics.net/lists/linux-input/msg34029.html
> >> > > and then we can do the more comprehensive solution in
> >> > > 3.19.
> >> > > 
> >> > > Thanks.
> >> > 
> >> > Hello, I think that patches 1/7 and 5/7 could do that
> >> > job. I did not tested them alone (without other
> >> > patches), but if you think that two patches are ok for
> >> > 3.18 & stable I can test them...
> >> 
> >> Dmitry, ping.
> >
> >Dmitry: ping again.
> 
> Hi Pali,
> 
> I'm on vacation and unfortunately connection here is
> horrendous so I likely won't be able to do anything until
> after 12/03.
> 
> Sorry about that.

Hi Dmitry,

I tested that two patches 1/7 and 5/7 on top of 3.18 and it fixed 
name of ALPS devices. Can you send those two patches to 3.18  
queue (just remove comment about speed as without other patches 
duplicate detection is still called)?

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

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

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

[Pali Rohár]

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

On Friday 14 November 2014 20:38:19 Pali Rohár wrote:
> This patch series fix detection and identifying trackstick on
> machines with ALPS devices. Last patch split trackstick and
> bare PS/2 mouse packets between dev2 and dev3 input devices
> which make sure that driver will send only trackstick data to
> trackstick input device.
> 
> Pali Rohár (7):
>   input: alps: Set correct name of psmouse device in
> alps_init() input: alps: Move trackstick detection to
> alps_hw_init_* input: alps: Move alps_dolphin_get_device_area
> into alps_hw_init_dolphin_v1
>   input: alps: Use NULL instead dummy argument for
> alps_identify input: alps: Fix name, product and version of
> dev2 input device input: alps: Add sanity checks for non
> DualPoint devices input: alps: Do not report both trackstick
> and external PS/2 mouse data to one input device
> 
>  drivers/input/mouse/alps.c |  361
> +++++++++++++++++++++++++++++++-------------
> drivers/input/mouse/alps.h |   14 +-
>  2 files changed, 264 insertions(+), 111 deletions(-)

Hi Dmitry, now when 3.18 was released, can you review this patch 
series for 3.19?

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

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

Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Dmitry Torokhov]

Hi Pali,

On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár wrote:
> On some laptops after starting them from off state (not after reboot), function
> alps_probe_trackstick_v3() (called from function alps_identify()) does not
> detect trackstick. To fix this problem we need to reset device. But function
> alps_identify() is called also from alps_detect() and we do not want to reset
> device in detect function because it will slow down initialization of all other
> non alps devices.
> 
> This patch moves code for setting correct device name & protocol from function
> alps_detect() to alps_init() which already doing full device reset.
> 
> So this patch removes need to do trackstick detection in alps_detect() function.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/input/mouse/alps.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 8d85c79..9ffa98d 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse *psmouse)
>  	if (input_register_device(priv->dev2))
>  		goto init_fail;
>  
> +	if (!(priv->flags & ALPS_DUALPOINT))
> +		psmouse->name = "GlidePoint TouchPad";
> +	psmouse->model = priv->proto_version;
> +
>  	psmouse->protocol_handler = alps_process_byte;
>  	psmouse->poll = alps_poll;
>  	psmouse->disconnect = alps_disconnect;
> @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
>  		return -1;
>  
>  	if (set_properties) {
> +		/*
> +		 * NOTE: To detect model and trackstick presence we need to do
> +		 *       full device reset. To speed up detection and prevent
> +		 *       calling duplicate initialization sequence (both in
> +		 *       alps_detect() and alps_init()) we set model/protocol
> +		 *       version and correct name in alps_init() (which will
> +		 *       do full device reset). For now set name to DualPoint.
> +		 */
>  		psmouse->vendor = "ALPS";
> -		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> -				"DualPoint TouchPad" : "GlidePoint";
> -		psmouse->model = dummy.proto_version << 8;
> +		psmouse->name = "DualPoint TouchPad";
>  	}
> +

I do not quite like the way we change the device description back and
forth. Do you think we could allocate the "real" priv structure in
alps_detect() and have alps_init() expect to find it (and free it if
set_properties is false). This way we'd go through initialization once
in detect, it will be authoritative, and we would set the name of the
device properly from the beginning.

Thanks.

-- 
Dmitry

Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Pali Rohár]

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

On Tuesday 16 December 2014 06:02:34 Dmitry Torokhov wrote:
> Hi Pali,
> 
> On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár wrote:
> > On some laptops after starting them from off state (not
> > after reboot), function alps_probe_trackstick_v3() (called
> > from function alps_identify()) does not detect trackstick.
> > To fix this problem we need to reset device. But function
> > alps_identify() is called also from alps_detect() and we do
> > not want to reset device in detect function because it will
> > slow down initialization of all other non alps devices.
> > 
> > This patch moves code for setting correct device name &
> > protocol from function alps_detect() to alps_init() which
> > already doing full device reset.
> > 
> > So this patch removes need to do trackstick detection in
> > alps_detect() function.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> > 
> >  drivers/input/mouse/alps.c |   17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/alps.c
> > b/drivers/input/mouse/alps.c index 8d85c79..9ffa98d 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse
> > *psmouse)
> > 
> >  	if (input_register_device(priv->dev2))
> >  	
> >  		goto init_fail;
> > 
> > +	if (!(priv->flags & ALPS_DUALPOINT))
> > +		psmouse->name = "GlidePoint TouchPad";
> > +	psmouse->model = priv->proto_version;
> > +
> > 
> >  	psmouse->protocol_handler = alps_process_byte;
> >  	psmouse->poll = alps_poll;
> >  	psmouse->disconnect = alps_disconnect;
> > 
> > @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse
> > *psmouse, bool set_properties)
> > 
> >  		return -1;
> >  	
> >  	if (set_properties) {
> > 
> > +		/*
> > +		 * NOTE: To detect model and trackstick presence we 
need
> > to do +		 *       full device reset. To speed up 
detection
> > and prevent +		 *       calling duplicate initialization
> > sequence (both in +		 *       alps_detect() and
> > alps_init()) we set model/protocol +		 *       version and
> > correct name in alps_init() (which will +		 *       do 
full
> > device reset). For now set name to DualPoint. +		 */
> > 
> >  		psmouse->vendor = "ALPS";
> > 
> > -		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> > -				"DualPoint TouchPad" : "GlidePoint";
> > -		psmouse->model = dummy.proto_version << 8;
> > +		psmouse->name = "DualPoint TouchPad";
> > 
> >  	}
> > 
> > +
> 
> I do not quite like the way we change the device description
> back and forth. Do you think we could allocate the "real"
> priv structure in alps_detect() and have alps_init() expect
> to find it (and free it if set_properties is false). This way
> we'd go through initialization once in detect, it will be
> authoritative, and we would set the name of the device
> properly from the beginning.
> 
> Thanks.

No without introducing another psmouse_reset call. I want to 
reduce time of loading driver, so I think this is better.

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

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

Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Pali Rohár]

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

On Tuesday 16 December 2014 12:58:20 Pali Rohár wrote:
> On Tuesday 16 December 2014 06:02:34 Dmitry Torokhov wrote:
> > Hi Pali,
> > 
> > On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár wrote:
> > > On some laptops after starting them from off state (not
> > > after reboot), function alps_probe_trackstick_v3() (called
> > > from function alps_identify()) does not detect trackstick.
> > > To fix this problem we need to reset device. But function
> > > alps_identify() is called also from alps_detect() and we
> > > do not want to reset device in detect function because it
> > > will slow down initialization of all other non alps
> > > devices.
> > > 
> > > This patch moves code for setting correct device name &
> > > protocol from function alps_detect() to alps_init() which
> > > already doing full device reset.
> > > 
> > > So this patch removes need to do trackstick detection in
> > > alps_detect() function.
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > ---
> > > 
> > >  drivers/input/mouse/alps.c |   17 ++++++++++++++---
> > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/input/mouse/alps.c
> > > b/drivers/input/mouse/alps.c index 8d85c79..9ffa98d 100644
> > > --- a/drivers/input/mouse/alps.c
> > > +++ b/drivers/input/mouse/alps.c
> > > @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse
> > > *psmouse)
> > > 
> > >  	if (input_register_device(priv->dev2))
> > >  	
> > >  		goto init_fail;
> > > 
> > > +	if (!(priv->flags & ALPS_DUALPOINT))
> > > +		psmouse->name = "GlidePoint TouchPad";
> > > +	psmouse->model = priv->proto_version;
> > > +
> > > 
> > >  	psmouse->protocol_handler = alps_process_byte;
> > >  	psmouse->poll = alps_poll;
> > >  	psmouse->disconnect = alps_disconnect;
> > > 
> > > @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse
> > > *psmouse, bool set_properties)
> > > 
> > >  		return -1;
> > >  	
> > >  	if (set_properties) {
> > > 
> > > +		/*
> > > +		 * NOTE: To detect model and trackstick presence we
> 
> need
> 
> > > to do +		 *       full device reset. To speed up
> 
> detection
> 
> > > and prevent +		 *       calling duplicate 
initialization
> > > sequence (both in +		 *       alps_detect() and
> > > alps_init()) we set model/protocol +		 *       version 
and
> > > correct name in alps_init() (which will +		 *       do
> 
> full
> 
> > > device reset). For now set name to DualPoint. +		 */
> > > 
> > >  		psmouse->vendor = "ALPS";
> > > 
> > > -		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> > > -				"DualPoint TouchPad" : "GlidePoint";
> > > -		psmouse->model = dummy.proto_version << 8;
> > > +		psmouse->name = "DualPoint TouchPad";
> > > 
> > >  	}
> > > 
> > > +
> > 
> > I do not quite like the way we change the device description
> > back and forth. Do you think we could allocate the "real"
> > priv structure in alps_detect() and have alps_init() expect
> > to find it (and free it if set_properties is false). This
> > way we'd go through initialization once in detect, it will
> > be authoritative, and we would set the name of the device
> > properly from the beginning.
> > 
> > Thanks.
> 
> No without introducing another psmouse_reset call. I want to
> reduce time of loading driver, so I think this is better.

Also psmouse-base.c call psmouse_reset between alps_detect() and 
alps_init(). We must do trackstick detection after psmouse_reset, 
but intorducing another psmouse_reset in alps_detect() will slow 
down initialization for ALPS devices. Also do not remember that 
psmouse_reset will lock both keyboard & mouse for one second 
(sometimes for two) on my laptop. I played with this and I think 
my patch is good approach how to do trackstick detection without 
slowdown or other side effects.

Also we do not need to know if ALPS touchpad has trackstick 
device in alps_detect() phase. In alps_detect() we just need to 
know if PS/2 device is ALPS or not. From psmouse/serio point of 
view there is only one ALPS device on PS/2/i8042 bus. Just 
alps.ko driver parse PS/2 packets and detect which packet has 
touchpad header and which trackpoint. So we really do not need to 
know if trackstick is there or not in alps_detect() function.

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

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

Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Pali Rohár]

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

On Saturday 20 December 2014 09:53:38 Pali Rohár wrote:
> On Tuesday 16 December 2014 12:58:20 Pali Rohár wrote:
> > On Tuesday 16 December 2014 06:02:34 Dmitry Torokhov wrote:
> > > Hi Pali,
> > > 
> > > On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár wrote:
> > > > On some laptops after starting them from off state (not
> > > > after reboot), function alps_probe_trackstick_v3()
> > > > (called from function alps_identify()) does not detect
> > > > trackstick. To fix this problem we need to reset
> > > > device. But function alps_identify() is called also
> > > > from alps_detect() and we do not want to reset device
> > > > in detect function because it will slow down
> > > > initialization of all other non alps devices.
> > > > 
> > > > This patch moves code for setting correct device name &
> > > > protocol from function alps_detect() to alps_init()
> > > > which already doing full device reset.
> > > > 
> > > > So this patch removes need to do trackstick detection in
> > > > alps_detect() function.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > ---
> > > > 
> > > >  drivers/input/mouse/alps.c |   17 ++++++++++++++---
> > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/input/mouse/alps.c
> > > > b/drivers/input/mouse/alps.c index 8d85c79..9ffa98d
> > > > 100644 --- a/drivers/input/mouse/alps.c
> > > > +++ b/drivers/input/mouse/alps.c
> > > > @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse
> > > > *psmouse)
> > > > 
> > > >  	if (input_register_device(priv->dev2))
> > > >  	
> > > >  		goto init_fail;
> > > > 
> > > > +	if (!(priv->flags & ALPS_DUALPOINT))
> > > > +		psmouse->name = "GlidePoint TouchPad";
> > > > +	psmouse->model = priv->proto_version;
> > > > +
> > > > 
> > > >  	psmouse->protocol_handler = alps_process_byte;
> > > >  	psmouse->poll = alps_poll;
> > > >  	psmouse->disconnect = alps_disconnect;
> > > > 
> > > > @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse
> > > > *psmouse, bool set_properties)
> > > > 
> > > >  		return -1;
> > > >  	
> > > >  	if (set_properties) {
> > > > 
> > > > +		/*
> > > > +		 * NOTE: To detect model and trackstick presence we
> > 
> > need
> > 
> > > > to do +		 *       full device reset. To speed up
> > 
> > detection
> > 
> > > > and prevent +		 *       calling duplicate
> 
> initialization
> 
> > > > sequence (both in +		 *       alps_detect() and
> > > > alps_init()) we set model/protocol +		 *       version
> 
> and
> 
> > > > correct name in alps_init() (which will +		 *       
do
> > 
> > full
> > 
> > > > device reset). For now set name to DualPoint. +		 */
> > > > 
> > > >  		psmouse->vendor = "ALPS";
> > > > 
> > > > -		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> > > > -				"DualPoint TouchPad" : "GlidePoint";
> > > > -		psmouse->model = dummy.proto_version << 8;
> > > > +		psmouse->name = "DualPoint TouchPad";
> > > > 
> > > >  	}
> > > > 
> > > > +
> > > 
> > > I do not quite like the way we change the device
> > > description back and forth. Do you think we could
> > > allocate the "real" priv structure in alps_detect() and
> > > have alps_init() expect to find it (and free it if
> > > set_properties is false). This way we'd go through
> > > initialization once in detect, it will be authoritative,
> > > and we would set the name of the device properly from the
> > > beginning.
> > > 
> > > Thanks.
> > 
> > No without introducing another psmouse_reset call. I want to
> > reduce time of loading driver, so I think this is better.
> 
> Also psmouse-base.c call psmouse_reset between alps_detect()
> and alps_init(). We must do trackstick detection after
> psmouse_reset, but intorducing another psmouse_reset in
> alps_detect() will slow down initialization for ALPS devices.
> Also do not remember that psmouse_reset will lock both
> keyboard & mouse for one second (sometimes for two) on my
> laptop. I played with this and I think my patch is good
> approach how to do trackstick detection without slowdown or
> other side effects.
> 
> Also we do not need to know if ALPS touchpad has trackstick
> device in alps_detect() phase. In alps_detect() we just need
> to know if PS/2 device is ALPS or not. From psmouse/serio
> point of view there is only one ALPS device on PS/2/i8042
> bus. Just alps.ko driver parse PS/2 packets and detect which
> packet has touchpad header and which trackpoint. So we really
> do not need to know if trackstick is there or not in
> alps_detect() function.

Dmitry ping, can you finally review this patch series? I it there 
for more then month...

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

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

Re: [PATCH 1/7] input: alps: Set correct name of psmouse device in alps_init()

[Pali Rohár]

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

On Wednesday 24 December 2014 08:48:42 Pali Rohár wrote:
> On Saturday 20 December 2014 09:53:38 Pali Rohár wrote:
> > On Tuesday 16 December 2014 12:58:20 Pali Rohár wrote:
> > > On Tuesday 16 December 2014 06:02:34 Dmitry Torokhov wrote:
> > > > Hi Pali,
> > > > 
> > > > On Fri, Nov 14, 2014 at 08:38:20PM +0100, Pali Rohár 
wrote:
> > > > > On some laptops after starting them from off state
> > > > > (not after reboot), function
> > > > > alps_probe_trackstick_v3() (called from function
> > > > > alps_identify()) does not detect trackstick. To fix
> > > > > this problem we need to reset device. But function
> > > > > alps_identify() is called also from alps_detect() and
> > > > > we do not want to reset device in detect function
> > > > > because it will slow down
> > > > > initialization of all other non alps devices.
> > > > > 
> > > > > This patch moves code for setting correct device name
> > > > > & protocol from function alps_detect() to alps_init()
> > > > > which already doing full device reset.
> > > > > 
> > > > > So this patch removes need to do trackstick detection
> > > > > in alps_detect() function.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > ---
> > > > > 
> > > > >  drivers/input/mouse/alps.c |   17 ++++++++++++++---
> > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/input/mouse/alps.c
> > > > > b/drivers/input/mouse/alps.c index 8d85c79..9ffa98d
> > > > > 100644 --- a/drivers/input/mouse/alps.c
> > > > > +++ b/drivers/input/mouse/alps.c
> > > > > @@ -2392,6 +2392,10 @@ int alps_init(struct psmouse
> > > > > *psmouse)
> > > > > 
> > > > >  	if (input_register_device(priv->dev2))
> > > > >  	
> > > > >  		goto init_fail;
> > > > > 
> > > > > +	if (!(priv->flags & ALPS_DUALPOINT))
> > > > > +		psmouse->name = "GlidePoint TouchPad";
> > > > > +	psmouse->model = priv->proto_version;
> > > > > +
> > > > > 
> > > > >  	psmouse->protocol_handler = alps_process_byte;
> > > > >  	psmouse->poll = alps_poll;
> > > > >  	psmouse->disconnect = alps_disconnect;
> > > > > 
> > > > > @@ -2422,11 +2426,18 @@ int alps_detect(struct psmouse
> > > > > *psmouse, bool set_properties)
> > > > > 
> > > > >  		return -1;
> > > > >  	
> > > > >  	if (set_properties) {
> > > > > 
> > > > > +		/*
> > > > > +		 * NOTE: To detect model and trackstick 
presence we
> > > 
> > > need
> > > 
> > > > > to do +		 *       full device reset. To speed up
> > > 
> > > detection
> > > 
> > > > > and prevent +		 *       calling duplicate
> > 
> > initialization
> > 
> > > > > sequence (both in +		 *       alps_detect() and
> > > > > alps_init()) we set model/protocol +		 *       
version
> > 
> > and
> > 
> > > > > correct name in alps_init() (which will +		 *
> 
> do
> 
> > > full
> > > 
> > > > > device reset). For now set name to DualPoint. +		 */
> > > > > 
> > > > >  		psmouse->vendor = "ALPS";
> > > > > 
> > > > > -		psmouse->name = dummy.flags & ALPS_DUALPOINT ?
> > > > > -				"DualPoint TouchPad" : "GlidePoint";
> > > > > -		psmouse->model = dummy.proto_version << 8;
> > > > > +		psmouse->name = "DualPoint TouchPad";
> > > > > 
> > > > >  	}
> > > > > 
> > > > > +
> > > > 
> > > > I do not quite like the way we change the device
> > > > description back and forth. Do you think we could
> > > > allocate the "real" priv structure in alps_detect() and
> > > > have alps_init() expect to find it (and free it if
> > > > set_properties is false). This way we'd go through
> > > > initialization once in detect, it will be authoritative,
> > > > and we would set the name of the device properly from
> > > > the beginning.
> > > > 
> > > > Thanks.
> > > 
> > > No without introducing another psmouse_reset call. I want
> > > to reduce time of loading driver, so I think this is
> > > better.
> > 
> > Also psmouse-base.c call psmouse_reset between alps_detect()
> > and alps_init(). We must do trackstick detection after
> > psmouse_reset, but intorducing another psmouse_reset in
> > alps_detect() will slow down initialization for ALPS
> > devices. Also do not remember that psmouse_reset will lock
> > both keyboard & mouse for one second (sometimes for two) on
> > my laptop. I played with this and I think my patch is good
> > approach how to do trackstick detection without slowdown or
> > other side effects.
> > 
> > Also we do not need to know if ALPS touchpad has trackstick
> > device in alps_detect() phase. In alps_detect() we just need
> > to know if PS/2 device is ALPS or not. From psmouse/serio
> > point of view there is only one ALPS device on PS/2/i8042
> > bus. Just alps.ko driver parse PS/2 packets and detect which
> > packet has touchpad header and which trackpoint. So we
> > really do not need to know if trackstick is there or not in
> > alps_detect() function.
> 
> Dmitry ping, can you finally review this patch series? I it
> there for more then month...

Dmitry ping, see ^^^^^

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

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

Re: [PATCH 5/7] input: alps: Fix name, product and version of dev2 input device

[Dmitry Torokhov]

On Fri, Nov 14, 2014 at 08:38:24PM +0100, Pali Rohár wrote:
> This patch fix name, product and version of dev2 input device based on format
> used in function psmouse_switch_protocol() in file psmouse-base.c.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Applied, thank you.

> ---
>  drivers/input/mouse/alps.c |   18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 0176425..2b7b74d 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2407,14 +2407,24 @@ int alps_init(struct psmouse *psmouse)
>  		dev1->keybit[BIT_WORD(BTN_MIDDLE)] |= BIT_MASK(BTN_MIDDLE);
>  	}
>  
> +	if (priv->flags & ALPS_DUALPOINT) {
> +		/*
> +		 * 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.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->name = (priv->flags & ALPS_DUALPOINT) ?
> -		     "DualPoint Stick" : "ALPS PS/2 Device";
>  	dev2->id.bustype = BUS_I8042;
>  	dev2->id.vendor  = 0x0002;
> -	dev2->id.product = PSMOUSE_ALPS;
> -	dev2->id.version = 0x0000;
>  	dev2->dev.parent = &psmouse->ps2dev.serio->dev;
>  
>  	dev2->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
> -- 
> 1.7.9.5
> 

-- 
Dmitry

Re: [PATCH 6/7] input: alps: Add sanity checks for non DualPoint devices

[Dmitry Torokhov]

On Fri, Nov 14, 2014 at 08:38:25PM +0100, Pali Rohár wrote:
> This patch adds sanity checks and reject trackstick packets from ALPS devices
> which do not have trackstick present (those without ALPS_DUALPOINT flag).
> 
> Make sure that driver does not process some bogus packets as trackstick data
> when there is no trackstick packet. Patch also write warning do dmesg so
> possible problems with driver (e.g received invalid data) will be visible
> for debugging.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Applied, thank you.

> ---
>  drivers/input/mouse/alps.c |   22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 2b7b74d..770bec5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -475,6 +475,13 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
>  	struct input_dev *dev = priv->dev2;
>  	int x, y, z, left, right, middle;
>  
> +	/* It should be a DualPoint when received trackstick packet */
> +	if (!(priv->flags & ALPS_DUALPOINT)) {
> +		psmouse_warn(psmouse,
> +			"Rejected trackstick packet from non DualPoint device");
> +		return;
> +	}
> +
>  	/* Sanity check packet */
>  	if (!(packet[0] & 0x40)) {
>  		psmouse_dbg(psmouse, "Bad trackstick packet, discarding\n");
> @@ -699,7 +706,8 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
>  
>  	alps_report_semi_mt_data(psmouse, fingers);
>  
> -	if (!(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
> +	if ((priv->flags & ALPS_DUALPOINT) &&
> +	    !(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
>  		input_report_key(dev2, BTN_LEFT, f->ts_left);
>  		input_report_key(dev2, BTN_RIGHT, f->ts_right);
>  		input_report_key(dev2, BTN_MIDDLE, f->ts_middle);
> @@ -743,8 +751,11 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
>  	 */
>  	if (packet[5] == 0x7F) {
>  		/* It should be a DualPoint when received Trackpoint packet */
> -		if (!(priv->flags & ALPS_DUALPOINT))
> +		if (!(priv->flags & ALPS_DUALPOINT)) {
> +			psmouse_warn(psmouse,
> +			"Rejected trackstick packet from non DualPoint device");
>  			return;
> +		}
>  
>  		/* Trackpoint packet */
>  		x = packet[1] | ((packet[3] & 0x20) << 2);
> @@ -962,6 +973,13 @@ static void alps_process_trackstick_packet_v7(struct psmouse *psmouse)
>  	struct input_dev *dev2 = priv->dev2;
>  	int x, y, z, left, right, middle;
>  
> +	/* It should be a DualPoint when received trackstick packet */
> +	if (!(priv->flags & ALPS_DUALPOINT)) {
> +		psmouse_warn(psmouse,
> +			"Rejected trackstick packet from non DualPoint device");
> +		return;
> +	}
> +
>  	/*
>  	 *        b7 b6 b5 b4 b3 b2 b1 b0
>  	 * Byte0   0  1  0  0  1  0  0  0
> -- 
> 1.7.9.5
> 

-- 
Dmitry

Re: [PATCH 6/7] input: alps: Add sanity checks for non DualPoint devices

[Pali Rohár]

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

On Monday 12 January 2015 01:31:00 Dmitry Torokhov wrote:
> On Fri, Nov 14, 2014 at 08:38:25PM +0100, Pali Rohár wrote:
> > This patch adds sanity checks and reject trackstick packets
> > from ALPS devices which do not have trackstick present
> > (those without ALPS_DUALPOINT flag).
> > 
> > Make sure that driver does not process some bogus packets as
> > trackstick data when there is no trackstick packet. Patch
> > also write warning do dmesg so possible problems with
> > driver (e.g received invalid data) will be visible for
> > debugging.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> Applied, thank you.
> 

Hello Dmitry, can you comment and review also other patches?

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

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