Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1596423 > unrolled thread

[PATCH 0/8] PS

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/8] PS Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-09 23:20 +0100
    [PATCH 7/8] Input: synaptics - split device info into a separate structure Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-09 23:20 +0100
    [PATCH 2/8] Input: serio - add fast reconnect option Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-09 23:20 +0100
    [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 01:00 +0100
      Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-10 19:00 +0100
        Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 19:20 +0100
    Re: [PATCH 0/8] PS Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-10 17:00 +0100
      Re: [PATCH 0/8] PS Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 19:00 +0100
        Re: [PATCH 0/8] PS Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-10 19:10 +0100
          Re: [PATCH 0/8] PS Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 19:20 +0100
            Re: [PATCH 0/8] PS Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 21:30 +0100
        Re: [PATCH 0/8] PS Andrew Duggan <aduggan@synaptics.com> - 2017-03-10 20:00 +0100
          Re: [PATCH 0/8] PS Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-10 21:20 +0100
            Re: [PATCH 0/8] PS Andrew Duggan <aduggan@synaptics.com> - 2017-03-10 21:40 +0100

#1596423 — [PATCH 0/8] PS

Hi,

This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
devices for better support of Synaptics RMI4 touchpads (and Elans later).

The main difference is that we do not have platform device, as it only
adds another indirection level, and have psmouse create SMBus companion
directly. Because serio ports complete registration asynchronously, we do
not deadlock on psmouse_mutex when even if we have a pass-through port.
(Frankly we need to revisit this whole serio and psmouse thing, use of
global serio_mutex and psmouse_mutex is hurting us; they were needed when
driver core could not recursively iterate over device and driver lists).

We also do not allow overriding serio driver, instead we teach psmouse
about "special" devices and let it continue own the serio port and make
sure nobody else touches it.

To work around issue with psmouse_reconnect() running sometimes too late,
we add "fast reconnect" option to serio. Not too pretty, but gets the job
done. We may need to revisit whole serio PM story later and stop "cheating"
and pretending that device is resumed when it is not, but for that we need
to teach PM core about devices that are OK not to wait for before resuming
userspace. Anyway, much bigger topic for later.

This seems to be working on X1 Carbon and also not breaking my HP 1040 with
forcepad (unfortunately it seems to be using some other SMBus controller
for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).

Thanks,
Dmitry

Benjamin Tissoires (2):
  Input: psmouse - add support for SMBus companions
  Input: synaptics - add support for Intertouch devices

Dmitry Torokhov (6):
  i2c: export i2c_client_type structure
  Input: serio - add fast reconnect option
  Input: psmouse - implement fast reconnect option
  Input: psmouse - store pointer to current protocol
  Input: psmouse - introduce notion of SMBus companions
  Input: synaptics - split device info into a separate structure

 drivers/i2c/i2c-core.c              |   4 +-
 drivers/input/mouse/Kconfig         |  16 +
 drivers/input/mouse/Makefile        |   2 +
 drivers/input/mouse/psmouse-base.c  | 213 ++++++---
 drivers/input/mouse/psmouse-smbus.c | 280 ++++++++++++
 drivers/input/mouse/psmouse.h       | 106 +++--
 drivers/input/mouse/synaptics.c     | 832 +++++++++++++++++++++++-------------
 drivers/input/mouse/synaptics.h     |  33 +-
 drivers/input/serio/serio.c         |  22 +-
 include/linux/i2c.h                 |   1 +
 include/linux/serio.h               |   1 +
 11 files changed, 1100 insertions(+), 410 deletions(-)
 create mode 100644 drivers/input/mouse/psmouse-smbus.c

[toc] | [next] | [standalone]

#1596424 — [PATCH 7/8] Input: synaptics - split device info into a separate structure

In preparation for SMBus/Intertouch device support, move static device
information that we query form the touchpad upon initialization into
separate structure. This will allow us to query the device without
allocating memory first.

Also stop using "unsigned long", everything fits into 32 bit chunks.

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

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index da7719bf887a..0eb01d1e8802 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -127,9 +127,9 @@ static bool cr48_profile_sensor;
 struct min_max_quirk {
 	const char * const *pnp_ids;
 	struct {
-		unsigned long int min, max;
+		u32 min, max;
 	} board_id;
-	int x_min, x_max, y_min, y_max;
+	u32 x_min, x_max, y_min, y_max;
 };
 
 static const struct min_max_quirk min_max_pnpid_table[] = {
@@ -248,27 +248,31 @@ static int synaptics_send_cmd(struct psmouse *psmouse, unsigned char c, unsigned
  * Read the model-id bytes from the touchpad
  * see also SYN_MODEL_* macros
  */
-static int synaptics_model_id(struct psmouse *psmouse)
+static int synaptics_model_id(struct psmouse *psmouse,
+			      struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char mi[3];
+	int error;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_MODEL, mi))
-		return -1;
-	priv->model_id = (mi[0]<<16) | (mi[1]<<8) | mi[2];
+	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;
 }
 
-static int synaptics_more_extended_queries(struct psmouse *psmouse)
+static int synaptics_more_extended_queries(struct psmouse *psmouse,
+					   struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char buf[3];
+	int error;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_MEXT_CAPAB_10, buf))
-		return -1;
-
-	priv->ext_cap_10 = (buf[0]<<16) | (buf[1]<<8) | buf[2];
+	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;
 }
 
@@ -276,21 +280,24 @@ static int synaptics_more_extended_queries(struct psmouse *psmouse)
  * Read the board id and the "More Extended Queries" from the touchpad
  * The board id is encoded in the "QUERY MODES" response
  */
-static int synaptics_query_modes(struct psmouse *psmouse)
+static int synaptics_query_modes(struct psmouse *psmouse,
+				 struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char bid[3];
+	int error;
 
 	/* firmwares prior 7.5 have no board_id encoded */
-	if (SYN_ID_FULL(priv->identity) < 0x705)
+	if (SYN_ID_FULL(info->identity) < 0x705)
 		return 0;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_MODES, bid))
-		return -1;
-	priv->board_id = ((bid[0] & 0xfc) << 6) | bid[1];
+	error = synaptics_send_cmd(psmouse, SYN_QUE_MODES, bid);
+	if (error)
+		return error;
+
+	info->board_id = ((bid[0] & 0xfc) << 6) | bid[1];
 
 	if (SYN_MEXT_CAP_BIT(bid[0]))
-		return synaptics_more_extended_queries(psmouse);
+		return synaptics_more_extended_queries(psmouse, info);
 
 	return 0;
 }
@@ -298,14 +305,17 @@ static int synaptics_query_modes(struct psmouse *psmouse)
 /*
  * Read the firmware id from the touchpad
  */
-static int synaptics_firmware_id(struct psmouse *psmouse)
+static int synaptics_firmware_id(struct psmouse *psmouse,
+				 struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char fwid[3];
+	int error;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid))
-		return -1;
-	priv->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
+	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;
 }
 
@@ -313,53 +323,57 @@ static int synaptics_firmware_id(struct psmouse *psmouse)
  * Read the capability-bits from the touchpad
  * see also the SYN_CAP_* macros
  */
-static int synaptics_capability(struct psmouse *psmouse)
+static int synaptics_capability(struct psmouse *psmouse,
+				struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char cap[3];
+	int error;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_CAPABILITIES, cap))
-		return -1;
-	priv->capabilities = (cap[0] << 16) | (cap[1] << 8) | cap[2];
-	priv->ext_cap = priv->ext_cap_0c = 0;
+	error = synaptics_send_cmd(psmouse, SYN_QUE_CAPABILITIES, cap);
+	if (error)
+		return error;
+
+	info->capabilities = (cap[0] << 16) | (cap[1] << 8) | cap[2];
+	info->ext_cap = info->ext_cap_0c = 0;
 
 	/*
 	 * Older firmwares had submodel ID fixed to 0x47
 	 */
-	if (SYN_ID_FULL(priv->identity) < 0x705 &&
-	    SYN_CAP_SUBMODEL_ID(priv->capabilities) != 0x47) {
-		return -1;
+	if (SYN_ID_FULL(info->identity) < 0x705 &&
+	    SYN_CAP_SUBMODEL_ID(info->capabilities) != 0x47) {
+		return -ENXIO;
 	}
 
 	/*
 	 * Unless capExtended is set the rest of the flags should be ignored
 	 */
-	if (!SYN_CAP_EXTENDED(priv->capabilities))
-		priv->capabilities = 0;
+	if (!SYN_CAP_EXTENDED(info->capabilities))
+		info->capabilities = 0;
 
-	if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 1) {
+	if (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 1) {
 		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB, cap)) {
 			psmouse_warn(psmouse,
 				     "device claims to have extended capabilities, but I'm not able to read them.\n");
 		} else {
-			priv->ext_cap = (cap[0] << 16) | (cap[1] << 8) | cap[2];
+			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(priv->ext_cap) > 8)
-				priv->ext_cap &= 0xff0fff;
+			if (SYN_CAP_MULTI_BUTTON_NO(info->ext_cap) > 8)
+				info->ext_cap &= 0xff0fff;
 		}
 	}
 
-	if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 4) {
-		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB_0C, cap)) {
+	if (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 4) {
+		error = synaptics_send_cmd(psmouse, SYN_QUE_EXT_CAPAB_0C, cap);
+		if (error)
 			psmouse_warn(psmouse,
 				     "device claims to have extended capability 0x0c, but I'm not able to read it.\n");
-		} else {
-			priv->ext_cap_0c = (cap[0] << 16) | (cap[1] << 8) | cap[2];
-		}
+		else
+			info->ext_cap_0c =
+				(cap[0] << 16) | (cap[1] << 8) | cap[2];
 	}
 
 	return 0;
@@ -369,17 +383,18 @@ static int synaptics_capability(struct psmouse *psmouse)
  * Identify Touchpad
  * See also the SYN_ID_* macros
  */
-static int synaptics_identify(struct psmouse *psmouse)
+static int synaptics_identify(struct psmouse *psmouse,
+			      struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char id[3];
+	int error;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_IDENTIFY, id))
-		return -1;
-	priv->identity = (id[0]<<16) | (id[1]<<8) | id[2];
-	if (SYN_ID_IS_SYNAPTICS(priv->identity))
-		return 0;
-	return -1;
+	error = synaptics_send_cmd(psmouse, SYN_QUE_IDENTIFY, id);
+	if (error)
+		return error;
+
+	info->identity = (id[0] << 16) | (id[1] << 8) | id[2];
+	return SYN_ID_IS_SYNAPTICS(info->identity) ? 0 : -ENXIO;
 }
 
 /*
@@ -387,52 +402,58 @@ static int synaptics_identify(struct psmouse *psmouse)
  * Resolution is left zero if touchpad does not support the query
  */
 
-static int synaptics_resolution(struct psmouse *psmouse)
+static int synaptics_resolution(struct psmouse *psmouse,
+				struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	unsigned char resp[3];
+	int error;
 
-	if (SYN_ID_MAJOR(priv->identity) < 4)
+	if (SYN_ID_MAJOR(info->identity) < 4)
 		return 0;
 
-	if (synaptics_send_cmd(psmouse, SYN_QUE_RESOLUTION, resp) == 0) {
+	error = synaptics_send_cmd(psmouse, SYN_QUE_RESOLUTION, resp);
+	if (!error) {
 		if (resp[0] != 0 && (resp[1] & 0x80) && resp[2] != 0) {
-			priv->x_res = resp[0]; /* x resolution in units/mm */
-			priv->y_res = resp[2]; /* y resolution in units/mm */
+			info->x_res = resp[0]; /* x resolution in units/mm */
+			info->y_res = resp[2]; /* y resolution in units/mm */
 		}
 	}
 
-	if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 5 &&
-	    SYN_CAP_MAX_DIMENSIONS(priv->ext_cap_0c)) {
-		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_MAX_COORDS, resp)) {
+	if (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 5 &&
+	    SYN_CAP_MAX_DIMENSIONS(info->ext_cap_0c)) {
+		error = synaptics_send_cmd(psmouse,
+					   SYN_QUE_EXT_MAX_COORDS, resp);
+		if (error) {
 			psmouse_warn(psmouse,
 				     "device claims to have max coordinates query, but I'm not able to read it.\n");
 		} else {
-			priv->x_max = (resp[0] << 5) | ((resp[1] & 0x0f) << 1);
-			priv->y_max = (resp[2] << 5) | ((resp[1] & 0xf0) >> 3);
+			info->x_max = (resp[0] << 5) | ((resp[1] & 0x0f) << 1);
+			info->y_max = (resp[2] << 5) | ((resp[1] & 0xf0) >> 3);
 			psmouse_info(psmouse,
 				     "queried max coordinates: x [..%d], y [..%d]\n",
-				     priv->x_max, priv->y_max);
+				     info->x_max, info->y_max);
 		}
 	}
 
-	if (SYN_CAP_MIN_DIMENSIONS(priv->ext_cap_0c) &&
-	    (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 7 ||
+	if (SYN_CAP_MIN_DIMENSIONS(info->ext_cap_0c) &&
+	    (SYN_EXT_CAP_REQUESTS(info->capabilities) >= 7 ||
 	     /*
 	      * Firmware v8.1 does not report proper number of extended
 	      * capabilities, but has been proven to report correct min
 	      * coordinates.
 	      */
-	     SYN_ID_FULL(priv->identity) == 0x801)) {
-		if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_MIN_COORDS, resp)) {
+	     SYN_ID_FULL(info->identity) == 0x801)) {
+		error = synaptics_send_cmd(psmouse,
+					   SYN_QUE_EXT_MIN_COORDS, resp);
+		if (error) {
 			psmouse_warn(psmouse,
 				     "device claims to have min coordinates query, but I'm not able to read it.\n");
 		} else {
-			priv->x_min = (resp[0] << 5) | ((resp[1] & 0x0f) << 1);
-			priv->y_min = (resp[2] << 5) | ((resp[1] & 0xf0) >> 3);
+			info->x_min = (resp[0] << 5) | ((resp[1] & 0x0f) << 1);
+			info->y_min = (resp[2] << 5) | ((resp[1] & 0xf0) >> 3);
 			psmouse_info(psmouse,
 				     "queried min coordinates: x [%d..], y [%d..]\n",
-				     priv->x_min, priv->y_min);
+				     info->x_min, info->y_min);
 		}
 	}
 
@@ -443,9 +464,9 @@ static int synaptics_resolution(struct psmouse *psmouse)
  * Apply quirk(s) if the hardware matches
  */
 
-static void synaptics_apply_quirks(struct psmouse *psmouse)
+static void synaptics_apply_quirks(struct psmouse *psmouse,
+				   struct synaptics_device_info *info)
 {
-	struct synaptics_data *priv = psmouse->private;
 	int i;
 
 	for (i = 0; min_max_pnpid_table[i].pnp_ids; i++) {
@@ -454,41 +475,53 @@ static void synaptics_apply_quirks(struct psmouse *psmouse)
 			continue;
 
 		if (min_max_pnpid_table[i].board_id.min != ANY_BOARD_ID &&
-		    priv->board_id < min_max_pnpid_table[i].board_id.min)
+		    info->board_id < min_max_pnpid_table[i].board_id.min)
 			continue;
 
 		if (min_max_pnpid_table[i].board_id.max != ANY_BOARD_ID &&
-		    priv->board_id > min_max_pnpid_table[i].board_id.max)
+		    info->board_id > min_max_pnpid_table[i].board_id.max)
 			continue;
 
-		priv->x_min = min_max_pnpid_table[i].x_min;
-		priv->x_max = min_max_pnpid_table[i].x_max;
-		priv->y_min = min_max_pnpid_table[i].y_min;
-		priv->y_max = min_max_pnpid_table[i].y_max;
+		info->x_min = min_max_pnpid_table[i].x_min;
+		info->x_max = min_max_pnpid_table[i].x_max;
+		info->y_min = min_max_pnpid_table[i].y_min;
+		info->y_max = min_max_pnpid_table[i].y_max;
 		psmouse_info(psmouse,
 			     "quirked min/max coordinates: x [%d..%d], y [%d..%d]\n",
-			     priv->x_min, priv->x_max,
-			     priv->y_min, priv->y_max);
+			     info->x_min, info->x_max,
+			     info->y_min, info->y_max);
 		break;
 	}
 }
 
-static int synaptics_query_hardware(struct psmouse *psmouse)
+static int synaptics_query_hardware(struct psmouse *psmouse,
+				    struct synaptics_device_info *info)
 {
-	if (synaptics_identify(psmouse))
-		return -1;
-	if (synaptics_model_id(psmouse))
-		return -1;
-	if (synaptics_firmware_id(psmouse))
-		return -1;
-	if (synaptics_query_modes(psmouse))
-		return -1;
-	if (synaptics_capability(psmouse))
-		return -1;
-	if (synaptics_resolution(psmouse))
-		return -1;
+	int error;
+
+	error = synaptics_identify(psmouse, info);
+	if (error)
+		return error;
+
+	error = synaptics_model_id(psmouse, info);
+	if (error)
+		return error;
+
+	error = synaptics_firmware_id(psmouse, info);
+	if (error)
+		return error;
 
-	synaptics_apply_quirks(psmouse);
+	error = synaptics_query_modes(psmouse, info);
+	if (error)
+		return error;
+
+	error = synaptics_capability(psmouse, info);
+	if (error)
+		return error;
+
+	error = synaptics_resolution(psmouse, info);
+	if (error)
+		return error;
 
 	return 0;
 }
@@ -498,8 +531,8 @@ static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 	static unsigned char param = 0xc8;
 	struct synaptics_data *priv = psmouse->private;
 
-	if (!(SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
-	      SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)))
+	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))
@@ -509,7 +542,7 @@ static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 		return -1;
 
 	/* Advanced gesture mode also sends multi finger data */
-	priv->capabilities |= BIT(1);
+	priv->info.capabilities |= BIT(1);
 
 	return 0;
 }
@@ -525,7 +558,7 @@ static int synaptics_set_mode(struct psmouse *psmouse)
 		priv->mode |= SYN_BIT_DISABLE_GESTURE;
 	if (psmouse->rate >= 80)
 		priv->mode |= SYN_BIT_HIGH_RATE;
-	if (SYN_CAP_EXTENDED(priv->capabilities))
+	if (SYN_CAP_EXTENDED(priv->info.capabilities))
 		priv->mode |= SYN_BIT_W_MODE;
 
 	if (synaptics_mode_cmd(psmouse, priv->mode))
@@ -695,7 +728,7 @@ static void synaptics_parse_ext_buttons(const unsigned char buf[],
 					struct synaptics_hw_state *hw)
 {
 	unsigned int ext_bits =
-		(SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1;
+		(SYN_CAP_MULTI_BUTTON_NO(priv->info.ext_cap) + 1) >> 1;
 	unsigned int ext_mask = GENMASK(ext_bits - 1, 0);
 
 	hw->ext_buttons = buf[4] & ext_mask;
@@ -708,13 +741,13 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 {
 	memset(hw, 0, sizeof(struct synaptics_hw_state));
 
-	if (SYN_MODEL_NEWABS(priv->model_id)) {
+	if (SYN_MODEL_NEWABS(priv->info.model_id)) {
 		hw->w = (((buf[0] & 0x30) >> 2) |
 			 ((buf[0] & 0x04) >> 1) |
 			 ((buf[3] & 0x04) >> 2));
 
-		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
-			SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) &&
+		if ((SYN_CAP_ADV_GESTURE(priv->info.ext_cap_0c) ||
+			SYN_CAP_IMAGE_SENSOR(priv->info.ext_cap_0c)) &&
 		    hw->w == 2) {
 			synaptics_parse_agm(buf, priv, hw);
 			return 1;
@@ -767,7 +800,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 
 			hw->left = priv->report_press;
 
-		} else if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+		} else if (SYN_CAP_CLICKPAD(priv->info.ext_cap_0c)) {
 			/*
 			 * Clickpad's button is transmitted as middle button,
 			 * however, since it is primary button, we will report
@@ -775,18 +808,18 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 			 */
 			hw->left = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
 
-		} else if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+		} 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]);
 		}
 
-		if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
+		if (SYN_CAP_FOUR_BUTTON(priv->info.capabilities)) {
 			hw->up   = ((buf[0] ^ buf[3]) & 0x01) ? 1 : 0;
 			hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
 		}
 
-		if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) > 0 &&
+		if (SYN_CAP_MULTI_BUTTON_NO(priv->info.ext_cap) > 0 &&
 		    ((buf[0] ^ buf[3]) & 0x02)) {
 			synaptics_parse_ext_buttons(buf, priv, hw);
 		}
@@ -855,19 +888,19 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
 {
 	struct input_dev *dev = psmouse->dev;
 	struct synaptics_data *priv = psmouse->private;
-	int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1;
+	int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->info.ext_cap) + 1) >> 1;
 	int i;
 
-	if (!SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap))
+	if (!SYN_CAP_MULTI_BUTTON_NO(priv->info.ext_cap))
 		return;
 
 	/* Bug in FW 8.1 & 8.2, buttons are reported only when ExtBit is 1 */
-	if ((SYN_ID_FULL(priv->identity) == 0x801 ||
-	     SYN_ID_FULL(priv->identity) == 0x802) &&
+	if ((SYN_ID_FULL(priv->info.identity) == 0x801 ||
+	     SYN_ID_FULL(priv->info.identity) == 0x802) &&
 	    !((psmouse->packet[0] ^ psmouse->packet[3]) & 0x02))
 		return;
 
-	if (!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10)) {
+	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));
@@ -905,10 +938,10 @@ static void synaptics_report_buttons(struct psmouse *psmouse,
 	input_report_key(dev, BTN_LEFT, hw->left);
 	input_report_key(dev, BTN_RIGHT, hw->right);
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities))
+	if (SYN_CAP_MIDDLE_BUTTON(priv->info.capabilities))
 		input_report_key(dev, BTN_MIDDLE, hw->middle);
 
-	if (SYN_CAP_FOUR_BUTTON(priv->capabilities)) {
+	if (SYN_CAP_FOUR_BUTTON(priv->info.capabilities)) {
 		input_report_key(dev, BTN_FORWARD, hw->up);
 		input_report_key(dev, BTN_BACK, hw->down);
 	}
@@ -933,7 +966,7 @@ static void synaptics_report_mt_data(struct psmouse *psmouse,
 		pos[i].y = synaptics_invert_y(hw[i]->y);
 	}
 
-	input_mt_assign_slots(dev, slot, pos, nsemi, DMAX * priv->x_res);
+	input_mt_assign_slots(dev, slot, pos, nsemi, DMAX * priv->info.x_res);
 
 	for (i = 0; i < nsemi; i++) {
 		input_mt_slot(dev, slot[i]);
@@ -1000,6 +1033,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 {
 	struct input_dev *dev = psmouse->dev;
 	struct synaptics_data *priv = psmouse->private;
+	struct synaptics_device_info *info = &priv->info;
 	struct synaptics_hw_state hw;
 	int num_fingers;
 	int finger_width;
@@ -1007,7 +1041,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
 		return;
 
-	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+	if (SYN_CAP_IMAGE_SENSOR(info->ext_cap_0c)) {
 		synaptics_image_sensor_process(psmouse, &hw);
 		return;
 	}
@@ -1035,18 +1069,18 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	if (hw.z > 0 && hw.x > 1) {
 		num_fingers = 1;
 		finger_width = 5;
-		if (SYN_CAP_EXTENDED(priv->capabilities)) {
+		if (SYN_CAP_EXTENDED(info->capabilities)) {
 			switch (hw.w) {
 			case 0 ... 1:
-				if (SYN_CAP_MULTIFINGER(priv->capabilities))
+				if (SYN_CAP_MULTIFINGER(info->capabilities))
 					num_fingers = hw.w + 2;
 				break;
 			case 2:
-				if (SYN_MODEL_PEN(priv->model_id))
+				if (SYN_MODEL_PEN(info->model_id))
 					;   /* Nothing, treat a pen as a single finger */
 				break;
 			case 4 ... 15:
-				if (SYN_CAP_PALMDETECT(priv->capabilities))
+				if (SYN_CAP_PALMDETECT(info->capabilities))
 					finger_width = hw.w;
 				break;
 			}
@@ -1061,7 +1095,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		return;
 	}
 
-	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
+	if (SYN_CAP_ADV_GESTURE(info->ext_cap_0c))
 		synaptics_report_semi_mt_data(dev, &hw, &priv->agm,
 					      num_fingers);
 
@@ -1078,11 +1112,11 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	}
 	input_report_abs(dev, ABS_PRESSURE, hw.z);
 
-	if (SYN_CAP_PALMDETECT(priv->capabilities))
+	if (SYN_CAP_PALMDETECT(info->capabilities))
 		input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
 
 	input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
-	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
+	if (SYN_CAP_MULTIFINGER(info->capabilities)) {
 		input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
 		input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
 	}
@@ -1144,7 +1178,7 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
 		if (unlikely(priv->pkt_type == SYN_NEWABS))
 			priv->pkt_type = synaptics_detect_pkt_type(psmouse);
 
-		if (SYN_CAP_PASS_THROUGH(priv->capabilities) &&
+		if (SYN_CAP_PASS_THROUGH(priv->info.capabilities) &&
 		    synaptics_is_pt_packet(psmouse->packet)) {
 			if (priv->pt_port)
 				synaptics_pass_pt_packet(priv->pt_port,
@@ -1163,26 +1197,27 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
  *	Driver initialization/cleanup functions
  ****************************************************************************/
 static void set_abs_position_params(struct input_dev *dev,
-				    struct synaptics_data *priv, int x_code,
-				    int y_code)
+				    struct synaptics_device_info *info,
+				    int x_code, int y_code)
 {
-	int x_min = priv->x_min ?: XMIN_NOMINAL;
-	int x_max = priv->x_max ?: XMAX_NOMINAL;
-	int y_min = priv->y_min ?: YMIN_NOMINAL;
-	int y_max = priv->y_max ?: YMAX_NOMINAL;
-	int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
+	int x_min = info->x_min ?: XMIN_NOMINAL;
+	int x_max = info->x_max ?: XMAX_NOMINAL;
+	int y_min = info->y_min ?: YMIN_NOMINAL;
+	int y_max = info->y_max ?: YMAX_NOMINAL;
+	int fuzz = SYN_CAP_REDUCED_FILTERING(info->ext_cap_0c) ?
 			SYN_REDUCED_FILTER_FUZZ : 0;
 
 	input_set_abs_params(dev, x_code, x_min, x_max, fuzz, 0);
 	input_set_abs_params(dev, y_code, y_min, y_max, fuzz, 0);
-	input_abs_set_res(dev, x_code, priv->x_res);
-	input_abs_set_res(dev, y_code, priv->y_res);
+	input_abs_set_res(dev, x_code, info->x_res);
+	input_abs_set_res(dev, y_code, info->y_res);
 }
 
 static void 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;
 
 	/* Things that apply to both modes */
@@ -1191,7 +1226,7 @@ static void set_input_params(struct psmouse *psmouse,
 	__set_bit(BTN_LEFT, dev->keybit);
 	__set_bit(BTN_RIGHT, dev->keybit);
 
-	if (SYN_CAP_MIDDLE_BUTTON(priv->capabilities))
+	if (SYN_CAP_MIDDLE_BUTTON(info->capabilities))
 		__set_bit(BTN_MIDDLE, dev->keybit);
 
 	if (!priv->absolute_mode) {
@@ -1204,15 +1239,15 @@ static void set_input_params(struct psmouse *psmouse,
 
 	/* Absolute mode */
 	__set_bit(EV_ABS, dev->evbit);
-	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
+	set_abs_position_params(dev, &priv->info, ABS_X, ABS_Y);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
 	if (cr48_profile_sensor)
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
 
-	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
-		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
-					ABS_MT_POSITION_Y);
+	if (SYN_CAP_IMAGE_SENSOR(info->ext_cap_0c)) {
+		set_abs_position_params(dev, info,
+					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);
@@ -1220,9 +1255,9 @@ static void set_input_params(struct psmouse *psmouse,
 		/* Image sensors can signal 4 and 5 finger clicks */
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
 		__set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
-	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
-		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
-					ABS_MT_POSITION_Y);
+	} else if (SYN_CAP_ADV_GESTURE(info->ext_cap_0c)) {
+		set_abs_position_params(dev, info,
+					ABS_MT_POSITION_X, ABS_MT_POSITION_Y);
 		/*
 		 * Profile sensor in CR-48 tracks contacts reasonably well,
 		 * other non-image sensors with AGM use semi-mt.
@@ -1233,35 +1268,35 @@ static void set_input_params(struct psmouse *psmouse,
 					INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
 	}
 
-	if (SYN_CAP_PALMDETECT(priv->capabilities))
+	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);
 
-	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
+	if (SYN_CAP_MULTIFINGER(info->capabilities)) {
 		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
 		__set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
 	}
 
-	if (SYN_CAP_FOUR_BUTTON(priv->capabilities) ||
-	    SYN_CAP_MIDDLE_BUTTON(priv->capabilities)) {
+	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);
 	}
 
-	if (!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10))
-		for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++)
+	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);
 
-	if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+	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(priv->ext_cap_10))
+		    !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);
@@ -1315,7 +1350,8 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
 
-	if (!priv->absolute_mode && SYN_ID_DISGEST_SUPPORTED(priv->identity))
+	if (!priv->absolute_mode &&
+			SYN_ID_DISGEST_SUPPORTED(priv->info.identity))
 		device_remove_file(&psmouse->ps2dev.serio->dev,
 				   &psmouse_attr_disable_gesture.dattr);
 
@@ -1327,7 +1363,7 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 static int synaptics_reconnect(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
-	struct synaptics_data old_priv = *priv;
+	struct synaptics_device_info info;
 	unsigned char param[2];
 	int retry = 0;
 	int error;
@@ -1354,7 +1390,7 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 	if (retry > 1)
 		psmouse_dbg(psmouse, "reconnected after %d tries\n", retry);
 
-	if (synaptics_query_hardware(psmouse)) {
+	if (synaptics_query_hardware(psmouse, &info)) {
 		psmouse_err(psmouse, "Unable to query device.\n");
 		return -1;
 	}
@@ -1364,16 +1400,16 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 		return -1;
 	}
 
-	if (old_priv.identity != priv->identity ||
-	    old_priv.model_id != priv->model_id ||
-	    old_priv.capabilities != priv->capabilities ||
-	    old_priv.ext_cap != priv->ext_cap) {
+	if (info.identity != priv->info.identity ||
+	    info.model_id != priv->info.model_id ||
+	    info.capabilities != priv->info.capabilities ||
+	    info.ext_cap != priv->info.ext_cap) {
 		psmouse_err(psmouse,
-			    "hardware appears to be different: id(%ld-%ld), model(%ld-%ld), caps(%lx-%lx), ext(%lx-%lx).\n",
-			    old_priv.identity, priv->identity,
-			    old_priv.model_id, priv->model_id,
-			    old_priv.capabilities, priv->capabilities,
-			    old_priv.ext_cap, priv->ext_cap);
+			    "hardware appears to be different: id(%u-%u), model(%u-%u), caps(%x-%x), ext(%x-%x).\n",
+			    priv->info.identity, info.identity,
+			    priv->info.model_id, info.model_id,
+			    priv->info.capabilities, info.capabilities,
+			    priv->info.ext_cap, info.ext_cap);
 		return -1;
 	}
 
@@ -1457,6 +1493,7 @@ void __init synaptics_module_init(void)
 static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 {
 	struct synaptics_data *priv;
+	struct synaptics_device_info *info;
 	int err = -1;
 
 	/*
@@ -1475,15 +1512,19 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 	if (!priv)
 		return -ENOMEM;
 
+	info = &priv->info;
+
 	psmouse_reset(psmouse);
 
-	if (synaptics_query_hardware(psmouse)) {
+	if (synaptics_query_hardware(psmouse, info)) {
 		psmouse_err(psmouse, "Unable to query device.\n");
 		goto init_fail;
 	}
 
+	synaptics_apply_quirks(psmouse, info);
+
 	priv->absolute_mode = absolute_mode;
-	if (SYN_ID_DISGEST_SUPPORTED(priv->identity))
+	if (SYN_ID_DISGEST_SUPPORTED(info->identity))
 		priv->disable_gesture = true;
 
 	/*
@@ -1497,15 +1538,16 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 		goto init_fail;
 	}
 
-	priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
+	priv->pkt_type = SYN_MODEL_NEWABS(info->model_id) ?
+					SYN_NEWABS : SYN_OLDABS;
 
 	psmouse_info(psmouse,
-		     "Touchpad model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx/%#lx, board id: %lu, fw id: %lu\n",
-		     SYN_ID_MODEL(priv->identity),
-		     SYN_ID_MAJOR(priv->identity), SYN_ID_MINOR(priv->identity),
-		     priv->model_id,
-		     priv->capabilities, priv->ext_cap, priv->ext_cap_0c,
-		     priv->ext_cap_10, priv->board_id, priv->firmware_id);
+		     "Touchpad model: %u, fw: %u.%u, 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,
+		     info->capabilities, info->ext_cap, info->ext_cap_0c,
+		     info->ext_cap_10, info->board_id, info->firmware_id);
 
 	set_input_params(psmouse, priv);
 
@@ -1516,8 +1558,8 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 	 * Hardware info bits seem to be good candidates as they
 	 * are documented to be for Synaptics corp. internal use.
 	 */
-	psmouse->model = ((priv->model_id & 0x00ff0000) >> 8) |
-			  (priv->model_id & 0x000000ff);
+	psmouse->model = ((info->model_id & 0x00ff0000) >> 8) |
+			  (info->model_id & 0x000000ff);
 
 	if (absolute_mode) {
 		psmouse->protocol_handler = synaptics_process_byte;
@@ -1535,7 +1577,7 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 	/* Synaptics can usually stay in sync without extra help */
 	psmouse->resync_time = 0;
 
-	if (SYN_CAP_PASS_THROUGH(priv->capabilities))
+	if (SYN_CAP_PASS_THROUGH(info->capabilities))
 		synaptics_pt_create(psmouse);
 
 	/*
@@ -1550,7 +1592,7 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 		psmouse->rate = 40;
 	}
 
-	if (!priv->absolute_mode && SYN_ID_DISGEST_SUPPORTED(priv->identity)) {
+	if (!priv->absolute_mode && SYN_ID_DISGEST_SUPPORTED(info->identity)) {
 		err = device_create_file(&psmouse->ps2dev.serio->dev,
 					 &psmouse_attr_disable_gesture.dattr);
 		if (err) {
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 116ae2546ace..4d1452718cbc 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -161,19 +161,23 @@ struct synaptics_hw_state {
 	signed char scroll;
 };
 
+/* Data read from the touchpad */
+struct synaptics_device_info {
+	u32 model_id;		/* Model-ID */
+	u32 firmware_id;	/* Firmware-ID */
+	u32 board_id;		/* Board-ID */
+	u32 capabilities;	/* Capabilities */
+	u32 ext_cap;		/* Extended Capabilities */
+	u32 ext_cap_0c;		/* Ext Caps from 0x0c query */
+	u32 ext_cap_10;		/* Ext Caps from 0x10 query */
+	u32 identity;		/* Identification */
+	u32 x_res, y_res;	/* X/Y resolution in units/mm */
+	u32 x_max, y_max;	/* Max coordinates (from FW) */
+	u32 x_min, y_min;	/* Min coordinates (from FW) */
+};
+
 struct synaptics_data {
-	/* Data read from the touchpad */
-	unsigned long int model_id;		/* Model-ID */
-	unsigned long int firmware_id;		/* Firmware-ID */
-	unsigned long int board_id;		/* Board-ID */
-	unsigned long int capabilities;		/* Capabilities */
-	unsigned long int ext_cap;		/* Extended Capabilities */
-	unsigned long int ext_cap_0c;		/* Ext Caps from 0x0c query */
-	unsigned long int ext_cap_10;		/* Ext Caps from 0x10 query */
-	unsigned long int identity;		/* Identification */
-	unsigned int x_res, y_res;		/* X/Y resolution in units/mm */
-	unsigned int x_max, y_max;		/* Max coordinates (from FW) */
-	unsigned int x_min, y_min;		/* Min coordinates (from FW) */
+	struct synaptics_device_info info;
 
 	unsigned char pkt_type;			/* packet type - old, new, etc */
 	unsigned char mode;			/* current mode byte */
-- 
2.12.0.246.ga2ecc84866-goog

[toc] | [prev] | [next] | [standalone]

#1596426 — [PATCH 2/8] Input: serio - add fast reconnect option

Devices connected to serio bus are quite slow, and to improve apparent
speed of resume process, serio core resumes (reconnects) its devices
asynchronously, by posting port reconnect requests to a workqueue.
Unfortunately this means that if there is a dependent device of a given
serio port (for example SMBus part of touchpad connected via both PS/2 and
SMBus), we do not have a good way of ensuring resume order.

This change allows drivers to define "fast reconnect" handlers that would
be called in-line during system resume. Drivers need to ensure that these
handlers are truly "fast".

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/serio.c | 22 +++++++++++++++++-----
 include/linux/serio.h       |  1 +
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 1ca7f551e2da..34793ce2df91 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -953,12 +953,24 @@ static int serio_suspend(struct device *dev)
 static int serio_resume(struct device *dev)
 {
 	struct serio *serio = to_serio_port(dev);
+	int error = -ENOENT;
 
-	/*
-	 * Driver reconnect can take a while, so better let kseriod
-	 * deal with it.
-	 */
-	serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT);
+	mutex_lock(&serio->drv_mutex);
+	if (serio->drv && serio->drv->fast_reconnect) {
+		error = serio->drv->fast_reconnect(serio);
+		if (error && error != -ENOENT)
+			dev_warn(dev, "fast reconnect failed with error %d\n",
+				 error);
+	}
+	mutex_unlock(&serio->drv_mutex);
+
+	if (error) {
+		/*
+		 * Driver reconnect can take a while, so better let
+		 * kseriod deal with it.
+		 */
+		serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT);
+	}
 
 	return 0;
 }
diff --git a/include/linux/serio.h b/include/linux/serio.h
index c733cff44e18..138a5efe863a 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -77,6 +77,7 @@ struct serio_driver {
 	irqreturn_t (*interrupt)(struct serio *, unsigned char, unsigned int);
 	int  (*connect)(struct serio *, struct serio_driver *drv);
 	int  (*reconnect)(struct serio *);
+	int  (*fast_reconnect)(struct serio *);
 	void (*disconnect)(struct serio *);
 	void (*cleanup)(struct serio *);
 
-- 
2.12.0.246.ga2ecc84866-goog

[toc] | [prev] | [next] | [standalone]

#1596492 — [PATCH 6/8] Input: psmouse - add support for SMBus companions

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

This provides glue between PS/2 devices that enumerate the RMI4 devices
and Elan touchpads to the RMI4 (or Elan) SMBus driver.

The SMBus devices keep their PS/2 connection alive. If the initialization
process goes too far (psmouse_activate called), the device disconnects
from the I2C bus and stays on the PS/2 bus, that is why we explicitly
disable PS/2 device reporting (by calling psmouse_deactivate) before
trying to register SMBus companion device.

The HID over I2C devices are enumerated through the ACPI DSDT, and
their PS/2 device also exports the InterTouch bit in the extended
capability 0x0C. However, the firmware keeps its I2C connection open
even after going further in the PS/2 initialization. We don't need
to take extra precautions with those device, especially because they
block their PS/2 communication when HID over I2C is used.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

[ Added Wolfram to the CC ]

 drivers/input/mouse/Kconfig         |   4 +
 drivers/input/mouse/Makefile        |   2 +
 drivers/input/mouse/psmouse-base.c  |  16 ++-
 drivers/input/mouse/psmouse-smbus.c | 280 ++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/psmouse.h       |  33 +++++
 5 files changed, 333 insertions(+), 2 deletions(-)
 create mode 100644 drivers/input/mouse/psmouse-smbus.c

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 096abb4ad5cd..87bde8a210c7 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -171,6 +171,10 @@ config MOUSE_PS2_VMMOUSE
 
 	  If unsure, say N.
 
+config MOUSE_PS2_SMBUS
+	bool
+	depends on MOUSE_PS2
+
 config MOUSE_SERIAL
 	tristate "Serial mouse"
 	select SERIO
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 6168b134937b..56bf0ad877c6 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -39,6 +39,8 @@ psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT)	+= touchkit_ps2.o
 psmouse-$(CONFIG_MOUSE_PS2_CYPRESS)	+= cypress_ps2.o
 psmouse-$(CONFIG_MOUSE_PS2_VMMOUSE)	+= vmmouse.o
 
+psmouse-$(CONFIG_MOUSE_PS2_SMBUS)	+= psmouse-smbus.o
+
 elan_i2c-objs := elan_i2c_core.o
 elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_I2C)	+= elan_i2c_i2c.o
 elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_SMBUS)	+= elan_i2c_smbus.o
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 40f09ce84f14..bdb48b2acc57 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1996,16 +1996,27 @@ static int __init psmouse_init(void)
 	synaptics_module_init();
 	hgpk_module_init();
 
+	err = psmouse_smbus_module_init();
+	if (err)
+		return err;
+
 	kpsmoused_wq = alloc_ordered_workqueue("kpsmoused", 0);
 	if (!kpsmoused_wq) {
 		pr_err("failed to create kpsmoused workqueue\n");
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto err_smbus_exit;
 	}
 
 	err = serio_register_driver(&psmouse_drv);
 	if (err)
-		destroy_workqueue(kpsmoused_wq);
+		goto err_destroy_wq;
 
+	return 0;
+
+err_destroy_wq:
+	destroy_workqueue(kpsmoused_wq);
+err_smbus_exit:
+	psmouse_smbus_module_exit();
 	return err;
 }
 
@@ -2013,6 +2024,7 @@ static void __exit psmouse_exit(void)
 {
 	serio_unregister_driver(&psmouse_drv);
 	destroy_workqueue(kpsmoused_wq);
+	psmouse_smbus_module_exit();
 }
 
 module_init(psmouse_init);
diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
new file mode 100644
index 000000000000..5bda551b752f
--- /dev/null
+++ b/drivers/input/mouse/psmouse-smbus.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (c) 2017 Red Hat, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/libps2.h>
+#include <linux/i2c.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include "psmouse.h"
+
+struct psmouse_smbus_dev {
+	struct psmouse *psmouse;
+	struct i2c_client *client;
+	struct list_head node;
+	unsigned short addr;
+	bool dead;
+};
+
+static LIST_HEAD(psmouse_smbus_list);
+static DEFINE_MUTEX(psmouse_smbus_mutex);
+
+static void psmouse_smbus_check_adapter(struct i2c_adapter *adapter)
+{
+	struct psmouse_smbus_dev *smbdev;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY))
+		return;
+
+	mutex_lock(&psmouse_smbus_mutex);
+
+	list_for_each_entry(smbdev, &psmouse_smbus_list, node) {
+		if (smbdev->dead)
+			continue;
+
+		if (smbdev->client)
+			continue;
+
+		if (i2c_probe_func_quick_read(adapter, smbdev->addr) < 0)
+			continue;
+
+		/* Device seems to be there, let's try switching over */
+		psmouse_dbg(smbdev->psmouse,
+			    "SMBus companion appeared, triggering rescan\n");
+		serio_rescan(smbdev->psmouse->ps2dev.serio);
+	}
+
+	mutex_unlock(&psmouse_smbus_mutex);
+}
+
+static void psmouse_smbus_detach_i2c_client(struct i2c_client *client)
+{
+	struct psmouse_smbus_dev *smbdev;
+
+	mutex_lock(&psmouse_smbus_mutex);
+
+	list_for_each_entry(smbdev, &psmouse_smbus_list, node) {
+		if (smbdev->client == client) {
+			psmouse_dbg(smbdev->psmouse,
+				    "Marking SMBus companion %s as gone\n",
+				    dev_name(&smbdev->client->dev));
+			smbdev->client = NULL;
+			smbdev->dead = true;
+			serio_rescan(smbdev->psmouse->ps2dev.serio);
+		}
+	}
+
+	mutex_unlock(&psmouse_smbus_mutex);
+}
+
+static int psmouse_smbus_notifier_call(struct notifier_block *nb,
+				       unsigned long action, void *data)
+{
+	struct device *dev = data;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		if (dev->type == &i2c_adapter_type)
+			psmouse_smbus_check_adapter(to_i2c_adapter(dev));
+		break;
+
+	case BUS_NOTIFY_REMOVED_DEVICE:
+		if (dev->type == &i2c_client_type)
+			psmouse_smbus_detach_i2c_client(to_i2c_client(dev));
+		break;
+	}
+
+	return 0;
+}
+
+static struct notifier_block psmouse_smbus_notifier = {
+	.notifier_call = psmouse_smbus_notifier_call,
+};
+
+static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse)
+{
+	return PSMOUSE_FULL_PACKET;
+}
+
+static int psmouse_smbus_reconnect(struct psmouse *psmouse)
+{
+	psmouse_deactivate(psmouse);
+
+	return 0;
+}
+
+struct psmouse_smbus_removal_work {
+	struct work_struct work;
+	struct i2c_client *client;
+};
+
+static void psmouse_smbus_remove_i2c_device(struct work_struct *work)
+{
+	struct psmouse_smbus_removal_work *rwork =
+		container_of(work, struct psmouse_smbus_removal_work, work);
+
+	dev_dbg(&rwork->client->dev, "destroying SMBus companion device\n");
+	i2c_unregister_device(rwork->client);
+
+	kfree(rwork);
+}
+
+/*
+ * This schedules removal of SMBus companion device. We have to do
+ * it in a separate tread to avoid deadlocking on psmouse_mutex in
+ * case the device has a trackstick (which is also driven by psmouse).
+ *
+ * Note that this may be racing with i2c adapter removal, but we
+ * can't do anything about that: i2c automatically destroys clients
+ * attached to an adapter that is being removed. This has to be
+ * fixed in i2c core.
+ */
+static void psmouse_smbus_schedule_remove(struct i2c_client *client)
+{
+	struct psmouse_smbus_removal_work *rwork;
+
+	rwork = kzalloc(sizeof(*rwork), GFP_KERNEL);
+	if (rwork) {
+		INIT_WORK(&rwork->work, psmouse_smbus_remove_i2c_device);
+		rwork->client = client;
+
+		schedule_work(&rwork->work);
+	}
+}
+
+static void psmouse_smbus_disconnect(struct psmouse *psmouse)
+{
+	struct psmouse_smbus_dev *smbdev = psmouse->private;
+
+	mutex_lock(&psmouse_smbus_mutex);
+	list_del(&smbdev->node);
+	mutex_unlock(&psmouse_smbus_mutex);
+
+	if (smbdev->client) {
+		psmouse_dbg(smbdev->psmouse,
+			    "posting removal request for SMBus companion %s\n",
+			    dev_name(&smbdev->client->dev));
+		psmouse_smbus_schedule_remove(smbdev->client);
+	}
+
+	kfree(smbdev);
+	psmouse->private = NULL;
+}
+
+struct psmouse_smbus_companion_req {
+	struct i2c_board_info *board;
+	struct i2c_client **client;
+};
+
+static int psmouse_smbus_create_companion(struct device *dev, void *data)
+{
+	struct psmouse_smbus_companion_req *req = data;
+	unsigned short addr[] = { req->board->addr, I2C_CLIENT_END };
+	struct i2c_adapter *adapter;
+
+	adapter = i2c_verify_adapter(dev);
+	if (!adapter)
+		return 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY))
+		return 0;
+
+	*req->client = i2c_new_probed_device(adapter, req->board, addr, NULL);
+	if (!*req->client)
+		return 0;
+
+	/* We have our(?) device, stop iterating i2c bus. */
+	return 1;
+}
+
+void psmouse_smbus_cleanup(struct psmouse *psmouse)
+{
+	struct psmouse_smbus_dev *smbdev, *tmp;
+
+	mutex_lock(&psmouse_smbus_mutex);
+
+	list_for_each_entry_safe(smbdev, tmp, &psmouse_smbus_list, node) {
+		if (psmouse == smbdev->psmouse) {
+			list_del(&smbdev->node);
+			kfree(smbdev);
+		}
+	}
+
+	mutex_unlock(&psmouse_smbus_mutex);
+}
+
+int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
+		       bool leave_breadcrumbs)
+{
+	struct psmouse_smbus_companion_req req;
+	struct psmouse_smbus_dev *smbdev;
+	int error;
+
+	smbdev = kzalloc(sizeof(*smbdev), GFP_KERNEL);
+	if (!smbdev)
+		return -ENOMEM;
+
+	smbdev->psmouse = psmouse;
+	smbdev->addr = board->addr;
+
+	psmouse->private = smbdev;
+	psmouse->protocol_handler = psmouse_smbus_process_byte;
+	psmouse->reconnect = psmouse_smbus_reconnect;
+	psmouse->fast_reconnect = psmouse_smbus_reconnect;
+	psmouse->disconnect = psmouse_smbus_disconnect;
+	psmouse->resync_time = 0;
+
+	psmouse_deactivate(psmouse);
+
+	mutex_lock(&psmouse_smbus_mutex);
+	list_add_tail(&smbdev->node, &psmouse_smbus_list);
+	mutex_unlock(&psmouse_smbus_mutex);
+
+	/* Bind to already existing adapters right away */
+	req.board = board;
+	req.client = &smbdev->client;
+	error = i2c_for_each_dev(&req, psmouse_smbus_create_companion);
+
+	if (smbdev->client) {
+		/* We have our companion device */
+		return 0;
+	}
+
+	if (error < 0 || !leave_breadcrumbs) {
+		mutex_lock(&psmouse_smbus_mutex);
+		list_del(&smbdev->node);
+		mutex_unlock(&psmouse_smbus_mutex);
+
+		kfree(smbdev);
+	}
+
+	return error < 0 ? error : -EAGAIN;
+}
+
+int __init psmouse_smbus_module_init(void)
+{
+	int error;
+
+	error = bus_register_notifier(&i2c_bus_type, &psmouse_smbus_notifier);
+	if (error) {
+		pr_err("failed to register i2c bus notifier: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+void __exit psmouse_smbus_module_exit(void)
+{
+	bus_unregister_notifier(&i2c_bus_type, &psmouse_smbus_notifier);
+	flush_scheduled_work();
+}
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index e853dee05e79..186c760e398e 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -209,5 +209,38 @@ static struct psmouse_attribute psmouse_attr_##_name = {			\
 		   &(psmouse)->ps2dev.serio->dev,	\
 		   psmouse_fmt(format), ##__VA_ARGS__)
 
+#ifdef CONFIG_MOUSE_PS2_SMBUS
+
+int psmouse_smbus_module_init(void);
+void psmouse_smbus_module_exit(void);
+
+struct i2c_board_info;
+
+int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
+		       bool leave_breadcrumbs);
+void psmouse_smbus_cleanup(struct psmouse *psmouse);
+
+#else /* !CONFIG_MOUSE_PS2_SMBUS */
+
+static inline int psmouse_smbus_module_init(void)
+{
+	return 0;
+}
+
+static inline void psmouse_smbus_module_exit(void)
+{
+}
+
+int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
+		       bool leave_breadcrumbs)
+{
+	return -ENOSYS;
+}
+
+void psmouse_smbus_cleanup(struct psmouse *psmouse)
+{
+}
+
+#endif /* CONFIG_MOUSE_PS2_SMBUS */
 
 #endif /* _PSMOUSE_H */
-- 
2.12.0.246.ga2ecc84866-goog

[toc] | [prev] | [next] | [standalone]

#1597983 — Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions

On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> This provides glue between PS/2 devices that enumerate the RMI4 devices
> and Elan touchpads to the RMI4 (or Elan) SMBus driver.
> 
> The SMBus devices keep their PS/2 connection alive. If the initialization
> process goes too far (psmouse_activate called), the device disconnects
> from the I2C bus and stays on the PS/2 bus, that is why we explicitly
> disable PS/2 device reporting (by calling psmouse_deactivate) before
> trying to register SMBus companion device.
> 
> The HID over I2C devices are enumerated through the ACPI DSDT, and
> their PS/2 device also exports the InterTouch bit in the extended
> capability 0x0C. However, the firmware keeps its I2C connection open
> even after going further in the PS/2 initialization. We don't need
> to take extra precautions with those device, especially because they
> block their PS/2 communication when HID over I2C is used.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>

Hi Dmitry,

There is an issue with this patch. The platform_data provided by the
caller of psmouse_smbus_init() may be dereference, leading to a dangling
pointer on the stack in rmi4.

There is no guarantees rmi-smbus will get probed directly and even if it
does, a later rmmod/modprobe might happen and won't have access to the
platform data.

See below for a patch that solves this. Feel free to squash it, change it
and remove the terrible commit message.

From a86f766de9c544a8d61da520719287c68a5f1bea Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Fri, 10 Mar 2017 18:31:01 +0100
Subject: [PATCH] Input: smbus companion - store the platform data for later
 use

The platform data should be available as long as the i2c_device exists.
In the current implementation, the platform data is allocated on the
stack, which gives a dangling pointer to the i2c_client once
psmouse_smbus_init() ends.

Duplicate the provided platform data in psmouse_smbus_init() so that
the allocation/free of pdata is handled by psmouse-smbus.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/psmouse-smbus.c | 18 ++++++++++++++++--
 drivers/input/mouse/psmouse.h       |  2 +-
 drivers/input/mouse/synaptics.c     |  5 ++---
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
index 5bda551..6f603ec 100644
--- a/drivers/input/mouse/psmouse-smbus.c
+++ b/drivers/input/mouse/psmouse-smbus.c
@@ -20,6 +20,7 @@
 struct psmouse_smbus_dev {
 	struct psmouse *psmouse;
 	struct i2c_client *client;
+	void *pdata;
 	struct list_head node;
 	unsigned short addr;
 	bool dead;
@@ -166,6 +167,7 @@ static void psmouse_smbus_disconnect(struct psmouse *psmouse)
 		psmouse_smbus_schedule_remove(smbdev->client);
 	}
 
+	kfree(smbdev->pdata);
 	kfree(smbdev);
 	psmouse->private = NULL;
 }
@@ -205,6 +207,7 @@ void psmouse_smbus_cleanup(struct psmouse *psmouse)
 	list_for_each_entry_safe(smbdev, tmp, &psmouse_smbus_list, node) {
 		if (psmouse == smbdev->psmouse) {
 			list_del(&smbdev->node);
+			kfree(smbdev->pdata);
 			kfree(smbdev);
 		}
 	}
@@ -213,16 +216,26 @@ void psmouse_smbus_cleanup(struct psmouse *psmouse)
 }
 
 int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
-		       bool leave_breadcrumbs)
+		       void *pdata, size_t pdata_size, bool leave_breadcrumbs)
 {
 	struct psmouse_smbus_companion_req req;
 	struct psmouse_smbus_dev *smbdev;
+	struct i2c_board_info _board;
 	int error;
 
 	smbdev = kzalloc(sizeof(*smbdev), GFP_KERNEL);
 	if (!smbdev)
 		return -ENOMEM;
 
+	smbdev->pdata = kmemdup(pdata, pdata_size, GFP_KERNEL);
+	if (!smbdev->pdata) {
+		kfree(smbdev);
+		return -ENOMEM;
+	}
+
+	_board = *board;
+	_board.platform_data = smbdev->pdata;
+
 	smbdev->psmouse = psmouse;
 	smbdev->addr = board->addr;
 
@@ -240,7 +253,7 @@ int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
 	mutex_unlock(&psmouse_smbus_mutex);
 
 	/* Bind to already existing adapters right away */
-	req.board = board;
+	req.board = &_board;
 	req.client = &smbdev->client;
 	error = i2c_for_each_dev(&req, psmouse_smbus_create_companion);
 
@@ -254,6 +267,7 @@ int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
 		list_del(&smbdev->node);
 		mutex_unlock(&psmouse_smbus_mutex);
 
+		kfree(smbdev->pdata);
 		kfree(smbdev);
 	}
 
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 60e5e8d..a572e66 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -219,7 +219,7 @@ void psmouse_smbus_module_exit(void);
 struct i2c_board_info;
 
 int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
-		       bool leave_breadcrumbs);
+		       void *pdata, size_t pdata_size, bool leave_breadcrumbs);
 void psmouse_smbus_cleanup(struct psmouse *psmouse);
 
 #else /* !CONFIG_MOUSE_PS2_SMBUS */
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 5807504..95cdf21 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1708,12 +1708,11 @@ static int synaptics_create_intertouch(struct psmouse *psmouse,
 	};
 	struct i2c_board_info intertouch_board = {
 		I2C_BOARD_INFO("rmi4_smbus", 0x2c),
-		.platform_data = &pdata,
 		.flags = I2C_CLIENT_HOST_NOTIFY,
 	};
 
-	return psmouse_smbus_init(psmouse, &intertouch_board,
-				  leave_breadcrumbs);
+	return psmouse_smbus_init(psmouse, &intertouch_board, &pdata,
+				  sizeof(pdata), leave_breadcrumbs);
 }
 
 /**
-- 
2.9.3

Cheers,
Benjamin

[toc] | [prev] | [next] | [standalone]

#1598000 — Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions

On Fri, Mar 10, 2017 at 06:55:09PM +0100, Benjamin Tissoires wrote:
> On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > 
> > This provides glue between PS/2 devices that enumerate the RMI4 devices
> > and Elan touchpads to the RMI4 (or Elan) SMBus driver.
> > 
> > The SMBus devices keep their PS/2 connection alive. If the initialization
> > process goes too far (psmouse_activate called), the device disconnects
> > from the I2C bus and stays on the PS/2 bus, that is why we explicitly
> > disable PS/2 device reporting (by calling psmouse_deactivate) before
> > trying to register SMBus companion device.
> > 
> > The HID over I2C devices are enumerated through the ACPI DSDT, and
> > their PS/2 device also exports the InterTouch bit in the extended
> > capability 0x0C. However, the firmware keeps its I2C connection open
> > even after going further in the PS/2 initialization. We don't need
> > to take extra precautions with those device, especially because they
> > block their PS/2 communication when HID over I2C is used.
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >
> 
> Hi Dmitry,
> 
> There is an issue with this patch. The platform_data provided by the
> caller of psmouse_smbus_init() may be dereference, leading to a dangling
> pointer on the stack in rmi4.
> 
> There is no guarantees rmi-smbus will get probed directly and even if it
> does, a later rmmod/modprobe might happen and won't have access to the
> platform data.
> 
> See below for a patch that solves this. Feel free to squash it, change it
> and remove the terrible commit message.
> 
> From a86f766de9c544a8d61da520719287c68a5f1bea Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Fri, 10 Mar 2017 18:31:01 +0100
> Subject: [PATCH] Input: smbus companion - store the platform data for later
>  use
> 
> The platform data should be available as long as the i2c_device exists.
> In the current implementation, the platform data is allocated on the
> stack, which gives a dangling pointer to the i2c_client once
> psmouse_smbus_init() ends.
> 
> Duplicate the provided platform data in psmouse_smbus_init() so that
> the allocation/free of pdata is handled by psmouse-smbus.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/mouse/psmouse-smbus.c | 18 ++++++++++++++++--
>  drivers/input/mouse/psmouse.h       |  2 +-
>  drivers/input/mouse/synaptics.c     |  5 ++---
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
> index 5bda551..6f603ec 100644
> --- a/drivers/input/mouse/psmouse-smbus.c
> +++ b/drivers/input/mouse/psmouse-smbus.c
> @@ -20,6 +20,7 @@
>  struct psmouse_smbus_dev {
>  	struct psmouse *psmouse;
>  	struct i2c_client *client;
> +	void *pdata;
>  	struct list_head node;
>  	unsigned short addr;
>  	bool dead;
> @@ -166,6 +167,7 @@ static void psmouse_smbus_disconnect(struct psmouse *psmouse)
>  		psmouse_smbus_schedule_remove(smbdev->client);
>  	}
>  
> +	kfree(smbdev->pdata);

We can 't do it here if we fired off a work to unregister i2c_client, as
it may still be referencing it. I guess we can do it in notifier code.
I'll update the patch.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1597935

Hi Dmitry,

On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> Hi,
> 
> This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> devices for better support of Synaptics RMI4 touchpads (and Elans later).

Thanks!

I have some issues/comments and am still working on those. Here are some
general comments:

> 
> The main difference is that we do not have platform device, as it only
> adds another indirection level, and have psmouse create SMBus companion

The purpose of having the platform device was to not have dependency
between psmouse and I2C. Right now I think that patch 6/8 will fail to
compile if I2C=m and PSMOUSE=y (I may be wrong).

> directly. Because serio ports complete registration asynchronously, we do
> not deadlock on psmouse_mutex when even if we have a pass-through port.
> (Frankly we need to revisit this whole serio and psmouse thing, use of
> global serio_mutex and psmouse_mutex is hurting us; they were needed when
> driver core could not recursively iterate over device and driver lists).

Agree, this is a giant PITA.

> 
> We also do not allow overriding serio driver, instead we teach psmouse
> about "special" devices and let it continue own the serio port and make
> sure nobody else touches it.
> 
> To work around issue with psmouse_reconnect() running sometimes too late,
> we add "fast reconnect" option to serio. Not too pretty, but gets the job
> done. We may need to revisit whole serio PM story later and stop "cheating"
> and pretending that device is resumed when it is not, but for that we need
> to teach PM core about devices that are OK not to wait for before resuming
> userspace. Anyway, much bigger topic for later.

I thought there was already the ability to say that a driver needs to be
run in a different thread for PM functions (IIRC i2c-hid uses
device_enable_async_suspend(&client->dev); and this "should" do the
trick).

> 
> This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> forcepad (unfortunately it seems to be using some other SMBus controller
> for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).

Well, on my T450, the SMBus connection is dead too. I can't seem to talk
to the device at all. This happens when the firmware believes it needs
to stay on PS/2 and gets completely deaf to I2C. I solved this by
calling psmouse_deactivate(), but this time, it looks like some other
function needs to be called.

I'll keep investigating and report back.

Cheers,
Benjamin

> 
> Thanks,
> Dmitry
> 
> Benjamin Tissoires (2):
>   Input: psmouse - add support for SMBus companions
>   Input: synaptics - add support for Intertouch devices
> 
> Dmitry Torokhov (6):
>   i2c: export i2c_client_type structure
>   Input: serio - add fast reconnect option
>   Input: psmouse - implement fast reconnect option
>   Input: psmouse - store pointer to current protocol
>   Input: psmouse - introduce notion of SMBus companions
>   Input: synaptics - split device info into a separate structure
> 
>  drivers/i2c/i2c-core.c              |   4 +-
>  drivers/input/mouse/Kconfig         |  16 +
>  drivers/input/mouse/Makefile        |   2 +
>  drivers/input/mouse/psmouse-base.c  | 213 ++++++---
>  drivers/input/mouse/psmouse-smbus.c | 280 ++++++++++++
>  drivers/input/mouse/psmouse.h       | 106 +++--
>  drivers/input/mouse/synaptics.c     | 832 +++++++++++++++++++++++-------------
>  drivers/input/mouse/synaptics.h     |  33 +-
>  drivers/input/serio/serio.c         |  22 +-
>  include/linux/i2c.h                 |   1 +
>  include/linux/serio.h               |   1 +
>  11 files changed, 1100 insertions(+), 410 deletions(-)
>  create mode 100644 drivers/input/mouse/psmouse-smbus.c
> 

[toc] | [prev] | [next] | [standalone]

#1597986

On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> Hi Dmitry,
> 
> On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > Hi,
> > 
> > This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> > devices for better support of Synaptics RMI4 touchpads (and Elans later).
> 
> Thanks!
> 
> I have some issues/comments and am still working on those. Here are some
> general comments:
> 
> > 
> > The main difference is that we do not have platform device, as it only
> > adds another indirection level, and have psmouse create SMBus companion
> 
> The purpose of having the platform device was to not have dependency
> between psmouse and I2C. Right now I think that patch 6/8 will fail to
> compile if I2C=m and PSMOUSE=y (I may be wrong).

This is taken care by the following guards in users if MOUSE_PS2_SMBUS:

depends on I2C=y || I2C=MOUSE_PS2

I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
adapters loaded for it to work (hopefully).

> 
> > directly. Because serio ports complete registration asynchronously, we do
> > not deadlock on psmouse_mutex when even if we have a pass-through port.
> > (Frankly we need to revisit this whole serio and psmouse thing, use of
> > global serio_mutex and psmouse_mutex is hurting us; they were needed when
> > driver core could not recursively iterate over device and driver lists).
> 
> Agree, this is a giant PITA.
> 
> > 
> > We also do not allow overriding serio driver, instead we teach psmouse
> > about "special" devices and let it continue own the serio port and make
> > sure nobody else touches it.
> > 
> > To work around issue with psmouse_reconnect() running sometimes too late,
> > we add "fast reconnect" option to serio. Not too pretty, but gets the job
> > done. We may need to revisit whole serio PM story later and stop "cheating"
> > and pretending that device is resumed when it is not, but for that we need
> > to teach PM core about devices that are OK not to wait for before resuming
> > userspace. Anyway, much bigger topic for later.
> 
> I thought there was already the ability to say that a driver needs to be
> run in a different thread for PM functions (IIRC i2c-hid uses
> device_enable_async_suspend(&client->dev); and this "should" do the
> trick).

The issue is that currently asynchronous resume still has to complete
before we start resuming userspace, as PS/2 is way too slow. So the
current solution marks device as resumed right away, and mouse may
become responsive 2 seconds later, but that is good as we do not idly
sit and wait but have userspace start turning on the screen and do other
useful stuff. Maybe user can already start typing their password into
screen locker.

We would need to give a way to drivers to indicate to PM core just how
asynchronous our resume can be.

> 
> > 
> > This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> > forcepad (unfortunately it seems to be using some other SMBus controller
> > for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
> 
> Well, on my T450, the SMBus connection is dead too. I can't seem to talk
> to the device at all. This happens when the firmware believes it needs
> to stay on PS/2 and gets completely deaf to I2C. I solved this by
> calling psmouse_deactivate(), but this time, it looks like some other
> function needs to be called.
> 
> I'll keep investigating and report back.

I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
its touchpad, it could be that 1040 is similar.

When your SMBus connection is dead do you see anything on the bus? At
that address? Or it is completely unresponsive?

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1597991

On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> > Hi Dmitry,
> > 
> > On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > > Hi,
> > > 
> > > This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> > > devices for better support of Synaptics RMI4 touchpads (and Elans later).
> > 
> > Thanks!
> > 
> > I have some issues/comments and am still working on those. Here are some
> > general comments:
> > 
> > > 
> > > The main difference is that we do not have platform device, as it only
> > > adds another indirection level, and have psmouse create SMBus companion
> > 
> > The purpose of having the platform device was to not have dependency
> > between psmouse and I2C. Right now I think that patch 6/8 will fail to
> > compile if I2C=m and PSMOUSE=y (I may be wrong).
> 
> This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
> 
> depends on I2C=y || I2C=MOUSE_PS2

I can see this guards for MOUSE_PS2_SYNAPTICS_SMBUS, but not for
MOUSE_PS2_SMBUS. So unless I am completely missing the point, if users
disable SYNAPTICS_SMBUS but keep PS2_SMBUS there might be a problem.

> 
> I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
> adapters loaded for it to work (hopefully).

K.

> 
> > 
> > > directly. Because serio ports complete registration asynchronously, we do
> > > not deadlock on psmouse_mutex when even if we have a pass-through port.
> > > (Frankly we need to revisit this whole serio and psmouse thing, use of
> > > global serio_mutex and psmouse_mutex is hurting us; they were needed when
> > > driver core could not recursively iterate over device and driver lists).
> > 
> > Agree, this is a giant PITA.
> > 
> > > 
> > > We also do not allow overriding serio driver, instead we teach psmouse
> > > about "special" devices and let it continue own the serio port and make
> > > sure nobody else touches it.
> > > 
> > > To work around issue with psmouse_reconnect() running sometimes too late,
> > > we add "fast reconnect" option to serio. Not too pretty, but gets the job
> > > done. We may need to revisit whole serio PM story later and stop "cheating"
> > > and pretending that device is resumed when it is not, but for that we need
> > > to teach PM core about devices that are OK not to wait for before resuming
> > > userspace. Anyway, much bigger topic for later.
> > 
> > I thought there was already the ability to say that a driver needs to be
> > run in a different thread for PM functions (IIRC i2c-hid uses
> > device_enable_async_suspend(&client->dev); and this "should" do the
> > trick).
> 
> The issue is that currently asynchronous resume still has to complete
> before we start resuming userspace, as PS/2 is way too slow. So the
> current solution marks device as resumed right away, and mouse may
> become responsive 2 seconds later, but that is good as we do not idly
> sit and wait but have userspace start turning on the screen and do other
> useful stuff. Maybe user can already start typing their password into
> screen locker.

Oh, I see. I haven't thought at the userspace issue.

> 
> We would need to give a way to drivers to indicate to PM core just how
> asynchronous our resume can be.
> 
> > 
> > > 
> > > This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> > > forcepad (unfortunately it seems to be using some other SMBus controller
> > > for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
> > 
> > Well, on my T450, the SMBus connection is dead too. I can't seem to talk
> > to the device at all. This happens when the firmware believes it needs
> > to stay on PS/2 and gets completely deaf to I2C. I solved this by
> > calling psmouse_deactivate(), but this time, it looks like some other
> > function needs to be called.
> > 
> > I'll keep investigating and report back.
> 
> I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
> its touchpad, it could be that 1040 is similar.

Oh, yes, I do remember this. However, I think the device was i2c_hid,
not SMBus. You can check if that's the case by looking at the DSDT, if
it has a HID over I2C touchpad, then that the same issue.

> 
> When your SMBus connection is dead do you see anything on the bus? At
> that address? Or it is completely unresponsive?

Completely unresponsive, nothing on the bus (as if there was nothing
physically wired).

I managed to discover that using psmouse as a module, not in bzImage
allows to have the bus properly set. So I guess there is some timing
issue (and that's going to be a pain to figure out).

In addition to the pdata fix I just sent in reply to 6/8, I have one
extra fix for rmi30 in case the pdata gets corrupted (or if f30 has
been deliberately disabled).

Cheers,
Benjamin

> 
> Thanks.
> 
> -- 
> Dmitry

[toc] | [prev] | [next] | [standalone]

#1597998

On Fri, Mar 10, 2017 at 07:04:22PM +0100, Benjamin Tissoires wrote:
> On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote:
> > On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> > > Hi Dmitry,
> > > 
> > > On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > > > Hi,
> > > > 
> > > > This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> > > > devices for better support of Synaptics RMI4 touchpads (and Elans later).
> > > 
> > > Thanks!
> > > 
> > > I have some issues/comments and am still working on those. Here are some
> > > general comments:
> > > 
> > > > 
> > > > The main difference is that we do not have platform device, as it only
> > > > adds another indirection level, and have psmouse create SMBus companion
> > > 
> > > The purpose of having the platform device was to not have dependency
> > > between psmouse and I2C. Right now I think that patch 6/8 will fail to
> > > compile if I2C=m and PSMOUSE=y (I may be wrong).
> > 
> > This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
> > 
> > depends on I2C=y || I2C=MOUSE_PS2
> 
> I can see this guards for MOUSE_PS2_SYNAPTICS_SMBUS, but not for
> MOUSE_PS2_SMBUS. So unless I am completely missing the point, if users
> disable SYNAPTICS_SMBUS but keep PS2_SMBUS there might be a problem.

Hmm, I'll need to play with this. MOUSE_PS2_SMBUS is not user visible.
Might end up adding condition there as well, or doing somethign more
elaborate, like Arnd did for RMI_F03_SERIO.

> 
> > 
> > I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
> > adapters loaded for it to work (hopefully).
> 
> K.
> 
> > 
> > > 
> > > > directly. Because serio ports complete registration asynchronously, we do
> > > > not deadlock on psmouse_mutex when even if we have a pass-through port.
> > > > (Frankly we need to revisit this whole serio and psmouse thing, use of
> > > > global serio_mutex and psmouse_mutex is hurting us; they were needed when
> > > > driver core could not recursively iterate over device and driver lists).
> > > 
> > > Agree, this is a giant PITA.
> > > 
> > > > 
> > > > We also do not allow overriding serio driver, instead we teach psmouse
> > > > about "special" devices and let it continue own the serio port and make
> > > > sure nobody else touches it.
> > > > 
> > > > To work around issue with psmouse_reconnect() running sometimes too late,
> > > > we add "fast reconnect" option to serio. Not too pretty, but gets the job
> > > > done. We may need to revisit whole serio PM story later and stop "cheating"
> > > > and pretending that device is resumed when it is not, but for that we need
> > > > to teach PM core about devices that are OK not to wait for before resuming
> > > > userspace. Anyway, much bigger topic for later.
> > > 
> > > I thought there was already the ability to say that a driver needs to be
> > > run in a different thread for PM functions (IIRC i2c-hid uses
> > > device_enable_async_suspend(&client->dev); and this "should" do the
> > > trick).
> > 
> > The issue is that currently asynchronous resume still has to complete
> > before we start resuming userspace, as PS/2 is way too slow. So the
> > current solution marks device as resumed right away, and mouse may
> > become responsive 2 seconds later, but that is good as we do not idly
> > sit and wait but have userspace start turning on the screen and do other
> > useful stuff. Maybe user can already start typing their password into
> > screen locker.
> 
> Oh, I see. I haven't thought at the userspace issue.
> 
> > 
> > We would need to give a way to drivers to indicate to PM core just how
> > asynchronous our resume can be.
> > 
> > > 
> > > > 
> > > > This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> > > > forcepad (unfortunately it seems to be using some other SMBus controller
> > > > for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
> > > 
> > > Well, on my T450, the SMBus connection is dead too. I can't seem to talk
> > > to the device at all. This happens when the firmware believes it needs
> > > to stay on PS/2 and gets completely deaf to I2C. I solved this by
> > > calling psmouse_deactivate(), but this time, it looks like some other
> > > function needs to be called.
> > > 
> > > I'll keep investigating and report back.
> > 
> > I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
> > its touchpad, it could be that 1040 is similar.
> 
> Oh, yes, I do remember this. However, I think the device was i2c_hid,
> not SMBus. You can check if that's the case by looking at the DSDT, if
> it has a HID over I2C touchpad, then that the same issue.
> 
> > 
> > When your SMBus connection is dead do you see anything on the bus? At
> > that address? Or it is completely unresponsive?
> 
> Completely unresponsive, nothing on the bus (as if there was nothing
> physically wired).
> 
> I managed to discover that using psmouse as a module, not in bzImage
> allows to have the bus properly set. So I guess there is some timing
> issue (and that's going to be a pain to figure out).
> 
> In addition to the pdata fix I just sent in reply to 6/8, I have one
> extra fix for rmi30 in case the pdata gets corrupted (or if f30 has
> been deliberately disabled).

OK, cool.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1598073

On Fri, Mar 10, 2017 at 10:10:30AM -0800, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 07:04:22PM +0100, Benjamin Tissoires wrote:
> > On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote:
> > > On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> > > > Hi Dmitry,
> > > > 
> > > > On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > > > > Hi,
> > > > > 
> > > > > This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> > > > > devices for better support of Synaptics RMI4 touchpads (and Elans later).
> > > > 
> > > > Thanks!
> > > > 
> > > > I have some issues/comments and am still working on those. Here are some
> > > > general comments:
> > > > 
> > > > > 
> > > > > The main difference is that we do not have platform device, as it only
> > > > > adds another indirection level, and have psmouse create SMBus companion
> > > > 
> > > > The purpose of having the platform device was to not have dependency
> > > > between psmouse and I2C. Right now I think that patch 6/8 will fail to
> > > > compile if I2C=m and PSMOUSE=y (I may be wrong).
> > > 
> > > This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
> > > 
> > > depends on I2C=y || I2C=MOUSE_PS2
> > 
> > I can see this guards for MOUSE_PS2_SYNAPTICS_SMBUS, but not for
> > MOUSE_PS2_SMBUS. So unless I am completely missing the point, if users
> > disable SYNAPTICS_SMBUS but keep PS2_SMBUS there might be a problem.
> 
> Hmm, I'll need to play with this. MOUSE_PS2_SMBUS is not user visible.
> Might end up adding condition there as well, or doing somethign more
> elaborate, like Arnd did for RMI_F03_SERIO.

So if I disable MOUSE_PS2_SYNAPTICS_SMBUS then MOUSE_PS2_SMBUS
disappears from .config, so I think we are good.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1598029

On 03/10/2017 09:52 AM, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
>> Hi Dmitry,
>>
>> On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
>>> Hi,
>>>
>>> This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
>>> devices for better support of Synaptics RMI4 touchpads (and Elans later).
>> Thanks!
>>
>> I have some issues/comments and am still working on those. Here are some
>> general comments:
>>
>>> The main difference is that we do not have platform device, as it only
>>> adds another indirection level, and have psmouse create SMBus companion
>> The purpose of having the platform device was to not have dependency
>> between psmouse and I2C. Right now I think that patch 6/8 will fail to
>> compile if I2C=m and PSMOUSE=y (I may be wrong).
> This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
>
> depends on I2C=y || I2C=MOUSE_PS2
>
> I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
> adapters loaded for it to work (hopefully).
>
>>> directly. Because serio ports complete registration asynchronously, we do
>>> not deadlock on psmouse_mutex when even if we have a pass-through port.
>>> (Frankly we need to revisit this whole serio and psmouse thing, use of
>>> global serio_mutex and psmouse_mutex is hurting us; they were needed when
>>> driver core could not recursively iterate over device and driver lists).
>> Agree, this is a giant PITA.
>>
>>> We also do not allow overriding serio driver, instead we teach psmouse
>>> about "special" devices and let it continue own the serio port and make
>>> sure nobody else touches it.
>>>
>>> To work around issue with psmouse_reconnect() running sometimes too late,
>>> we add "fast reconnect" option to serio. Not too pretty, but gets the job
>>> done. We may need to revisit whole serio PM story later and stop "cheating"
>>> and pretending that device is resumed when it is not, but for that we need
>>> to teach PM core about devices that are OK not to wait for before resuming
>>> userspace. Anyway, much bigger topic for later.
>> I thought there was already the ability to say that a driver needs to be
>> run in a different thread for PM functions (IIRC i2c-hid uses
>> device_enable_async_suspend(&client->dev); and this "should" do the
>> trick).
> The issue is that currently asynchronous resume still has to complete
> before we start resuming userspace, as PS/2 is way too slow. So the
> current solution marks device as resumed right away, and mouse may
> become responsive 2 seconds later, but that is good as we do not idly
> sit and wait but have userspace start turning on the screen and do other
> useful stuff. Maybe user can already start typing their password into
> screen locker.
>
> We would need to give a way to drivers to indicate to PM core just how
> asynchronous our resume can be.
>
>>> This seems to be working on X1 Carbon and also not breaking my HP 1040 with
>>> forcepad (unfortunately it seems to be using some other SMBus controller
>>> for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
>> Well, on my T450, the SMBus connection is dead too. I can't seem to talk
>> to the device at all. This happens when the firmware believes it needs
>> to stay on PS/2 and gets completely deaf to I2C. I solved this by
>> calling psmouse_deactivate(), but this time, it looks like some other
>> function needs to be called.
>>
>> I'll keep investigating and report back.
> I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
> its touchpad, it could be that 1040 is similar.
>
> When your SMBus connection is dead do you see anything on the bus? At
> that address? Or it is completely unresponsive?

Try the I2C address 0x20 for the HP forcepad. I have gotten previous 
versions of Benjamin's SMBus patches working on a similar system. It is 
a HP Elitebook Folio 940 and the forcepad was at address 0x20 and it was 
on the i801 bus. The HP 1020 does have a microchip I2C controller, but 
thats connected to a HID / I2C touchpad. The 1020 was a one off so the 
1040 should be the more common SMBus implementation.

I also have not been able to get this patch series to successfully 
switch over to SMBus mode. But, I have not had a chance to do anything 
besides apply the patches and build. This is the output from a Lenovo W541:

[    9.674826] psmouse serio1: synaptics: queried max coordinates: x 
[..5676], y [..4758]
[    9.705273] psmouse serio1: synaptics: queried min coordinates: x 
[1266..], y [1096..]
[    9.705276] psmouse serio1: synaptics: Trying to set up SMBus access
[    9.707946] psmouse serio1: synaptics: SMbus companion is not ready yet
[    9.764848] psmouse serio1: synaptics: Touchpad model: 1, fw: 8.1, 
id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board id: 3053, 
fw id: 2560
[    9.764853] psmouse serio1: synaptics: serio: Synaptics pass-through 
port at isa0060/serio1/input0
[   10.418268] psmouse serio2: trackpoint: IBM TrackPoint firmware: 
0x0e, buttons: 3/3
...
[   27.112954] psmouse serio1: synaptics: queried max coordinates: x 
[..5676], y [..4758]
[   27.142555] psmouse serio1: synaptics: queried min coordinates: x 
[1266..], y [1096..]
[   27.142559] psmouse serio1: synaptics: Trying to set up SMBus access
[   27.169776] psmouse serio1: synaptics: SMbus companion is not ready yet
[   27.226071] psmouse serio1: synaptics: Touchpad model: 1, fw: 8.1, 
id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board id: 3053, 
fw id: 2560
[   27.226087] psmouse serio1: synaptics: serio: Synaptics pass-through 
port at isa0060/serio1/input0
[   27.880282] psmouse serio3: trackpoint: IBM TrackPoint firmware: 
0x0e, buttons: 3/3

Andrew

> Thanks.
>

[toc] | [prev] | [next] | [standalone]

#1598068

On Fri, Mar 10, 2017 at 10:56:33AM -0800, Andrew Duggan wrote:
> On 03/10/2017 09:52 AM, Dmitry Torokhov wrote:
> >On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> >>Hi Dmitry,
> >>
> >>On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> >>>Hi,
> >>>
> >>>This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> >>>devices for better support of Synaptics RMI4 touchpads (and Elans later).
> >>Thanks!
> >>
> >>I have some issues/comments and am still working on those. Here are some
> >>general comments:
> >>
> >>>The main difference is that we do not have platform device, as it only
> >>>adds another indirection level, and have psmouse create SMBus companion
> >>The purpose of having the platform device was to not have dependency
> >>between psmouse and I2C. Right now I think that patch 6/8 will fail to
> >>compile if I2C=m and PSMOUSE=y (I may be wrong).
> >This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
> >
> >depends on I2C=y || I2C=MOUSE_PS2
> >
> >I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
> >adapters loaded for it to work (hopefully).
> >
> >>>directly. Because serio ports complete registration asynchronously, we do
> >>>not deadlock on psmouse_mutex when even if we have a pass-through port.
> >>>(Frankly we need to revisit this whole serio and psmouse thing, use of
> >>>global serio_mutex and psmouse_mutex is hurting us; they were needed when
> >>>driver core could not recursively iterate over device and driver lists).
> >>Agree, this is a giant PITA.
> >>
> >>>We also do not allow overriding serio driver, instead we teach psmouse
> >>>about "special" devices and let it continue own the serio port and make
> >>>sure nobody else touches it.
> >>>
> >>>To work around issue with psmouse_reconnect() running sometimes too late,
> >>>we add "fast reconnect" option to serio. Not too pretty, but gets the job
> >>>done. We may need to revisit whole serio PM story later and stop "cheating"
> >>>and pretending that device is resumed when it is not, but for that we need
> >>>to teach PM core about devices that are OK not to wait for before resuming
> >>>userspace. Anyway, much bigger topic for later.
> >>I thought there was already the ability to say that a driver needs to be
> >>run in a different thread for PM functions (IIRC i2c-hid uses
> >>device_enable_async_suspend(&client->dev); and this "should" do the
> >>trick).
> >The issue is that currently asynchronous resume still has to complete
> >before we start resuming userspace, as PS/2 is way too slow. So the
> >current solution marks device as resumed right away, and mouse may
> >become responsive 2 seconds later, but that is good as we do not idly
> >sit and wait but have userspace start turning on the screen and do other
> >useful stuff. Maybe user can already start typing their password into
> >screen locker.
> >
> >We would need to give a way to drivers to indicate to PM core just how
> >asynchronous our resume can be.
> >
> >>>This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> >>>forcepad (unfortunately it seems to be using some other SMBus controller
> >>>for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
> >>Well, on my T450, the SMBus connection is dead too. I can't seem to talk
> >>to the device at all. This happens when the firmware believes it needs
> >>to stay on PS/2 and gets completely deaf to I2C. I solved this by
> >>calling psmouse_deactivate(), but this time, it looks like some other
> >>function needs to be called.
> >>
> >>I'll keep investigating and report back.
> >I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
> >its touchpad, it could be that 1040 is similar.
> >
> >When your SMBus connection is dead do you see anything on the bus? At
> >that address? Or it is completely unresponsive?
> 
> Try the I2C address 0x20 for the HP forcepad. I have gotten previous
> versions of Benjamin's SMBus patches working on a similar system. It
> is a HP Elitebook Folio 940 and the forcepad was at address 0x20 and
> it was on the i801 bus. The HP 1020 does have a microchip I2C
> controller, but thats connected to a HID / I2C touchpad. The 1020
> was a one off so the 1040 should be the more common SMBus
> implementation.
> 
> I also have not been able to get this patch series to successfully
> switch over to SMBus mode. But, I have not had a chance to do
> anything besides apply the patches and build. This is the output
> from a Lenovo W541:
> 
> [    9.674826] psmouse serio1: synaptics: queried max coordinates: x
> [..5676], y [..4758]
> [    9.705273] psmouse serio1: synaptics: queried min coordinates: x
> [1266..], y [1096..]
> [    9.705276] psmouse serio1: synaptics: Trying to set up SMBus access
> [    9.707946] psmouse serio1: synaptics: SMbus companion is not ready yet
> [    9.764848] psmouse serio1: synaptics: Touchpad model: 1, fw:
> 8.1, id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board
> id: 3053, fw id: 2560
> [    9.764853] psmouse serio1: synaptics: serio: Synaptics
> pass-through port at isa0060/serio1/input0
> [   10.418268] psmouse serio2: trackpoint: IBM TrackPoint firmware:
> 0x0e, buttons: 3/3
> ...
> [   27.112954] psmouse serio1: synaptics: queried max coordinates: x
> [..5676], y [..4758]
> [   27.142555] psmouse serio1: synaptics: queried min coordinates: x
> [1266..], y [1096..]
> [   27.142559] psmouse serio1: synaptics: Trying to set up SMBus access
> [   27.169776] psmouse serio1: synaptics: SMbus companion is not ready yet
> [   27.226071] psmouse serio1: synaptics: Touchpad model: 1, fw:
> 8.1, id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board
> id: 3053, fw id: 2560
> [   27.226087] psmouse serio1: synaptics: serio: Synaptics
> pass-through port at isa0060/serio1/input0
> [   27.880282] psmouse serio3: trackpoint: IBM TrackPoint firmware:
> 0x0e, buttons: 3/3

Yay, that works:

[  980.124573] psmouse serio3: synaptics: queried max coordinates: x [..5676], y [..4690]
[  980.155902] psmouse serio3: synaptics: queried min coordinates: x [1266..], y [1162..]
[  980.155915] psmouse serio3: synaptics: Trying to set up SMBus access
[  980.229405] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2685-009, fw id: 1608298
[  980.293225] input: Synaptics TM2685-009 as /devices/rmi4-00/input/input42
[  980.305824] rmi4_smbus 9-0020: registered rmi smb driver at 0x20.

Thanks Andrew!

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1598076

On 03/10/2017 12:12 PM, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 10:56:33AM -0800, Andrew Duggan wrote:
>> On 03/10/2017 09:52 AM, Dmitry Torokhov wrote:
>>> On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
>>>> Hi Dmitry,
>>>>
>>>> On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
>>>>> Hi,
>>>>>
>>>>> This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
>>>>> devices for better support of Synaptics RMI4 touchpads (and Elans later).
>>>> Thanks!
>>>>
>>>> I have some issues/comments and am still working on those. Here are some
>>>> general comments:
>>>>
>>>>> The main difference is that we do not have platform device, as it only
>>>>> adds another indirection level, and have psmouse create SMBus companion
>>>> The purpose of having the platform device was to not have dependency
>>>> between psmouse and I2C. Right now I think that patch 6/8 will fail to
>>>> compile if I2C=m and PSMOUSE=y (I may be wrong).
>>> This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
>>>
>>> depends on I2C=y || I2C=MOUSE_PS2
>>>
>>> I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
>>> adapters loaded for it to work (hopefully).
>>>
>>>>> directly. Because serio ports complete registration asynchronously, we do
>>>>> not deadlock on psmouse_mutex when even if we have a pass-through port.
>>>>> (Frankly we need to revisit this whole serio and psmouse thing, use of
>>>>> global serio_mutex and psmouse_mutex is hurting us; they were needed when
>>>>> driver core could not recursively iterate over device and driver lists).
>>>> Agree, this is a giant PITA.
>>>>
>>>>> We also do not allow overriding serio driver, instead we teach psmouse
>>>>> about "special" devices and let it continue own the serio port and make
>>>>> sure nobody else touches it.
>>>>>
>>>>> To work around issue with psmouse_reconnect() running sometimes too late,
>>>>> we add "fast reconnect" option to serio. Not too pretty, but gets the job
>>>>> done. We may need to revisit whole serio PM story later and stop "cheating"
>>>>> and pretending that device is resumed when it is not, but for that we need
>>>>> to teach PM core about devices that are OK not to wait for before resuming
>>>>> userspace. Anyway, much bigger topic for later.
>>>> I thought there was already the ability to say that a driver needs to be
>>>> run in a different thread for PM functions (IIRC i2c-hid uses
>>>> device_enable_async_suspend(&client->dev); and this "should" do the
>>>> trick).
>>> The issue is that currently asynchronous resume still has to complete
>>> before we start resuming userspace, as PS/2 is way too slow. So the
>>> current solution marks device as resumed right away, and mouse may
>>> become responsive 2 seconds later, but that is good as we do not idly
>>> sit and wait but have userspace start turning on the screen and do other
>>> useful stuff. Maybe user can already start typing their password into
>>> screen locker.
>>>
>>> We would need to give a way to drivers to indicate to PM core just how
>>> asynchronous our resume can be.
>>>
>>>>> This seems to be working on X1 Carbon and also not breaking my HP 1040 with
>>>>> forcepad (unfortunately it seems to be using some other SMBus controller
>>>>> for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
>>>> Well, on my T450, the SMBus connection is dead too. I can't seem to talk
>>>> to the device at all. This happens when the firmware believes it needs
>>>> to stay on PS/2 and gets completely deaf to I2C. I solved this by
>>>> calling psmouse_deactivate(), but this time, it looks like some other
>>>> function needs to be called.
>>>>
>>>> I'll keep investigating and report back.
>>> I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
>>> its touchpad, it could be that 1040 is similar.
>>>
>>> When your SMBus connection is dead do you see anything on the bus? At
>>> that address? Or it is completely unresponsive?
>> Try the I2C address 0x20 for the HP forcepad. I have gotten previous
>> versions of Benjamin's SMBus patches working on a similar system. It
>> is a HP Elitebook Folio 940 and the forcepad was at address 0x20 and
>> it was on the i801 bus. The HP 1020 does have a microchip I2C
>> controller, but thats connected to a HID / I2C touchpad. The 1020
>> was a one off so the 1040 should be the more common SMBus
>> implementation.
>>
>> I also have not been able to get this patch series to successfully
>> switch over to SMBus mode. But, I have not had a chance to do
>> anything besides apply the patches and build. This is the output
>> from a Lenovo W541:
>>
>> [    9.674826] psmouse serio1: synaptics: queried max coordinates: x
>> [..5676], y [..4758]
>> [    9.705273] psmouse serio1: synaptics: queried min coordinates: x
>> [1266..], y [1096..]
>> [    9.705276] psmouse serio1: synaptics: Trying to set up SMBus access
>> [    9.707946] psmouse serio1: synaptics: SMbus companion is not ready yet
>> [    9.764848] psmouse serio1: synaptics: Touchpad model: 1, fw:
>> 8.1, id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board
>> id: 3053, fw id: 2560
>> [    9.764853] psmouse serio1: synaptics: serio: Synaptics
>> pass-through port at isa0060/serio1/input0
>> [   10.418268] psmouse serio2: trackpoint: IBM TrackPoint firmware:
>> 0x0e, buttons: 3/3
>> ...
>> [   27.112954] psmouse serio1: synaptics: queried max coordinates: x
>> [..5676], y [..4758]
>> [   27.142555] psmouse serio1: synaptics: queried min coordinates: x
>> [1266..], y [1096..]
>> [   27.142559] psmouse serio1: synaptics: Trying to set up SMBus access
>> [   27.169776] psmouse serio1: synaptics: SMbus companion is not ready yet
>> [   27.226071] psmouse serio1: synaptics: Touchpad model: 1, fw:
>> 8.1, id: 0x1e2b1, caps: 0xf003a3/0x943300/0x12e800/0x10000, board
>> id: 3053, fw id: 2560
>> [   27.226087] psmouse serio1: synaptics: serio: Synaptics
>> pass-through port at isa0060/serio1/input0
>> [   27.880282] psmouse serio3: trackpoint: IBM TrackPoint firmware:
>> 0x0e, buttons: 3/3
> Yay, that works:
>
> [  980.124573] psmouse serio3: synaptics: queried max coordinates: x [..5676], y [..4690]
> [  980.155902] psmouse serio3: synaptics: queried min coordinates: x [1266..], y [1162..]
> [  980.155915] psmouse serio3: synaptics: Trying to set up SMBus access
> [  980.229405] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2685-009, fw id: 1608298
> [  980.293225] input: Synaptics TM2685-009 as /devices/rmi4-00/input/input42
> [  980.305824] rmi4_smbus 9-0020: registered rmi smb driver at 0x20.
>
> Thanks Andrew!
>
Cool! FYI, forcepads don't have a F30 so currently there is nothing to 
report BTN_LEFT or BUTTONPAD which will probably confuse userspace about 
what type of device it is. I have a patch which I worked on a year ago 
for F21 which is the force function for these devices. It will report a 
click when the firmware determines that a certain amount of force has 
been applied. I can take a look at that patch again and submit it when I 
get a chance now that we are closer to getting these device working in 
RMI mode.

Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web