linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] PS
@ 2017-03-09 22:16 Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
                   ` (9 more replies)
  0 siblings, 10 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 23:12   ` Wolfram Sang
  2017-04-01 16:06   ` Wolfram Sang
  2017-03-09 22:16 ` [PATCH 2/8] Input: serio - add fast reconnect option Dmitry Torokhov
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires, Wolfram Sang; +Cc: Andrew Duggan, linux-kernel, linux-input

i2c bus has 2 different types of device belonging to the same bus and
bus notifiers use device type to distinguish between adapters and clients.
Previously we only had i2c_adapter_type exported, which made code wanting
to work with i2c_client devices test for type not equal to adapter type.
This unfortunately is not safe if we ever add another type to the bus,
so let's export i2c_client_type as well.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Wolfram, this is the patch I was talking about in the other mail.

 drivers/i2c/i2c-core.c | 4 ++--
 include/linux/i2c.h    | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 34a5115484dd..446e341e9508 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -74,7 +74,6 @@
 static DEFINE_MUTEX(core_lock);
 static DEFINE_IDR(i2c_adapter_idr);
 
-static struct device_type i2c_client_type;
 static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
 
 static struct static_key i2c_trace_msg = STATIC_KEY_INIT_FALSE;
@@ -1096,11 +1095,12 @@ struct bus_type i2c_bus_type = {
 };
 EXPORT_SYMBOL_GPL(i2c_bus_type);
 
-static struct device_type i2c_client_type = {
+struct device_type i2c_client_type = {
 	.groups		= i2c_dev_groups,
 	.uevent		= i2c_device_uevent,
 	.release	= i2c_client_dev_release,
 };
+EXPORT_SYMBOL_GPL(i2c_client_type);
 
 
 /**
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 2cc3988d127b..89ca5e56b433 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -37,6 +37,7 @@
 
 extern struct bus_type i2c_bus_type;
 extern struct device_type i2c_adapter_type;
+extern struct device_type i2c_client_type;
 
 /* --- General options ------------------------------------------------	*/
 
-- 
2.12.0.246.ga2ecc84866-goog

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

* [PATCH 2/8] Input: serio - add fast reconnect option
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 3/8] Input: psmouse - implement " Dmitry Torokhov
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* [PATCH 3/8] Input: psmouse - implement fast reconnect option
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 2/8] Input: serio - add fast reconnect option Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 4/8] Input: psmouse - store pointer to current protocol Dmitry Torokhov
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

Make use of serio's fast reconnect option and allow psmouse protocol
handler's to implement fast reconnect handlers that will be called during
system resume.

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

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index a598b7223cef..47fd2976da7f 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -966,6 +966,7 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
 	psmouse->protocol_handler = psmouse_process_byte;
 	psmouse->pktsize = 3;
 	psmouse->reconnect = NULL;
+	psmouse->fast_reconnect = NULL;
 	psmouse->disconnect = NULL;
 	psmouse->cleanup = NULL;
 	psmouse->pt_activate = NULL;
@@ -1628,15 +1629,26 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
 	goto out;
 }
 
-static int psmouse_reconnect(struct serio *serio)
+static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
 {
 	struct psmouse *psmouse = serio_get_drvdata(serio);
 	struct psmouse *parent = NULL;
+	int (*reconnect_handler)(struct psmouse *);
 	unsigned char type;
 	int rc = -1;
 
 	mutex_lock(&psmouse_mutex);
 
+	if (fast_reconnect) {
+		reconnect_handler = psmouse->fast_reconnect;
+		if (!reconnect_handler) {
+			rc = -ENOENT;
+			goto out_unlock;
+		}
+	} else {
+		reconnect_handler = psmouse->reconnect;
+	}
+
 	if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
 		parent = serio_get_drvdata(serio->parent);
 		psmouse_deactivate(parent);
@@ -1644,8 +1656,8 @@ static int psmouse_reconnect(struct serio *serio)
 
 	psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
 
-	if (psmouse->reconnect) {
-		if (psmouse->reconnect(psmouse))
+	if (reconnect_handler) {
+		if (reconnect_handler(psmouse))
 			goto out;
 	} else {
 		psmouse_reset(psmouse);
@@ -1677,10 +1689,21 @@ static int psmouse_reconnect(struct serio *serio)
 	if (parent)
 		psmouse_activate(parent);
 
+out_unlock:
 	mutex_unlock(&psmouse_mutex);
 	return rc;
 }
 
+static int psmouse_reconnect(struct serio *serio)
+{
+	return __psmouse_reconnect(serio, false);
+}
+
+static int psmouse_fast_reconnect(struct serio *serio)
+{
+	return __psmouse_reconnect(serio, true);
+}
+
 static struct serio_device_id psmouse_serio_ids[] = {
 	{
 		.type	= SERIO_8042,
@@ -1708,6 +1731,7 @@ static struct serio_driver psmouse_drv = {
 	.interrupt	= psmouse_interrupt,
 	.connect	= psmouse_connect,
 	.reconnect	= psmouse_reconnect,
+	.fast_reconnect	= psmouse_fast_reconnect,
 	.disconnect	= psmouse_disconnect,
 	.cleanup	= psmouse_cleanup,
 };
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 8c83b8e2505c..bc76e771812b 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -80,6 +80,7 @@ struct psmouse {
 	void (*set_scale)(struct psmouse *psmouse, enum psmouse_scale scale);
 
 	int (*reconnect)(struct psmouse *psmouse);
+	int (*fast_reconnect)(struct psmouse *psmouse);
 	void (*disconnect)(struct psmouse *psmouse);
 	void (*cleanup)(struct psmouse *psmouse);
 	int (*poll)(struct psmouse *psmouse);
-- 
2.12.0.246.ga2ecc84866-goog

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

* [PATCH 4/8] Input: psmouse - store pointer to current protocol
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 3/8] Input: psmouse - implement " Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 5/8] Input: psmouse - introduce notion of SMBus companions Dmitry Torokhov
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

Instead of storing only protocol "type" in pmsouse structure, store pointer
to the protocol structure, so that we have access to more data without
having to copy it over to psmouse structure.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/psmouse-base.c | 43 +++++++++--------------
 drivers/input/mouse/psmouse.h      | 70 ++++++++++++++++++++++----------------
 2 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 47fd2976da7f..bb5d164849ea 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -116,17 +116,6 @@ static DEFINE_MUTEX(psmouse_mutex);
 
 static struct workqueue_struct *kpsmoused_wq;
 
-struct psmouse_protocol {
-	enum psmouse_type type;
-	bool maxproto;
-	bool ignore_parity; /* Protocol should ignore parity errors from KBC */
-	bool try_passthru; /* Try protocol also on passthrough ports */
-	const char *name;
-	const char *alias;
-	int (*detect)(struct psmouse *, bool);
-	int (*init)(struct psmouse *);
-};
-
 static void psmouse_report_standard_buttons(struct input_dev *dev, u8 buttons)
 {
 	input_report_key(dev, BTN_LEFT,   buttons & BIT(0));
@@ -148,7 +137,7 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
 
 	/* Full packet accumulated, process it */
 
-	switch (psmouse->type) {
+	switch (psmouse->protocol->type) {
 	case PSMOUSE_IMPS:
 		/* IntelliMouse has scroll wheel */
 		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
@@ -325,7 +314,8 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
 		goto out;
 
 	if (unlikely((flags & SERIO_TIMEOUT) ||
-		     ((flags & SERIO_PARITY) && !psmouse->ignore_parity))) {
+		     ((flags & SERIO_PARITY) &&
+		      !psmouse->protocol->ignore_parity))) {
 
 		if (psmouse->state == PSMOUSE_ACTIVATED)
 			psmouse_warn(psmouse,
@@ -372,7 +362,7 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
 		}
 
 		if (psmouse->packet[1] == PSMOUSE_RET_ID ||
-		    (psmouse->type == PSMOUSE_HGPK &&
+		    (psmouse->protocol->type == PSMOUSE_HGPK &&
 		     psmouse->packet[1] == PSMOUSE_RET_BAT)) {
 			__psmouse_set_state(psmouse, PSMOUSE_IGNORE);
 			serio_reconnect(serio);
@@ -959,6 +949,8 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
 
 	__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
 
+	psmouse->protocol = &psmouse_protocols[0];
+
 	psmouse->set_rate = psmouse_set_rate;
 	psmouse->set_resolution = psmouse_set_resolution;
 	psmouse->set_scale = psmouse_set_scale;
@@ -1476,6 +1468,7 @@ static int psmouse_switch_protocol(struct psmouse *psmouse,
 {
 	const struct psmouse_protocol *selected_proto;
 	struct input_dev *input_dev = psmouse->dev;
+	enum psmouse_type type;
 
 	input_dev->dev.parent = &psmouse->ps2dev.serio->dev;
 
@@ -1488,15 +1481,13 @@ static int psmouse_switch_protocol(struct psmouse *psmouse,
 		if (proto->init && proto->init(psmouse) < 0)
 			return -1;
 
-		psmouse->type = proto->type;
 		selected_proto = proto;
 	} else {
-		psmouse->type = psmouse_extensions(psmouse,
-						   psmouse_max_proto, true);
-		selected_proto = psmouse_protocol_by_type(psmouse->type);
+		type = psmouse_extensions(psmouse, psmouse_max_proto, true);
+		selected_proto = psmouse_protocol_by_type(type);
 	}
 
-	psmouse->ignore_parity = selected_proto->ignore_parity;
+	psmouse->protocol = selected_proto;
 
 	/*
 	 * If mouse's packet size is 3 there is no point in polling the
@@ -1522,7 +1513,7 @@ static int psmouse_switch_protocol(struct psmouse *psmouse,
 	input_dev->phys = psmouse->phys;
 	input_dev->id.bustype = BUS_I8042;
 	input_dev->id.vendor = 0x0002;
-	input_dev->id.product = psmouse->type;
+	input_dev->id.product = psmouse->protocol->type;
 	input_dev->id.version = psmouse->model;
 
 	return 0;
@@ -1634,7 +1625,7 @@ static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
 	struct psmouse *psmouse = serio_get_drvdata(serio);
 	struct psmouse *parent = NULL;
 	int (*reconnect_handler)(struct psmouse *);
-	unsigned char type;
+	enum psmouse_type type;
 	int rc = -1;
 
 	mutex_lock(&psmouse_mutex);
@@ -1666,7 +1657,7 @@ static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
 			goto out;
 
 		type = psmouse_extensions(psmouse, psmouse_max_proto, false);
-		if (psmouse->type != type)
+		if (psmouse->protocol->type != type)
 			goto out;
 	}
 
@@ -1816,7 +1807,7 @@ static ssize_t psmouse_set_int_attr(struct psmouse *psmouse, void *offset, const
 
 static ssize_t psmouse_attr_show_protocol(struct psmouse *psmouse, void *data, char *buf)
 {
-	return sprintf(buf, "%s\n", psmouse_protocol_by_type(psmouse->type)->name);
+	return sprintf(buf, "%s\n", psmouse->protocol->name);
 }
 
 static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, const char *buf, size_t count)
@@ -1832,7 +1823,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co
 	if (!proto)
 		return -EINVAL;
 
-	if (psmouse->type == proto->type)
+	if (psmouse->protocol == proto)
 		return count;
 
 	new_dev = input_allocate_device();
@@ -1856,7 +1847,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co
 			return -ENODEV;
 		}
 
-		if (psmouse->type == proto->type) {
+		if (psmouse->protocol == proto) {
 			input_free_device(new_dev);
 			return count; /* switched by other thread */
 		}
@@ -1869,7 +1860,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co
 	}
 
 	old_dev = psmouse->dev;
-	old_proto = psmouse_protocol_by_type(psmouse->type);
+	old_proto = psmouse->protocol;
 
 	if (psmouse->disconnect)
 		psmouse->disconnect(psmouse);
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index bc76e771812b..36bd42179456 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -44,21 +44,58 @@ enum psmouse_scale {
 	PSMOUSE_SCALE21
 };
 
+enum psmouse_type {
+	PSMOUSE_NONE,
+	PSMOUSE_PS2,
+	PSMOUSE_PS2PP,
+	PSMOUSE_THINKPS,
+	PSMOUSE_GENPS,
+	PSMOUSE_IMPS,
+	PSMOUSE_IMEX,
+	PSMOUSE_SYNAPTICS,
+	PSMOUSE_ALPS,
+	PSMOUSE_LIFEBOOK,
+	PSMOUSE_TRACKPOINT,
+	PSMOUSE_TOUCHKIT_PS2,
+	PSMOUSE_CORTRON,
+	PSMOUSE_HGPK,
+	PSMOUSE_ELANTECH,
+	PSMOUSE_FSP,
+	PSMOUSE_SYNAPTICS_RELATIVE,
+	PSMOUSE_CYPRESS,
+	PSMOUSE_FOCALTECH,
+	PSMOUSE_VMMOUSE,
+	PSMOUSE_BYD,
+	PSMOUSE_AUTO		/* This one should always be last */
+};
+
+struct psmouse;
+
+struct psmouse_protocol {
+	enum psmouse_type type;
+	bool maxproto;
+	bool ignore_parity; /* Protocol should ignore parity errors from KBC */
+	bool try_passthru; /* Try protocol also on passthrough ports */
+	const char *name;
+	const char *alias;
+	int (*detect)(struct psmouse *, bool);
+	int (*init)(struct psmouse *);
+};
+
 struct psmouse {
 	void *private;
 	struct input_dev *dev;
 	struct ps2dev ps2dev;
 	struct delayed_work resync_work;
-	char *vendor;
-	char *name;
+	const char *vendor;
+	const char *name;
+	const struct psmouse_protocol *protocol;
 	unsigned char packet[8];
 	unsigned char badbyte;
 	unsigned char pktcnt;
 	unsigned char pktsize;
-	unsigned char type;
 	unsigned char oob_data_type;
 	unsigned char extra_buttons;
-	bool ignore_parity;
 	bool acks_disable_command;
 	unsigned int model;
 	unsigned long last;
@@ -89,31 +126,6 @@ struct psmouse {
 	void (*pt_deactivate)(struct psmouse *psmouse);
 };
 
-enum psmouse_type {
-	PSMOUSE_NONE,
-	PSMOUSE_PS2,
-	PSMOUSE_PS2PP,
-	PSMOUSE_THINKPS,
-	PSMOUSE_GENPS,
-	PSMOUSE_IMPS,
-	PSMOUSE_IMEX,
-	PSMOUSE_SYNAPTICS,
-	PSMOUSE_ALPS,
-	PSMOUSE_LIFEBOOK,
-	PSMOUSE_TRACKPOINT,
-	PSMOUSE_TOUCHKIT_PS2,
-	PSMOUSE_CORTRON,
-	PSMOUSE_HGPK,
-	PSMOUSE_ELANTECH,
-	PSMOUSE_FSP,
-	PSMOUSE_SYNAPTICS_RELATIVE,
-	PSMOUSE_CYPRESS,
-	PSMOUSE_FOCALTECH,
-	PSMOUSE_VMMOUSE,
-	PSMOUSE_BYD,
-	PSMOUSE_AUTO		/* This one should always be last */
-};
-
 void psmouse_queue_work(struct psmouse *psmouse, struct delayed_work *work,
 		unsigned long delay);
 int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command);
-- 
2.12.0.246.ga2ecc84866-goog

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

* [PATCH 5/8] Input: psmouse - introduce notion of SMBus companions
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 4/8] Input: psmouse - store pointer to current protocol Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 6/8] Input: psmouse - add support for " Dmitry Torokhov
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

Prepare PS/2 mouse drivers to work with devices that are accessible both
via PS/2 and SMBus, which provides higher bandwidth, and thus suits better
for modern multi-touch devices.

We expect that SMBus drivers will take control over the device, so when
we detect SMBus "protocol" we forego registering input device, or enabling
PS/2 device reports (as it usually makes device unresponsive to access over
SMBus).

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/psmouse-base.c | 100 +++++++++++++++++++++++++------------
 drivers/input/mouse/psmouse.h      |   1 +
 2 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index bb5d164849ea..40f09ce84f14 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1424,9 +1424,8 @@ static void psmouse_cleanup(struct serio *serio)
  */
 static void psmouse_disconnect(struct serio *serio)
 {
-	struct psmouse *psmouse, *parent = NULL;
-
-	psmouse = serio_get_drvdata(serio);
+	struct psmouse *psmouse = serio_get_drvdata(serio);
+	struct psmouse *parent = NULL;
 
 	sysfs_remove_group(&serio->dev.kobj, &psmouse_attribute_group);
 
@@ -1454,7 +1453,10 @@ static void psmouse_disconnect(struct serio *serio)
 
 	serio_close(serio);
 	serio_set_drvdata(serio, NULL);
-	input_unregister_device(psmouse->dev);
+
+	if (psmouse->dev)
+		input_unregister_device(psmouse->dev);
+
 	kfree(psmouse);
 
 	if (parent)
@@ -1575,12 +1577,18 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
 
 	psmouse_switch_protocol(psmouse, NULL);
 
-	psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
-	psmouse_initialize(psmouse);
+	if (!psmouse->protocol->smbus_companion) {
+		psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
+		psmouse_initialize(psmouse);
 
-	error = input_register_device(psmouse->dev);
-	if (error)
-		goto err_protocol_disconnect;
+		error = input_register_device(input_dev);
+		if (error)
+			goto err_protocol_disconnect;
+	} else {
+		/* Smbus companion will be reporting events, not us. */
+		input_free_device(input_dev);
+		psmouse->dev = input_dev = NULL;
+	}
 
 	if (parent && parent->pt_activate)
 		parent->pt_activate(parent);
@@ -1589,7 +1597,12 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
 	if (error)
 		goto err_pt_deactivate;
 
-	psmouse_activate(psmouse);
+	/*
+	 * PS/2 devices having SMBus companions should stay disabled
+	 * on PS/2 side, in order to have SMBus part operable.
+	 */
+	if (!psmouse->protocol->smbus_companion)
+		psmouse_activate(psmouse);
 
  out:
 	/* If this is a pass-through port the parent needs to be re-activated */
@@ -1602,8 +1615,10 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
  err_pt_deactivate:
 	if (parent && parent->pt_deactivate)
 		parent->pt_deactivate(parent);
-	input_unregister_device(psmouse->dev);
-	input_dev = NULL; /* so we don't try to free it below */
+	if (input_dev) {
+		input_unregister_device(input_dev);
+		input_dev = NULL; /* so we don't try to free it below */
+	}
  err_protocol_disconnect:
 	if (psmouse->disconnect)
 		psmouse->disconnect(psmouse);
@@ -1665,14 +1680,21 @@ static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
 	 * OK, the device type (and capabilities) match the old one,
 	 * we can continue using it, complete initialization
 	 */
-	psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
-
-	psmouse_initialize(psmouse);
+	if (!psmouse->protocol->smbus_companion) {
+		psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
+		psmouse_initialize(psmouse);
+	}
 
 	if (parent && parent->pt_activate)
 		parent->pt_activate(parent);
 
-	psmouse_activate(psmouse);
+	/*
+	 * PS/2 devices having SMBus companions should stay disabled
+	 * on PS/2 side, in order to have SMBus part operable.
+	 */
+	if (!psmouse->protocol->smbus_companion)
+		psmouse_activate(psmouse);
+
 	rc = 0;
 
 out:
@@ -1732,9 +1754,11 @@ ssize_t psmouse_attr_show_helper(struct device *dev, struct device_attribute *de
 {
 	struct serio *serio = to_serio_port(dev);
 	struct psmouse_attribute *attr = to_psmouse_attr(devattr);
-	struct psmouse *psmouse;
+	struct psmouse *psmouse = serio_get_drvdata(serio);
 
-	psmouse = serio_get_drvdata(serio);
+	if (psmouse->protocol->smbus_companion &&
+			devattr != &psmouse_attr_protocol.dattr)
+		return -ENOENT;
 
 	return attr->show(psmouse, attr->data, buf);
 }
@@ -1753,6 +1777,12 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev
 
 	psmouse = serio_get_drvdata(serio);
 
+	if (psmouse->protocol->smbus_companion &&
+			devattr != &psmouse_attr_protocol.dattr) {
+		retval = -ENOENT;
+		goto out_unlock;
+	}
+
 	if (attr->protect) {
 		if (psmouse->state == PSMOUSE_IGNORE) {
 			retval = -ENODEV;
@@ -1764,13 +1794,14 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev
 			psmouse_deactivate(parent);
 		}
 
-		psmouse_deactivate(psmouse);
+		if (!psmouse->protocol->smbus_companion)
+			psmouse_deactivate(psmouse);
 	}
 
 	retval = attr->set(psmouse, attr->data, buf, count);
 
 	if (attr->protect) {
-		if (retval != -ENODEV)
+		if (retval != -ENODEV && !psmouse->protocol->smbus_companion)
 			psmouse_activate(psmouse);
 
 		if (parent)
@@ -1879,23 +1910,26 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co
 	psmouse_initialize(psmouse);
 	psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
 
-	error = input_register_device(psmouse->dev);
-	if (error) {
-		if (psmouse->disconnect)
-			psmouse->disconnect(psmouse);
+	if (!psmouse->protocol->smbus_companion) {
+		error = input_register_device(psmouse->dev);
+		if (error) {
+			if (psmouse->disconnect)
+				psmouse->disconnect(psmouse);
 
-		psmouse_set_state(psmouse, PSMOUSE_IGNORE);
-		input_free_device(new_dev);
-		psmouse->dev = old_dev;
-		psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
-		psmouse_switch_protocol(psmouse, old_proto);
-		psmouse_initialize(psmouse);
-		psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
+			psmouse_set_state(psmouse, PSMOUSE_IGNORE);
+			input_free_device(new_dev);
+			psmouse->dev = old_dev;
+			psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
+			psmouse_switch_protocol(psmouse, old_proto);
+			psmouse_initialize(psmouse);
+			psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
 
-		return error;
+			return error;
+		}
 	}
 
-	input_unregister_device(old_dev);
+	if (old_dev)
+		input_unregister_device(old_dev);
 
 	if (parent && parent->pt_activate)
 		parent->pt_activate(parent);
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 36bd42179456..e853dee05e79 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -76,6 +76,7 @@ struct psmouse_protocol {
 	bool maxproto;
 	bool ignore_parity; /* Protocol should ignore parity errors from KBC */
 	bool try_passthru; /* Try protocol also on passthrough ports */
+	bool smbus_companion; /* "Protocol" is a stub, device is on SMBus */
 	const char *name;
 	const char *alias;
 	int (*detect)(struct psmouse *, bool);
-- 
2.12.0.246.ga2ecc84866-goog

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

* [PATCH 6/8] Input: psmouse - add support for SMBus companions
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 5/8] Input: psmouse - introduce notion of SMBus companions Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-11 15:13   ` kbuild test robot
  2017-03-09 22:16 ` [PATCH 7/8] Input: synaptics - split device info into a separate structure Dmitry Torokhov
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* [PATCH 7/8] Input: synaptics - split device info into a separate structure
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (5 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 6/8] Input: psmouse - add support for " Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 22:16 ` [PATCH 8/8] Input: synaptics - add support for Intertouch devices Dmitry Torokhov
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* [PATCH 8/8] Input: synaptics - add support for Intertouch devices
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (6 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 7/8] Input: synaptics - split device info into a separate structure Dmitry Torokhov
@ 2017-03-09 22:16 ` Dmitry Torokhov
  2017-03-09 23:53 ` [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov
  2017-03-10 15:57 ` [PATCH 0/8] PS Benjamin Tissoires
  9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 22:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

Most of the Synaptics devices are connected through PS/2 and a different
bus (SMBus or HID over I2C). The secondary bus capability is indicated by
the InterTouch bit in extended capability 0x0C.

We only enable the InterTouch device to be created for the laptops
registered with the top software button property or those we know that are
functional. In the future, we might change the default to always rely on
the InterTouch bus. Currently, users can enable/disable the feature with
the psmouse parameter synaptics_intertouch.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/Kconfig        |  12 +
 drivers/input/mouse/psmouse-base.c |  24 +-
 drivers/input/mouse/psmouse.h      |   1 +
 drivers/input/mouse/synaptics.c    | 542 +++++++++++++++++++++++++------------
 drivers/input/mouse/synaptics.h    |   5 +-
 5 files changed, 410 insertions(+), 174 deletions(-)

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 87bde8a210c7..89ebb8f39fee 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -78,6 +78,18 @@ config MOUSE_PS2_SYNAPTICS
 
 	  If unsure, say Y.
 
+config MOUSE_PS2_SYNAPTICS_SMBUS
+	bool "Synaptics PS/2 SMbus companion" if EXPERT
+	default y
+	depends on MOUSE_PS2
+	depends on I2C=y || I2C=MOUSE_PS2
+	select MOUSE_PS2_SMBUS
+	help
+	  Say Y here if you have a Synaptics RMI4 touchpad connected to
+	  to an SMBus, but enumerated through PS/2.
+
+	  If unsure, say Y.
+
 config MOUSE_PS2_CYPRESS
        bool "Cypress PS/2 mouse protocol extension" if EXPERT
        default y
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index bdb48b2acc57..68aa00689c5d 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -773,7 +773,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
 		.name		= "SynPS/2",
 		.alias		= "synaptics",
 		.detect		= synaptics_detect,
-		.init		= synaptics_init,
+		.init		= synaptics_init_absolute,
 	},
 	{
 		.type		= PSMOUSE_SYNAPTICS_RELATIVE,
@@ -783,6 +783,16 @@ static const struct psmouse_protocol psmouse_protocols[] = {
 		.init		= synaptics_init_relative,
 	},
 #endif
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS
+	{
+		.type		= PSMOUSE_SYNAPTICS_SMBUS,
+		.name		= "SynSMBus",
+		.alias		= "synaptics-smbus",
+		.detect		= synaptics_detect,
+		.init		= synaptics_init_smbus,
+		.smbus_companion = true,
+	},
+#endif
 #ifdef CONFIG_MOUSE_PS2_ALPS
 	{
 		.type		= PSMOUSE_ALPS,
@@ -1011,6 +1021,7 @@ static int psmouse_extensions(struct psmouse *psmouse,
 			      unsigned int max_proto, bool set_properties)
 {
 	bool synaptics_hardware = false;
+	int ret;
 
 	/*
 	 * Always check for focaltech, this is safe as it uses pnp-id
@@ -1073,9 +1084,14 @@ static int psmouse_extensions(struct psmouse *psmouse,
 			 * enabled first, since we try detecting Synaptics
 			 * even when protocol is disabled.
 			 */
-			if (IS_ENABLED(CONFIG_MOUSE_PS2_SYNAPTICS) &&
-			    (!set_properties || synaptics_init(psmouse) == 0)) {
-				return PSMOUSE_SYNAPTICS;
+			if (IS_ENABLED(CONFIG_MOUSE_PS2_SYNAPTICS) ||
+			    IS_ENABLED(CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS)) {
+				if (!set_properties)
+					return PSMOUSE_SYNAPTICS;
+
+				ret = synaptics_init(psmouse);
+				if (ret >= 0)
+					return ret;
 			}
 
 			/*
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 186c760e398e..ca4e96baf5e3 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -66,6 +66,7 @@ enum psmouse_type {
 	PSMOUSE_FOCALTECH,
 	PSMOUSE_VMMOUSE,
 	PSMOUSE_BYD,
+	PSMOUSE_SYNAPTICS_SMBUS,
 	PSMOUSE_AUTO		/* This one should always be last */
 };
 
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 0eb01d1e8802..877f9eb3d901 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -29,6 +29,8 @@
 #include <linux/input/mt.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/rmi.h>
+#include <linux/i2c.h>
 #include <linux/slab.h>
 #include "psmouse.h"
 #include "synaptics.h"
@@ -119,59 +121,8 @@ void synaptics_reset(struct psmouse *psmouse)
 	synaptics_mode_cmd(psmouse, 0);
 }
 
-#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
-
-static bool cr48_profile_sensor;
-
-#define ANY_BOARD_ID 0
-struct min_max_quirk {
-	const char * const *pnp_ids;
-	struct {
-		u32 min, max;
-	} board_id;
-	u32 x_min, x_max, y_min, y_max;
-};
-
-static const struct min_max_quirk min_max_pnpid_table[] = {
-	{
-		(const char * const []){"LEN0033", NULL},
-		{ANY_BOARD_ID, ANY_BOARD_ID},
-		1024, 5052, 2258, 4832
-	},
-	{
-		(const char * const []){"LEN0042", NULL},
-		{ANY_BOARD_ID, ANY_BOARD_ID},
-		1232, 5710, 1156, 4696
-	},
-	{
-		(const char * const []){"LEN0034", "LEN0036", "LEN0037",
-					"LEN0039", "LEN2002", "LEN2004",
-					NULL},
-		{ANY_BOARD_ID, 2961},
-		1024, 5112, 2024, 4832
-	},
-	{
-		(const char * const []){"LEN2000", NULL},
-		{ANY_BOARD_ID, ANY_BOARD_ID},
-		1024, 5113, 2021, 4832
-	},
-	{
-		(const char * const []){"LEN2001", NULL},
-		{ANY_BOARD_ID, ANY_BOARD_ID},
-		1024, 5022, 2508, 4832
-	},
-	{
-		(const char * const []){"LEN2006", NULL},
-		{2691, 2691},
-		1024, 5045, 2457, 4832
-	},
-	{
-		(const char * const []){"LEN2006", NULL},
-		{ANY_BOARD_ID, ANY_BOARD_ID},
-		1264, 5675, 1171, 4688
-	},
-	{ }
-};
+#if defined(CONFIG_MOUSE_PS2_SYNAPTICS) || \
+    defined(CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS)
 
 /* This list has been kindly provided by Synaptics. */
 static const char * const topbuttonpad_pnp_ids[] = {
@@ -211,37 +162,50 @@ static const char * const topbuttonpad_pnp_ids[] = {
 	NULL
 };
 
-/* This list has been kindly provided by Synaptics. */
-static const char * const forcepad_pnp_ids[] = {
-	"SYN300D",
-	"SYN3014",
+static const char * const smbus_pnp_ids[] = {
+	/* all of the topbuttonpad_pnp_ids are valid, we just add some extras */
+	"LEN0048", /* X1 Carbon 3 */
+	"LEN0046", /* X250 */
+	"LEN004a", /* W541 */
+	"LEN200f", /* T450s */
 	NULL
 };
 
-/*****************************************************************************
- *	Synaptics communications functions
- ****************************************************************************/
-
 /*
- * Synaptics touchpads report the y coordinate from bottom to top, which is
- * opposite from what userspace expects.
- * This function is used to invert y before reporting.
+ * Send a command to the synpatics touchpad by special commands
  */
-static int synaptics_invert_y(int y)
+static int synaptics_send_cmd(struct psmouse *psmouse,
+			      unsigned char c, unsigned char *param)
 {
-	return YMAX_NOMINAL + YMIN_NOMINAL - y;
+	int error;
+
+	error = psmouse_sliced_command(psmouse, c);
+	if (error)
+		return error;
+
+	error = ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_GETINFO);
+	if (error)
+		return error;
+
+	return 0;
 }
 
 /*
- * Send a command to the synpatics touchpad by special commands
+ * Identify Touchpad
+ * See also the SYN_ID_* macros
  */
-static int synaptics_send_cmd(struct psmouse *psmouse, unsigned char c, unsigned char *param)
+static int synaptics_identify(struct psmouse *psmouse,
+			      struct synaptics_device_info *info)
 {
-	if (psmouse_sliced_command(psmouse, c))
-		return -1;
-	if (ps2_command(&psmouse->ps2dev, param, PSMOUSE_CMD_GETINFO))
-		return -1;
-	return 0;
+	unsigned char id[3];
+	int error;
+
+	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;
 }
 
 /*
@@ -262,6 +226,23 @@ static int synaptics_model_id(struct psmouse *psmouse,
 	return 0;
 }
 
+/*
+ * Read the firmware id from the touchpad
+ */
+static int synaptics_firmware_id(struct psmouse *psmouse,
+				 struct synaptics_device_info *info)
+{
+	unsigned char fwid[3];
+	int error;
+
+	error = synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid);
+	if (error)
+		return error;
+
+	info->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
+	return 0;
+}
+
 static int synaptics_more_extended_queries(struct psmouse *psmouse,
 					   struct synaptics_device_info *info)
 {
@@ -303,23 +284,6 @@ static int synaptics_query_modes(struct psmouse *psmouse,
 }
 
 /*
- * Read the firmware id from the touchpad
- */
-static int synaptics_firmware_id(struct psmouse *psmouse,
-				 struct synaptics_device_info *info)
-{
-	unsigned char fwid[3];
-	int error;
-
-	error = synaptics_send_cmd(psmouse, SYN_QUE_FIRMWARE_ID, fwid);
-	if (error)
-		return error;
-
-	info->firmware_id = (fwid[0] << 16) | (fwid[1] << 8) | fwid[2];
-	return 0;
-}
-
-/*
  * Read the capability-bits from the touchpad
  * see also the SYN_CAP_* macros
  */
@@ -380,28 +344,9 @@ static int synaptics_capability(struct psmouse *psmouse,
 }
 
 /*
- * Identify Touchpad
- * See also the SYN_ID_* macros
- */
-static int synaptics_identify(struct psmouse *psmouse,
-			      struct synaptics_device_info *info)
-{
-	unsigned char id[3];
-	int error;
-
-	error = synaptics_send_cmd(psmouse, SYN_QUE_IDENTIFY, id);
-	if (error)
-		return error;
-
-	info->identity = (id[0] << 16) | (id[1] << 8) | id[2];
-	return SYN_ID_IS_SYNAPTICS(info->identity) ? 0 : -ENXIO;
-}
-
-/*
  * Read touchpad resolution and maximum reported coordinates
  * Resolution is left zero if touchpad does not support the query
  */
-
 static int synaptics_resolution(struct psmouse *psmouse,
 				struct synaptics_device_info *info)
 {
@@ -460,10 +405,118 @@ static int synaptics_resolution(struct psmouse *psmouse,
 	return 0;
 }
 
+static int synaptics_query_hardware(struct psmouse *psmouse,
+				    struct synaptics_device_info *info)
+{
+	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;
+
+	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;
+}
+
+#endif /* CONFIG_MOUSE_PS2_SYNAPTICS || CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS */
+
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
+
+static bool cr48_profile_sensor;
+
+#define ANY_BOARD_ID 0
+struct min_max_quirk {
+	const char * const *pnp_ids;
+	struct {
+		u32 min, max;
+	} board_id;
+	u32 x_min, x_max, y_min, y_max;
+};
+
+static const struct min_max_quirk min_max_pnpid_table[] = {
+	{
+		(const char * const []){"LEN0033", NULL},
+		{ANY_BOARD_ID, ANY_BOARD_ID},
+		1024, 5052, 2258, 4832
+	},
+	{
+		(const char * const []){"LEN0042", NULL},
+		{ANY_BOARD_ID, ANY_BOARD_ID},
+		1232, 5710, 1156, 4696
+	},
+	{
+		(const char * const []){"LEN0034", "LEN0036", "LEN0037",
+					"LEN0039", "LEN2002", "LEN2004",
+					NULL},
+		{ANY_BOARD_ID, 2961},
+		1024, 5112, 2024, 4832
+	},
+	{
+		(const char * const []){"LEN2000", NULL},
+		{ANY_BOARD_ID, ANY_BOARD_ID},
+		1024, 5113, 2021, 4832
+	},
+	{
+		(const char * const []){"LEN2001", NULL},
+		{ANY_BOARD_ID, ANY_BOARD_ID},
+		1024, 5022, 2508, 4832
+	},
+	{
+		(const char * const []){"LEN2006", NULL},
+		{2691, 2691},
+		1024, 5045, 2457, 4832
+	},
+	{
+		(const char * const []){"LEN2006", NULL},
+		{ANY_BOARD_ID, ANY_BOARD_ID},
+		1264, 5675, 1171, 4688
+	},
+	{ }
+};
+
+/* This list has been kindly provided by Synaptics. */
+static const char * const forcepad_pnp_ids[] = {
+	"SYN300D",
+	"SYN3014",
+	NULL
+};
+
+/*****************************************************************************
+ *	Synaptics communications functions
+ ****************************************************************************/
+
 /*
- * Apply quirk(s) if the hardware matches
+ * Synaptics touchpads report the y coordinate from bottom to top, which is
+ * opposite from what userspace expects.
+ * This function is used to invert y before reporting.
  */
+static int synaptics_invert_y(int y)
+{
+	return YMAX_NOMINAL + YMIN_NOMINAL - y;
+}
 
+/*
+ * Apply quirk(s) if the hardware matches
+ */
 static void synaptics_apply_quirks(struct psmouse *psmouse,
 				   struct synaptics_device_info *info)
 {
@@ -494,38 +547,6 @@ static void synaptics_apply_quirks(struct psmouse *psmouse,
 	}
 }
 
-static int synaptics_query_hardware(struct psmouse *psmouse,
-				    struct synaptics_device_info *info)
-{
-	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;
-
-	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;
-}
-
 static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 {
 	static unsigned char param = 0xc8;
@@ -1350,6 +1371,12 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
 
+	/*
+	 * We might have left a breadcrumb when trying to
+	 * set up SMbus companion.
+	 */
+	psmouse_smbus_cleanup(psmouse);
+
 	if (!priv->absolute_mode &&
 			SYN_ID_DISGEST_SUPPORTED(priv->info.identity))
 		device_remove_file(&psmouse->ps2dev.serio->dev,
@@ -1490,39 +1517,20 @@ void __init synaptics_module_init(void)
 	cr48_profile_sensor = dmi_check_system(cr48_dmi_table);
 }
 
-static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
+static int synaptics_init_ps2(struct psmouse *psmouse,
+			      struct synaptics_device_info *info,
+			      bool absolute_mode)
 {
 	struct synaptics_data *priv;
-	struct synaptics_device_info *info;
-	int err = -1;
+	int err;
 
-	/*
-	 * The OLPC XO has issues with Synaptics' absolute mode; the constant
-	 * packet spew overloads the EC such that key presses on the keyboard
-	 * are missed.  Given that, don't even attempt to use Absolute mode.
-	 * Relative mode seems to work just fine.
-	 */
-	if (absolute_mode && broken_olpc_ec) {
-		psmouse_info(psmouse,
-			     "OLPC XO detected, not enabling Synaptics protocol.\n");
-		return -ENODEV;
-	}
+	synaptics_apply_quirks(psmouse, info);
 
 	psmouse->private = priv = kzalloc(sizeof(struct synaptics_data), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	info = &priv->info;
-
-	psmouse_reset(psmouse);
-
-	if (synaptics_query_hardware(psmouse, info)) {
-		psmouse_err(psmouse, "Unable to query device.\n");
-		goto init_fail;
-	}
-
-	synaptics_apply_quirks(psmouse, info);
-
+	priv->info = *info;
 	priv->absolute_mode = absolute_mode;
 	if (SYN_ID_DISGEST_SUPPORTED(info->identity))
 		priv->disable_gesture = true;
@@ -1610,7 +1618,23 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 	return err;
 }
 
-int synaptics_init(struct psmouse *psmouse)
+static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
+{
+	struct synaptics_device_info info;
+	int error;
+
+	psmouse_reset(psmouse);
+
+	error = synaptics_query_hardware(psmouse, &info);
+	if (error) {
+		psmouse_err(psmouse, "Unable to query device: %d\n", error);
+		return error;
+	}
+
+	return synaptics_init_ps2(psmouse, &info, absolute_mode);
+}
+
+int synaptics_init_absolute(struct psmouse *psmouse)
 {
 	return __synaptics_init(psmouse, true);
 }
@@ -1620,15 +1644,195 @@ int synaptics_init_relative(struct psmouse *psmouse)
 	return __synaptics_init(psmouse, false);
 }
 
+static int synaptics_setup_ps2(struct psmouse *psmouse,
+			       struct synaptics_device_info *info)
+{
+	bool absolute_mode = true;
+	int error;
+
+	/*
+	 * The OLPC XO has issues with Synaptics' absolute mode; the constant
+	 * packet spew overloads the EC such that key presses on the keyboard
+	 * are missed.  Given that, don't even attempt to use Absolute mode.
+	 * Relative mode seems to work just fine.
+	 */
+	if (broken_olpc_ec) {
+		psmouse_info(psmouse,
+			     "OLPC XO detected, forcing relative protocol.\n");
+		absolute_mode = false;
+	}
+
+	error = synaptics_init_ps2(psmouse, info, absolute_mode);
+	if (error)
+		return error;
+
+	return absolute_mode ? PSMOUSE_SYNAPTICS : PSMOUSE_SYNAPTICS_RELATIVE;
+}
+
 #else /* CONFIG_MOUSE_PS2_SYNAPTICS */
 
 void __init synaptics_module_init(void)
 {
 }
 
-int synaptics_init(struct psmouse *psmouse)
+static int synaptics_setup_ps2(struct psmouse *psmouse,
+			       struct synaptics_device_info *info)
 {
 	return -ENOSYS;
 }
 
 #endif /* CONFIG_MOUSE_PS2_SYNAPTICS */
+
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS
+
+/*
+ * The newest Synaptics device can use a secondary bus (called InterTouch) which
+ * provides a better bandwidth and allow a better control of the touchpads.
+ * This is used to decide if we need to use this bus or not.
+ */
+enum {
+	SYNAPTICS_INTERTOUCH_NOT_SET = -1,
+	SYNAPTICS_INTERTOUCH_OFF,
+	SYNAPTICS_INTERTOUCH_ON,
+};
+
+static int synaptics_intertouch = SYNAPTICS_INTERTOUCH_NOT_SET;
+module_param_named(synaptics_intertouch, synaptics_intertouch, int, 0644);
+MODULE_PARM_DESC(synaptics_intertouch, "Use a secondary bus for the Synaptics device.");
+
+static int synaptics_create_intertouch(struct psmouse *psmouse,
+				       struct synaptics_device_info *info,
+				       bool leave_breadcrumbs)
+{
+	bool topbuttonpad =
+		psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
+		!SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10);
+	struct rmi_device_platform_data pdata = {
+		.sensor_pdata = {
+			.sensor_type = rmi_sensor_touchpad,
+			.axis_align.flip_y = true,
+			/* to prevent cursors jumps: */
+			.kernel_tracking = true,
+			.topbuttonpad = topbuttonpad,
+		},
+		.f30_data = {
+			.buttonpad = SYN_CAP_CLICKPAD(info->ext_cap_0c),
+			.trackstick_buttons =
+				!!SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10),
+		},
+	};
+	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);
+}
+
+/**
+ * synaptics_setup_intertouch - called once the PS/2 devices are enumerated
+ * and decides to instantiate a SMBus InterTouch device.
+ */
+static int synaptics_setup_intertouch(struct psmouse *psmouse,
+				      struct synaptics_device_info *info,
+				      bool leave_breadcrumbs)
+{
+	int error;
+
+	if (synaptics_intertouch == SYNAPTICS_INTERTOUCH_OFF)
+		return -ENXIO;
+
+	if (synaptics_intertouch == SYNAPTICS_INTERTOUCH_NOT_SET) {
+		if (!psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
+		    !psmouse_matches_pnp_id(psmouse, smbus_pnp_ids))
+			return -ENXIO;
+	}
+
+	psmouse_info(psmouse, "Trying to set up SMBus access\n");
+
+	error = synaptics_create_intertouch(psmouse, info, leave_breadcrumbs);
+	if (error) {
+		if (error == -EAGAIN)
+			psmouse_info(psmouse, "SMbus companion is not ready yet\n");
+		else
+			psmouse_err(psmouse, "unable to create intertouch device\n");
+
+		return error;
+	}
+
+	return 0;
+}
+
+int synaptics_init_smbus(struct psmouse *psmouse)
+{
+	struct synaptics_device_info info;
+	int error;
+
+	psmouse_reset(psmouse);
+
+	error = synaptics_query_hardware(psmouse, &info);
+	if (error) {
+		psmouse_err(psmouse, "Unable to query device: %d\n", error);
+		return error;
+	}
+
+	if (!SYN_CAP_INTERTOUCH(info.ext_cap_0c))
+		return -ENXIO;
+
+	return synaptics_create_intertouch(psmouse, &info, false);
+}
+
+#else /* CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS */
+
+int synaptics_init_smbus(struct psmouse *psmouse)
+{
+	return -ENOSYS;
+}
+
+#endif /* CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS */
+
+#if defined(CONFIG_MOUSE_PS2_SYNAPTICS) || \
+    defined(CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS)
+
+int synaptics_init(struct psmouse *psmouse)
+{
+	struct synaptics_device_info info;
+	int error;
+	int retval;
+
+	psmouse_reset(psmouse);
+
+	error = synaptics_query_hardware(psmouse, &info);
+	if (error) {
+		psmouse_err(psmouse, "Unable to query device: %d\n", error);
+		return error;
+	}
+
+	if (SYN_CAP_INTERTOUCH(info.ext_cap_0c)) {
+		error = synaptics_setup_intertouch(psmouse, &info, true);
+		if (!error)
+			return PSMOUSE_SYNAPTICS_SMBUS;
+	}
+
+	retval = synaptics_setup_ps2(psmouse, &info);
+	if (retval < 0) {
+		/*
+		 * Not using any flavor of Synaptics support, so clean up
+		 * SMbus breadcrumbs, if any.
+		 */
+		psmouse_smbus_cleanup(psmouse);
+	}
+
+	return retval;
+}
+
+#else /* CONFIG_MOUSE_PS2_SYNAPTICS || CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS */
+
+int synaptics_init(struct psmouse *psmouse)
+{
+	return -ENOSYS;
+}
+
+#endif /* CONFIG_MOUSE_PS2_SYNAPTICS || CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS */
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 4d1452718cbc..fc93481cf183 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -90,6 +90,7 @@
 #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
 #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
 #define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
+#define SYN_CAP_INTERTOUCH(ex0c)	((ex0c) & 0x004000)
 
 /*
  * The following descibes response for the 0x10 query.
@@ -204,8 +205,10 @@ struct synaptics_data {
 
 void synaptics_module_init(void);
 int synaptics_detect(struct psmouse *psmouse, bool set_properties);
-int synaptics_init(struct psmouse *psmouse);
+int synaptics_init_absolute(struct psmouse *psmouse);
 int synaptics_init_relative(struct psmouse *psmouse);
+int synaptics_init_smbus(struct psmouse *psmouse);
+int synaptics_init(struct psmouse *psmouse);
 void synaptics_reset(struct psmouse *psmouse);
 
 #endif /* _SYNAPTICS_H */
-- 
2.12.0.246.ga2ecc84866-goog

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
@ 2017-03-09 23:12   ` Wolfram Sang
  2017-03-09 23:46     ` Dmitry Torokhov
  2017-03-13 13:50     ` Jean Delvare
  2017-04-01 16:06   ` Wolfram Sang
  1 sibling, 2 replies; 28+ messages in thread
From: Wolfram Sang @ 2017-03-09 23:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Jean Delvare
  Cc: Benjamin Tissoires, Andrew Duggan, linux-kernel, linux-input

[-- Attachment #1: Type: text/plain, Size: 2400 bytes --]

Dmitry,

On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> i2c bus has 2 different types of device belonging to the same bus and
> bus notifiers use device type to distinguish between adapters and clients.
> Previously we only had i2c_adapter_type exported, which made code wanting
> to work with i2c_client devices test for type not equal to adapter type.
> This unfortunately is not safe if we ever add another type to the bus,
> so let's export i2c_client_type as well.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Wolfram, this is the patch I was talking about in the other mail.

I see. From a glimpse, I am fine with the patch. I'll add Jean Delvare
to CC, though, in case I missed some detail he still knows. Furthermore,
while I agree that testing for "not adapter" when one means "is client"
is not nice, is there a bigger benefit than being correct in your queue?

Regards,

   Wolfram

> 
>  drivers/i2c/i2c-core.c | 4 ++--
>  include/linux/i2c.h    | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 34a5115484dd..446e341e9508 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -74,7 +74,6 @@
>  static DEFINE_MUTEX(core_lock);
>  static DEFINE_IDR(i2c_adapter_idr);
>  
> -static struct device_type i2c_client_type;
>  static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
>  
>  static struct static_key i2c_trace_msg = STATIC_KEY_INIT_FALSE;
> @@ -1096,11 +1095,12 @@ struct bus_type i2c_bus_type = {
>  };
>  EXPORT_SYMBOL_GPL(i2c_bus_type);
>  
> -static struct device_type i2c_client_type = {
> +struct device_type i2c_client_type = {
>  	.groups		= i2c_dev_groups,
>  	.uevent		= i2c_device_uevent,
>  	.release	= i2c_client_dev_release,
>  };
> +EXPORT_SYMBOL_GPL(i2c_client_type);
>  
>  
>  /**
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 2cc3988d127b..89ca5e56b433 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -37,6 +37,7 @@
>  
>  extern struct bus_type i2c_bus_type;
>  extern struct device_type i2c_adapter_type;
> +extern struct device_type i2c_client_type;
>  
>  /* --- General options ------------------------------------------------	*/
>  
> -- 
> 2.12.0.246.ga2ecc84866-goog
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 23:12   ` Wolfram Sang
@ 2017-03-09 23:46     ` Dmitry Torokhov
  2017-03-09 23:51       ` Dmitry Torokhov
  2017-03-13 13:50     ` Jean Delvare
  1 sibling, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 23:46 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jean Delvare, Benjamin Tissoires, Andrew Duggan, linux-kernel,
	linux-input

On Fri, Mar 10, 2017 at 12:12:46AM +0100, Wolfram Sang wrote:
> Dmitry,
> 
> On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> > i2c bus has 2 different types of device belonging to the same bus and
> > bus notifiers use device type to distinguish between adapters and clients.
> > Previously we only had i2c_adapter_type exported, which made code wanting
> > to work with i2c_client devices test for type not equal to adapter type.
> > This unfortunately is not safe if we ever add another type to the bus,
> > so let's export i2c_client_type as well.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 
> > Wolfram, this is the patch I was talking about in the other mail.
> 
> I see. From a glimpse, I am fine with the patch. I'll add Jean Delvare
> to CC, though, in case I missed some detail he still knows. Furthermore,
> while I agree that testing for "not adapter" when one means "is client"
> is not nice, is there a bigger benefit than being correct in your queue?

No, just my dislike of testing for "dev->type != &i2c_adapter_type" in
the new i2c bus notifier when we want to work with i2c clients.

Thanks.

-- 
Dmitry

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 23:46     ` Dmitry Torokhov
@ 2017-03-09 23:51       ` Dmitry Torokhov
  0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 23:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jean Delvare, Benjamin Tissoires, Andrew Duggan, linux-kernel,
	linux-input

On Thu, Mar 09, 2017 at 03:46:24PM -0800, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 12:12:46AM +0100, Wolfram Sang wrote:
> > Dmitry,
> > 
> > On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> > > i2c bus has 2 different types of device belonging to the same bus and
> > > bus notifiers use device type to distinguish between adapters and clients.
> > > Previously we only had i2c_adapter_type exported, which made code wanting
> > > to work with i2c_client devices test for type not equal to adapter type.
> > > This unfortunately is not safe if we ever add another type to the bus,
> > > so let's export i2c_client_type as well.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > 
> > > Wolfram, this is the patch I was talking about in the other mail.
> > 
> > I see. From a glimpse, I am fine with the patch. I'll add Jean Delvare
> > to CC, though, in case I missed some detail he still knows. Furthermore,
> > while I agree that testing for "not adapter" when one means "is client"
> > is not nice, is there a bigger benefit than being correct in your queue?
> 
> No, just my dislike of testing for "dev->type != &i2c_adapter_type" in
> the new i2c bus notifier when we want to work with i2c clients.

I might not answered your question ;) I use i2c_client_type export in
patch #6 of this series (I'll add you to CC in a moment), and I have
some other patches that should use it as well.

Thanks.

-- 
Dmitry

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

* [PATCH 6/8] Input: psmouse - add support for SMBus companions
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (7 preceding siblings ...)
  2017-03-09 22:16 ` [PATCH 8/8] Input: synaptics - add support for Intertouch devices Dmitry Torokhov
@ 2017-03-09 23:53 ` Dmitry Torokhov
  2017-03-10 17:55   ` Benjamin Tissoires
  2017-03-10 15:57 ` [PATCH 0/8] PS Benjamin Tissoires
  9 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 23:53 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input, Wolfram Sang

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

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

* Re: [PATCH 0/8] PS
  2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
                   ` (8 preceding siblings ...)
  2017-03-09 23:53 ` [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov
@ 2017-03-10 15:57 ` Benjamin Tissoires
  2017-03-10 17:52   ` Dmitry Torokhov
  9 siblings, 1 reply; 28+ messages in thread
From: Benjamin Tissoires @ 2017-03-10 15:57 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Andrew Duggan, linux-kernel, linux-input

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
> 

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

* Re: [PATCH 0/8] PS
  2017-03-10 15:57 ` [PATCH 0/8] PS Benjamin Tissoires
@ 2017-03-10 17:52   ` Dmitry Torokhov
  2017-03-10 18:04     ` Benjamin Tissoires
  2017-03-10 18:56     ` Andrew Duggan
  0 siblings, 2 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 17:52 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions
  2017-03-09 23:53 ` [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov
@ 2017-03-10 17:55   ` Benjamin Tissoires
  2017-03-10 18:16     ` Dmitry Torokhov
  0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Tissoires @ 2017-03-10 17:55 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Andrew Duggan, linux-kernel, linux-input, Wolfram Sang

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 17:52   ` Dmitry Torokhov
@ 2017-03-10 18:04     ` Benjamin Tissoires
  2017-03-10 18:10       ` Dmitry Torokhov
  2017-03-10 18:56     ` Andrew Duggan
  1 sibling, 1 reply; 28+ messages in thread
From: Benjamin Tissoires @ 2017-03-10 18:04 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 18:04     ` Benjamin Tissoires
@ 2017-03-10 18:10       ` Dmitry Torokhov
  2017-03-10 20:25         ` Dmitry Torokhov
  0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 18:10 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions
  2017-03-10 17:55   ` Benjamin Tissoires
@ 2017-03-10 18:16     ` Dmitry Torokhov
  0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 18:16 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input, Wolfram Sang

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 17:52   ` Dmitry Torokhov
  2017-03-10 18:04     ` Benjamin Tissoires
@ 2017-03-10 18:56     ` Andrew Duggan
  2017-03-10 20:12       ` Dmitry Torokhov
  1 sibling, 1 reply; 28+ messages in thread
From: Andrew Duggan @ 2017-03-10 18:56 UTC (permalink / raw)
  To: Dmitry Torokhov, Benjamin Tissoires; +Cc: linux-kernel, linux-input

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 18:56     ` Andrew Duggan
@ 2017-03-10 20:12       ` Dmitry Torokhov
  2017-03-10 20:31         ` Andrew Duggan
  0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 20:12 UTC (permalink / raw)
  To: Andrew Duggan; +Cc: Benjamin Tissoires, linux-kernel, linux-input

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 18:10       ` Dmitry Torokhov
@ 2017-03-10 20:25         ` Dmitry Torokhov
  0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 20:25 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-kernel, linux-input

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

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

* Re: [PATCH 0/8] PS
  2017-03-10 20:12       ` Dmitry Torokhov
@ 2017-03-10 20:31         ` Andrew Duggan
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Duggan @ 2017-03-10 20:31 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Benjamin Tissoires, linux-kernel, linux-input

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

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

* Re: [PATCH 6/8] Input: psmouse - add support for SMBus companions
  2017-03-09 22:16 ` [PATCH 6/8] Input: psmouse - add support for " Dmitry Torokhov
@ 2017-03-11 15:13   ` kbuild test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kbuild test robot @ 2017-03-11 15:13 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: kbuild-all, Benjamin Tissoires, Andrew Duggan, linux-kernel, linux-input

[-- Attachment #1: Type: text/plain, Size: 4108 bytes --]

Hi Benjamin,

[auto build test ERROR on input/next]
[also build test ERROR on v4.11-rc1 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/PS/20170311-222141
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-randconfig-x004-201710 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Dmitry-Torokhov/PS/20170311-222141 HEAD b84adc980e4fc51bf4be2aac8853afcc68ceb13b builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   In file included from drivers/input/mouse/psmouse-base.c:27:0:
>> drivers/input/mouse/psmouse.h:234:56: warning: 'struct i2c_board_info' declared inside parameter list will not be visible outside of this definition or declaration
    int psmouse_smbus_init(struct psmouse *psmouse, struct i2c_board_info *board,
                                                           ^~~~~~~~~~~~~~
--
   drivers/input/mouse/synaptics.o: In function `psmouse_smbus_init':
>> synaptics.c:(.text+0x0): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/synaptics.o: In function `psmouse_smbus_cleanup':
>> synaptics.c:(.text+0x10): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here
   drivers/input/mouse/focaltech.o: In function `psmouse_smbus_init':
   focaltech.c:(.text+0x0): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/focaltech.o: In function `psmouse_smbus_cleanup':
   focaltech.c:(.text+0x10): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here
   drivers/input/mouse/byd.o: In function `psmouse_smbus_init':
   byd.c:(.text+0x3e8): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/byd.o: In function `psmouse_smbus_cleanup':
   byd.c:(.text+0x3f8): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here
   drivers/input/mouse/elantech.o: In function `psmouse_smbus_init':
   elantech.c:(.text+0x1cc6): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/elantech.o: In function `psmouse_smbus_cleanup':
   elantech.c:(.text+0x1cd6): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here
   drivers/input/mouse/sentelic.o: In function `psmouse_smbus_init':
   sentelic.c:(.text+0x14f1): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/sentelic.o: In function `psmouse_smbus_cleanup':
   sentelic.c:(.text+0x1501): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here
   drivers/input/mouse/trackpoint.o: In function `psmouse_smbus_init':
   trackpoint.c:(.text+0xb2b): multiple definition of `psmouse_smbus_init'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11c4): first defined here
   drivers/input/mouse/trackpoint.o: In function `psmouse_smbus_cleanup':
   trackpoint.c:(.text+0xb3b): multiple definition of `psmouse_smbus_cleanup'
   drivers/input/mouse/psmouse-base.o:psmouse-base.c:(.text+0x11d4): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19966 bytes --]

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 23:12   ` Wolfram Sang
  2017-03-09 23:46     ` Dmitry Torokhov
@ 2017-03-13 13:50     ` Jean Delvare
  2017-03-15 23:50       ` Dmitry Torokhov
  1 sibling, 1 reply; 28+ messages in thread
From: Jean Delvare @ 2017-03-13 13:50 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Dmitry Torokhov, Benjamin Tissoires, Andrew Duggan, linux-kernel,
	linux-input

Hi Dmitry,

On Fri, 10 Mar 2017 00:12:46 +0100, Wolfram Sang wrote:
> On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> > i2c bus has 2 different types of device belonging to the same bus and
> > bus notifiers use device type to distinguish between adapters and clients.

Out of curiosity, is this something unusual, or other bus types do the
same?

> > Previously we only had i2c_adapter_type exported, which made code wanting
> > to work with i2c_client devices test for type not equal to adapter type.
> > This unfortunately is not safe if we ever add another type to the bus,
> > so let's export i2c_client_type as well.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 
> > Wolfram, this is the patch I was talking about in the other mail.
> 
> I see. From a glimpse, I am fine with the patch. I'll add Jean Delvare
> to CC, though, in case I missed some detail he still knows. Furthermore,
> while I agree that testing for "not adapter" when one means "is client"
> is not nice, is there a bigger benefit than being correct in your queue?

No objection from me.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

> > 
> >  drivers/i2c/i2c-core.c | 4 ++--
> >  include/linux/i2c.h    | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 34a5115484dd..446e341e9508 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -74,7 +74,6 @@
> >  static DEFINE_MUTEX(core_lock);
> >  static DEFINE_IDR(i2c_adapter_idr);
> >  
> > -static struct device_type i2c_client_type;
> >  static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
> >  
> >  static struct static_key i2c_trace_msg = STATIC_KEY_INIT_FALSE;
> > @@ -1096,11 +1095,12 @@ struct bus_type i2c_bus_type = {
> >  };
> >  EXPORT_SYMBOL_GPL(i2c_bus_type);
> >  
> > -static struct device_type i2c_client_type = {
> > +struct device_type i2c_client_type = {
> >  	.groups		= i2c_dev_groups,
> >  	.uevent		= i2c_device_uevent,
> >  	.release	= i2c_client_dev_release,
> >  };
> > +EXPORT_SYMBOL_GPL(i2c_client_type);
> >  
> >  
> >  /**
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index 2cc3988d127b..89ca5e56b433 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -37,6 +37,7 @@
> >  
> >  extern struct bus_type i2c_bus_type;
> >  extern struct device_type i2c_adapter_type;
> > +extern struct device_type i2c_client_type;
> >  
> >  /* --- General options ------------------------------------------------	*/
> >  
> > -- 
> > 2.12.0.246.ga2ecc84866-goog
> > 


-- 
Jean Delvare
SUSE L3 Support

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-13 13:50     ` Jean Delvare
@ 2017-03-15 23:50       ` Dmitry Torokhov
  0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-03-15 23:50 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Wolfram Sang, Benjamin Tissoires, Andrew Duggan, linux-kernel,
	linux-input

Hi Jean,

On Mon, Mar 13, 2017 at 02:50:45PM +0100, Jean Delvare wrote:
> Hi Dmitry,
> 
> On Fri, 10 Mar 2017 00:12:46 +0100, Wolfram Sang wrote:
> > On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> > > i2c bus has 2 different types of device belonging to the same bus and
> > > bus notifiers use device type to distinguish between adapters and clients.
> 
> Out of curiosity, is this something unusual, or other bus types do the
> same?

I think I2C is somewhat special because of compounding of multiple
factors:

1. It uses several device types (clients and adapters) on the same bus.
It is not unique in this way, many buses do that (w1 for example), but
others do not (spi).

2. We are starting using i2c bus notifiers more and more because x86
platform is shitty at describing i2c devices. Even if device is
described in ACPI, there is high chance that description is incomplete,
and if it is "complete", then it is quite likely wrong. Or they may not
be described in ACPI at all and require vendor drivers on Windows to
activate and operate (as with these SMBus companions to PS/2 devices).
Thankfully SPI is not as popular on X86, or we'd have the same mess
there.

So we create bunch of notifiers and try to "fix" stuff, but we need to
verify that we are dealing with the right device type. We already have
to_verified_client() and to_verified_adapter(), and i2c_adapter_type was
already exported, so in cases where I need to work with both adapters
and clients I wanted to export i2c_client_type as well. So if we ever
add a 3rd type we do not have to go back and adjust existing code. I
think it looks makes code easier to read if you test for equality to
client if you want clients.

> 
> > > Previously we only had i2c_adapter_type exported, which made code wanting
> > > to work with i2c_client devices test for type not equal to adapter type.
> > > This unfortunately is not safe if we ever add another type to the bus,
> > > so let's export i2c_client_type as well.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > 
> > > Wolfram, this is the patch I was talking about in the other mail.
> > 
> > I see. From a glimpse, I am fine with the patch. I'll add Jean Delvare
> > to CC, though, in case I missed some detail he still knows. Furthermore,
> > while I agree that testing for "not adapter" when one means "is client"
> > is not nice, is there a bigger benefit than being correct in your queue?
> 
> No objection from me.
> 
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Thanks!

> 
> > > 
> > >  drivers/i2c/i2c-core.c | 4 ++--
> > >  include/linux/i2c.h    | 1 +
> > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > index 34a5115484dd..446e341e9508 100644
> > > --- a/drivers/i2c/i2c-core.c
> > > +++ b/drivers/i2c/i2c-core.c
> > > @@ -74,7 +74,6 @@
> > >  static DEFINE_MUTEX(core_lock);
> > >  static DEFINE_IDR(i2c_adapter_idr);
> > >  
> > > -static struct device_type i2c_client_type;
> > >  static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
> > >  
> > >  static struct static_key i2c_trace_msg = STATIC_KEY_INIT_FALSE;
> > > @@ -1096,11 +1095,12 @@ struct bus_type i2c_bus_type = {
> > >  };
> > >  EXPORT_SYMBOL_GPL(i2c_bus_type);
> > >  
> > > -static struct device_type i2c_client_type = {
> > > +struct device_type i2c_client_type = {
> > >  	.groups		= i2c_dev_groups,
> > >  	.uevent		= i2c_device_uevent,
> > >  	.release	= i2c_client_dev_release,
> > >  };
> > > +EXPORT_SYMBOL_GPL(i2c_client_type);
> > >  
> > >  
> > >  /**
> > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > > index 2cc3988d127b..89ca5e56b433 100644
> > > --- a/include/linux/i2c.h
> > > +++ b/include/linux/i2c.h
> > > @@ -37,6 +37,7 @@
> > >  
> > >  extern struct bus_type i2c_bus_type;
> > >  extern struct device_type i2c_adapter_type;
> > > +extern struct device_type i2c_client_type;
> > >  
> > >  /* --- General options ------------------------------------------------	*/
> > >  
> > > -- 
> > > 2.12.0.246.ga2ecc84866-goog
> > > 
> 
> 
> -- 
> Jean Delvare
> SUSE L3 Support

-- 
Dmitry

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
  2017-03-09 23:12   ` Wolfram Sang
@ 2017-04-01 16:06   ` Wolfram Sang
  2017-04-01 16:43     ` Dmitry Torokhov
  1 sibling, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2017-04-01 16:06 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Benjamin Tissoires, Andrew Duggan, linux-kernel, linux-input

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> i2c bus has 2 different types of device belonging to the same bus and
> bus notifiers use device type to distinguish between adapters and clients.
> Previously we only had i2c_adapter_type exported, which made code wanting
> to work with i2c_client devices test for type not equal to adapter type.
> This unfortunately is not safe if we ever add another type to the bus,
> so let's export i2c_client_type as well.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Wolfram, this is the patch I was talking about in the other mail.

Reviewed-by: Wolfram Sang <wsa@the-dreams.de>

You can add the above tag to all patches in your branch
'4.11-rc3-i2c-irq-resources' and after that we declare it immutable and
I'll pull it into my for-next. Sorry for the delay. I got side-tracked
and ill. But I am better now.

And to keep your heads up: We agreed yesterday that i2c_drivers will get
a flag to signal I2C core to skip all the irq assignment stuff and let
the driver do it itself [1]. Maybe this will become useful for you, too.

[1] http://www.spinics.net/lists/linux-acpi/msg73197.html


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 1/8] i2c: export i2c_client_type structure
  2017-04-01 16:06   ` Wolfram Sang
@ 2017-04-01 16:43     ` Dmitry Torokhov
  0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2017-04-01 16:43 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Benjamin Tissoires, Andrew Duggan, linux-kernel, linux-input

On Sat, Apr 01, 2017 at 06:06:46PM +0200, Wolfram Sang wrote:
> On Thu, Mar 09, 2017 at 02:16:37PM -0800, Dmitry Torokhov wrote:
> > i2c bus has 2 different types of device belonging to the same bus and
> > bus notifiers use device type to distinguish between adapters and clients.
> > Previously we only had i2c_adapter_type exported, which made code wanting
> > to work with i2c_client devices test for type not equal to adapter type.
> > This unfortunately is not safe if we ever add another type to the bus,
> > so let's export i2c_client_type as well.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 
> > Wolfram, this is the patch I was talking about in the other mail.
> 
> Reviewed-by: Wolfram Sang <wsa@the-dreams.de>
> 
> You can add the above tag to all patches in your branch
> '4.11-rc3-i2c-irq-resources' and after that we declare it immutable and
> I'll pull it into my for-next. Sorry for the delay. I got side-tracked
> and ill. But I am better now.

No worries about the delay - we still have plenty of time till merge
window, and I am glad to hear that you are better now.

I published the branch with your reviewed-bys:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git ib/4.11-rc3-i2c-irq-resources

Note the 'ib' prefix - I am tagging immutable branches with it. I also
dropped the old branch to avoid confusion.

> 
> And to keep your heads up: We agreed yesterday that i2c_drivers will get
> a flag to signal I2C core to skip all the irq assignment stuff and let
> the driver do it itself [1]. Maybe this will become useful for you, too.
> 
> [1] http://www.spinics.net/lists/linux-acpi/msg73197.html
> 

Thanks for heads up. I actually do not think it will be useful for me,
as I am trying to make drivers generic and do not care about platform
quirks, and instead enhance board/platform code to give sane data to the
drivers.

I replied in thread.

Thanks.

-- 
Dmitry

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

end of thread, other threads:[~2017-04-01 16:43 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09 22:16 [PATCH 0/8] PS Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
2017-03-09 23:12   ` Wolfram Sang
2017-03-09 23:46     ` Dmitry Torokhov
2017-03-09 23:51       ` Dmitry Torokhov
2017-03-13 13:50     ` Jean Delvare
2017-03-15 23:50       ` Dmitry Torokhov
2017-04-01 16:06   ` Wolfram Sang
2017-04-01 16:43     ` Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 2/8] Input: serio - add fast reconnect option Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 3/8] Input: psmouse - implement " Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 4/8] Input: psmouse - store pointer to current protocol Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 5/8] Input: psmouse - introduce notion of SMBus companions Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 6/8] Input: psmouse - add support for " Dmitry Torokhov
2017-03-11 15:13   ` kbuild test robot
2017-03-09 22:16 ` [PATCH 7/8] Input: synaptics - split device info into a separate structure Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 8/8] Input: synaptics - add support for Intertouch devices Dmitry Torokhov
2017-03-09 23:53 ` [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov
2017-03-10 17:55   ` Benjamin Tissoires
2017-03-10 18:16     ` Dmitry Torokhov
2017-03-10 15:57 ` [PATCH 0/8] PS Benjamin Tissoires
2017-03-10 17:52   ` Dmitry Torokhov
2017-03-10 18:04     ` Benjamin Tissoires
2017-03-10 18:10       ` Dmitry Torokhov
2017-03-10 20:25         ` Dmitry Torokhov
2017-03-10 18:56     ` Andrew Duggan
2017-03-10 20:12       ` Dmitry Torokhov
2017-03-10 20:31         ` Andrew Duggan

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