[PATCH v2 0/3] Bind RMI4 over SMBus from PS/2

[PATCH v2 0/3] Bind RMI4 over SMBus from PS/2

[Benjamin Tissoires]

Hi Dmitry,

quoting your last email about this topic:

> I think that by trying to "unwind" unsuccessful SMbus initialization you
> make the code much more complicated and fragile. I think we should
> select a path (PS/2 or SMbus) and commit to it. If we commit to SMBus
> then we need to communicate that fact to psmouse core so that it does
> not create input devices or psmouse attributes, and "short-circuit" the
> reconnect() routines to simply ignore requests and always report
> success.

I have tried in the past to unwind the psmouse input devices, and it was a pain.
So I thought of using a dummy serio driver that basically just calls
PSMOUSE_CMD_DISABLE at connect and returns 0 everywhere else.

I tried this in the past without much luck but I think I found a reliable
way today.

The good thing is that it seems that in that case, the rmi4_smbus driver
doesn't need any changes now that most serio states are ignored.

The new ps2smbus driver gets a little bit more complex, especially because
of kseriod. We need to wait for the .connect() of the serio driver to end
and have a stable PS/2 connection before starting the smbus work.

>From the tests I made today, it seems reliable: cold boot & suspend/resume
works, various "rescan" with drvctl with different synaptics_intertouch
parameter state work too.

I hope you'll find this solution acceptable.

Cheers,
Benjamin

Benjamin Tissoires (3):
  input: serio - allow others to specify a driver for a serio device
  Input: synaptics - allocate a Synaptics Intertouch device
  Input: add a PS/2 to SMBus platform module

 drivers/input/misc/Kconfig      |  11 ++
 drivers/input/misc/Makefile     |   1 +
 drivers/input/misc/ps2_smbus.c  | 382 ++++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.c | 106 +++++++++++
 drivers/input/mouse/synaptics.h |   1 +
 drivers/input/rmi4/Kconfig      |   1 +
 drivers/input/serio/serio.c     |  20 +++
 include/linux/serio.h           |   5 +
 8 files changed, 527 insertions(+)
 create mode 100644 drivers/input/misc/ps2_smbus.c

-- 
2.9.3

[PATCH v2 1/3] input: serio - allow others to specify a driver for a serio device

[Benjamin Tissoires]

The Lenovo Thinkpads use RMI4 over SMBus in addition to PS/2 for their
trackpad. The problem is that the device doesn't enumerate itself besides
some registers in PS/2.

Once the initial PS/2 initialization has been made, we need a way to unbind
psmouse from the touchpad and use a different (dummy) serio driver to
not interfere with SMBus.

This patch adds the mechanisms to unbind psmouse and use a different serio
driver, driver which can be marked as manual_bind to not be picked up
by inadvertence.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

new in v2

 drivers/input/serio/serio.c | 20 ++++++++++++++++++++
 include/linux/serio.h       |  5 +++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 1ca7f55..14cc383 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -704,6 +704,23 @@ void serio_reconnect(struct serio *serio)
 }
 EXPORT_SYMBOL(serio_reconnect);
 
+void serio_bind_manual_driver(struct serio *serio, struct serio_driver *drv)
+{
+	mutex_lock(&serio_mutex);
+	serio->manual_bind = true;
+	serio->manual_drv = &drv->driver;
+	mutex_unlock(&serio_mutex);
+	serio_rescan(serio);
+}
+EXPORT_SYMBOL(serio_bind_manual_driver);
+
+void serio_clear_manual_driver(struct serio *serio)
+{
+	serio->manual_bind = false;
+	serio->manual_drv = NULL;
+}
+EXPORT_SYMBOL(serio_clear_manual_driver);
+
 /*
  * Submits register request to kseriod for subsequent execution.
  * Note that port registration is always asynchronous.
@@ -902,6 +919,9 @@ static int serio_bus_match(struct device *dev, struct device_driver *drv)
 	struct serio *serio = to_serio_port(dev);
 	struct serio_driver *serio_drv = to_serio_driver(drv);
 
+	if (serio->manual_drv && serio->manual_drv == drv)
+		return serio_match_port(serio_drv->id_table, serio);
+
 	if (serio->manual_bind || serio_drv->manual_bind)
 		return 0;
 
diff --git a/include/linux/serio.h b/include/linux/serio.h
index c733cff..1a6f4db 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -64,6 +64,9 @@ struct serio {
 	 * may get indigestion when exposed to concurrent access (i8042).
 	 */
 	struct mutex *ps2_cmd_mutex;
+
+	/* Used when forcing a driver instead of the default one. */
+	struct device_driver *manual_drv;
 };
 #define to_serio_port(d)	container_of(d, struct serio, dev)
 
@@ -88,6 +91,8 @@ int serio_open(struct serio *serio, struct serio_driver *drv);
 void serio_close(struct serio *serio);
 void serio_rescan(struct serio *serio);
 void serio_reconnect(struct serio *serio);
+void serio_bind_manual_driver(struct serio *serio, struct serio_driver *drv);
+void serio_clear_manual_driver(struct serio *serio);
 irqreturn_t serio_interrupt(struct serio *serio, unsigned char data, unsigned int flags);
 
 void __serio_register_port(struct serio *serio, struct module *owner);
-- 
2.9.3

[PATCH v2 2/3] Input: synaptics - allocate a Synaptics Intertouch device

[Benjamin Tissoires]

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.

When we encounter such a device, we can create a platform device with
the information gathered through the PS/2 enumeration as some information
might be missing through the other bus. Using a platform device allows
to not add any dependency on the psmouse driver.

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.

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. We need to be sure the psmouse
driver will stop communicating with the device (and the pass-through
trackstick too). This part is not addressed here but will be in a
following patch.

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>

---

changes in v2:
- do not store the platform device as psmouse will be disconnected
  to be replaced by a dummy serio driver.
---
 drivers/input/mouse/synaptics.c | 106 ++++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |   1 +
 2 files changed, 107 insertions(+)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 597ee4b..c71a500 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/platform_device.h>
+#include <linux/rmi.h>
 #include <linux/slab.h>
 #include "psmouse.h"
 #include "synaptics.h"
@@ -70,6 +72,21 @@
 /* maximum ABS_MT_POSITION displacement (in mm) */
 #define DMAX 10
 
+/*
+ * 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.");
+
 /*****************************************************************************
  *	Stuff we need even when we do not want native Synaptics support
  ****************************************************************************/
@@ -218,6 +235,92 @@ static const char * const forcepad_pnp_ids[] = {
 	NULL
 };
 
+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 */
+};
+
+static int rmi4_id;
+
+static int synaptics_create_intertouch(struct psmouse *psmouse)
+{
+	struct synaptics_data *priv = psmouse->private;
+	struct platform_device_info pdevinfo;
+	struct platform_device *pdev;
+	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,
+		},
+		.f30_data = {
+			.buttonpad = SYN_CAP_CLICKPAD(priv->ext_cap_0c),
+			.trackstick_buttons =
+			  !!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10),
+		},
+	};
+
+	pdata.sensor_pdata.topbuttonpad =
+			psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
+			!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10);
+
+	memset(&pdevinfo, 0, sizeof(pdevinfo));
+	pdevinfo.name = "rmi4";
+	pdevinfo.id = rmi4_id++;
+	pdevinfo.data = &pdata;
+	pdevinfo.size_data = sizeof(pdata);
+	pdevinfo.parent = &psmouse->ps2dev.serio->dev;
+
+	pdev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
+}
+
+static int synaptics_remove_intertouch_device(struct device *dev, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	if (strncmp("rmi4", dev_name(dev), 4))
+		return 0;
+
+	platform_device_unregister(pdev);
+
+	return 0;
+}
+
+/**
+ * synaptics_setup_intertouch - called once the PS/2 devices are enumerated
+ * and decides to instantiate a SMBus InterTouch device.
+ */
+static void synaptics_setup_intertouch(struct psmouse *psmouse)
+{
+	int ret;
+
+	/* first remove any remnant platform intertouch devices */
+	bus_for_each_dev(&platform_bus_type, NULL, NULL,
+			 synaptics_remove_intertouch_device);
+
+	if (synaptics_intertouch == SYNAPTICS_INTERTOUCH_OFF)
+		return;
+
+	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;
+	}
+
+	psmouse_info(psmouse, "device also supported by an other bus.\n");
+	ret = synaptics_create_intertouch(psmouse);
+	if (ret)
+		psmouse_info(psmouse,
+			     "unable to create intertouch device.\n");
+}
 /*****************************************************************************
  *	Synaptics communications functions
  ****************************************************************************/
@@ -1546,6 +1649,9 @@ static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
 		}
 	}
 
+	if (SYN_CAP_INTERTOUCH(priv->ext_cap_0c))
+		synaptics_setup_intertouch(psmouse);
+
 	return 0;
 
  init_fail:
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 116ae25..ccbd3c5 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.
-- 
2.9.3

Re: [PATCH v2 2/3] Input: synaptics - allocate a Synaptics Intertouch device

[kbuild test robot]

Hi Benjamin,

[auto build test WARNING on input/next]
[also build test WARNING on v4.10-rc8 next-20170216]
[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/Benjamin-Tissoires/Bind-RMI4-over-SMBus-from-PS-2/20170217-043234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/input/mouse/synaptics.c:279:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Please review and possibly fold the followup patch.

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

[PATCH] Input: fix ptr_ret.cocci warnings

[kbuild test robot]

drivers/input/mouse/synaptics.c:279:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 synaptics.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -276,10 +276,7 @@ static int synaptics_create_intertouch(s
 	pdevinfo.parent = &psmouse->ps2dev.serio->dev;
 
 	pdev = platform_device_register_full(&pdevinfo);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(pdev);
 }
 
 static int synaptics_remove_intertouch_device(struct device *dev, void *data)

[PATCH v2 3/3] Input: add a PS/2 to SMBus platform module

[Benjamin Tissoires]

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

We use an intermediate platform device to not add a
dependency between psmouse and I2C. It also handles
the subtleties of going around the serio mutex lock by
deferring the i2c creation/destruction in a separate
thread.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v2:
- use our own ps2 driver (detach psmouse and reattach ours)
---
 drivers/input/misc/Kconfig     |  11 ++
 drivers/input/misc/Makefile    |   1 +
 drivers/input/misc/ps2_smbus.c | 382 +++++++++++++++++++++++++++++++++++++++++
 drivers/input/rmi4/Kconfig     |   1 +
 4 files changed, 395 insertions(+)
 create mode 100644 drivers/input/misc/ps2_smbus.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 5b6c522..64362ba 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -822,4 +822,15 @@ config INPUT_HISI_POWERKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called hisi_powerkey.
 
+config PS2_SMBUS
+	tristate "Platform Support for PS/2 Nodes also connected over SMBus"
+	depends on I2C
+	help
+	  Say Y here if you want to support RMI4 or Elan devices connected to
+	  an SMB bus but enumerated through PS/2.
+
+	  if unsure, say N.
+	  To compile this driver as a module, choose M here: the module will be
+	  called ps2_smbus.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index b10523f..59406da 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_INPUT_PCSPKR)		+= pcspkr.o
 obj-$(CONFIG_INPUT_PM8941_PWRKEY)	+= pm8941-pwrkey.o
 obj-$(CONFIG_INPUT_PM8XXX_VIBRATOR)	+= pm8xxx-vibrator.o
 obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY)	+= pmic8xxx-pwrkey.o
+obj-$(CONFIG_PS2_SMBUS)			+= ps2_smbus.o
 obj-$(CONFIG_INPUT_POWERMATE)		+= powermate.o
 obj-$(CONFIG_INPUT_PWM_BEEPER)		+= pwm-beeper.o
 obj-$(CONFIG_INPUT_RB532_BUTTON)	+= rb532_button.o
diff --git a/drivers/input/misc/ps2_smbus.c b/drivers/input/misc/ps2_smbus.c
new file mode 100644
index 0000000..b58c113
--- /dev/null
+++ b/drivers/input/misc/ps2_smbus.c
@@ -0,0 +1,382 @@
+/*
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/libps2.h>
+#include <linux/platform_device.h>
+#include <linux/rmi.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@redhat.com>");
+MODULE_DESCRIPTION("Platform PS/2 - SMBus bridge driver");
+MODULE_LICENSE("GPL");
+
+static struct workqueue_struct *kps2smbus_wq;
+static DECLARE_WAIT_QUEUE_HEAD(ps2smbus_serio_wait);
+static DEFINE_MUTEX(ps2smbus_mutex);
+
+enum ps2smbus_type {
+	PS2SMBUS_SYNAPTICS_RMI4,
+};
+
+struct ps2smbus {
+	struct serio *serio;
+	struct i2c_client *smbus_client;
+	struct notifier_block i2c_notifier;
+	enum ps2smbus_type type;
+	void *pdata;
+};
+
+enum ps2smbus_event_type {
+	PS2SMBUS_REGISTER_DEVICE,
+	PS2SMBUS_UNREGISTER_DEVICE,
+};
+
+struct ps2smbus_work {
+	struct work_struct work;
+	enum ps2smbus_event_type type;
+	struct ps2smbus *ps2smbus;
+	struct i2c_adapter *adap;
+};
+
+struct ps2smbus_serio {
+	struct ps2dev ps2dev;
+};
+
+static struct serio_device_id ps2smbus_serio_ids[] = {
+	{
+		.type	= SERIO_8042,
+		.proto	= SERIO_ANY,
+		.id	= SERIO_ANY,
+		.extra	= SERIO_ANY,
+	},
+	{ 0 }
+};
+
+MODULE_DEVICE_TABLE(serio, ps2smbus_serio_ids);
+
+static irqreturn_t ps2smbus_interrupt(struct serio *serio,
+		unsigned char data, unsigned int flags)
+{
+	struct ps2smbus_serio *ps2smbus = serio_get_drvdata(serio);
+
+	if (unlikely((flags & SERIO_TIMEOUT) || (flags & SERIO_PARITY))) {
+		ps2_cmd_aborted(&ps2smbus->ps2dev);
+		goto out;
+	}
+
+	if (unlikely(ps2smbus->ps2dev.flags & PS2_FLAG_ACK))
+		if (ps2_handle_ack(&ps2smbus->ps2dev, data))
+			goto out;
+
+	if (unlikely(ps2smbus->ps2dev.flags & PS2_FLAG_CMD))
+		if (ps2_handle_response(&ps2smbus->ps2dev, data))
+			goto out;
+
+out:
+	return IRQ_HANDLED;
+}
+
+#define PSMOUSE_CMD_DISABLE	0x00f5
+
+static int ps2smbus_connect(struct serio *serio, struct serio_driver *drv)
+{
+	int error;
+	struct ps2smbus_serio *ps2smbus;
+
+	ps2smbus = kzalloc(sizeof(struct ps2smbus_serio), GFP_KERNEL);
+
+	ps2_init(&ps2smbus->ps2dev, serio);
+
+	serio_set_drvdata(serio, ps2smbus);
+
+	error = serio_open(serio, drv);
+	if (error)
+		goto fail_free;
+
+	error = ps2_command(&ps2smbus->ps2dev, NULL, PSMOUSE_CMD_DISABLE);
+	if (error) {
+		dev_warn(&serio->dev, "Failed to deactivate mouse on %s\n",
+			 serio->phys);
+		goto fail;
+	}
+
+	wake_up_interruptible(&ps2smbus_serio_wait);
+
+	return 0;
+
+fail:
+	serio_close(serio);
+fail_free:
+	serio_set_drvdata(serio, NULL);
+	kfree(ps2smbus);
+	return error;
+}
+
+static int ps2smbus_reconnect(struct serio *serio)
+{
+	return 0;
+}
+
+static void ps2smbus_disconnect(struct serio *serio)
+{
+	struct ps2smbus_serio *ps2smbus = serio_get_drvdata(serio);
+
+	serio_clear_manual_driver(serio);
+	serio_close(serio);
+	serio_set_drvdata(serio, NULL);
+	kfree(ps2smbus);
+}
+
+static struct serio_driver ps2smbus_serio_drv = {
+	.driver		= {
+		.name	= "ps2smbus",
+	},
+	.description	= "PS/2 SMBus bridge",
+	.id_table	= ps2smbus_serio_ids,
+	.interrupt	= ps2smbus_interrupt,
+	.connect	= ps2smbus_connect,
+	.reconnect	= ps2smbus_reconnect,
+	.disconnect	= ps2smbus_disconnect,
+	.manual_bind	= true,
+};
+
+static void ps2smbus_create_rmi4(struct ps2smbus *ps2smbus,
+				 struct i2c_adapter *adap)
+{
+	const struct i2c_board_info i2c_info = {
+		I2C_BOARD_INFO("rmi4_smbus", 0x2c),
+		.platform_data = ps2smbus->pdata,
+		.flags = I2C_CLIENT_HOST_NOTIFY,
+	};
+
+	ps2smbus->smbus_client = i2c_new_device(adap, &i2c_info);
+}
+
+static void ps2smbus_worker(struct work_struct *work)
+{
+	struct ps2smbus_work *ps2smbus_work;
+	struct i2c_client *client;
+	struct serio *serio;
+	int error;
+
+	ps2smbus_work = container_of(work, struct ps2smbus_work, work);
+	client = ps2smbus_work->ps2smbus->smbus_client;
+	serio = ps2smbus_work->ps2smbus->serio;
+
+	mutex_lock(&ps2smbus_mutex);
+
+	switch (ps2smbus_work->type) {
+	case PS2SMBUS_REGISTER_DEVICE:
+		serio_bind_manual_driver(serio, &ps2smbus_serio_drv);
+		error = wait_event_interruptible_timeout(ps2smbus_serio_wait,
+				serio->drv == &ps2smbus_serio_drv,
+				msecs_to_jiffies(2000));
+		if (error <= 1) {
+			dev_warn(&serio->dev,
+				 "error while waiting for the PS/2 node to be ready: %d\n",
+				 error);
+			goto out;
+		}
+
+		if (ps2smbus_work->ps2smbus->type == PS2SMBUS_SYNAPTICS_RMI4)
+			ps2smbus_create_rmi4(ps2smbus_work->ps2smbus,
+					     ps2smbus_work->adap);
+		break;
+	case PS2SMBUS_UNREGISTER_DEVICE:
+		if (client)
+			i2c_unregister_device(client);
+		break;
+	}
+
+out:
+	kfree(ps2smbus_work);
+
+	mutex_unlock(&ps2smbus_mutex);
+}
+
+static int ps2smbus_schedule_work(enum ps2smbus_event_type type,
+				  struct ps2smbus *ps2smbus,
+				  struct i2c_adapter *adap)
+{
+	struct ps2smbus_work *ps2smbus_work;
+
+	ps2smbus_work = kzalloc(sizeof(*ps2smbus_work), GFP_KERNEL);
+	if (!ps2smbus_work)
+		return -ENOMEM;
+
+	ps2smbus_work->type = type;
+	ps2smbus_work->ps2smbus = ps2smbus;
+	ps2smbus_work->adap = adap;
+
+	INIT_WORK(&ps2smbus_work->work, ps2smbus_worker);
+
+	queue_work(kps2smbus_wq, &ps2smbus_work->work);
+
+	return 0;
+}
+
+static int ps2smbus_attach_i2c_device(struct device *dev, void *data)
+{
+	struct ps2smbus *ps2smbus = data;
+	struct i2c_adapter *adap;
+
+	if (dev->type != &i2c_adapter_type)
+		return 0;
+
+	adap = to_i2c_adapter(dev);
+
+	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_HOST_NOTIFY))
+		return 0;
+
+	if (ps2smbus->smbus_client)
+		return 0;
+
+	ps2smbus_schedule_work(PS2SMBUS_REGISTER_DEVICE, ps2smbus, adap);
+
+	pr_debug("ps2smbus: adapter [%s] registered\n", adap->name);
+	return 0;
+}
+
+static int ps2smbus_detach_i2c_device(struct device *dev,
+				      struct ps2smbus *ps2smbus)
+{
+	struct i2c_client *client;
+
+	if (dev->type == &i2c_adapter_type)
+		return 0;
+
+	mutex_lock(&ps2smbus_mutex);
+
+	client = to_i2c_client(dev);
+	if (client == ps2smbus->smbus_client)
+		ps2smbus->smbus_client = NULL;
+
+	mutex_unlock(&ps2smbus_mutex);
+
+	pr_debug("ps2smbus: client [%s] unregistered\n", client->name);
+	return 0;
+}
+
+static int ps2smbus_notifier_call(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct device *dev = data;
+	struct ps2smbus *ps2smbus;
+
+	ps2smbus = container_of(nb, struct ps2smbus, i2c_notifier);
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		return ps2smbus_attach_i2c_device(dev, ps2smbus);
+	case BUS_NOTIFY_DEL_DEVICE:
+		return ps2smbus_detach_i2c_device(dev, ps2smbus);
+	}
+
+	return 0;
+}
+
+static int ps2smbus_probe(struct platform_device *pdev)
+{
+	struct ps2smbus *ps2smbus;
+	int error;
+
+	if (!pdev->dev.parent)
+		return -EINVAL;
+
+	ps2smbus = devm_kzalloc(&pdev->dev, sizeof(struct ps2smbus),
+				GFP_KERNEL);
+	if (!ps2smbus)
+		return -ENOMEM;
+
+	ps2smbus->i2c_notifier.notifier_call = ps2smbus_notifier_call;
+	ps2smbus->pdata = pdev->dev.platform_data;
+	ps2smbus->type = pdev->id_entry->driver_data;
+	ps2smbus->serio = to_serio_port(pdev->dev.parent);
+
+	/* Keep track of adapters which will be added or removed later */
+	error = bus_register_notifier(&i2c_bus_type, &ps2smbus->i2c_notifier);
+	if (error)
+		return error;
+
+	/* Bind to already existing adapters right away */
+	i2c_for_each_dev(ps2smbus, ps2smbus_attach_i2c_device);
+
+	platform_set_drvdata(pdev, ps2smbus);
+
+	return 0;
+}
+
+static int ps2smbus_remove(struct platform_device *pdev)
+{
+	struct ps2smbus *ps2smbus = platform_get_drvdata(pdev);
+
+	bus_unregister_notifier(&i2c_bus_type, &ps2smbus->i2c_notifier);
+
+	ps2smbus_schedule_work(PS2SMBUS_UNREGISTER_DEVICE, ps2smbus, NULL);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct platform_device_id ps2smbus_id_table[] = {
+	{ .name = "rmi4", .driver_data = PS2SMBUS_SYNAPTICS_RMI4 },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, ps2smbus_id_table);
+
+static struct platform_driver ps2smbus_drv = {
+	.driver		= {
+		.name	= "ps2smbus",
+	},
+	.probe		= ps2smbus_probe,
+	.remove		= ps2smbus_remove,
+	.id_table	= ps2smbus_id_table,
+};
+
+static int __init ps2smbus_init(void)
+{
+	int err;
+
+	err = serio_register_driver(&ps2smbus_serio_drv);
+	if (err)
+		return err;
+
+	kps2smbus_wq = alloc_ordered_workqueue("kps2smbusd", WQ_MEM_RECLAIM);
+	if (!kps2smbus_wq) {
+		pr_err("failed to create kps2smbusd workqueue\n");
+		err = -ENOMEM;
+		goto fail_workqueue;
+	}
+
+	err = platform_driver_register(&ps2smbus_drv);
+	if (err)
+		goto fail_register_pdrv;
+
+	return 0;
+
+fail_register_pdrv:
+	destroy_workqueue(kps2smbus_wq);
+fail_workqueue:
+	serio_unregister_driver(&ps2smbus_serio_drv);
+	return err;
+}
+
+static void __exit ps2smbus_exit(void)
+{
+	platform_driver_unregister(&ps2smbus_drv);
+	destroy_workqueue(kps2smbus_wq);
+	serio_unregister_driver(&ps2smbus_serio_drv);
+}
+
+module_init(ps2smbus_init);
+module_exit(ps2smbus_exit);
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 7341bf0..6c067f7 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -32,6 +32,7 @@ config RMI4_SPI
 config RMI4_SMB
 	tristate "RMI4 SMB Support"
 	depends on I2C
+	select PS2_SMBUS
 	help
 	  Say Y here if you want to support RMI4 devices connected to an SMB
 	  bus.
-- 
2.9.3

Re: [PATCH v2 3/3] Input: add a PS/2 to SMBus platform module

[kbuild test robot]

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

Hi Benjamin,

[auto build test WARNING on input/next]
[also build test WARNING on next-20170216]
[cannot apply to v4.10-rc8]
[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/Benjamin-Tissoires/Bind-RMI4-over-SMBus-from-PS-2/20170217-043234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-x001-201707 (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=i386 

All warnings (new ones prefixed by >>):

warning: (RMI4_SMB) selects PS2_SMBUS which has unmet direct dependencies (!UML && INPUT && INPUT_MISC && I2C)

---
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: 25385 bytes --]

[PATCH v2 4/3] Input: ps2smbus - force PS/2 disable before SMBus gets resumed

[Benjamin Tissoires]

On some cases, the touchpad can be reset during resume. We need to
send the PS/2 command PSMOUSE_CMD_DISABLE before attempting to contact
the touchpad over SMBus. Given that the .connect() callback is called
in a separate thread in kseriod, we need to wait for it in the main
thread before leaving the resume of the platform device.

>From what I can see, the I2C client is then blocked until the platform
device gets resumed, even if the I2C client is not a child of the platform
device.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Hi Dmitry,

this morning the touchpad was dead after the resume. So we need to
actually be sure the PS/2 state is disabled before attempting to
use the SMBus connection.

I am not 100% sure the I2C client will be waiting for the platform
device to be resumed given that I can't find a way to mark the I2C
as a child of the other one. However, it seems that the ordering
is correct nevertheless.

Cheers,
Benjamin


new in v2

 drivers/input/misc/ps2_smbus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/input/misc/ps2_smbus.c b/drivers/input/misc/ps2_smbus.c
index b58c113..0b03224 100644
--- a/drivers/input/misc/ps2_smbus.c
+++ b/drivers/input/misc/ps2_smbus.c
@@ -49,6 +49,7 @@ struct ps2smbus_work {
 
 struct ps2smbus_serio {
 	struct ps2dev ps2dev;
+	bool suspended;
 };
 
 static struct serio_device_id ps2smbus_serio_ids[] = {
@@ -121,8 +122,26 @@ static int ps2smbus_connect(struct serio *serio, struct serio_driver *drv)
 	return error;
 }
 
+static void ps2smbus_cleanup(struct serio *serio)
+{
+	struct ps2smbus_serio *ps2smbus = serio_get_drvdata(serio);
+
+	ps2smbus->suspended = true;
+}
+
 static int ps2smbus_reconnect(struct serio *serio)
 {
+	struct ps2smbus_serio *ps2smbus = serio_get_drvdata(serio);
+	int error;
+
+	error = ps2_command(&ps2smbus->ps2dev, NULL, PSMOUSE_CMD_DISABLE);
+	if (error)
+		dev_warn(&serio->dev, "Failed to deactivate PS/2 mouse on %s\n",
+			 serio->phys);
+
+	ps2smbus->suspended = false;
+	wake_up_interruptible(&ps2smbus_serio_wait);
+
 	return 0;
 }
 
@@ -144,6 +163,7 @@ static struct serio_driver ps2smbus_serio_drv = {
 	.id_table	= ps2smbus_serio_ids,
 	.interrupt	= ps2smbus_interrupt,
 	.connect	= ps2smbus_connect,
+	.cleanup	= ps2smbus_cleanup,
 	.reconnect	= ps2smbus_reconnect,
 	.disconnect	= ps2smbus_disconnect,
 	.manual_bind	= true,
@@ -328,6 +348,27 @@ static int ps2smbus_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused ps2smbus_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ps2smbus *ps2smbus = platform_get_drvdata(pdev);
+	struct serio *serio = ps2smbus->serio;
+	struct ps2smbus_serio *ps2smbus_serio = serio_get_drvdata(serio);
+	int error;
+
+	error = wait_event_interruptible_timeout(ps2smbus_serio_wait,
+				ps2smbus_serio->suspended == false,
+				msecs_to_jiffies(1000));
+	if (error <= 10)
+		dev_warn(&serio->dev,
+			 "error while waiting for the PS/2 node to be ready: %d\n",
+			 error);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ps2smbus_pm_ops, NULL, ps2smbus_resume);
+
 static const struct platform_device_id ps2smbus_id_table[] = {
 	{ .name = "rmi4", .driver_data = PS2SMBUS_SYNAPTICS_RMI4 },
 	{ }
@@ -337,6 +378,7 @@ MODULE_DEVICE_TABLE(platform, ps2smbus_id_table);
 static struct platform_driver ps2smbus_drv = {
 	.driver		= {
 		.name	= "ps2smbus",
+		.pm	= &ps2smbus_pm_ops,
 	},
 	.probe		= ps2smbus_probe,
 	.remove		= ps2smbus_remove,
-- 
2.9.3