[PATCH 1/4] Input: synaptics - add synaptics_query_int()

[PATCH 1/4] Input: synaptics - add synaptics_query_int()

[Dmitry Torokhov]

Factor out querying and parsing 3-byte response into an integer value.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/synaptics.c | 77 ++++++++++++++++-------------------------
 drivers/input/mouse/synaptics.h |  3 +-
 2 files changed, 32 insertions(+), 48 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index dd6caa0e7e09..7e35da8abcde 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -197,6 +197,22 @@ static int synaptics_send_cmd(struct psmouse *psmouse,
 	return 0;
 }
 
+static int synaptics_query_int(struct psmouse *psmouse, u8 query_cmd, u32 *val)
+{
+	int error;
+	union {
+		__be32 be_val;
+		char buf[4];
+	} resp = { 0 };
+
+	error = synaptics_send_cmd(psmouse, query_cmd, resp.buf + 1);
+	if (error)
+		return error;
+
+	*val = be32_to_cpu(resp.be_val);
+	return 0;
+}
+
 /*
  * Identify Touchpad
  * See also the SYN_ID_* macros
@@ -204,14 +220,12 @@ static int synaptics_send_cmd(struct psmouse *psmouse,
 static int synaptics_identify(struct psmouse *psmouse,
 			      struct synaptics_device_info *info)
 {
-	unsigned char id[3];
 	int error;
 
-	error = synaptics_send_cmd(psmouse, SYN_QUE_IDENTIFY, id);
+	error = synaptics_query_int(psmouse, SYN_QUE_IDENTIFY, &info->identity);
 	if (error)
 		return error;
 
-	info->identity = (id[0] << 16) | (id[1] << 8) | id[2];
 	return SYN_ID_IS_SYNAPTICS(info->identity) ? 0 : -ENXIO;
 }
 
@@ -222,15 +236,7 @@ static int synaptics_identify(struct psmouse *psmouse,
 static int synaptics_model_id(struct psmouse *psmouse,
 			      struct synaptics_device_info *info)
 {
-	unsigned char mi[3];
-	int error;
-
-	error = synaptics_send_cmd(psmouse, SYN_QUE_MODEL, mi);
-	if (error)
-		return error;
-
-	info->model_id = (mi[0] << 16) | (mi[1] << 8) | mi[2];
-	return 0;
+	return synaptics_query_int(psmouse, SYN_QUE_MODEL, &info->model_id);
 }
 
 /*
@@ -239,29 +245,8 @@ static int synaptics_model_id(struct psmouse *psmouse,
 static int synaptics_firmware_id(struct psmouse *psmouse,
 				 struct synaptics_device_info *info)
 {
-	unsigned char fwid[3];
-	int error;
-
-	error = synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid);
-	if (error)
-		return error;
-
-	info->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
-	return 0;
-}
-
-static int synaptics_more_extended_queries(struct psmouse *psmouse,
-					   struct synaptics_device_info *info)
-{
-	unsigned char buf[3];
-	int error;
-
-	error = synaptics_send_cmd(psmouse, SYN_QUE_MEXT_CAPAB_10, buf);
-	if (error)
-		return error;
-
-	info->ext_cap_10 = (buf[0] << 16) | (buf[1] << 8) | buf[2];
-	return 0;
+	return synaptics_query_int(psmouse, SYN_QUE_FIRMWARE_ID,
+				   &info->firmware_id);
 }
 
 /*
@@ -285,7 +270,8 @@ static int synaptics_query_modes(struct psmouse *psmouse,
 	info->board_id = ((bid[0] & 0xfc) << 6) | bid[1];
 
 	if (SYN_MEXT_CAP_BIT(bid[0]))
-		return synaptics_more_extended_queries(psmouse, info);
+		return synaptics_query_int(psmouse, SYN_QUE_MEXT_CAPAB_10,
+					   &info->ext_cap_10);
 
 	return 0;
 }
@@ -297,14 +283,13 @@ static int synaptics_query_modes(struct psmouse *psmouse,
 static int synaptics_capability(struct psmouse *psmouse,
 				struct synaptics_device_info *info)
 {
-	unsigned char cap[3];
 	int error;
 
-	error = synaptics_send_cmd(psmouse, SYN_QUE_CAPABILITIES, cap);
+	error = synaptics_query_int(psmouse, SYN_QUE_CAPABILITIES,
+				    &info->capabilities);
 	if (error)
 		return error;
 
-	info->capabilities = (cap[0] << 16) | (cap[1] << 8) | cap[2];
 	info->ext_cap = info->ext_cap_0c = 0;
 
 	/*
@@ -322,29 +307,27 @@ static int synaptics_capability(struct psmouse *psmouse,
 		info->capabilities = 0;
 
 	if (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 1) {
-		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB, cap)) {
+		error = synaptics_query_int(psmouse, SYN_QUE_EXT_CAPAB,
+					    &info->ext_cap);
+		if (error) {
 			psmouse_warn(psmouse,
 				     "device claims to have extended capabilities, but I'm not able to read them.\n");
 		} else {
-			info->ext_cap = (cap[0] << 16) | (cap[1] << 8) | cap[2];
-
 			/*
 			 * if nExtBtn is greater than 8 it should be considered
 			 * invalid and treated as 0
 			 */
 			if (SYN_CAP_MULTI_BUTTON_NO(info->ext_cap) > 8)
-				info->ext_cap &= 0xff0fff;
+				info->ext_cap &= ~SYN_CAP_MB_MASK;
 		}
 	}
 
 	if (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 4) {
-		error = synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB_0C, cap);
+		error = synaptics_query_int(psmouse, SYN_QUE_EXT_CAPAB_0C,
+					    &info->ext_cap_0c);
 		if (error)
 			psmouse_warn(psmouse,
 				     "device claims to have extended capability 0x0c, but I'm not able to read it.\n");
-		else
-			info->ext_cap_0c =
-				(cap[0] << 16) | (cap[1] << 8) | cap[2];
 	}
 
 	return 0;
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 31652d98b8f7..87fbe5b53803 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -52,7 +52,8 @@
 #define SYN_CAP_PALMDETECT(c)		((c) & (1 << 0))
 #define SYN_CAP_SUBMODEL_ID(c)		(((c) & 0x00ff00) >> 8)
 #define SYN_EXT_CAP_REQUESTS(c)		(((c) & 0x700000) >> 20)
-#define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & 0x00f000) >> 12)
+#define SYN_CAP_MB_MASK			GENMASK(15, 12)
+#define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & SYN_CAP_MB_MASK) >> 12)
 #define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
 #define SYN_MEXT_CAP_BIT(m)		((m) & (1 << 1))
 
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 2/4] Input: synaptics - use BIT() and GENMASK() macros

[Dmitry Torokhov]

Use standard infrastructure, such as BIT and GENMASK, instead of rolling
bitmasks by hand.

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

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 7e35da8abcde..be2265bc386c 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -907,9 +907,9 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
 	if (!SYN_CAP_EXT_BUTTONS_STICK(priv->info.ext_cap_10)) {
 		for (i = 0; i < ext_bits; i++) {
 			input_report_key(dev, BTN_0 + 2 * i,
-				hw->ext_buttons & (1 << i));
+				hw->ext_buttons & BIT(i));
 			input_report_key(dev, BTN_1 + 2 * i,
-				hw->ext_buttons & (1 << (i + ext_bits)));
+				hw->ext_buttons & BIT(i + ext_bits));
 		}
 		return;
 	}
@@ -1534,7 +1534,7 @@ static int synaptics_init_ps2(struct psmouse *psmouse,
 					SYN_NEWABS : SYN_OLDABS;
 
 	psmouse_info(psmouse,
-		     "Touchpad model: %u, fw: %u.%u, id: %#x, caps: %#x/%#x/%#x/%#x, board id: %u, fw id: %u\n",
+		     "Touchpad model: %lu, fw: %lu.%lu, id: %#x, caps: %#x/%#x/%#x/%#x, board id: %u, fw id: %u\n",
 		     SYN_ID_MODEL(info->identity),
 		     SYN_ID_MAJOR(info->identity), SYN_ID_MINOR(info->identity),
 		     info->model_id,
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 87fbe5b53803..7a998fbfa6b0 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -25,37 +25,37 @@
 #define SYN_QUE_MEXT_CAPAB_10		0x10
 
 /* synatics modes */
-#define SYN_BIT_ABSOLUTE_MODE		(1 << 7)
-#define SYN_BIT_HIGH_RATE		(1 << 6)
-#define SYN_BIT_SLEEP_MODE		(1 << 3)
-#define SYN_BIT_DISABLE_GESTURE		(1 << 2)
-#define SYN_BIT_FOUR_BYTE_CLIENT	(1 << 1)
-#define SYN_BIT_W_MODE			(1 << 0)
+#define SYN_BIT_ABSOLUTE_MODE		BIT(7)
+#define SYN_BIT_HIGH_RATE		BIT(6)
+#define SYN_BIT_SLEEP_MODE		BIT(3)
+#define SYN_BIT_DISABLE_GESTURE		BIT(2)
+#define SYN_BIT_FOUR_BYTE_CLIENT	BIT(1)
+#define SYN_BIT_W_MODE			BIT(0)
 
 /* synaptics model ID bits */
-#define SYN_MODEL_ROT180(m)		((m) & (1 << 23))
-#define SYN_MODEL_PORTRAIT(m)		((m) & (1 << 22))
-#define SYN_MODEL_SENSOR(m)		(((m) >> 16) & 0x3f)
-#define SYN_MODEL_HARDWARE(m)		(((m) >> 9) & 0x7f)
-#define SYN_MODEL_NEWABS(m)		((m) & (1 << 7))
-#define SYN_MODEL_PEN(m)		((m) & (1 << 6))
-#define SYN_MODEL_SIMPLIC(m)		((m) & (1 << 5))
-#define SYN_MODEL_GEOMETRY(m)		((m) & 0x0f)
+#define SYN_MODEL_ROT180(m)		((m) & BIT(23))
+#define SYN_MODEL_PORTRAIT(m)		((m) & BIT(22))
+#define SYN_MODEL_SENSOR(m)		(((m) & GENMASK(21, 16)) >> 16)
+#define SYN_MODEL_HARDWARE(m)		(((m) & GENMASK(15, 9)) >> 9)
+#define SYN_MODEL_NEWABS(m)		((m) & BIT(7))
+#define SYN_MODEL_PEN(m)		((m) & BIT(6))
+#define SYN_MODEL_SIMPLIC(m)		((m) & BIT(5))
+#define SYN_MODEL_GEOMETRY(m)		((m) & GENMASK(3, 0))
 
 /* synaptics capability bits */
-#define SYN_CAP_EXTENDED(c)		((c) & (1 << 23))
-#define SYN_CAP_MIDDLE_BUTTON(c)	((c) & (1 << 18))
-#define SYN_CAP_PASS_THROUGH(c)		((c) & (1 << 7))
-#define SYN_CAP_SLEEP(c)		((c) & (1 << 4))
-#define SYN_CAP_FOUR_BUTTON(c)		((c) & (1 << 3))
-#define SYN_CAP_MULTIFINGER(c)		((c) & (1 << 1))
-#define SYN_CAP_PALMDETECT(c)		((c) & (1 << 0))
-#define SYN_CAP_SUBMODEL_ID(c)		(((c) & 0x00ff00) >> 8)
-#define SYN_EXT_CAP_REQUESTS(c)		(((c) & 0x700000) >> 20)
+#define SYN_CAP_EXTENDED(c)		((c) & BIT(23))
+#define SYN_CAP_MIDDLE_BUTTON(c)	((c) & BIT(18))
+#define SYN_CAP_PASS_THROUGH(c)		((c) & BIT(7))
+#define SYN_CAP_SLEEP(c)		((c) & BIT(4))
+#define SYN_CAP_FOUR_BUTTON(c)		((c) & BIT(3))
+#define SYN_CAP_MULTIFINGER(c)		((c) & BIT(1))
+#define SYN_CAP_PALMDETECT(c)		((c) & BIT(0))
+#define SYN_CAP_SUBMODEL_ID(c)		(((c) & GENMASK(15, 8)) >> 8)
+#define SYN_EXT_CAP_REQUESTS(c)		(((c) & GENMASK(22, 20)) >> 20)
 #define SYN_CAP_MB_MASK			GENMASK(15, 12)
 #define SYN_CAP_MULTI_BUTTON_NO(ec)	(((ec) & SYN_CAP_MB_MASK) >> 12)
-#define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
-#define SYN_MEXT_CAP_BIT(m)		((m) & (1 << 1))
+#define SYN_CAP_PRODUCT_ID(ec)		(((ec) & GENMASK(23, 16)) >> 16)
+#define SYN_MEXT_CAP_BIT(m)		((m) & BIT(1))
 
 /*
  * The following describes response for the 0x0c query.
@@ -84,14 +84,14 @@
  *					hinged at the top.
  * 2	0x20	report min		query 0x0f gives min coord reported
  */
-#define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
-#define SYN_CAP_CLICKPAD2BTN(ex0c)	((ex0c) & 0x000100) /* 2-button ClickPad */
-#define SYN_CAP_MAX_DIMENSIONS(ex0c)	((ex0c) & 0x020000)
-#define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & 0x002000)
-#define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
-#define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
-#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
-#define SYN_CAP_INTERTOUCH(ex0c)	((ex0c) & 0x004000)
+#define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & BIT(20)) /* 1-button ClickPad */
+#define SYN_CAP_CLICKPAD2BTN(ex0c)	((ex0c) & BIT(8))  /* 2-button ClickPad */
+#define SYN_CAP_MAX_DIMENSIONS(ex0c)	((ex0c) & BIT(17))
+#define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & BIT(13))
+#define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & BIT(19))
+#define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & BIT(10))
+#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & BIT(11))
+#define SYN_CAP_INTERTOUCH(ex0c)	((ex0c) & BIT(14))
 
 /*
  * The following descibes response for the 0x10 query.
@@ -110,27 +110,27 @@
  * 3	0xff	SecurePad height	the height of the SecurePad fingerprint
  *					reader.
  */
-#define SYN_CAP_EXT_BUTTONS_STICK(ex10)	((ex10) & 0x010000)
-#define SYN_CAP_SECUREPAD(ex10)		((ex10) & 0x020000)
+#define SYN_CAP_EXT_BUTTONS_STICK(ex10)	((ex10) & BIT(16))
+#define SYN_CAP_SECUREPAD(ex10)		((ex10) & BIT(17))
 
 #define SYN_EXT_BUTTON_STICK_L(eb)	(((eb) & BIT(0)) >> 0)
 #define SYN_EXT_BUTTON_STICK_M(eb)	(((eb) & BIT(1)) >> 1)
 #define SYN_EXT_BUTTON_STICK_R(eb)	(((eb) & BIT(2)) >> 2)
 
 /* synaptics modes query bits */
-#define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
-#define SYN_MODE_RATE(m)		((m) & (1 << 6))
-#define SYN_MODE_BAUD_SLEEP(m)		((m) & (1 << 3))
-#define SYN_MODE_DISABLE_GESTURE(m)	((m) & (1 << 2))
-#define SYN_MODE_PACKSIZE(m)		((m) & (1 << 1))
-#define SYN_MODE_WMODE(m)		((m) & (1 << 0))
+#define SYN_MODE_ABSOLUTE(m)		((m) & BIT(7))
+#define SYN_MODE_RATE(m)		((m) & BIT(6))
+#define SYN_MODE_BAUD_SLEEP(m)		((m) & BIT(3))
+#define SYN_MODE_DISABLE_GESTURE(m)	((m) & BIT(2))
+#define SYN_MODE_PACKSIZE(m)		((m) & BIT(1))
+#define SYN_MODE_WMODE(m)		((m) & BIT(0))
 
 /* synaptics identify query bits */
-#define SYN_ID_MODEL(i)			(((i) >> 4) & 0x0f)
-#define SYN_ID_MAJOR(i)			((i) & 0x0f)
-#define SYN_ID_MINOR(i)			(((i) >> 16) & 0xff)
+#define SYN_ID_MODEL(i)			(((i) & GENMASK(7, 4)) >> 4)
+#define SYN_ID_MAJOR(i)			(((i) & GENMASK(3, 0)) >> 0)
+#define SYN_ID_MINOR(i)			(((i) & GENMASK(23, 16)) >> 16)
 #define SYN_ID_FULL(i)			((SYN_ID_MAJOR(i) << 8) | SYN_ID_MINOR(i))
-#define SYN_ID_IS_SYNAPTICS(i)		((((i) >> 8) & 0xff) == 0x47)
+#define SYN_ID_IS_SYNAPTICS(i)		(((i) & GENMASK(15, 8)) == 0x004700U)
 #define SYN_ID_DISGEST_SUPPORTED(i)	(SYN_ID_MAJOR(i) >= 4)
 
 /* synaptics special commands */
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 3/4] Input: synaptics - do not abuse -1 as return value

[Dmitry Torokhov]

Let's stop using -1 as a universal return value and instead propagate
errors from underlying calls up the stack.

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

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index be2265bc386c..224269c849bb 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -82,12 +82,17 @@
 static int synaptics_mode_cmd(struct psmouse *psmouse, unsigned char mode)
 {
 	unsigned char param[1];
+	int error;
+
+	error = psmouse_sliced_command(psmouse, mode);
+	if (error)
+		return error;
 
-	if (psmouse_sliced_command(psmouse, mode))
-		return -1;
 	param[0] = SYN_PS_SET_MODE2;
-	if (ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_SETRATE))
-		return -1;
+	error = ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_SETRATE);
+	if (error)
+		return error;
+
 	return 0;
 }
 
@@ -534,16 +539,19 @@ static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 {
 	static unsigned char param = 0xc8;
 	struct synaptics_data *priv = psmouse->private;
+	int error;
 
 	if (!(SYN_CAP_ADV_GESTURE(priv->info.ext_cap_0c) ||
 	      SYN_CAP_IMAGE_SENSOR(priv->info.ext_cap_0c)))
 		return 0;
 
-	if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL))
-		return -1;
+	error = psmouse_sliced_command(psmouse, SYN_QUE_MODEL);
+	if (error)
+		return error;
 
-	if (ps2_command(&psmouse->ps2dev, &param, PSMOUSE_CMD_SETRATE))
-		return -1;
+	error = ps2_command(&psmouse->ps2dev, &param, PSMOUSE_CMD_SETRATE);
+	if (error)
+		return error;
 
 	/* Advanced gesture mode also sends multi finger data */
 	priv->info.capabilities |= BIT(1);
@@ -554,6 +562,7 @@ static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 static int synaptics_set_mode(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
+	int error;
 
 	priv->mode = 0;
 	if (priv->absolute_mode)
@@ -565,13 +574,18 @@ static int synaptics_set_mode(struct psmouse *psmouse)
 	if (SYN_CAP_EXTENDED(priv->info.capabilities))
 		priv->mode |= SYN_BIT_W_MODE;
 
-	if (synaptics_mode_cmd(psmouse, priv->mode))
-		return -1;
+	error = synaptics_mode_cmd(psmouse, priv->mode);
+	if (error)
+		return error;
 
-	if (priv->absolute_mode &&
-	    synaptics_set_advanced_gesture_mode(psmouse)) {
-		psmouse_err(psmouse, "Advanced gesture mode init failed.\n");
-		return -1;
+	if (priv->absolute_mode) {
+		error = synaptics_set_advanced_gesture_mode(psmouse);
+		if (error) {
+			psmouse_err(psmouse,
+				    "Advanced gesture mode init failed: %d\n",
+				    error);
+			return error;
+		}
 	}
 
 	return 0;
@@ -598,12 +612,17 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
 static int synaptics_pt_write(struct serio *serio, unsigned char c)
 {
 	struct psmouse *parent = serio_get_drvdata(serio->parent);
-	char rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
+	u8 rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
+	int error;
+
+	error = psmouse_sliced_command(parent, c);
+	if (error)
+		return error;
+
+	error = ps2_command(&parent->ps2dev, &rate_param, PSMOUSE_CMD_SETRATE);
+	if (error)
+		return error;
 
-	if (psmouse_sliced_command(parent, c))
-		return -1;
-	if (ps2_command(&parent->ps2dev, &rate_param, PSMOUSE_CMD_SETRATE))
-		return -1;
 	return 0;
 }
 
@@ -1395,19 +1414,21 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 	} while (error && ++retry < 3);
 
 	if (error)
-		return -1;
+		return error;
 
 	if (retry > 1)
 		psmouse_dbg(psmouse, "reconnected after %d tries\n", retry);
 
-	if (synaptics_query_hardware(psmouse, &info)) {
+	error = synaptics_query_hardware(psmouse, &info);
+	if (error) {
 		psmouse_err(psmouse, "Unable to query device.\n");
-		return -1;
+		return error;
 	}
 
-	if (synaptics_set_mode(psmouse)) {
+	error = synaptics_set_mode(psmouse);
+	if (error) {
 		psmouse_err(psmouse, "Unable to initialize device.\n");
-		return -1;
+		return error;
 	}
 
 	if (info.identity != priv->info.identity ||
@@ -1420,7 +1441,7 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 			    priv->info.model_id, info.model_id,
 			    priv->info.capabilities, info.capabilities,
 			    priv->info.ext_cap, info.ext_cap);
-		return -1;
+		return -ENXIO;
 	}
 
 	return 0;
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 4/4] Input: synaptics - use u8 instead of unsigned char

[Dmitry Torokhov]

The rest of the kernel uses u8, u16, etc for data coming form hardware,
let's switch ti using u8 here as well.

Also turn pkt_type into an enum.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/synaptics.c | 60 ++++++++++++++++++++---------------------
 drivers/input/mouse/synaptics.h | 22 ++++++++-------
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 224269c849bb..0b99c8732306 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -79,9 +79,9 @@
 /*
  * Set the synaptics touchpad mode byte by special commands
  */
-static int synaptics_mode_cmd(struct psmouse *psmouse, unsigned char mode)
+static int synaptics_mode_cmd(struct psmouse *psmouse, u8 mode)
 {
-	unsigned char param[1];
+	u8 param[1];
 	int error;
 
 	error = psmouse_sliced_command(psmouse, mode);
@@ -99,7 +99,7 @@ static int synaptics_mode_cmd(struct psmouse *psmouse, unsigned char mode)
 int synaptics_detect(struct psmouse *psmouse, bool set_properties)
 {
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
-	unsigned char param[4];
+	u8 param[4];
 
 	param[0] = 0;
 
@@ -186,12 +186,11 @@ static const char * const forcepad_pnp_ids[] = {
 /*
  * Send a command to the synpatics touchpad by special commands
  */
-static int synaptics_send_cmd(struct psmouse *psmouse,
-			      unsigned char c, unsigned char *param)
+static int synaptics_send_cmd(struct psmouse *psmouse, u8 cmd, u8 *param)
 {
 	int error;
 
-	error = psmouse_sliced_command(psmouse, c);
+	error = psmouse_sliced_command(psmouse, cmd);
 	if (error)
 		return error;
 
@@ -261,7 +260,7 @@ static int synaptics_firmware_id(struct psmouse *psmouse,
 static int synaptics_query_modes(struct psmouse *psmouse,
 				 struct synaptics_device_info *info)
 {
-	unsigned char bid[3];
+	u8 bid[3];
 	int error;
 
 	/* firmwares prior 7.5 have no board_id encoded */
@@ -345,7 +344,7 @@ static int synaptics_capability(struct psmouse *psmouse,
 static int synaptics_resolution(struct psmouse *psmouse,
 				struct synaptics_device_info *info)
 {
-	unsigned char resp[3];
+	u8 resp[3];
 	int error;
 
 	if (SYN_ID_MAJOR(info->identity) < 4)
@@ -537,7 +536,7 @@ static void synaptics_apply_quirks(struct psmouse *psmouse,
 
 static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 {
-	static unsigned char param = 0xc8;
+	static u8 param = 0xc8;
 	struct synaptics_data *priv = psmouse->private;
 	int error;
 
@@ -609,7 +608,7 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
 /*****************************************************************************
  *	Synaptics pass-through PS/2 port support
  ****************************************************************************/
-static int synaptics_pt_write(struct serio *serio, unsigned char c)
+static int synaptics_pt_write(struct serio *serio, u8 c)
 {
 	struct psmouse *parent = serio_get_drvdata(serio->parent);
 	u8 rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
@@ -648,13 +647,12 @@ static void synaptics_pt_stop(struct serio *serio)
 	serio_continue_rx(parent->ps2dev.serio);
 }
 
-static int synaptics_is_pt_packet(unsigned char *buf)
+static int synaptics_is_pt_packet(u8 *buf)
 {
 	return (buf[0] & 0xFC) == 0x84 && (buf[3] & 0xCC) == 0xC4;
 }
 
-static void synaptics_pass_pt_packet(struct serio *ptport,
-				     unsigned char *packet)
+static void synaptics_pass_pt_packet(struct serio *ptport, u8 *packet)
 {
 	struct psmouse *child = serio_get_drvdata(ptport);
 
@@ -717,7 +715,7 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
-static void synaptics_parse_agm(const unsigned char buf[],
+static void synaptics_parse_agm(const u8 buf[],
 				struct synaptics_data *priv,
 				struct synaptics_hw_state *hw)
 {
@@ -746,7 +744,7 @@ static void synaptics_parse_agm(const unsigned char buf[],
 	}
 }
 
-static void synaptics_parse_ext_buttons(const unsigned char buf[],
+static void synaptics_parse_ext_buttons(const u8 buf[],
 					struct synaptics_data *priv,
 					struct synaptics_hw_state *hw)
 {
@@ -758,7 +756,7 @@ static void synaptics_parse_ext_buttons(const unsigned char buf[],
 	hw->ext_buttons |= (buf[5] & ext_mask) << ext_bits;
 }
 
-static int synaptics_parse_hw_state(const unsigned char buf[],
+static int synaptics_parse_hw_state(const u8 buf[],
 				    struct synaptics_data *priv,
 				    struct synaptics_hw_state *hw)
 {
@@ -834,7 +832,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 		} else if (SYN_CAP_MIDDLE_BUTTON(priv->info.capabilities)) {
 			hw->middle = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
 			if (hw->w == 2)
-				hw->scroll = (signed char)(buf[1]);
+				hw->scroll = (s8)buf[1];
 		}
 
 		if (SYN_CAP_FOUR_BUTTON(priv->info.capabilities)) {
@@ -1149,18 +1147,18 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
-static int synaptics_validate_byte(struct psmouse *psmouse,
-				   int idx, unsigned char pkt_type)
+static bool synaptics_validate_byte(struct psmouse *psmouse,
+				    int idx, enum synaptics_pkt_type pkt_type)
 {
-	static const unsigned char newabs_mask[]	= { 0xC8, 0x00, 0x00, 0xC8, 0x00 };
-	static const unsigned char newabs_rel_mask[]	= { 0xC0, 0x00, 0x00, 0xC0, 0x00 };
-	static const unsigned char newabs_rslt[]	= { 0x80, 0x00, 0x00, 0xC0, 0x00 };
-	static const unsigned char oldabs_mask[]	= { 0xC0, 0x60, 0x00, 0xC0, 0x60 };
-	static const unsigned char oldabs_rslt[]	= { 0xC0, 0x00, 0x00, 0x80, 0x00 };
-	const char *packet = psmouse->packet;
+	static const u8 newabs_mask[]	  = { 0xC8, 0x00, 0x00, 0xC8, 0x00 };
+	static const u8 newabs_rel_mask[] = { 0xC0, 0x00, 0x00, 0xC0, 0x00 };
+	static const u8 newabs_rslt[]	  = { 0x80, 0x00, 0x00, 0xC0, 0x00 };
+	static const u8 oldabs_mask[]	  = { 0xC0, 0x60, 0x00, 0xC0, 0x60 };
+	static const u8 oldabs_rslt[]	  = { 0xC0, 0x00, 0x00, 0x80, 0x00 };
+	const u8 *packet = psmouse->packet;
 
 	if (idx < 0 || idx > 4)
-		return 0;
+		return false;
 
 	switch (pkt_type) {
 
@@ -1176,19 +1174,21 @@ static int synaptics_validate_byte(struct psmouse *psmouse,
 
 	default:
 		psmouse_err(psmouse, "unknown packet type %d\n", pkt_type);
-		return 0;
+		return false;
 	}
 }
 
-static unsigned char synaptics_detect_pkt_type(struct psmouse *psmouse)
+static enum synaptics_pkt_type
+synaptics_detect_pkt_type(struct psmouse *psmouse)
 {
 	int i;
 
-	for (i = 0; i < 5; i++)
+	for (i = 0; i < 5; i++) {
 		if (!synaptics_validate_byte(psmouse, i, SYN_NEWABS_STRICT)) {
 			psmouse_info(psmouse, "using relaxed packet validation\n");
 			return SYN_NEWABS_RELAXED;
 		}
+	}
 
 	return SYN_NEWABS_STRICT;
 }
@@ -1393,7 +1393,7 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
 	struct synaptics_device_info info;
-	unsigned char param[2];
+	u8 param[2];
 	int retry = 0;
 	int error;
 
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 7a998fbfa6b0..fc00e005c611 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -137,15 +137,17 @@
 #define SYN_PS_SET_MODE2		0x14
 #define SYN_PS_CLIENT_CMD		0x28
 
-/* synaptics packet types */
-#define SYN_NEWABS			0
-#define SYN_NEWABS_STRICT		1
-#define SYN_NEWABS_RELAXED		2
-#define SYN_OLDABS			3
-
 /* amount to fuzz position data when touchpad reports reduced filtering */
 #define SYN_REDUCED_FILTER_FUZZ		8
 
+/* synaptics packet types */
+enum synaptics_pkt_type {
+	SYN_NEWABS,
+	SYN_NEWABS_STRICT,
+	SYN_NEWABS_RELAXED,
+	SYN_OLDABS,
+};
+
 /*
  * A structure to describe the state of the touchpad hardware (buttons and pad)
  */
@@ -159,8 +161,8 @@ struct synaptics_hw_state {
 	unsigned int middle:1;
 	unsigned int up:1;
 	unsigned int down:1;
-	unsigned char ext_buttons;
-	signed char scroll;
+	u8 ext_buttons;
+	s8 scroll;
 };
 
 /* Data read from the touchpad */
@@ -181,8 +183,8 @@ struct synaptics_device_info {
 struct synaptics_data {
 	struct synaptics_device_info info;
 
-	unsigned char pkt_type;			/* packet type - old, new, etc */
-	unsigned char mode;			/* current mode byte */
+	enum synaptics_pkt_type pkt_type;	/* packet type - old, new, etc */
+	u8 mode;				/* current mode byte */
 	int scroll;
 
 	bool absolute_mode;			/* run in Absolute mode */
-- 
2.12.1.578.ge9c3154ca4-goog

Re: [PATCH 4/4] Input: synaptics - use u8 instead of unsigned char

[Benjamin Tissoires]

On Mar 24 2017 or thereabouts, Dmitry Torokhov wrote:
> The rest of the kernel uses u8, u16, etc for data coming form hardware,
> let's switch ti using u8 here as well.
> 
> Also turn pkt_type into an enum.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

The series looks good to me:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/input/mouse/synaptics.c | 60 ++++++++++++++++++++---------------------
>  drivers/input/mouse/synaptics.h | 22 ++++++++-------
>  2 files changed, 42 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 224269c849bb..0b99c8732306 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -79,9 +79,9 @@
>  /*
>   * Set the synaptics touchpad mode byte by special commands
>   */
> -static int synaptics_mode_cmd(struct psmouse *psmouse, unsigned char mode)
> +static int synaptics_mode_cmd(struct psmouse *psmouse, u8 mode)
>  {
> -	unsigned char param[1];
> +	u8 param[1];
>  	int error;
>  
>  	error = psmouse_sliced_command(psmouse, mode);
> @@ -99,7 +99,7 @@ static int synaptics_mode_cmd(struct psmouse *psmouse, unsigned char mode)
>  int synaptics_detect(struct psmouse *psmouse, bool set_properties)
>  {
>  	struct ps2dev *ps2dev = &psmouse->ps2dev;
> -	unsigned char param[4];
> +	u8 param[4];
>  
>  	param[0] = 0;
>  
> @@ -186,12 +186,11 @@ static const char * const forcepad_pnp_ids[] = {
>  /*
>   * Send a command to the synpatics touchpad by special commands
>   */
> -static int synaptics_send_cmd(struct psmouse *psmouse,
> -			      unsigned char c, unsigned char *param)
> +static int synaptics_send_cmd(struct psmouse *psmouse, u8 cmd, u8 *param)
>  {
>  	int error;
>  
> -	error = psmouse_sliced_command(psmouse, c);
> +	error = psmouse_sliced_command(psmouse, cmd);
>  	if (error)
>  		return error;
>  
> @@ -261,7 +260,7 @@ static int synaptics_firmware_id(struct psmouse *psmouse,
>  static int synaptics_query_modes(struct psmouse *psmouse,
>  				 struct synaptics_device_info *info)
>  {
> -	unsigned char bid[3];
> +	u8 bid[3];
>  	int error;
>  
>  	/* firmwares prior 7.5 have no board_id encoded */
> @@ -345,7 +344,7 @@ static int synaptics_capability(struct psmouse *psmouse,
>  static int synaptics_resolution(struct psmouse *psmouse,
>  				struct synaptics_device_info *info)
>  {
> -	unsigned char resp[3];
> +	u8 resp[3];
>  	int error;
>  
>  	if (SYN_ID_MAJOR(info->identity) < 4)
> @@ -537,7 +536,7 @@ static void synaptics_apply_quirks(struct psmouse *psmouse,
>  
>  static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
>  {
> -	static unsigned char param = 0xc8;
> +	static u8 param = 0xc8;
>  	struct synaptics_data *priv = psmouse->private;
>  	int error;
>  
> @@ -609,7 +608,7 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
>  /*****************************************************************************
>   *	Synaptics pass-through PS/2 port support
>   ****************************************************************************/
> -static int synaptics_pt_write(struct serio *serio, unsigned char c)
> +static int synaptics_pt_write(struct serio *serio, u8 c)
>  {
>  	struct psmouse *parent = serio_get_drvdata(serio->parent);
>  	u8 rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
> @@ -648,13 +647,12 @@ static void synaptics_pt_stop(struct serio *serio)
>  	serio_continue_rx(parent->ps2dev.serio);
>  }
>  
> -static int synaptics_is_pt_packet(unsigned char *buf)
> +static int synaptics_is_pt_packet(u8 *buf)
>  {
>  	return (buf[0] & 0xFC) == 0x84 && (buf[3] & 0xCC) == 0xC4;
>  }
>  
> -static void synaptics_pass_pt_packet(struct serio *ptport,
> -				     unsigned char *packet)
> +static void synaptics_pass_pt_packet(struct serio *ptport, u8 *packet)
>  {
>  	struct psmouse *child = serio_get_drvdata(ptport);
>  
> @@ -717,7 +715,7 @@ static void synaptics_pt_create(struct psmouse *psmouse)
>   *	Functions to interpret the absolute mode packets
>   ****************************************************************************/
>  
> -static void synaptics_parse_agm(const unsigned char buf[],
> +static void synaptics_parse_agm(const u8 buf[],
>  				struct synaptics_data *priv,
>  				struct synaptics_hw_state *hw)
>  {
> @@ -746,7 +744,7 @@ static void synaptics_parse_agm(const unsigned char buf[],
>  	}
>  }
>  
> -static void synaptics_parse_ext_buttons(const unsigned char buf[],
> +static void synaptics_parse_ext_buttons(const u8 buf[],
>  					struct synaptics_data *priv,
>  					struct synaptics_hw_state *hw)
>  {
> @@ -758,7 +756,7 @@ static void synaptics_parse_ext_buttons(const unsigned char buf[],
>  	hw->ext_buttons |= (buf[5] & ext_mask) << ext_bits;
>  }
>  
> -static int synaptics_parse_hw_state(const unsigned char buf[],
> +static int synaptics_parse_hw_state(const u8 buf[],
>  				    struct synaptics_data *priv,
>  				    struct synaptics_hw_state *hw)
>  {
> @@ -834,7 +832,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>  		} else if (SYN_CAP_MIDDLE_BUTTON(priv->info.capabilities)) {
>  			hw->middle = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
>  			if (hw->w == 2)
> -				hw->scroll = (signed char)(buf[1]);
> +				hw->scroll = (s8)buf[1];
>  		}
>  
>  		if (SYN_CAP_FOUR_BUTTON(priv->info.capabilities)) {
> @@ -1149,18 +1147,18 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>  	input_sync(dev);
>  }
>  
> -static int synaptics_validate_byte(struct psmouse *psmouse,
> -				   int idx, unsigned char pkt_type)
> +static bool synaptics_validate_byte(struct psmouse *psmouse,
> +				    int idx, enum synaptics_pkt_type pkt_type)
>  {
> -	static const unsigned char newabs_mask[]	= { 0xC8, 0x00, 0x00, 0xC8, 0x00 };
> -	static const unsigned char newabs_rel_mask[]	= { 0xC0, 0x00, 0x00, 0xC0, 0x00 };
> -	static const unsigned char newabs_rslt[]	= { 0x80, 0x00, 0x00, 0xC0, 0x00 };
> -	static const unsigned char oldabs_mask[]	= { 0xC0, 0x60, 0x00, 0xC0, 0x60 };
> -	static const unsigned char oldabs_rslt[]	= { 0xC0, 0x00, 0x00, 0x80, 0x00 };
> -	const char *packet = psmouse->packet;
> +	static const u8 newabs_mask[]	  = { 0xC8, 0x00, 0x00, 0xC8, 0x00 };
> +	static const u8 newabs_rel_mask[] = { 0xC0, 0x00, 0x00, 0xC0, 0x00 };
> +	static const u8 newabs_rslt[]	  = { 0x80, 0x00, 0x00, 0xC0, 0x00 };
> +	static const u8 oldabs_mask[]	  = { 0xC0, 0x60, 0x00, 0xC0, 0x60 };
> +	static const u8 oldabs_rslt[]	  = { 0xC0, 0x00, 0x00, 0x80, 0x00 };
> +	const u8 *packet = psmouse->packet;
>  
>  	if (idx < 0 || idx > 4)
> -		return 0;
> +		return false;
>  
>  	switch (pkt_type) {
>  
> @@ -1176,19 +1174,21 @@ static int synaptics_validate_byte(struct psmouse *psmouse,
>  
>  	default:
>  		psmouse_err(psmouse, "unknown packet type %d\n", pkt_type);
> -		return 0;
> +		return false;
>  	}
>  }
>  
> -static unsigned char synaptics_detect_pkt_type(struct psmouse *psmouse)
> +static enum synaptics_pkt_type
> +synaptics_detect_pkt_type(struct psmouse *psmouse)
>  {
>  	int i;
>  
> -	for (i = 0; i < 5; i++)
> +	for (i = 0; i < 5; i++) {
>  		if (!synaptics_validate_byte(psmouse, i, SYN_NEWABS_STRICT)) {
>  			psmouse_info(psmouse, "using relaxed packet validation\n");
>  			return SYN_NEWABS_RELAXED;
>  		}
> +	}
>  
>  	return SYN_NEWABS_STRICT;
>  }
> @@ -1393,7 +1393,7 @@ static int synaptics_reconnect(struct psmouse *psmouse)
>  {
>  	struct synaptics_data *priv = psmouse->private;
>  	struct synaptics_device_info info;
> -	unsigned char param[2];
> +	u8 param[2];
>  	int retry = 0;
>  	int error;
>  
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index 7a998fbfa6b0..fc00e005c611 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -137,15 +137,17 @@
>  #define SYN_PS_SET_MODE2		0x14
>  #define SYN_PS_CLIENT_CMD		0x28
>  
> -/* synaptics packet types */
> -#define SYN_NEWABS			0
> -#define SYN_NEWABS_STRICT		1
> -#define SYN_NEWABS_RELAXED		2
> -#define SYN_OLDABS			3
> -
>  /* amount to fuzz position data when touchpad reports reduced filtering */
>  #define SYN_REDUCED_FILTER_FUZZ		8
>  
> +/* synaptics packet types */
> +enum synaptics_pkt_type {
> +	SYN_NEWABS,
> +	SYN_NEWABS_STRICT,
> +	SYN_NEWABS_RELAXED,
> +	SYN_OLDABS,
> +};
> +
>  /*
>   * A structure to describe the state of the touchpad hardware (buttons and pad)
>   */
> @@ -159,8 +161,8 @@ struct synaptics_hw_state {
>  	unsigned int middle:1;
>  	unsigned int up:1;
>  	unsigned int down:1;
> -	unsigned char ext_buttons;
> -	signed char scroll;
> +	u8 ext_buttons;
> +	s8 scroll;
>  };
>  
>  /* Data read from the touchpad */
> @@ -181,8 +183,8 @@ struct synaptics_device_info {
>  struct synaptics_data {
>  	struct synaptics_device_info info;
>  
> -	unsigned char pkt_type;			/* packet type - old, new, etc */
> -	unsigned char mode;			/* current mode byte */
> +	enum synaptics_pkt_type pkt_type;	/* packet type - old, new, etc */
> +	u8 mode;				/* current mode byte */
>  	int scroll;
>  
>  	bool absolute_mode;			/* run in Absolute mode */
> -- 
> 2.12.1.578.ge9c3154ca4-goog
>