linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] psmouse assorted cleanups
@ 2018-01-19 23:06 Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 1/7] Input: psmouse - create helper for reporting standard buttons/motion Dmitry Torokhov
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

Hi,

Just a few cleanups to psmouse code: BIT(), sign_extend(), u8 for data,
etc. Plus a patch from Stephen Lyons to handle A4Tech mice with 2 scroll
wheels.

Thanks.

Dmitry Torokhov (6):
  Input: psmouse - create helper for reporting standard buttons/motion
  Input: psmouse - clean up code
  Input: logips2pp - clean up code
  Input: lifebook - clean up code
  Input: synaptics - switch to using input_set_capability
  Input: synaptics - handle errors from input_mt_init_slots()

Stephen Lyons (1):
  Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel
    mice

 drivers/input/mouse/alps.c         |  30 ++----
 drivers/input/mouse/elantech.c     |  28 ++----
 drivers/input/mouse/lifebook.c     |  62 +++++++------
 drivers/input/mouse/logips2pp.c    | 152 ++++++++++++++++++-------------
 drivers/input/mouse/psmouse-base.c | 181 +++++++++++++++++++++++--------------
 drivers/input/mouse/psmouse.h      |   4 +
 drivers/input/mouse/sentelic.c     |  11 +--
 drivers/input/mouse/synaptics.c    |  82 ++++++++++-------
 8 files changed, 302 insertions(+), 248 deletions(-)

-- 
Dmitry

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

* [PATCH 1/7] Input: psmouse - create helper for reporting standard buttons/motion
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 2/7] Input: psmouse - clean up code Dmitry Torokhov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

Many protocol driver re-implement code to parse buttons or motion data from
the standard PS/2 protocol. Let's split the parsing into separate
functions and reuse them in protocol drivers.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/alps.c         | 30 +++++++-----------------------
 drivers/input/mouse/elantech.c     | 28 ++++++++++------------------
 drivers/input/mouse/lifebook.c     | 12 ++++--------
 drivers/input/mouse/logips2pp.c    | 10 ++++------
 drivers/input/mouse/psmouse-base.c | 26 ++++++++++++++++++++------
 drivers/input/mouse/psmouse.h      |  4 ++++
 drivers/input/mouse/sentelic.c     | 11 +----------
 7 files changed, 50 insertions(+), 71 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index dbe57da8c1a1b..f9c7f24522644 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -827,7 +827,7 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 	unsigned char *packet = psmouse->packet;
 	struct input_dev *dev = psmouse->dev;
 	struct input_dev *dev2 = priv->dev2;
-	int x, y, z, left, right, middle;
+	int x, y, z;
 
 	/*
 	 * We can use Byte5 to distinguish if the packet is from Touchpad
@@ -847,9 +847,6 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 		x = packet[1] | ((packet[3] & 0x20) << 2);
 		y = packet[2] | ((packet[3] & 0x40) << 1);
 		z = packet[4];
-		left = packet[3] & 0x01;
-		right = packet[3] & 0x02;
-		middle = packet[3] & 0x04;
 
 		/* To prevent the cursor jump when finger lifted */
 		if (x == 0x7F && y == 0x7F && z == 0x7F)
@@ -859,9 +856,7 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X, (char)x / 4);
 		input_report_rel(dev2, REL_Y, -((char)y / 4));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		psmouse_report_standard_buttons(dev2, packet[3]);
 
 		input_sync(dev2);
 		return;
@@ -871,8 +866,6 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 	x = packet[1] | ((packet[3] & 0x78) << 4);
 	y = packet[2] | ((packet[4] & 0x78) << 4);
 	z = packet[5];
-	left = packet[3] & 0x01;
-	right = packet[3] & 0x02;
 
 	if (z > 30)
 		input_report_key(dev, BTN_TOUCH, 1);
@@ -888,8 +881,8 @@ static void alps_process_packet_v6(struct psmouse *psmouse)
 	input_report_key(dev, BTN_TOOL_FINGER, z > 0);
 
 	/* v6 touchpad does not have middle button */
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
+	packet[3] &= ~BIT(2);
+	psmouse_report_standard_buttons(dev2, packet[3]);
 
 	input_sync(dev);
 }
@@ -1098,7 +1091,7 @@ static void alps_process_trackstick_packet_v7(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 	unsigned char *packet = psmouse->packet;
 	struct input_dev *dev2 = priv->dev2;
-	int x, y, z, left, right, middle;
+	int x, y, z;
 
 	/* It should be a DualPoint when received trackstick packet */
 	if (!(priv->flags & ALPS_DUALPOINT)) {
@@ -1112,16 +1105,10 @@ static void alps_process_trackstick_packet_v7(struct psmouse *psmouse)
 	    ((packet[3] & 0x20) << 1);
 	z = (packet[5] & 0x3f) | ((packet[3] & 0x80) >> 1);
 
-	left = (packet[1] & 0x01);
-	right = (packet[1] & 0x02) >> 1;
-	middle = (packet[1] & 0x04) >> 2;
-
 	input_report_rel(dev2, REL_X, (char)x);
 	input_report_rel(dev2, REL_Y, -((char)y));
 
-	input_report_key(dev2, BTN_LEFT, left);
-	input_report_key(dev2, BTN_RIGHT, right);
-	input_report_key(dev2, BTN_MIDDLE, middle);
+	psmouse_report_standard_buttons(dev2, packet[1]);
 
 	input_sync(dev2);
 }
@@ -1503,10 +1490,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
 		alps_report_buttons(dev, dev2,
 				packet[0] & 1, packet[0] & 2, packet[0] & 4);
 
-	input_report_rel(dev, REL_X,
-		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-	input_report_rel(dev, REL_Y,
-		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+	psmouse_report_standard_motion(dev, packet);
 
 	input_sync(dev);
 }
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index a4aaa748e987f..af7fc17c14d96 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -279,8 +279,8 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	input_report_key(dev, BTN_TOOL_FINGER, fingers == 1);
 	input_report_key(dev, BTN_TOOL_DOUBLETAP, fingers == 2);
 	input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3);
-	input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
-	input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
+
+	psmouse_report_standard_buttons(dev, packet[0]);
 
 	if (etd->fw_version < 0x020000 &&
 	    (etd->capabilities[0] & ETP_CAP_HAS_ROCKER)) {
@@ -390,8 +390,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	input_report_key(dev, BTN_TOOL_DOUBLETAP, fingers == 2);
 	input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3);
 	input_report_key(dev, BTN_TOOL_QUADTAP, fingers == 4);
-	input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
-	input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
+	psmouse_report_standard_buttons(dev, packet[0]);
 	if (etd->reports_pressure) {
 		input_report_abs(dev, ABS_PRESSURE, pres);
 		input_report_abs(dev, ABS_TOOL_WIDTH, width);
@@ -434,9 +433,7 @@ static void elantech_report_trackpoint(struct psmouse *psmouse,
 		x = packet[4] - (int)((packet[1]^0x80) << 1);
 		y = (int)((packet[2]^0x80) << 1) - packet[5];
 
-		input_report_key(tp_dev, BTN_LEFT, packet[0] & 0x01);
-		input_report_key(tp_dev, BTN_RIGHT, packet[0] & 0x02);
-		input_report_key(tp_dev, BTN_MIDDLE, packet[0] & 0x04);
+		psmouse_report_standard_buttons(tp_dev, packet[0]);
 
 		input_report_rel(tp_dev, REL_X, x);
 		input_report_rel(tp_dev, REL_Y, y);
@@ -526,12 +523,10 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
 	input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3);
 
 	/* For clickpads map both buttons to BTN_LEFT */
-	if (etd->fw_version & 0x001000) {
+	if (etd->fw_version & 0x001000)
 		input_report_key(dev, BTN_LEFT, packet[0] & 0x03);
-	} else {
-		input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
-		input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
-	}
+	else
+		psmouse_report_standard_buttons(dev, packet[0]);
 
 	input_report_abs(dev, ABS_PRESSURE, pres);
 	input_report_abs(dev, ABS_TOOL_WIDTH, width);
@@ -546,13 +541,10 @@ static void elantech_input_sync_v4(struct psmouse *psmouse)
 	unsigned char *packet = psmouse->packet;
 
 	/* For clickpads map both buttons to BTN_LEFT */
-	if (etd->fw_version & 0x001000) {
+	if (etd->fw_version & 0x001000)
 		input_report_key(dev, BTN_LEFT, packet[0] & 0x03);
-	} else {
-		input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
-		input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
-		input_report_key(dev, BTN_MIDDLE, packet[0] & 0x04);
-	}
+	else
+		psmouse_report_standard_buttons(dev, packet[0]);
 
 	input_mt_report_pointer_emulation(dev, true);
 	input_sync(dev);
diff --git a/drivers/input/mouse/lifebook.c b/drivers/input/mouse/lifebook.c
index 13d324cef7df5..65efaade0820d 100644
--- a/drivers/input/mouse/lifebook.c
+++ b/drivers/input/mouse/lifebook.c
@@ -188,14 +188,10 @@ static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse)
 	}
 
 	if (dev2) {
-		if (relative_packet) {
-			input_report_rel(dev2, REL_X,
-				((packet[0] & 0x10) ? packet[1] - 256 : packet[1]));
-			input_report_rel(dev2, REL_Y,
-				 -(int)((packet[0] & 0x20) ? packet[2] - 256 : packet[2]));
-		}
-		input_report_key(dev2, BTN_LEFT, packet[0] & 0x01);
-		input_report_key(dev2, BTN_RIGHT, packet[0] & 0x02);
+		if (relative_packet)
+			psmouse_report_standard_motion(dev2, packet);
+
+		psmouse_report_standard_buttons(dev2, packet[0]);
 		input_sync(dev2);
 	}
 
diff --git a/drivers/input/mouse/logips2pp.c b/drivers/input/mouse/logips2pp.c
index ef9c97f5e3d78..b7d17db632fc0 100644
--- a/drivers/input/mouse/logips2pp.c
+++ b/drivers/input/mouse/logips2pp.c
@@ -88,16 +88,14 @@ static psmouse_ret_t ps2pp_process_byte(struct psmouse *psmouse)
 				    (packet[1] >> 4) | (packet[0] & 0x30));
 			break;
 		}
+
+		psmouse_report_standard_buttons(dev, packet[0]);
+
 	} else {
 		/* Standard PS/2 motion data */
-		input_report_rel(dev, REL_X, packet[1] ? (int) packet[1] - (int) ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev, REL_Y, packet[2] ? (int) ((packet[0] << 3) & 0x100) - (int) packet[2] : 0);
+		psmouse_report_standard_packet(dev, packet);
 	}
 
-	input_report_key(dev, BTN_LEFT,    packet[0]       & 1);
-	input_report_key(dev, BTN_MIDDLE, (packet[0] >> 2) & 1);
-	input_report_key(dev, BTN_RIGHT,  (packet[0] >> 1) & 1);
-
 	input_sync(dev);
 
 	return PSMOUSE_FULL_PACKET;
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 8ac9e03c05b45..19f7727555911 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -116,13 +116,30 @@ static DEFINE_MUTEX(psmouse_mutex);
 
 static struct workqueue_struct *kpsmoused_wq;
 
-static void psmouse_report_standard_buttons(struct input_dev *dev, u8 buttons)
+void psmouse_report_standard_buttons(struct input_dev *dev, u8 buttons)
 {
 	input_report_key(dev, BTN_LEFT,   buttons & BIT(0));
 	input_report_key(dev, BTN_MIDDLE, buttons & BIT(2));
 	input_report_key(dev, BTN_RIGHT,  buttons & BIT(1));
 }
 
+void psmouse_report_standard_motion(struct input_dev *dev, u8 *packet)
+{
+	int x, y;
+
+	x = packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0;
+	y = packet[2] ? packet[2] - ((packet[0] << 3) & 0x100) : 0;
+
+	input_report_rel(dev, REL_X, x);
+	input_report_rel(dev, REL_Y, -y);
+}
+
+void psmouse_report_standard_packet(struct input_dev *dev, u8 *packet)
+{
+	psmouse_report_standard_buttons(dev, packet[0]);
+	psmouse_report_standard_motion(dev, packet);
+}
+
 /*
  * psmouse_process_byte() analyzes the PS/2 data stream and reports
  * relevant events to the input module once full packet has arrived.
@@ -195,11 +212,8 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 	}
 
 	/* Generic PS/2 Mouse */
-	psmouse_report_standard_buttons(dev,
-					packet[0] | psmouse->extra_buttons);
-
-	input_report_rel(dev, REL_X, packet[1] ? (int) packet[1] - (int) ((packet[0] << 4) & 0x100) : 0);
-	input_report_rel(dev, REL_Y, packet[2] ? (int) ((packet[0] << 3) & 0x100) - (int) packet[2] : 0);
+	packet[0] |= psmouse->extra_buttons;
+	psmouse_report_standard_packet(dev, packet);
 
 	input_sync(dev);
 
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 8cd453808cc7d..8bc99691494e9 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -140,6 +140,10 @@ int psmouse_activate(struct psmouse *psmouse);
 int psmouse_deactivate(struct psmouse *psmouse);
 bool psmouse_matches_pnp_id(struct psmouse *psmouse, const char * const ids[]);
 
+void psmouse_report_standard_buttons(struct input_dev *, u8 buttons);
+void psmouse_report_standard_motion(struct input_dev *, u8 *packet);
+void psmouse_report_standard_packet(struct input_dev *, u8 *packet);
+
 struct psmouse_attribute {
 	struct device_attribute dattr;
 	void *data;
diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index 11c32ac8234b2..1d6010d463e2c 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c
@@ -710,7 +710,6 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 	unsigned char *packet = psmouse->packet;
 	unsigned char button_status = 0, lscroll = 0, rscroll = 0;
 	unsigned short abs_x, abs_y, fgrs = 0;
-	int rel_x, rel_y;
 
 	if (psmouse->pktcnt < 4)
 		return PSMOUSE_GOOD_DATA;
@@ -840,15 +839,7 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 		/*
 		 * Standard PS/2 Mouse
 		 */
-		input_report_key(dev, BTN_LEFT, packet[0] & 1);
-		input_report_key(dev, BTN_MIDDLE, (packet[0] >> 2) & 1);
-		input_report_key(dev, BTN_RIGHT, (packet[0] >> 1) & 1);
-
-		rel_x = packet[1] ? (int)packet[1] - (int)((packet[0] << 4) & 0x100) : 0;
-		rel_y = packet[2] ? (int)((packet[0] << 3) & 0x100) - (int)packet[2] : 0;
-
-		input_report_rel(dev, REL_X, rel_x);
-		input_report_rel(dev, REL_Y, rel_y);
+		psmouse_report_standard_packet(dev, packet);
 		break;
 	}
 
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 2/7] Input: psmouse - clean up code
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 1/7] Input: psmouse - create helper for reporting standard buttons/motion Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  2018-06-25 18:35   ` Jiri Slaby
  2018-01-19 23:06 ` [PATCH 3/7] Input: logips2pp " Dmitry Torokhov
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

- switch to using BIT() macros
- use u8 instead of unsigned char for byte data
- use input_set_capability() instead of manipulating capabilities bits
  directly
- use sign_extend32() when extracting wheel data.
- do not abuse -1 as error code, propagate errors from various calls.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/psmouse-base.c | 140 ++++++++++++++++++++-----------------
 1 file changed, 77 insertions(+), 63 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 19f7727555911..cbca668bb931f 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -14,6 +14,7 @@
 #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
 #define psmouse_fmt(fmt)	fmt
 
+#include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -23,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/libps2.h>
 #include <linux/mutex.h>
+#include <linux/types.h>
 
 #include "psmouse.h"
 #include "synaptics.h"
@@ -147,7 +149,7 @@ void psmouse_report_standard_packet(struct input_dev *dev, u8 *packet)
 psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 {
 	struct input_dev *dev = psmouse->dev;
-	unsigned char *packet = psmouse->packet;
+	u8 *packet = psmouse->packet;
 
 	if (psmouse->pktcnt < psmouse->pktsize)
 		return PSMOUSE_GOOD_DATA;
@@ -157,39 +159,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 	switch (psmouse->protocol->type) {
 	case PSMOUSE_IMPS:
 		/* IntelliMouse has scroll wheel */
-		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
+		input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
 		break;
 
 	case PSMOUSE_IMEX:
 		/* Scroll wheel and buttons on IntelliMouse Explorer */
 		switch (packet[3] & 0xC0) {
 		case 0x80: /* vertical scroll on IntelliMouse Explorer 4.0 */
-			input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 32) - (int) (packet[3] & 31));
+			input_report_rel(dev, REL_WHEEL,
+					 -sign_extend32(packet[3], 5));
 			break;
 		case 0x40: /* horizontal scroll on IntelliMouse Explorer 4.0 */
-			input_report_rel(dev, REL_HWHEEL, (int) (packet[3] & 32) - (int) (packet[3] & 31));
+			input_report_rel(dev, REL_HWHEEL,
+					 -sign_extend32(packet[3], 5));
 			break;
 		case 0x00:
 		case 0xC0:
-			input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 8) - (int) (packet[3] & 7));
-			input_report_key(dev, BTN_SIDE, (packet[3] >> 4) & 1);
-			input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
+			input_report_rel(dev, REL_WHEEL,
+					 -sign_extend32(packet[3], 3));
+			input_report_key(dev, BTN_SIDE,  BIT(4));
+			input_report_key(dev, BTN_EXTRA, BIT(5));
 			break;
 		}
 		break;
 
 	case PSMOUSE_GENPS:
 		/* Report scroll buttons on NetMice */
-		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
+		input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
 
 		/* Extra buttons on Genius NewNet 3D */
-		input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
-		input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
+		input_report_key(dev, BTN_SIDE,  BIT(6));
+		input_report_key(dev, BTN_EXTRA, BIT(7));
 		break;
 
 	case PSMOUSE_THINKPS:
 		/* Extra button on ThinkingMouse */
-		input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
+		input_report_key(dev, BTN_EXTRA, BIT(3));
 
 		/*
 		 * Without this bit of weirdness moving up gives wildly
@@ -203,8 +208,8 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 		 * Cortron PS2 Trackball reports SIDE button in the
 		 * 4th bit of the first byte.
 		 */
-		input_report_key(dev, BTN_SIDE, (packet[0] >> 3) & 1);
-		packet[0] |= 0x08;
+		input_report_key(dev, BTN_SIDE, BIT(3));
+		packet[0] |= BIT(3);
 		break;
 
 	default:
@@ -269,7 +274,7 @@ static int psmouse_handle_byte(struct psmouse *psmouse)
 				psmouse_notice(psmouse,
 						"issuing reconnect request\n");
 				serio_reconnect(psmouse->ps2dev.serio);
-				return -1;
+				return -EIO;
 			}
 		}
 		psmouse->pktcnt = 0;
@@ -320,7 +325,7 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
  * for normal processing or gathering them as command response.
  */
 static irqreturn_t psmouse_interrupt(struct serio *serio,
-		unsigned char data, unsigned int flags)
+				     u8 data, unsigned int flags)
 {
 	struct psmouse *psmouse = serio_get_drvdata(serio);
 
@@ -418,17 +423,20 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
  * 0xE6 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
  * is the command.
  */
-int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command)
+int psmouse_sliced_command(struct psmouse *psmouse, u8 command)
 {
 	int i;
+	int error;
 
-	if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11))
-		return -1;
+	error = ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
+	if (error)
+		return error;
 
 	for (i = 6; i >= 0; i -= 2) {
-		unsigned char d = (command >> i) & 3;
-		if (ps2_command(&psmouse->ps2dev, &d, PSMOUSE_CMD_SETRES))
-			return -1;
+		u8 d = (command >> i) & 3;
+		error = ps2_command(&psmouse->ps2dev, &d, PSMOUSE_CMD_SETRES);
+		if (error)
+			return error;
 	}
 
 	return 0;
@@ -439,13 +447,15 @@ int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command)
  */
 int psmouse_reset(struct psmouse *psmouse)
 {
-	unsigned char param[2];
+	u8 param[2];
+	int error;
 
-	if (ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_RESET_BAT))
-		return -1;
+	error = ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_RESET_BAT);
+	if (error)
+		return error;
 
 	if (param[0] != PSMOUSE_RET_BAT && param[1] != PSMOUSE_RET_ID)
-		return -1;
+		return -EIO;
 
 	return 0;
 }
@@ -455,8 +465,8 @@ int psmouse_reset(struct psmouse *psmouse)
  */
 void psmouse_set_resolution(struct psmouse *psmouse, unsigned int resolution)
 {
-	static const unsigned char params[] = { 0, 1, 2, 2, 3 };
-	unsigned char p;
+	static const u8 params[] = { 0, 1, 2, 2, 3 };
+	u8 p;
 
 	if (resolution == 0 || resolution > 200)
 		resolution = 200;
@@ -471,11 +481,12 @@ void psmouse_set_resolution(struct psmouse *psmouse, unsigned int resolution)
  */
 static void psmouse_set_rate(struct psmouse *psmouse, unsigned int rate)
 {
-	static const unsigned char rates[] = { 200, 100, 80, 60, 40, 20, 10, 0 };
-	unsigned char r;
+	static const u8 rates[] = { 200, 100, 80, 60, 40, 20, 10, 0 };
+	u8 r;
 	int i = 0;
 
-	while (rates[i] > rate) i++;
+	while (rates[i] > rate)
+		i++;
 	r = rates[i];
 	ps2_command(&psmouse->ps2dev, &r, PSMOUSE_CMD_SETRATE);
 	psmouse->rate = r;
@@ -547,7 +558,7 @@ bool psmouse_matches_pnp_id(struct psmouse *psmouse, const char * const ids[])
 static int genius_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[4];
+	u8 param[4];
 
 	param[0] = 3;
 	ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
@@ -557,7 +568,7 @@ static int genius_detect(struct psmouse *psmouse, bool set_properties)
 	ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO);
 
 	if (param[0] != 0x00 || param[1] != 0x33 || param[2] != 0x55)
-		return -1;
+		return -ENODEV;
 
 	if (set_properties) {
 		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
@@ -579,7 +590,7 @@ static int genius_detect(struct psmouse *psmouse, bool set_properties)
 static int intellimouse_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[2];
+	u8 param[2];
 
 	param[0] = 200;
 	ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
@@ -590,7 +601,7 @@ static int intellimouse_detect(struct psmouse *psmouse, bool set_properties)
 	ps2_command(ps2dev, param, PSMOUSE_CMD_GETID);
 
 	if (param[0] != 3)
-		return -1;
+		return -ENODEV;
 
 	if (set_properties) {
 		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
@@ -612,7 +623,7 @@ static int intellimouse_detect(struct psmouse *psmouse, bool set_properties)
 static int im_explorer_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[2];
+	u8 param[2];
 
 	intellimouse_detect(psmouse, 0);
 
@@ -625,7 +636,7 @@ static int im_explorer_detect(struct psmouse *psmouse, bool set_properties)
 	ps2_command(ps2dev, param, PSMOUSE_CMD_GETID);
 
 	if (param[0] != 4)
-		return -1;
+		return -ENODEV;
 
 	/* Magic to enable horizontal scrolling on IntelliMouse 4.0 */
 	param[0] = 200;
@@ -658,8 +669,8 @@ static int im_explorer_detect(struct psmouse *psmouse, bool set_properties)
 static int thinking_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[2];
-	static const unsigned char seq[] = { 20, 60, 40, 20, 20, 60, 40, 20, 20 };
+	u8 param[2];
+	static const u8 seq[] = { 20, 60, 40, 20, 20, 60, 40, 20, 20 };
 	int i;
 
 	param[0] = 10;
@@ -673,7 +684,7 @@ static int thinking_detect(struct psmouse *psmouse, bool set_properties)
 	ps2_command(ps2dev, param, PSMOUSE_CMD_GETID);
 
 	if (param[0] != 2)
-		return -1;
+		return -ENODEV;
 
 	if (set_properties) {
 		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
@@ -701,7 +712,7 @@ static int ps2bare_detect(struct psmouse *psmouse, bool set_properties)
 		 * We have no way of figuring true number of buttons so let's
 		 * assume that the device has 3.
 		 */
-		__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
+		input_set_capability(psmouse->dev, EV_KEY, BTN_MIDDLE);
 	}
 
 	return 0;
@@ -956,20 +967,17 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
 {
 	struct input_dev *input_dev = psmouse->dev;
 
-	memset(input_dev->evbit, 0, sizeof(input_dev->evbit));
-	memset(input_dev->keybit, 0, sizeof(input_dev->keybit));
-	memset(input_dev->relbit, 0, sizeof(input_dev->relbit));
-	memset(input_dev->absbit, 0, sizeof(input_dev->absbit));
-	memset(input_dev->mscbit, 0, sizeof(input_dev->mscbit));
-
-	__set_bit(EV_KEY, input_dev->evbit);
-	__set_bit(EV_REL, input_dev->evbit);
+	bitmap_zero(input_dev->evbit, EV_CNT);
+	bitmap_zero(input_dev->keybit, KEY_CNT);
+	bitmap_zero(input_dev->relbit, REL_CNT);
+	bitmap_zero(input_dev->absbit, ABS_CNT);
+	bitmap_zero(input_dev->mscbit, MSC_CNT);
 
-	__set_bit(BTN_LEFT, input_dev->keybit);
-	__set_bit(BTN_RIGHT, input_dev->keybit);
+	input_set_capability(input_dev, EV_KEY, BTN_LEFT);
+	input_set_capability(input_dev, EV_KEY, BTN_RIGHT);
 
-	__set_bit(REL_X, input_dev->relbit);
-	__set_bit(REL_Y, input_dev->relbit);
+	input_set_capability(input_dev, EV_REL, REL_X);
+	input_set_capability(input_dev, EV_REL, REL_Y);
 
 	__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
 
@@ -1239,7 +1247,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
 static int psmouse_probe(struct psmouse *psmouse)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[2];
+	u8 param[2];
+	int error;
 
 	/*
 	 * First, we check if it's a mouse. It should send 0x00 or 0x03 in
@@ -1248,20 +1257,22 @@ static int psmouse_probe(struct psmouse *psmouse)
 	 * subsequent ID queries, probably due to a firmware bug.
 	 */
 	param[0] = 0xa5;
-	if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
-		return -1;
+	error = ps2_command(ps2dev, param, PSMOUSE_CMD_GETID);
+	if (error)
+		return error;
 
 	if (param[0] != 0x00 && param[0] != 0x03 &&
 	    param[0] != 0x04 && param[0] != 0xff)
-		return -1;
+		return -ENODEV;
 
 	/*
 	 * Then we reset and disable the mouse so that it doesn't generate
 	 * events.
 	 */
-	if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_DIS))
-		psmouse_warn(psmouse, "Failed to reset mouse on %s\n",
-			     ps2dev->serio->phys);
+	error = ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
+	if (error)
+		psmouse_warn(psmouse, "Failed to reset mouse on %s: %d\n",
+			     ps2dev->serio->phys, error);
 
 	return 0;
 }
@@ -1302,10 +1313,13 @@ int psmouse_activate(struct psmouse *psmouse)
  */
 int psmouse_deactivate(struct psmouse *psmouse)
 {
-	if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_DISABLE)) {
-		psmouse_warn(psmouse, "Failed to deactivate mouse on %s\n",
-			     psmouse->ps2dev.serio->phys);
-		return -1;
+	int error;
+
+	error = ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_DISABLE);
+	if (error) {
+		psmouse_warn(psmouse, "Failed to deactivate mouse on %s: %d\n",
+			     psmouse->ps2dev.serio->phys, error);
+		return error;
 	}
 
 	psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 3/7] Input: logips2pp - clean up code
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 1/7] Input: psmouse - create helper for reporting standard buttons/motion Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 2/7] Input: psmouse - clean up code Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 4/7] Input: lifebook " Dmitry Torokhov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

- switch to using BIT() macros
- use u8 instead of unsigned char for byte data
- use input_set_capability() instead of manipulating capabilities bits
  directly
- use sign_extend32() when extracting wheel data.
- do not abuse -1 as error code, propagate errors from various calls.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/logips2pp.c | 142 +++++++++++++++++++++++-----------------
 1 file changed, 83 insertions(+), 59 deletions(-)

diff --git a/drivers/input/mouse/logips2pp.c b/drivers/input/mouse/logips2pp.c
index b7d17db632fc0..3c8d7051ef5e0 100644
--- a/drivers/input/mouse/logips2pp.c
+++ b/drivers/input/mouse/logips2pp.c
@@ -9,9 +9,11 @@
  * the Free Software Foundation.
  */
 
+#include <linux/bitops.h>
 #include <linux/input.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/types.h>
 #include "psmouse.h"
 #include "logips2pp.h"
 
@@ -22,12 +24,12 @@
 #define PS2PP_KIND_TRACKMAN	4
 
 /* Logitech mouse features */
-#define PS2PP_WHEEL		0x01
-#define PS2PP_HWHEEL		0x02
-#define PS2PP_SIDE_BTN		0x04
-#define PS2PP_EXTRA_BTN		0x08
-#define PS2PP_TASK_BTN		0x10
-#define PS2PP_NAV_BTN		0x20
+#define PS2PP_WHEEL		BIT(0)
+#define PS2PP_HWHEEL		BIT(1)
+#define PS2PP_SIDE_BTN		BIT(2)
+#define PS2PP_EXTRA_BTN		BIT(3)
+#define PS2PP_TASK_BTN		BIT(4)
+#define PS2PP_NAV_BTN		BIT(5)
 
 struct ps2pp_info {
 	u8 model;
@@ -42,7 +44,7 @@ struct ps2pp_info {
 static psmouse_ret_t ps2pp_process_byte(struct psmouse *psmouse)
 {
 	struct input_dev *dev = psmouse->dev;
-	unsigned char *packet = psmouse->packet;
+	u8 *packet = psmouse->packet;
 
 	if (psmouse->pktcnt < 3)
 		return PSMOUSE_GOOD_DATA;
@@ -58,28 +60,30 @@ static psmouse_ret_t ps2pp_process_byte(struct psmouse *psmouse)
 
 		case 0x0d: /* Mouse extra info */
 
-			input_report_rel(dev, packet[2] & 0x80 ? REL_HWHEEL : REL_WHEEL,
-				(int) (packet[2] & 8) - (int) (packet[2] & 7));
-			input_report_key(dev, BTN_SIDE, (packet[2] >> 4) & 1);
-			input_report_key(dev, BTN_EXTRA, (packet[2] >> 5) & 1);
+			input_report_rel(dev,
+				packet[2] & 0x80 ? REL_HWHEEL : REL_WHEEL,
+				-sign_extend32(packet[2], 3));
+			input_report_key(dev, BTN_SIDE,  packet[2] & BIT(4));
+			input_report_key(dev, BTN_EXTRA, packet[2] & BIT(5));
 
 			break;
 
 		case 0x0e: /* buttons 4, 5, 6, 7, 8, 9, 10 info */
 
-			input_report_key(dev, BTN_SIDE, (packet[2]) & 1);
-			input_report_key(dev, BTN_EXTRA, (packet[2] >> 1) & 1);
-			input_report_key(dev, BTN_BACK, (packet[2] >> 3) & 1);
-			input_report_key(dev, BTN_FORWARD, (packet[2] >> 4) & 1);
-			input_report_key(dev, BTN_TASK, (packet[2] >> 2) & 1);
+			input_report_key(dev, BTN_SIDE,    packet[2] & BIT(0));
+			input_report_key(dev, BTN_EXTRA,   packet[2] & BIT(1));
+			input_report_key(dev, BTN_TASK,    packet[2] & BIT(2));
+			input_report_key(dev, BTN_BACK,    packet[2] & BIT(3));
+			input_report_key(dev, BTN_FORWARD, packet[2] & BIT(4));
 
 			break;
 
 		case 0x0f: /* TouchPad extra info */
 
-			input_report_rel(dev, packet[2] & 0x08 ? REL_HWHEEL : REL_WHEEL,
-				(int) ((packet[2] >> 4) & 8) - (int) ((packet[2] >> 4) & 7));
-			packet[0] = packet[2] | 0x08;
+			input_report_rel(dev,
+				packet[2] & 0x08 ? REL_HWHEEL : REL_WHEEL,
+				-sign_extend32(packet[2] >> 4, 3));
+			packet[0] = packet[2] | BIT(3);
 			break;
 
 		default:
@@ -109,13 +113,17 @@ static psmouse_ret_t ps2pp_process_byte(struct psmouse *psmouse)
  * Ugly.
  */
 
-static int ps2pp_cmd(struct psmouse *psmouse, unsigned char *param, unsigned char command)
+static int ps2pp_cmd(struct psmouse *psmouse, u8 *param, u8 command)
 {
-	if (psmouse_sliced_command(psmouse, command))
-		return -1;
+	int error;
+
+	error = psmouse_sliced_command(psmouse, command);
+	if (error)
+		return error;
 
-	if (ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_POLL | 0x0300))
-		return -1;
+	error = ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_POLL | 0x0300);
+	if (error)
+		return error;
 
 	return 0;
 }
@@ -131,7 +139,7 @@ static int ps2pp_cmd(struct psmouse *psmouse, unsigned char *param, unsigned cha
 static void ps2pp_set_smartscroll(struct psmouse *psmouse, bool smartscroll)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[4];
+	u8 param[4];
 
 	ps2pp_cmd(psmouse, param, 0x32);
 
@@ -169,7 +177,7 @@ static ssize_t ps2pp_attr_set_smartscroll(struct psmouse *psmouse, void *data,
 }
 
 PSMOUSE_DEFINE_ATTR(smartscroll, S_IWUSR | S_IRUGO, NULL,
-			ps2pp_attr_show_smartscroll, ps2pp_attr_set_smartscroll);
+		    ps2pp_attr_show_smartscroll, ps2pp_attr_set_smartscroll);
 
 /*
  * Support 800 dpi resolution _only_ if the user wants it (there are good
@@ -177,11 +185,12 @@ PSMOUSE_DEFINE_ATTR(smartscroll, S_IWUSR | S_IRUGO, NULL,
  * also good reasons to use it, let the user decide).
  */
 
-static void ps2pp_set_resolution(struct psmouse *psmouse, unsigned int resolution)
+static void ps2pp_set_resolution(struct psmouse *psmouse,
+				 unsigned int resolution)
 {
 	if (resolution > 400) {
 		struct ps2dev *ps2dev = &psmouse->ps2dev;
-		unsigned char param = 3;
+		u8 param = 3;
 
 		ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
 		ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
@@ -194,7 +203,8 @@ static void ps2pp_set_resolution(struct psmouse *psmouse, unsigned int resolutio
 
 static void ps2pp_disconnect(struct psmouse *psmouse)
 {
-	device_remove_file(&psmouse->ps2dev.serio->dev, &psmouse_attr_smartscroll.dattr);
+	device_remove_file(&psmouse->ps2dev.serio->dev,
+			   &psmouse_attr_smartscroll.dattr);
 }
 
 static const struct ps2pp_info *get_model_info(unsigned char model)
@@ -267,24 +277,24 @@ static void ps2pp_set_model_properties(struct psmouse *psmouse,
 	struct input_dev *input_dev = psmouse->dev;
 
 	if (model_info->features & PS2PP_SIDE_BTN)
-		__set_bit(BTN_SIDE, input_dev->keybit);
+		input_set_capability(input_dev, EV_KEY, BTN_SIDE);
 
 	if (model_info->features & PS2PP_EXTRA_BTN)
-		__set_bit(BTN_EXTRA, input_dev->keybit);
+		input_set_capability(input_dev, EV_KEY, BTN_EXTRA);
 
 	if (model_info->features & PS2PP_TASK_BTN)
-		__set_bit(BTN_TASK, input_dev->keybit);
+		input_set_capability(input_dev, EV_KEY, BTN_TASK);
 
 	if (model_info->features & PS2PP_NAV_BTN) {
-		__set_bit(BTN_FORWARD, input_dev->keybit);
-		__set_bit(BTN_BACK, input_dev->keybit);
+		input_set_capability(input_dev, EV_KEY, BTN_FORWARD);
+		input_set_capability(input_dev, EV_KEY, BTN_BACK);
 	}
 
 	if (model_info->features & PS2PP_WHEEL)
-		__set_bit(REL_WHEEL, input_dev->relbit);
+		input_set_capability(input_dev, EV_REL, REL_WHEEL);
 
 	if (model_info->features & PS2PP_HWHEEL)
-		__set_bit(REL_HWHEEL, input_dev->relbit);
+		input_set_capability(input_dev, EV_REL, REL_HWHEEL);
 
 	switch (model_info->kind) {
 
@@ -316,6 +326,30 @@ static void ps2pp_set_model_properties(struct psmouse *psmouse,
 	}
 }
 
+static int ps2pp_setup_protocol(struct psmouse *psmouse,
+				const struct ps2pp_info *model_info)
+{
+	int error;
+
+	psmouse->protocol_handler = ps2pp_process_byte;
+	psmouse->pktsize = 3;
+
+	if (model_info->kind != PS2PP_KIND_TP3) {
+		psmouse->set_resolution = ps2pp_set_resolution;
+		psmouse->disconnect = ps2pp_disconnect;
+
+		error = device_create_file(&psmouse->ps2dev.serio->dev,
+					   &psmouse_attr_smartscroll.dattr);
+		if (error) {
+			psmouse_err(psmouse,
+				    "failed to create smartscroll sysfs attribute, error: %d\n",
+				    error);
+			return error;
+		}
+	}
+
+	return 0;
+}
 
 /*
  * Logitech magic init. Detect whether the mouse is a Logitech one
@@ -326,9 +360,9 @@ static void ps2pp_set_model_properties(struct psmouse *psmouse,
 int ps2pp_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[4];
-	unsigned char model, buttons;
 	const struct ps2pp_info *model_info;
+	u8 param[4];
+	u8 model, buttons;
 	bool use_ps2pp = false;
 	int error;
 
@@ -344,7 +378,7 @@ int ps2pp_detect(struct psmouse *psmouse, bool set_properties)
 	buttons = param[1];
 
 	if (!model || !buttons)
-		return -1;
+		return -ENXIO;
 
 	model_info = get_model_info(model);
 	if (model_info) {
@@ -366,7 +400,8 @@ int ps2pp_detect(struct psmouse *psmouse, bool set_properties)
 
 			param[0] = 0;
 			if (!ps2_command(ps2dev, param, 0x13d1) &&
-			    param[0] == 0x06 && param[1] == 0x00 && param[2] == 0x14) {
+			    param[0] == 0x06 && param[1] == 0x00 &&
+			    param[2] == 0x14) {
 				use_ps2pp = true;
 			}
 
@@ -385,7 +420,9 @@ int ps2pp_detect(struct psmouse *psmouse, bool set_properties)
 		}
 
 	} else {
-		psmouse_warn(psmouse, "Detected unknown Logitech mouse model %d\n", model);
+		psmouse_warn(psmouse,
+			     "Detected unknown Logitech mouse model %d\n",
+			     model);
 	}
 
 	if (set_properties) {
@@ -393,31 +430,18 @@ int ps2pp_detect(struct psmouse *psmouse, bool set_properties)
 		psmouse->model = model;
 
 		if (use_ps2pp) {
-			psmouse->protocol_handler = ps2pp_process_byte;
-			psmouse->pktsize = 3;
-
-			if (model_info->kind != PS2PP_KIND_TP3) {
-				psmouse->set_resolution = ps2pp_set_resolution;
-				psmouse->disconnect = ps2pp_disconnect;
-
-				error = device_create_file(&ps2dev->serio->dev,
-							   &psmouse_attr_smartscroll.dattr);
-				if (error) {
-					psmouse_err(psmouse,
-						    "failed to create smartscroll sysfs attribute, error: %d\n",
-						    error);
-					return -1;
-				}
-			}
+			error = ps2pp_setup_protocol(psmouse, model_info);
+			if (error)
+				return error;
 		}
 
 		if (buttons >= 3)
-			__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
+			input_set_capability(psmouse->dev, EV_KEY, BTN_MIDDLE);
 
 		if (model_info)
 			ps2pp_set_model_properties(psmouse, model_info, use_ps2pp);
 	}
 
-	return use_ps2pp ? 0 : -1;
+	return use_ps2pp ? 0 : -ENXIO;
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 4/7] Input: lifebook - clean up code
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2018-01-19 23:06 ` [PATCH 3/7] Input: logips2pp " Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice Dmitry Torokhov
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

- use u8 instead of unsigned char for byte data
- use input_set_capability() instead of manipulating capabilities bits
  directly
- do not abuse -1 as error code, propagate errors from various calls.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/lifebook.c | 50 ++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/input/mouse/lifebook.c b/drivers/input/mouse/lifebook.c
index 65efaade0820d..a5765f747c020 100644
--- a/drivers/input/mouse/lifebook.c
+++ b/drivers/input/mouse/lifebook.c
@@ -17,6 +17,7 @@
 #include <linux/libps2.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/types.h>
 
 #include "psmouse.h"
 #include "lifebook.h"
@@ -136,7 +137,7 @@ static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse)
 	struct lifebook_data *priv = psmouse->private;
 	struct input_dev *dev1 = psmouse->dev;
 	struct input_dev *dev2 = priv ? priv->dev2 : NULL;
-	unsigned char *packet = psmouse->packet;
+	u8 *packet = psmouse->packet;
 	bool relative_packet = packet[0] & 0x08;
 
 	if (relative_packet || !lifebook_use_6byte_proto) {
@@ -201,10 +202,12 @@ static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse)
 static int lifebook_absolute_mode(struct psmouse *psmouse)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param;
+	u8 param;
+	int error;
 
-	if (psmouse_reset(psmouse))
-		return -1;
+	error = psmouse_reset(psmouse);
+	if (error)
+		return error;
 
 	/*
 	 * Enable absolute output -- ps2_command fails always but if
@@ -220,15 +223,15 @@ static int lifebook_absolute_mode(struct psmouse *psmouse)
 static void lifebook_relative_mode(struct psmouse *psmouse)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param = 0x06;
+	u8 param = 0x06;
 
 	ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
 }
 
 static void lifebook_set_resolution(struct psmouse *psmouse, unsigned int resolution)
 {
-	static const unsigned char params[] = { 0, 1, 2, 2, 3 };
-	unsigned char p;
+	static const u8 params[] = { 0, 1, 2, 2, 3 };
+	u8 p;
 
 	if (resolution == 0 || resolution > 400)
 		resolution = 400;
@@ -253,11 +256,11 @@ static void lifebook_disconnect(struct psmouse *psmouse)
 int lifebook_detect(struct psmouse *psmouse, bool set_properties)
 {
 	if (!lifebook_present)
-		return -1;
+		return -ENXIO;
 
 	if (desired_serio_phys &&
 	    strcmp(psmouse->ps2dev.serio->phys, desired_serio_phys))
-		return -1;
+		return -ENXIO;
 
 	if (set_properties) {
 		psmouse->vendor = "Fujitsu";
@@ -290,10 +293,10 @@ static int lifebook_create_relative_device(struct psmouse *psmouse)
 	dev2->id.version = 0x0000;
 	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_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);
 
 	error = input_register_device(priv->dev2);
 	if (error)
@@ -312,21 +315,26 @@ int lifebook_init(struct psmouse *psmouse)
 {
 	struct input_dev *dev1 = psmouse->dev;
 	int max_coord = lifebook_use_6byte_proto ? 4096 : 1024;
+	int error;
+
+	error = lifebook_absolute_mode(psmouse);
+	if (error)
+		return error;
 
-	if (lifebook_absolute_mode(psmouse))
-		return -1;
+	/* Clear default capabilities */
+	bitmap_zero(dev1->evbit, EV_CNT);
+	bitmap_zero(dev1->relbit, REL_CNT);
+	bitmap_zero(dev1->keybit, KEY_CNT);
 
-	dev1->evbit[0] = BIT_MASK(EV_ABS) | BIT_MASK(EV_KEY);
-	dev1->relbit[0] = 0;
-	dev1->keybit[BIT_WORD(BTN_MOUSE)] = 0;
-	dev1->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+	input_set_capability(dev1, EV_KEY, BTN_TOUCH);
 	input_set_abs_params(dev1, ABS_X, 0, max_coord, 0, 0);
 	input_set_abs_params(dev1, ABS_Y, 0, max_coord, 0, 0);
 
 	if (!desired_serio_phys) {
-		if (lifebook_create_relative_device(psmouse)) {
+		error = lifebook_create_relative_device(psmouse);
+		if (error) {
 			lifebook_relative_mode(psmouse);
-			return -1;
+			return error;
 		}
 	}
 
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2018-01-19 23:06 ` [PATCH 4/7] Input: lifebook " Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
       [not found]   ` <CAAZ5spDzNfrosHfCYFtPcQ2bZiE5iSn8Ac40yNNHR-mdjzt83w@mail.gmail.com>
  2018-01-19 23:06 ` [PATCH 6/7] Input: synaptics - switch to using input_set_capability Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 7/7] Input: synaptics - handle errors from input_mt_init_slots() Dmitry Torokhov
  6 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul
  Cc: Stephen Lyons, linux-input, linux-kernel

From: Stephen Lyons <slysven@virginmedia.com>

This Far-Eastern company's PS/2 mice use a deviant format for the data
relating to movement of the scroll wheels for, at least, their dual wheel
mice, such as their "Optical GreatEye Wheelmouse" model "WOP-35".  This
product has five "buttons" (one of which is the click action on the first
wheel) and TWO scroll wheels.  However for a byte comprising d0-d7 instead
of setting one of d6-7 in the forth byte of the mouse data packet and a
twos complement number of scroll steps in the remaining d5-d0 (or d3-d0
should there be a fourth (BTN_SIDE - d4) or fifth (BTN_EXTRA - d5) button
to report; they only report a single +/- event for each wheel and use a bit
pattern that corresponds to +/-1 for the first wheel and +/- 2 for the
second in the lower nibble of the fourth byte.

The effect with existing code is that the second mouse wheel merely repeats
the effect of the first but providing two steps per click rather than the
one of the first wheel - so there is no HORIZONTAL scroll wheel movement
detected from the device as far as the rest of the kernel sees it.

This patch, if enabled by the "a4tech_workaround" module parameter modifies
the handling just for mice of type PSMOUSE_IMEX so that the second scroll
wheel movement gets correctly reported as REL_HWHEEL events.  Should this
module parameter be activated for other mice of the same PSMOUSE_IMEX type
then it is possible that at the point where the mouse reports more than a
single movement step the user may start seeing horizontal rather than
vertical wheel events, but should the movement steps get to be more than
two at a time the hack will get immediately deactivated and the behaviour
will revert to the past code.

This was discussed around *fifteen* *years* *ago* on the LKML and the best
summary is in post https://lkml.org/lkml/2002/7/18/111 "Re: PS2 Input Core
Support" by Vojtech Pavlik. I was not able to locate any discussion later
than this on this topic.

Given that most users of the "psmouse" module will NOT want this additional
feature enabled I have taken the apparently erroneous step of defaulting
the module parameter that enables it to be "disabled" - this functionality
may interfere with the operation of "normal" mice of this type (until a
large enough scroll wheel movement is detected) so I cannot see how it
would want to be enabled for "normal" users - i.e.  everyone without this
brand of mouse.

I am using this patch at the moment and I can confirm that it is working
for me as both a module and compiled into the kernel for my mouse that is
of the type (WOP-35) described - I note that it is still available from
certain on-line retailers and that the manufacturers site does not list
GNU/Linux as being supported on the product page - this patch however does
enable full use of this product:
http://www.a4tech.com/product.asp?cid=3D1&scid=3D8&id=3D22

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/psmouse-base.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index cbca668bb931f..d0e45124e7f43 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -70,6 +70,10 @@ static bool psmouse_smartscroll = true;
 module_param_named(smartscroll, psmouse_smartscroll, bool, 0644);
 MODULE_PARM_DESC(smartscroll, "Logitech Smartscroll autorepeat, 1 = enabled (default), 0 = disabled.");
 
+static bool psmouse_a4tech_2wheels;
+module_param_named(a4tech_workaround, psmouse_a4tech_2wheels, bool, 0644);
+MODULE_PARM_DESC(a4tech_workaround, "A4Tech second scroll wheel workaround, 1 = enabled, 0 = disabled (default).");
+
 static unsigned int psmouse_resetafter = 5;
 module_param_named(resetafter, psmouse_resetafter, uint, 0644);
 MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets (0 = never).");
@@ -150,6 +154,7 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 {
 	struct input_dev *dev = psmouse->dev;
 	u8 *packet = psmouse->packet;
+	int wheel;
 
 	if (psmouse->pktcnt < psmouse->pktsize)
 		return PSMOUSE_GOOD_DATA;
@@ -175,8 +180,18 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 			break;
 		case 0x00:
 		case 0xC0:
-			input_report_rel(dev, REL_WHEEL,
-					 -sign_extend32(packet[3], 3));
+			wheel = sign_extend32(packet[3], 3);
+
+			/*
+			 * Some A4Tech mice have two scroll wheels, with first
+			 * one reporting +/-1 in the lower nibble, and second
+			 * one reporting +/-2.
+			 */
+			if (psmouse_a4tech_2wheels && abs(wheel) > 1)
+				input_report_rel(dev, REL_HWHEEL, wheel / 2);
+			else
+				input_report_rel(dev, REL_WHEEL, -wheel);
+
 			input_report_key(dev, BTN_SIDE,  BIT(4));
 			input_report_key(dev, BTN_EXTRA, BIT(5));
 			break;
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 6/7] Input: synaptics - switch to using input_set_capability
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2018-01-19 23:06 ` [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  2018-01-19 23:06 ` [PATCH 7/7] Input: synaptics - handle errors from input_mt_init_slots() Dmitry Torokhov
  6 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

Instead of manipulating capability bits directly, use
input_set_capability().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/synaptics.c | 49 ++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 3d2e23a0ae39d..feb9c04d0eae2 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1235,25 +1235,31 @@ static void set_input_params(struct psmouse *psmouse,
 	struct synaptics_device_info *info = &priv->info;
 	int i;
 
+	/* Reset default psmouse capabilities */
+	__clear_bit(EV_REL, dev->evbit);
+	bitmap_zero(dev->relbit, REL_CNT);
+	bitmap_zero(dev->keybit, KEY_CNT);
+
 	/* Things that apply to both modes */
 	__set_bit(INPUT_PROP_POINTER, dev->propbit);
-	__set_bit(EV_KEY, dev->evbit);
-	__set_bit(BTN_LEFT, dev->keybit);
-	__set_bit(BTN_RIGHT, dev->keybit);
 
-	if (SYN_CAP_MIDDLE_BUTTON(info->capabilities))
-		__set_bit(BTN_MIDDLE, dev->keybit);
+	input_set_capability(dev, EV_KEY, BTN_LEFT);
+
+	/* Clickpads report only left button */
+	if (!SYN_CAP_CLICKPAD(info->ext_cap_0c)) {
+		input_set_capability(dev, EV_KEY, BTN_RIGHT);
+		if (SYN_CAP_MIDDLE_BUTTON(info->capabilities))
+			input_set_capability(dev, EV_KEY, BTN_MIDDLE);
+	}
 
 	if (!priv->absolute_mode) {
 		/* Relative mode */
-		__set_bit(EV_REL, dev->evbit);
-		__set_bit(REL_X, dev->relbit);
-		__set_bit(REL_Y, dev->relbit);
+		input_set_capability(dev, EV_REL, REL_X);
+		input_set_capability(dev, EV_REL, REL_Y);
 		return;
 	}
 
 	/* Absolute mode */
-	__set_bit(EV_ABS, dev->evbit);
 	set_abs_position_params(dev, &priv->info, ABS_X, ABS_Y);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
@@ -1268,8 +1274,8 @@ static void set_input_params(struct psmouse *psmouse,
 		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
 
 		/* Image sensors can signal 4 and 5 finger clicks */
-		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
-		__set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
+		input_set_capability(dev, EV_KEY, BTN_TOOL_QUADTAP);
+		input_set_capability(dev, EV_KEY, BTN_TOOL_QUINTTAP);
 	} else if (SYN_CAP_ADV_GESTURE(info->ext_cap_0c)) {
 		set_abs_position_params(dev, info,
 					ABS_MT_POSITION_X, ABS_MT_POSITION_Y);
@@ -1296,36 +1302,29 @@ static void set_input_params(struct psmouse *psmouse,
 	if (SYN_CAP_PALMDETECT(info->capabilities))
 		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 
-	__set_bit(BTN_TOUCH, dev->keybit);
-	__set_bit(BTN_TOOL_FINGER, dev->keybit);
+	input_set_capability(dev, EV_KEY, BTN_TOUCH);
+	input_set_capability(dev, EV_KEY, BTN_TOOL_FINGER);
 
 	if (synaptics_has_multifinger(priv)) {
-		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
-		__set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
+		input_set_capability(dev, EV_KEY, BTN_TOOL_DOUBLETAP);
+		input_set_capability(dev, EV_KEY, BTN_TOOL_TRIPLETAP);
 	}
 
 	if (SYN_CAP_FOUR_BUTTON(info->capabilities) ||
 	    SYN_CAP_MIDDLE_BUTTON(info->capabilities)) {
-		__set_bit(BTN_FORWARD, dev->keybit);
-		__set_bit(BTN_BACK, dev->keybit);
+		input_set_capability(dev, EV_KEY, BTN_FORWARD);
+		input_set_capability(dev, EV_KEY, BTN_BACK);
 	}
 
 	if (!SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10))
 		for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(info->ext_cap); i++)
-			__set_bit(BTN_0 + i, dev->keybit);
-
-	__clear_bit(EV_REL, dev->evbit);
-	__clear_bit(REL_X, dev->relbit);
-	__clear_bit(REL_Y, dev->relbit);
+			input_set_capability(dev, EV_KEY, BTN_0 + i);
 
 	if (SYN_CAP_CLICKPAD(info->ext_cap_0c)) {
 		__set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
 		if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
 		    !SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10))
 			__set_bit(INPUT_PROP_TOPBUTTONPAD, dev->propbit);
-		/* Clickpads report only left button */
-		__clear_bit(BTN_RIGHT, dev->keybit);
-		__clear_bit(BTN_MIDDLE, dev->keybit);
 	}
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* [PATCH 7/7] Input: synaptics - handle errors from input_mt_init_slots()
  2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
                   ` (5 preceding siblings ...)
  2018-01-19 23:06 ` [PATCH 6/7] Input: synaptics - switch to using input_set_capability Dmitry Torokhov
@ 2018-01-19 23:06 ` Dmitry Torokhov
  6 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-19 23:06 UTC (permalink / raw)
  To: Benjamin Tissoires, Hans de Goede, Lyude Paul; +Cc: linux-input, linux-kernel

input_mt_init_slots() may fail, we need to handle this condition.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/synaptics.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index feb9c04d0eae2..dcb8e0cfaa1af 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1228,12 +1228,13 @@ static void set_abs_position_params(struct input_dev *dev,
 	input_abs_set_res(dev, y_code, info->y_res);
 }
 
-static void set_input_params(struct psmouse *psmouse,
-			     struct synaptics_data *priv)
+static int set_input_params(struct psmouse *psmouse,
+			    struct synaptics_data *priv)
 {
 	struct input_dev *dev = psmouse->dev;
 	struct synaptics_device_info *info = &priv->info;
 	int i;
+	int error;
 
 	/* Reset default psmouse capabilities */
 	__clear_bit(EV_REL, dev->evbit);
@@ -1256,7 +1257,7 @@ static void set_input_params(struct psmouse *psmouse,
 		/* Relative mode */
 		input_set_capability(dev, EV_REL, REL_X);
 		input_set_capability(dev, EV_REL, REL_Y);
-		return;
+		return 0;
 	}
 
 	/* Absolute mode */
@@ -1271,7 +1272,11 @@ static void set_input_params(struct psmouse *psmouse,
 					ABS_MT_POSITION_X, ABS_MT_POSITION_Y);
 		/* Image sensors can report per-contact pressure */
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
-		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
+
+		error = input_mt_init_slots(dev, 2,
+					    INPUT_MT_POINTER | INPUT_MT_TRACK);
+		if (error)
+			return error;
 
 		/* Image sensors can signal 4 and 5 finger clicks */
 		input_set_capability(dev, EV_KEY, BTN_TOOL_QUADTAP);
@@ -1283,10 +1288,13 @@ static void set_input_params(struct psmouse *psmouse,
 		 * Profile sensor in CR-48 tracks contacts reasonably well,
 		 * other non-image sensors with AGM use semi-mt.
 		 */
-		input_mt_init_slots(dev, 2,
-				    INPUT_MT_POINTER |
-				    (cr48_profile_sensor ?
-					INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
+		error = input_mt_init_slots(dev, 2,
+					    INPUT_MT_POINTER |
+					     (cr48_profile_sensor ?
+					      INPUT_MT_TRACK :
+					      INPUT_MT_SEMI_MT));
+		if (error)
+			return error;
 
 		/*
 		 * For semi-mt devices we send ABS_X/Y ourselves instead of
@@ -1326,6 +1334,8 @@ static void set_input_params(struct psmouse *psmouse,
 		    !SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10))
 			__set_bit(INPUT_PROP_TOPBUTTONPAD, dev->propbit);
 	}
+
+	return 0;
 }
 
 static ssize_t synaptics_show_disable_gesture(struct psmouse *psmouse,
@@ -1563,7 +1573,12 @@ static int synaptics_init_ps2(struct psmouse *psmouse,
 		     info->capabilities, info->ext_cap, info->ext_cap_0c,
 		     info->ext_cap_10, info->board_id, info->firmware_id);
 
-	set_input_params(psmouse, priv);
+	err = set_input_params(psmouse, priv);
+	if (err) {
+		psmouse_err(psmouse,
+			    "failed to set up capabilities: %d\n", err);
+		goto init_fail;
+	}
 
 	/*
 	 * Encode touchpad model so that it can be used to set
-- 
2.16.0.rc1.238.g530d649a79-goog

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

* Re: [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice
       [not found]   ` <CAAZ5spDzNfrosHfCYFtPcQ2bZiE5iSn8Ac40yNNHR-mdjzt83w@mail.gmail.com>
@ 2018-01-22 18:39     ` Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-01-22 18:39 UTC (permalink / raw)
  To: Michel Hermier
  Cc: Benjamin Tissoires, Hans de Goede, Lyude Paul, Stephen Lyons,
	linux-input, LKML

Hi Michel,

On Sat, Jan 20, 2018 at 11:02:50AM +0100, Michel Hermier wrote:
> Hi,
> I think this expose a little problem in the driver: The lack of a
> feature/quirk flags in the struct psmouse_protocol, to replace the 4 bools.
> Maybe I'm mistaken but this driver is legacy, so I doubt your patch would
> be accepted, or a more proper refactoring.
> My 2 cents as a powerless reviewer.

Flags are more compact, separate module parameters are more user
friendly. If we see a need for more quirks yet we may revisit the
situation and add flags.

As far as the dirver being legacy - yes, this is somewhat true, we are
slowly abandoning PS/2 in favor of I2C (and HID). Most of the PS/2
support goes into advanced protocols like ALPS and trackpoint, not basic
PS/2 handling.

> Cheers
> 
> Le 20 janv. 2018 12:09 AM, "Dmitry Torokhov" <dmitry.torokhov@gmail.com> a
> écrit :
> 
> > From: Stephen Lyons <slysven@virginmedia.com>
> >
> > This Far-Eastern company's PS/2 mice use a deviant format for the data
> > relating to movement of the scroll wheels for, at least, their dual wheel
> > mice, such as their "Optical GreatEye Wheelmouse" model "WOP-35".  This
> > product has five "buttons" (one of which is the click action on the first
> > wheel) and TWO scroll wheels.  However for a byte comprising d0-d7 instead
> > of setting one of d6-7 in the forth byte of the mouse data packet and a
> > twos complement number of scroll steps in the remaining d5-d0 (or d3-d0
> > should there be a fourth (BTN_SIDE - d4) or fifth (BTN_EXTRA - d5) button
> > to report; they only report a single +/- event for each wheel and use a bit
> > pattern that corresponds to +/-1 for the first wheel and +/- 2 for the
> > second in the lower nibble of the fourth byte.
> >
> > The effect with existing code is that the second mouse wheel merely repeats
> > the effect of the first but providing two steps per click rather than the
> > one of the first wheel - so there is no HORIZONTAL scroll wheel movement
> > detected from the device as far as the rest of the kernel sees it.
> >
> > This patch, if enabled by the "a4tech_workaround" module parameter modifies
> > the handling just for mice of type PSMOUSE_IMEX so that the second scroll
> > wheel movement gets correctly reported as REL_HWHEEL events.  Should this
> > module parameter be activated for other mice of the same PSMOUSE_IMEX type
> > then it is possible that at the point where the mouse reports more than a
> > single movement step the user may start seeing horizontal rather than
> > vertical wheel events, but should the movement steps get to be more than
> > two at a time the hack will get immediately deactivated and the behaviour
> > will revert to the past code.
> >
> > This was discussed around *fifteen* *years* *ago* on the LKML and the best
> > summary is in post https://lkml.org/lkml/2002/7/18/111 "Re: PS2 Input Core
> > Support" by Vojtech Pavlik. I was not able to locate any discussion later
> > than this on this topic.
> >
> > Given that most users of the "psmouse" module will NOT want this additional
> > feature enabled I have taken the apparently erroneous step of defaulting
> > the module parameter that enables it to be "disabled" - this functionality
> > may interfere with the operation of "normal" mice of this type (until a
> > large enough scroll wheel movement is detected) so I cannot see how it
> > would want to be enabled for "normal" users - i.e.  everyone without this
> > brand of mouse.
> >
> > I am using this patch at the moment and I can confirm that it is working
> > for me as both a module and compiled into the kernel for my mouse that is
> > of the type (WOP-35) described - I note that it is still available from
> > certain on-line retailers and that the manufacturers site does not list
> > GNU/Linux as being supported on the product page - this patch however does
> > enable full use of this product:
> > http://www.a4tech.com/product.asp?cid=3D1&scid=3D8&id=3D22
> >
> > Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/input/mouse/psmouse-base.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/psmouse-base.c
> > b/drivers/input/mouse/psmouse-base.c
> > index cbca668bb931f..d0e45124e7f43 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -70,6 +70,10 @@ static bool psmouse_smartscroll = true;
> >  module_param_named(smartscroll, psmouse_smartscroll, bool, 0644);
> >  MODULE_PARM_DESC(smartscroll, "Logitech Smartscroll autorepeat, 1 =
> > enabled (default), 0 = disabled.");
> >
> > +static bool psmouse_a4tech_2wheels;
> > +module_param_named(a4tech_workaround, psmouse_a4tech_2wheels, bool,
> > 0644);
> > +MODULE_PARM_DESC(a4tech_workaround, "A4Tech second scroll wheel
> > workaround, 1 = enabled, 0 = disabled (default).");
> > +
> >  static unsigned int psmouse_resetafter = 5;
> >  module_param_named(resetafter, psmouse_resetafter, uint, 0644);
> >  MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets (0 =
> > never).");
> > @@ -150,6 +154,7 @@ psmouse_ret_t psmouse_process_byte(struct psmouse
> > *psmouse)
> >  {
> >         struct input_dev *dev = psmouse->dev;
> >         u8 *packet = psmouse->packet;
> > +       int wheel;
> >
> >         if (psmouse->pktcnt < psmouse->pktsize)
> >                 return PSMOUSE_GOOD_DATA;
> > @@ -175,8 +180,18 @@ psmouse_ret_t psmouse_process_byte(struct psmouse
> > *psmouse)
> >                         break;
> >                 case 0x00:
> >                 case 0xC0:
> > -                       input_report_rel(dev, REL_WHEEL,
> > -                                        -sign_extend32(packet[3], 3));
> > +                       wheel = sign_extend32(packet[3], 3);
> > +
> > +                       /*
> > +                        * Some A4Tech mice have two scroll wheels, with
> > first
> > +                        * one reporting +/-1 in the lower nibble, and
> > second
> > +                        * one reporting +/-2.
> > +                        */
> > +                       if (psmouse_a4tech_2wheels && abs(wheel) > 1)
> > +                               input_report_rel(dev, REL_HWHEEL, wheel /
> > 2);
> > +                       else
> > +                               input_report_rel(dev, REL_WHEEL, -wheel);
> > +
> >                         input_report_key(dev, BTN_SIDE,  BIT(4));
> >                         input_report_key(dev, BTN_EXTRA, BIT(5));
> >                         break;
> > --
> > 2.16.0.rc1.238.g530d649a79-goog
> >
> > --
> > 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
> >

Thanks.

-- 
Dmitry

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

* Re: [PATCH 2/7] Input: psmouse - clean up code
  2018-01-19 23:06 ` [PATCH 2/7] Input: psmouse - clean up code Dmitry Torokhov
@ 2018-06-25 18:35   ` Jiri Slaby
  2018-06-25 18:52     ` Dmitry Torokhov
  0 siblings, 1 reply; 11+ messages in thread
From: Jiri Slaby @ 2018-06-25 18:35 UTC (permalink / raw)
  To: Dmitry Torokhov, Benjamin Tissoires, Hans de Goede, Lyude Paul
  Cc: linux-input, linux-kernel

On 01/20/2018, 12:06 AM, Dmitry Torokhov wrote:
> - switch to using BIT() macros
> - use u8 instead of unsigned char for byte data
> - use input_set_capability() instead of manipulating capabilities bits
>   directly
> - use sign_extend32() when extracting wheel data.
> - do not abuse -1 as error code, propagate errors from various calls.
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -157,39 +159,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
>  		case 0x00:
>  		case 0xC0:
> -			input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 8) - (int) (packet[3] & 7));
> -			input_report_key(dev, BTN_SIDE, (packet[3] >> 4) & 1);
> -			input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
> +			input_report_rel(dev, REL_WHEEL,
> +					 -sign_extend32(packet[3], 3));
> +			input_report_key(dev, BTN_SIDE,  BIT(4));
> +			input_report_key(dev, BTN_EXTRA, BIT(5));
>  			break;
>  		}
>  		break;
>  
>  	case PSMOUSE_GENPS:
>  		/* Report scroll buttons on NetMice */
> -		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
> +		input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
>  
>  		/* Extra buttons on Genius NewNet 3D */
> -		input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
> -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
> +		input_report_key(dev, BTN_SIDE,  BIT(6));
> +		input_report_key(dev, BTN_EXTRA, BIT(7));
>  		break;
>  
>  	case PSMOUSE_THINKPS:
>  		/* Extra button on ThinkingMouse */
> -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
> +		input_report_key(dev, BTN_EXTRA, BIT(3));

While hunting a 4.17 bug where some openSUSE users lost their ability to
mouse-click, I came across this commit. Putting aside it's a total mess
of multiple changes, were these changes above intentional at all? I mean
changing
  (packet[0] >> 3) & 1
to hardwired
  BIT(3)
does not look quite right. And if it is for some to me unknown reason,
it should have been properly documented in the commit log. I believe
your intent was:
  packet[0] & BIT(3)
?

thanks,
-- 
js
suse labs

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

* Re: [PATCH 2/7] Input: psmouse - clean up code
  2018-06-25 18:35   ` Jiri Slaby
@ 2018-06-25 18:52     ` Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2018-06-25 18:52 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Benjamin Tissoires, Hans de Goede, Lyude Paul, linux-input, linux-kernel

On Mon, Jun 25, 2018 at 08:35:14PM +0200, Jiri Slaby wrote:
> On 01/20/2018, 12:06 AM, Dmitry Torokhov wrote:
> > - switch to using BIT() macros
> > - use u8 instead of unsigned char for byte data
> > - use input_set_capability() instead of manipulating capabilities bits
> >   directly
> > - use sign_extend32() when extracting wheel data.
> > - do not abuse -1 as error code, propagate errors from various calls.
> …
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> …
> > @@ -157,39 +159,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
> …
> >  		case 0x00:
> >  		case 0xC0:
> > -			input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 8) - (int) (packet[3] & 7));
> > -			input_report_key(dev, BTN_SIDE, (packet[3] >> 4) & 1);
> > -			input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
> > +			input_report_rel(dev, REL_WHEEL,
> > +					 -sign_extend32(packet[3], 3));
> > +			input_report_key(dev, BTN_SIDE,  BIT(4));
> > +			input_report_key(dev, BTN_EXTRA, BIT(5));
> >  			break;
> >  		}
> >  		break;
> >  
> >  	case PSMOUSE_GENPS:
> >  		/* Report scroll buttons on NetMice */
> > -		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
> > +		input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
> >  
> >  		/* Extra buttons on Genius NewNet 3D */
> > -		input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
> > -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
> > +		input_report_key(dev, BTN_SIDE,  BIT(6));
> > +		input_report_key(dev, BTN_EXTRA, BIT(7));
> >  		break;
> >  
> >  	case PSMOUSE_THINKPS:
> >  		/* Extra button on ThinkingMouse */
> > -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
> > +		input_report_key(dev, BTN_EXTRA, BIT(3));
> 
> While hunting a 4.17 bug where some openSUSE users lost their ability to
> mouse-click, I came across this commit. Putting aside it's a total mess
> of multiple changes, were these changes above intentional at all? I mean
> changing
>   (packet[0] >> 3) & 1
> to hardwired
>   BIT(3)
> does not look quite right. And if it is for some to me unknown reason,
> it should have been properly documented in the commit log. I believe
> your intent was:
>   packet[0] & BIT(3)
> ?

Yes, that was, I am not sure what I was thinking; I'll fix that up.

Thanks.

-- 
Dmitry

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

end of thread, other threads:[~2018-06-25 18:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-19 23:06 [PATCH 0/7] psmouse assorted cleanups Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 1/7] Input: psmouse - create helper for reporting standard buttons/motion Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 2/7] Input: psmouse - clean up code Dmitry Torokhov
2018-06-25 18:35   ` Jiri Slaby
2018-06-25 18:52     ` Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 3/7] Input: logips2pp " Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 4/7] Input: lifebook " Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice Dmitry Torokhov
     [not found]   ` <CAAZ5spDzNfrosHfCYFtPcQ2bZiE5iSn8Ac40yNNHR-mdjzt83w@mail.gmail.com>
2018-01-22 18:39     ` Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 6/7] Input: synaptics - switch to using input_set_capability Dmitry Torokhov
2018-01-19 23:06 ` [PATCH 7/7] Input: synaptics - handle errors from input_mt_init_slots() Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).