[PATCH 00/11] Synaptics RMI4 over SMBus

[PATCH 00/11] Synaptics RMI4 over SMBus

[Benjamin Tissoires]

Hi,

this is finally my first whole submission of RMI4 over SMbus and the binding
of such devices found on the Thinkpad T series.

It has been a long time since we wanted to have those drivers in, but few
hiccups were on the road:
- lack of SMBus Host Notify (now merged upstream, still few fixes needed, but
  nothing preventing us to include this series).
- a unexpected behavior where the touchpad disables the SMBus commands if
  psmouse_activate has been called.
- some hairy problems with the 2 big locks found in the serio and psmouse
  driver.

This series also supports the RMI4 PS/2 pass-through required for the Thinkpad
X1 tablet even if it uses RMI4 over HID over USB.

Cheers,
Benjamin

Benjamin Tissoires (8):
  Input: synaptics-rmi4 - add SMBus support
  Input: serio - store the pt_buttons in the struct serio directly
  Input: synaptics-rmi4 - have only one struct platform data
  Input: synaptics-rmi4 - Add rmi_find_function()
  Input: synaptics - allocate a Synaptics Intertouch device
  Input: synaptics-rmi4 - add rmi_platform
  Input: synaptics-rmi4 - smbus: call psmouse_deactivate before
    binding/resume
  Input: synaptics-rmi4 - smbus: on resume, try 3 times if init fails

Dennis Wassenberg (1):
  Input: synaptics-rmi4 - f03: grab data passed by transport device

Lyude Paul (2):
  Input: synaptics-rmi4 - add support for F03
  Input: synaptics-rmi4 - f30/f03: Forward mechanical buttons on
    buttonpads to PS/2 guest

 drivers/input/mouse/psmouse-base.c |  12 +
 drivers/input/mouse/psmouse.h      |   1 +
 drivers/input/mouse/synaptics.c    | 150 +++++++++++-
 drivers/input/mouse/synaptics.h    |   5 +-
 drivers/input/rmi4/Kconfig         |  33 +++
 drivers/input/rmi4/Makefile        |   3 +
 drivers/input/rmi4/rmi_bus.c       |   3 +
 drivers/input/rmi4/rmi_bus.h       |  12 +
 drivers/input/rmi4/rmi_driver.c    |  13 +
 drivers/input/rmi4/rmi_driver.h    |  15 ++
 drivers/input/rmi4/rmi_f03.c       | 271 +++++++++++++++++++++
 drivers/input/rmi4/rmi_f11.c       |   4 +-
 drivers/input/rmi4/rmi_f12.c       |   4 +-
 drivers/input/rmi4/rmi_f30.c       |  71 +++++-
 drivers/input/rmi4/rmi_platform.c  | 235 ++++++++++++++++++
 drivers/input/rmi4/rmi_smbus.c     | 478 +++++++++++++++++++++++++++++++++++++
 include/linux/rmi.h                |  17 +-
 include/linux/serio.h              |   8 +
 include/uapi/linux/serio.h         |   1 +
 19 files changed, 1308 insertions(+), 28 deletions(-)
 create mode 100644 drivers/input/rmi4/rmi_f03.c
 create mode 100644 drivers/input/rmi4/rmi_platform.c
 create mode 100644 drivers/input/rmi4/rmi_smbus.c

-- 
2.5.5

[PATCH 01/11] Input: synaptics-rmi4 - add SMBus support

[Benjamin Tissoires]

Code obtained from https://raw.githubusercontent.com/mightybigcar/synaptics-rmi4/jf/drivers/input/rmi4/rmi_smbus.c
and updated to match upstream. And fixed to make it work.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

---

Changes in this series:
- rely on PM functions for resume handling

Changes from the Host Notify series:
new in v5

no changes in v6

changes in v7:
- fixed typos as per Andrew's requests
- changed module author from Allie to Andrew as per Andrew's request
- add suspend/resume callbacks to match upstream rmi4 behavior

no changes in v8
---
 drivers/input/rmi4/Kconfig     |  12 ++
 drivers/input/rmi4/Makefile    |   1 +
 drivers/input/rmi4/rmi_bus.h   |  12 ++
 drivers/input/rmi4/rmi_smbus.c | 449 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 474 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_smbus.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index f73df24..86a180b 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -27,6 +27,18 @@ config RMI4_SPI
 
 	  If unsure, say N.
 
+config RMI4_SMB
+	tristate "RMI4 SMB Support"
+	depends on RMI4_CORE && I2C
+	help
+	  Say Y here if you want to support RMI4 devices connected to an SMB
+	  bus.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called rmi_smbus.
+
 config RMI4_2D_SENSOR
 	bool
 	depends on RMI4_CORE
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 95c00a7..3c8ebf2 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -11,3 +11,4 @@ rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
 # Transports
 obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
 obj-$(CONFIG_RMI4_SPI) += rmi_spi.o
+obj-$(CONFIG_RMI4_SMB) += rmi_smbus.o
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 8995798..b7625a9 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -105,6 +105,18 @@ rmi_get_platform_data(struct rmi_device *d)
 bool rmi_is_physical_device(struct device *dev);
 
 /**
+ * rmi_reset - reset a RMI4 device
+ * @d: Pointer to an RMI device
+ *
+ * Calls for a reset of each function implemented by a specific device.
+ * Returns 0 on success or a negative error code.
+ */
+static inline int rmi_reset(struct rmi_device *d)
+{
+	return d->driver->reset_handler(d);
+}
+
+/**
  * rmi_read - read a single byte
  * @d: Pointer to an RMI device
  * @addr: The address to read from
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
new file mode 100644
index 0000000..4d6f228
--- /dev/null
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -0,0 +1,449 @@
+/*
+ * Copyright (c) 2015 - 2016 Red Hat, Inc
+ * Copyright (c) 2011, 2012 Synaptics Incorporated
+ * Copyright (c) 2011 Unixphere
+ *
+ * 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/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kconfig.h>
+#include <linux/lockdep.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/rmi.h>
+#include <linux/slab.h>
+#include "rmi_driver.h"
+
+#define SMB_PROTOCOL_VERSION_ADDRESS	0xfd
+#define SMB_MAX_COUNT			32
+#define RMI_SMB2_MAP_SIZE		8 /* 8 entry of 4 bytes each */
+#define RMI_SMB2_MAP_FLAGS_WE		0x01
+
+struct mapping_table_entry {
+	__le16 rmiaddr;
+	u8 readcount;
+	u8 flags;
+};
+
+struct rmi_smb_xport {
+	struct rmi_transport_dev xport;
+	struct i2c_client *client;
+
+	struct mutex page_mutex;
+	int page;
+	u8 table_index;
+	struct mutex mappingtable_mutex;
+	struct mapping_table_entry mapping_table[RMI_SMB2_MAP_SIZE];
+};
+
+static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
+{
+	struct i2c_client *client = rmi_smb->client;
+	int retval;
+
+	/* Check if for SMBus new version device by reading version byte. */
+	retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
+	if (retval < 0) {
+		dev_err(&client->dev, "failed to get SMBus version number!\n");
+		return retval;
+	}
+	return retval + 1;
+}
+
+/* SMB block write - wrapper over ic2_smb_write_block */
+static int smb_block_write(struct rmi_transport_dev *xport,
+			      u8 commandcode, const void *buf, size_t len)
+{
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+	struct i2c_client *client = rmi_smb->client;
+	int retval;
+
+	retval = i2c_smbus_write_block_data(client, commandcode, len, buf);
+
+	rmi_dbg(RMI_DEBUG_XPORT, &client->dev,
+		"wrote %zd bytes at %#04x: %d (%*ph)\n",
+		len, commandcode, retval, (int)len, buf);
+
+	return retval;
+}
+
+/*
+ * The function to get command code for smbus operations and keeps
+ * records to the driver mapping table
+ */
+static int rmi_smb_get_command_code(struct rmi_transport_dev *xport,
+		u16 rmiaddr, int bytecount, bool isread, u8 *commandcode)
+{
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+	int i;
+	int retval;
+	struct mapping_table_entry mapping_data[1];
+
+	mutex_lock(&rmi_smb->mappingtable_mutex);
+	for (i = 0; i < RMI_SMB2_MAP_SIZE; i++) {
+		if (rmi_smb->mapping_table[i].rmiaddr == rmiaddr) {
+			if (isread) {
+				if (rmi_smb->mapping_table[i].readcount
+							== bytecount) {
+					*commandcode = i;
+					retval = 0;
+					goto exit;
+				}
+			} else {
+				if (rmi_smb->mapping_table[i].flags &
+							RMI_SMB2_MAP_FLAGS_WE) {
+					*commandcode = i;
+					retval = 0;
+					goto exit;
+				}
+			}
+		}
+	}
+	i = rmi_smb->table_index;
+	rmi_smb->table_index = (i + 1) % RMI_SMB2_MAP_SIZE;
+
+	/* constructs mapping table data entry. 4 bytes each entry */
+	memset(mapping_data, 0, sizeof(mapping_data));
+
+	mapping_data[0].rmiaddr = cpu_to_le16(rmiaddr);
+	mapping_data[0].readcount = bytecount;
+	mapping_data[0].flags = !isread ? RMI_SMB2_MAP_FLAGS_WE : 0;
+
+	retval = smb_block_write(xport, i + 0x80, mapping_data,
+				 sizeof(mapping_data));
+
+	if (retval < 0) {
+		/*
+		 * if not written to device mapping table
+		 * clear the driver mapping table records
+		 */
+		rmi_smb->mapping_table[i].rmiaddr = 0x0000;
+		rmi_smb->mapping_table[i].readcount = 0;
+		rmi_smb->mapping_table[i].flags = 0;
+		goto exit;
+	}
+	/* save to the driver level mapping table */
+	rmi_smb->mapping_table[i].rmiaddr = rmiaddr;
+	rmi_smb->mapping_table[i].readcount = bytecount;
+	rmi_smb->mapping_table[i].flags = !isread ? RMI_SMB2_MAP_FLAGS_WE : 0;
+	*commandcode = i;
+
+exit:
+	mutex_unlock(&rmi_smb->mappingtable_mutex);
+
+	return retval;
+}
+
+static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
+				const void *databuff, size_t len)
+{
+	int retval = 0;
+	u8 commandcode;
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+	int cur_len = (int)len;
+
+	mutex_lock(&rmi_smb->page_mutex);
+
+	while (cur_len > 0) {
+		/*
+		 * break into 32 bytes chunks to write get command code
+		 */
+		int block_len = min_t(int, len, SMB_MAX_COUNT);
+
+		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
+						  false, &commandcode);
+		if (retval < 0)
+			goto exit;
+
+		retval = smb_block_write(xport, commandcode,
+					 databuff, block_len);
+		if (retval < 0)
+			goto exit;
+
+		/* prepare to write next block of bytes */
+		cur_len -= SMB_MAX_COUNT;
+		databuff += SMB_MAX_COUNT;
+		rmiaddr += SMB_MAX_COUNT;
+	}
+exit:
+	mutex_unlock(&rmi_smb->page_mutex);
+	return retval;
+}
+
+/* SMB block read - wrapper over ic2_smb_read_block */
+static int smb_block_read(struct rmi_transport_dev *xport,
+			     u8 commandcode, void *buf, size_t len)
+{
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+	struct i2c_client *client = rmi_smb->client;
+	int retval;
+
+	retval = i2c_smbus_read_block_data(client, commandcode, buf);
+	if (retval < 0)
+		return retval;
+
+	return retval;
+}
+
+static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
+			      void *databuff, size_t len)
+{
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+	int retval;
+	u8 commandcode;
+	int cur_len = (int)len;
+
+	mutex_lock(&rmi_smb->page_mutex);
+	memset(databuff, 0, len);
+
+	while (cur_len > 0) {
+		/* break into 32 bytes chunks to write get command code */
+		int block_len =  min_t(int, cur_len, SMB_MAX_COUNT);
+
+		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
+						  true, &commandcode);
+		if (retval < 0)
+			goto exit;
+
+		retval = smb_block_read(xport, commandcode,
+					databuff, block_len);
+		if (retval < 0)
+			goto exit;
+
+		/* prepare to read next block of bytes */
+		cur_len -= SMB_MAX_COUNT;
+		databuff += SMB_MAX_COUNT;
+		rmiaddr += SMB_MAX_COUNT;
+	}
+
+	retval = 0;
+
+exit:
+	mutex_unlock(&rmi_smb->page_mutex);
+	return retval;
+}
+
+static void rmi_smb_clear_state(struct rmi_smb_xport *rmi_smb)
+{
+	/* the mapping table has been flushed, discard the current one */
+	mutex_lock(&rmi_smb->mappingtable_mutex);
+	memset(rmi_smb->mapping_table, 0, sizeof(rmi_smb->mapping_table));
+	mutex_unlock(&rmi_smb->mappingtable_mutex);
+}
+
+static int rmi_smb_enable_smbus_mode(struct rmi_smb_xport *rmi_smb)
+{
+	int retval;
+
+	/* we need to get the smbus version to activate the touchpad */
+	retval = rmi_smb_get_version(rmi_smb);
+	if (retval < 0)
+		return retval;
+
+	return 0;
+}
+
+static int rmi_smb_reset(struct rmi_transport_dev *xport, u16 reset_addr)
+{
+	struct rmi_smb_xport *rmi_smb =
+		container_of(xport, struct rmi_smb_xport, xport);
+
+	rmi_smb_clear_state(rmi_smb);
+
+	/*
+	 * we do not call the actual reset command, it has to be handled in
+	 * PS/2 or there will be races between PS/2 and SMBus.
+	 * PS/2 should ensure that a psmouse_reset is called before
+	 * intializing the device and after it has been removed to be in a known
+	 * state.
+	 */
+	return rmi_smb_enable_smbus_mode(rmi_smb);
+}
+
+static const struct rmi_transport_ops rmi_smb_ops = {
+	.write_block	= rmi_smb_write_block,
+	.read_block	= rmi_smb_read_block,
+	.reset		= rmi_smb_reset,
+};
+
+static void rmi_smb_alert(struct i2c_client *client,
+			  enum i2c_alert_protocol type, unsigned int data)
+{
+	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
+	struct rmi_device *rmi_dev = rmi_smb->xport.rmi_dev;
+	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
+
+	if (type != I2C_PROTOCOL_SMBUS_HOST_NOTIFY)
+		return;
+
+	if (!drv_data) {
+		dev_err(&client->dev,
+			"Something went wrong, driver data is NULL.\n");
+		return;
+	}
+
+	rmi_process_interrupt_requests(rmi_dev);
+}
+
+static int rmi_smb_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
+	struct rmi_smb_xport *rmi_smb;
+	int retval;
+	int smbus_version;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+				     I2C_FUNC_SMBUS_HOST_NOTIFY)) {
+		dev_err(&client->dev,
+			"adapter does not support required functionality.\n");
+		return -ENODEV;
+	}
+
+	rmi_smb = devm_kzalloc(&client->dev, sizeof(struct rmi_smb_xport),
+				GFP_KERNEL);
+	if (!rmi_smb)
+		return -ENOMEM;
+
+	if (!pdata) {
+		dev_err(&client->dev, "no platform data, aborting\n");
+		return -ENOMEM;
+	}
+
+	rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Probing %s.\n",
+		dev_name(&client->dev));
+
+	rmi_smb->client = client;
+	mutex_init(&rmi_smb->page_mutex);
+	mutex_init(&rmi_smb->mappingtable_mutex);
+
+	rmi_smb->xport.dev = &client->dev;
+	rmi_smb->xport.pdata = *pdata;
+	rmi_smb->xport.proto_name = "smb2";
+	rmi_smb->xport.ops = &rmi_smb_ops;
+
+	retval = rmi_smb_get_version(rmi_smb);
+	if (retval < 0)
+		return retval;
+
+	smbus_version = retval;
+	rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Smbus version is %d",
+		smbus_version);
+
+	if (smbus_version != 2) {
+		dev_err(&client->dev, "Unrecognized SMB version %d.\n",
+				smbus_version);
+		return -ENODEV;
+	}
+
+	i2c_set_clientdata(client, rmi_smb);
+
+	retval = rmi_register_transport_device(&rmi_smb->xport);
+	if (retval) {
+		dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
+			client->addr);
+		i2c_set_clientdata(client, NULL);
+		return retval;
+	}
+
+	dev_info(&client->dev, "registered rmi smb driver at %#04x.\n",
+			client->addr);
+	return 0;
+
+}
+
+static int rmi_smb_remove(struct i2c_client *client)
+{
+	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
+
+	rmi_unregister_transport_device(&rmi_smb->xport);
+
+	return 0;
+}
+
+static int __maybe_unused rmi_smb_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
+	int ret;
+
+	ret = rmi_driver_suspend(rmi_smb->xport.rmi_dev);
+	if (ret)
+		dev_warn(dev, "Failed to suspend device: %d\n", ret);
+
+	return ret;
+}
+
+static int __maybe_unused rmi_smb_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
+	struct rmi_device *rmi_dev = rmi_smb->xport.rmi_dev;
+	int ret;
+
+	rmi_smb_reset(&rmi_smb->xport, 0);
+
+	rmi_reset(rmi_dev);
+
+	ret = rmi_driver_resume(rmi_smb->xport.rmi_dev);
+	if (ret)
+		dev_warn(dev, "Failed to resume device: %d\n", ret);
+
+	return 0;
+}
+
+static int __maybe_unused rmi_smb_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
+	int ret;
+
+	ret = rmi_driver_resume(rmi_smb->xport.rmi_dev);
+	if (ret)
+		dev_warn(dev, "Failed to resume device: %d\n", ret);
+
+	return 0;
+}
+
+static const struct dev_pm_ops rmi_smb_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(rmi_smb_suspend, rmi_smb_resume)
+	SET_RUNTIME_PM_OPS(rmi_smb_suspend, rmi_smb_runtime_resume,
+			   NULL)
+};
+
+static const struct i2c_device_id rmi_id[] = {
+	{ "rmi4_smbus", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, rmi_id);
+
+static struct i2c_driver rmi_smb_driver = {
+	.driver = {
+		.owner	= THIS_MODULE,
+		.name	= "rmi4_smbus",
+		.pm	= &rmi_smb_pm,
+	},
+	.id_table	= rmi_id,
+	.probe		= rmi_smb_probe,
+	.remove		= rmi_smb_remove,
+	.alert		= rmi_smb_alert,
+};
+
+module_i2c_driver(rmi_smb_driver);
+
+MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com>");
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@redhat.com>");
+MODULE_DESCRIPTION("RMI4 SMBus driver");
+MODULE_LICENSE("GPL");
-- 
2.5.5

[PATCH 02/11] Input: serio - store the pt_buttons in the struct serio directly

[Benjamin Tissoires]

With RMI4 over SMBus, the pass-through device can be instantiated
in a SMBus driver. However, compared to the psmouse-synaptics driver,
this pass-through PS/2 driver has no clue whether the current
serio_interrupt() is the beginning of the frame or not. Instead of
adding a protocol analysis in RMI4 function F03, we can add an extra
byte in struct serio to handle the extra data we want to append to the
first byte.

Convert the psmouse-synaptics device to use it too.

Partially reverts cdd9dc1 ("Input: synaptics - re-route tracksticks
buttons on the Lenovo 2015 series")

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/psmouse-base.c |  3 +++
 drivers/input/mouse/synaptics.c    | 19 ++++++++++---------
 drivers/input/mouse/synaptics.h    |  1 -
 include/linux/serio.h              |  8 ++++++++
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 5784e20..dbc002a 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -365,6 +365,9 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
 		goto out;
 	}
 
+	if (psmouse->pktcnt == 1)
+		psmouse->packet[0] |= serio->extra_byte;
+
 	psmouse->last = jiffies;
 	psmouse_handle_byte(psmouse);
 
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index a41d832..8781e23 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -597,15 +597,13 @@ static int synaptics_is_pt_packet(unsigned char *buf)
 	return (buf[0] & 0xFC) == 0x84 && (buf[3] & 0xCC) == 0xC4;
 }
 
-static void synaptics_pass_pt_packet(struct psmouse *psmouse,
-				     struct serio *ptport,
+static void synaptics_pass_pt_packet(struct serio *ptport,
 				     unsigned char *packet)
 {
-	struct synaptics_data *priv = psmouse->private;
 	struct psmouse *child = serio_get_drvdata(ptport);
 
 	if (child && child->state == PSMOUSE_ACTIVATED) {
-		serio_interrupt(ptport, packet[1] | priv->pt_buttons, 0);
+		serio_interrupt(ptport, packet[1], 0);
 		serio_interrupt(ptport, packet[4], 0);
 		serio_interrupt(ptport, packet[5], 0);
 		if (child->pktsize == 4)
@@ -857,6 +855,7 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
 	struct synaptics_data *priv = psmouse->private;
 	int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1;
 	char buf[6] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+	int pt_buttons;
 	int i;
 
 	if (!SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap))
@@ -887,11 +886,13 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
 		return;
 
 	/* The trackstick expects at most 3 buttons */
-	priv->pt_buttons = SYN_CAP_EXT_BUTTON_STICK_L(hw->ext_buttons)      |
-			   SYN_CAP_EXT_BUTTON_STICK_R(hw->ext_buttons) << 1 |
-			   SYN_CAP_EXT_BUTTON_STICK_M(hw->ext_buttons) << 2;
+	pt_buttons = SYN_CAP_EXT_BUTTON_STICK_L(hw->ext_buttons)      |
+		     SYN_CAP_EXT_BUTTON_STICK_R(hw->ext_buttons) << 1 |
+		     SYN_CAP_EXT_BUTTON_STICK_M(hw->ext_buttons) << 2;
+
+	priv->pt_port->extra_byte = pt_buttons;
 
-	synaptics_pass_pt_packet(psmouse, priv->pt_port, buf);
+	synaptics_pass_pt_packet(priv->pt_port, buf);
 }
 
 static void synaptics_report_buttons(struct psmouse *psmouse,
@@ -1132,7 +1133,7 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
 		if (SYN_CAP_PASS_THROUGH(priv->capabilities) &&
 		    synaptics_is_pt_packet(psmouse->packet)) {
 			if (priv->pt_port)
-				synaptics_pass_pt_packet(psmouse, priv->pt_port,
+				synaptics_pass_pt_packet(priv->pt_port,
 							 psmouse->packet);
 		} else
 			synaptics_process_packet(psmouse);
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 56faa7e..116ae25 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -183,7 +183,6 @@ struct synaptics_data {
 	bool disable_gesture;			/* disable gestures */
 
 	struct serio *pt_port;			/* Pass-through serio port */
-	unsigned char pt_buttons;		/* Pass-through buttons */
 
 	/*
 	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet
diff --git a/include/linux/serio.h b/include/linux/serio.h
index c733cff..f3b75c8 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -64,6 +64,14 @@ struct serio {
 	 * may get indigestion when exposed to concurrent access (i8042).
 	 */
 	struct mutex *ps2_cmd_mutex;
+
+	/*
+	 * For use with Synaptics devices that have the trackstick buttons
+	 * not actually wired to the trackstick PS/2 device.
+	 * This byte will be OR-ed with the first byte of the incoming packet
+	 * that contains actual data (not commands).
+	 */
+	u8 extra_byte;
 };
 #define to_serio_port(d)	container_of(d, struct serio, dev)
 
-- 
2.5.5

[PATCH 03/11] Input: synaptics-rmi4 - have only one struct platform data

[Benjamin Tissoires]

If struct rmi_device_platform_data contains pointers to other struct,
it gets difficult to allocate a fixed size struct and copy it over between
drivers.

Change the pointers into a struct and change the code in rmi4 accordingly.

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

---

this patch will conflict with Andrew's patch to switch hid-rmi
to use rmi4_core...
---
 drivers/input/rmi4/rmi_f11.c | 4 ++--
 drivers/input/rmi4/rmi_f12.c | 4 ++--
 drivers/input/rmi4/rmi_f30.c | 7 +++----
 include/linux/rmi.h          | 4 ++--
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 20c7134..b14a7b6 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1063,8 +1063,8 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 		rc = rmi_2d_sensor_of_probe(&fn->dev, &f11->sensor_pdata);
 		if (rc)
 			return rc;
-	} else if (pdata->sensor_pdata) {
-		f11->sensor_pdata = *pdata->sensor_pdata;
+	} else {
+		f11->sensor_pdata = pdata->sensor_pdata;
 	}
 
 	f11->rezero_wait_ms = f11->sensor_pdata.rezero_wait;
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 332c02f..a631bed 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -274,8 +274,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
 		ret = rmi_2d_sensor_of_probe(&fn->dev, &f12->sensor_pdata);
 		if (ret)
 			return ret;
-	} else if (pdata->sensor_pdata) {
-		f12->sensor_pdata = *pdata->sensor_pdata;
+	} else {
+		f12->sensor_pdata = pdata->sensor_pdata;
 	}
 
 	ret = rmi_read_register_desc(rmi_dev, query_addr,
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 760aff1..7990bb0 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -192,7 +192,7 @@ static int rmi_f30_config(struct rmi_function *fn)
 				rmi_get_platform_data(fn->rmi_dev);
 	int error;
 
-	if (pdata->f30_data && pdata->f30_data->disable) {
+	if (pdata && pdata->f30_data.disable) {
 		drv->clear_irq_bits(fn->rmi_dev, fn->irq_mask);
 	} else {
 		/* Write Control Register values back to device */
@@ -362,8 +362,7 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
 				 * f30->has_mech_mouse_btns, but I am
 				 * not sure, so use only the pdata info
 				 */
-				if (pdata->f30_data &&
-				    pdata->f30_data->buttonpad)
+				if (pdata && pdata->f30_data.buttonpad)
 					break;
 			}
 		}
@@ -378,7 +377,7 @@ static int rmi_f30_probe(struct rmi_function *fn)
 	const struct rmi_device_platform_data *pdata =
 				rmi_get_platform_data(fn->rmi_dev);
 
-	if (pdata->f30_data && pdata->f30_data->disable)
+	if (pdata && pdata->f30_data.disable)
 		return 0;
 
 	rc = rmi_f30_initialize(fn);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index e0aca14..4a071e8 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -211,9 +211,9 @@ struct rmi_device_platform_data {
 	struct rmi_device_platform_data_spi spi_data;
 
 	/* function handler pdata */
-	struct rmi_2d_sensor_platform_data *sensor_pdata;
+	struct rmi_2d_sensor_platform_data sensor_pdata;
 	struct rmi_f01_power_management power_management;
-	struct rmi_f30_data *f30_data;
+	struct rmi_f30_data f30_data;
 };
 
 /**
-- 
2.5.5

[PATCH 04/11] Input: synaptics-rmi4 - add support for F03

[Benjamin Tissoires]

From: Lyude Paul <thatslyude@gmail.com>

This adds basic functionality for PS/2 passthrough on Synaptics
Touchpads using RMI4 through smbus.

Signed-off-by: Lyude Paul <thatslyude@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/psmouse-base.c |   6 +
 drivers/input/rmi4/Kconfig         |   9 ++
 drivers/input/rmi4/Makefile        |   1 +
 drivers/input/rmi4/rmi_bus.c       |   3 +
 drivers/input/rmi4/rmi_driver.h    |   1 +
 drivers/input/rmi4/rmi_f03.c       | 226 +++++++++++++++++++++++++++++++++++++
 include/uapi/linux/serio.h         |   1 +
 7 files changed, 247 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_f03.c

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index dbc002a..fa2d700 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1666,6 +1666,12 @@ static struct serio_device_id psmouse_serio_ids[] = {
 		.id	= SERIO_ANY,
 		.extra	= SERIO_ANY,
 	},
+	{
+		.type	= SERIO_RMI_PSTHRU,
+		.proto	= SERIO_ANY,
+		.id	= SERIO_ANY,
+		.extra	= SERIO_ANY,
+	},
 	{ 0 }
 };
 
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 86a180b..cfc14b3 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -39,6 +39,15 @@ config RMI4_SMB
 	  To compile this driver as a module, choose M here: the module will be
 	  called rmi_smbus.
 
+config RMI4_F03
+        bool "RMI4 Function 03 (PS2 Guest)"
+        depends on RMI4_CORE
+        help
+          Say Y here if you want to add support for RMI4 function 03.
+
+          Function 03 provides PS2 guest support for RMI4 devices. This
+          includes support for TrackPoints on TouchPads.
+
 config RMI4_2D_SENSOR
 	bool
 	depends on RMI4_CORE
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 3c8ebf2..676e636 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
 rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
 
 # Function drivers
+rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
 rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
 rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
 rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index a735806..e1e3b80 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -303,6 +303,9 @@ struct bus_type rmi_bus_type = {
 
 static struct rmi_function_handler *fn_handlers[] = {
 	&rmi_f01_handler,
+#ifdef CONFIG_RMI4_F03
+	&rmi_f03_handler,
+#endif
 #ifdef CONFIG_RMI4_F11
 	&rmi_f11_handler,
 #endif
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 6e140fa..a7cb383 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -99,6 +99,7 @@ void rmi_unregister_physical_driver(void);
 char *rmi_f01_get_product_ID(struct rmi_function *fn);
 
 extern struct rmi_function_handler rmi_f01_handler;
+extern struct rmi_function_handler rmi_f03_handler;
 extern struct rmi_function_handler rmi_f11_handler;
 extern struct rmi_function_handler rmi_f12_handler;
 extern struct rmi_function_handler rmi_f30_handler;
diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
new file mode 100644
index 0000000..9945512
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -0,0 +1,226 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat
+ * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
+ *
+ * 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/slab.h>
+#include <linux/serio.h>
+#include <linux/notifier.h>
+#include "rmi_driver.h"
+
+#define RMI_F03_RX_DATA_OFB		0x01
+#define RMI_F03_OB_SIZE			2
+
+#define RMI_F03_OB_OFFSET		2
+#define RMI_F03_OB_DATA_OFFSET		1
+#define RMI_F03_OB_FLAG_TIMEOUT		(1 << 6)
+#define RMI_F03_OB_FLAG_PARITY		(1 << 7)
+
+#define RMI_F03_DEVICE_COUNT		0x07
+#define RMI_F03_BYTES_PER_DEVICE_MASK	0x70
+#define RMI_F03_BYTES_PER_DEVICE_SHIFT	4
+#define RMI_F03_QUEUE_LENGTH		0x0F
+
+struct f03_data {
+	struct rmi_function *fn;
+
+	struct serio *serio;
+
+	unsigned int overwrite_buttons;
+
+	u8 device_count;
+	u8 rx_queue_length;
+};
+
+static int rmi_f03_pt_write(struct serio *id, unsigned char val)
+{
+	struct f03_data *f03 = id->port_data;
+	int rc;
+
+	dev_dbg(&f03->fn->dev, "%s: Wrote %.2hhx to PS/2 passthrough address",
+		__func__, val);
+
+	rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);
+	if (rc) {
+		dev_err(&f03->fn->dev,
+			"%s: Failed to write to F03 TX register.\n", __func__);
+		return rc;
+	}
+
+	return 0;
+}
+
+static inline int rmi_f03_initialize(struct rmi_function *fn)
+{
+	struct f03_data *f03;
+	struct device *dev = &fn->dev;
+	int rc;
+	u8 bytes_per_device;
+	u8 query1;
+	size_t query2_len;
+
+	rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1);
+	if (rc) {
+		dev_err(dev, "Failed to read query register.\n");
+		return rc;
+	}
+
+	f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL);
+	if (!f03)
+		return -ENOMEM;
+
+	f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
+	bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >>
+		RMI_F03_BYTES_PER_DEVICE_SHIFT;
+
+	query2_len = f03->device_count * bytes_per_device;
+
+	/*
+	 * The first generation of image sensors don't have a second part to
+	 * their f03 query, as such we have to set some of these values manually
+	 */
+	if (query2_len < 1) {
+		f03->device_count = 1;
+		f03->rx_queue_length = 7;
+	} else {
+		u8 query2[query2_len];
+
+		rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1,
+				    query2, query2_len);
+		if (rc) {
+			dev_err(dev, "Failed to read second set of query registers.\n");
+			return rc;
+		}
+
+		f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH;
+	}
+
+	f03->fn = fn;
+
+	dev_set_drvdata(dev, f03);
+
+	return f03->device_count;
+}
+
+static inline int rmi_f03_register_pt(struct rmi_function *fn)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+
+	if (!serio)
+		return -ENOMEM;
+
+	serio->id.type = SERIO_RMI_PSTHRU;
+	serio->write = rmi_f03_pt_write;
+	serio->port_data = f03;
+
+	strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through",
+		sizeof(serio->name));
+	strlcpy(serio->phys, "synaptics-rmi4-pt/serio1",
+		sizeof(serio->phys));
+	 serio->dev.parent = &fn->dev;
+
+	f03->serio = serio;
+
+	serio_register_port(serio);
+
+	return 0;
+}
+
+static int rmi_f03_probe(struct rmi_function *fn)
+{
+	int rc;
+
+	rc = rmi_f03_initialize(fn);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(&fn->dev, "%d devices on PS/2 passthrough", rc);
+
+	rc = rmi_f03_register_pt(fn);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int rmi_f03_config(struct rmi_function *fn)
+{
+	fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+	return 0;
+}
+
+static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	u16 data_addr = fn->fd.data_base_addr;
+	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
+	u8 obs[ob_len];
+	u8 ob_status;
+	u8 ob_data;
+	unsigned int serio_flags;
+	int i;
+	int retval;
+
+	/* Grab all of the data registers, and check them for data */
+	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
+				&obs, ob_len);
+	if (retval) {
+		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
+			__func__);
+		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
+		return retval;
+	}
+
+	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
+		ob_status = obs[i];
+		ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
+		serio_flags = 0;
+
+		if (!(ob_status & RMI_F03_RX_DATA_OFB))
+			continue;
+
+		if (ob_status & RMI_F03_OB_FLAG_TIMEOUT)
+			serio_flags |= SERIO_TIMEOUT;
+		if (ob_status & RMI_F03_OB_FLAG_PARITY)
+			serio_flags |= SERIO_PARITY;
+
+		dev_dbg(&fn->dev,
+			"%s: Received %.2hhx from PS2 guest T: %c P: %c\n",
+			__func__, ob_data,
+			serio_flags & SERIO_TIMEOUT ?  'Y' : 'N',
+			serio_flags & SERIO_PARITY ? 'Y' : 'N');
+
+		serio_interrupt(f03->serio, ob_data, serio_flags);
+	}
+
+	return 0;
+}
+
+static void rmi_f03_remove(struct rmi_function *fn)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+
+	serio_unregister_port(f03->serio);
+}
+
+struct rmi_function_handler rmi_f03_handler = {
+	.driver = {
+		.name = "rmi4_f03",
+	},
+	.func = 0x03,
+	.probe = rmi_f03_probe,
+	.config = rmi_f03_config,
+	.attention = rmi_f03_attention,
+	.remove = rmi_f03_remove,
+};
+
+MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("RMI F03 module");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
index f2447a8..7012178 100644
--- a/include/uapi/linux/serio.h
+++ b/include/uapi/linux/serio.h
@@ -30,6 +30,7 @@
 #define SERIO_HIL_MLC	0x03
 #define SERIO_PS_PSTHRU	0x05
 #define SERIO_8042_XL	0x06
+#define SERIO_RMI_PSTHRU	0x07
 
 /*
  * Serio protocols
-- 
2.5.5

[PATCH 05/11] Input: synaptics-rmi4 - f03: grab data passed by transport device

[Benjamin Tissoires]

From: Dennis Wassenberg <dennis.wassenberg@secunet.com>

First check if there are data available passed by the transport device.
If data available use these data. If there are no data available
try to read the rmi block if dsata are passed this way.

This is the way the other rmi function handlers will do this.
That why apply this to f03 as well.

This will fix corrupted or missing data issues.

Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f03.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
index 9945512..daae1c95 100644
--- a/drivers/input/rmi4/rmi_f03.c
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -158,6 +158,7 @@ static int rmi_f03_config(struct rmi_function *fn)
 
 static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 {
+	struct rmi_device *rmi_dev = fn->rmi_dev;
 	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
 	u16 data_addr = fn->fd.data_base_addr;
 	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
@@ -166,16 +167,32 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	u8 ob_data;
 	unsigned int serio_flags;
 	int i;
-	int retval;
-
-	/* Grab all of the data registers, and check them for data */
-	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
-				&obs, ob_len);
-	if (retval) {
-		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
-			__func__);
-		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
-		return retval;
+	int ret;
+
+	if (!rmi_dev || !rmi_dev->xport)
+		return -ENODEV;
+
+	if (rmi_dev->xport->attn_data) {
+		/* First grab the data passed by the transport device */
+		if (rmi_dev->xport->attn_size < ob_len) {
+			dev_warn(&fn->dev, "F03 interrupted, but data is missing!\n");
+			return 0;
+		}
+
+		memcpy(obs, rmi_dev->xport->attn_data, ob_len);
+
+		rmi_dev->xport->attn_data += ob_len;
+		rmi_dev->xport->attn_size -= ob_len;
+	} else {
+		/* Grab all of the data registers, and check them for data */
+		ret = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
+				     &obs, ob_len);
+		if (ret) {
+			dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
+				__func__);
+			serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
+			return ret;
+		}
 	}
 
 	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
-- 
2.5.5

[PATCH 06/11] Input: synaptics-rmi4 - Add rmi_find_function()

[Benjamin Tissoires]

If a function needs to communicate with an other, it's better to have
a way to retrieve this other.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 13 +++++++++++++
 drivers/input/rmi4/rmi_driver.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index faa295e..304f142 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -181,6 +181,19 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 }
 EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
 
+struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
+	struct rmi_function *entry;
+
+	list_for_each_entry(entry, &data->function_list, node) {
+		if (entry->fd.function_number == number)
+			return entry;
+	}
+
+	return NULL;
+}
+
 static int suspend_one_function(struct rmi_function *fn)
 {
 	struct rmi_function_handler *fh;
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index a7cb383..e4be773 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -95,6 +95,7 @@ bool rmi_register_desc_has_subpacket(const struct rmi_register_desc_item *item,
 bool rmi_is_physical_driver(struct device_driver *);
 int rmi_register_physical_driver(void);
 void rmi_unregister_physical_driver(void);
+struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
 
 char *rmi_f01_get_product_ID(struct rmi_function *fn);
 
-- 
2.5.5

[PATCH 07/11] Input: synaptics-rmi4 - f30/f03: Forward mechanical buttons on buttonpads to PS/2 guest

[Benjamin Tissoires]

From: Lyude Paul <thatslyude@gmail.com>

On the latest series of ThinkPads, the button events for the TrackPoint
are reported through the touchpad itself as opposed to the TrackPoint
device. In order to report these buttons properly, we need to forward
them to the TrackPoint device and send the button presses/releases
through there instead.

Signed-off-by: Lyude Paul <thatslyude@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.h | 13 +++++++++
 drivers/input/rmi4/rmi_f03.c    | 28 ++++++++++++++++++
 drivers/input/rmi4/rmi_f30.c    | 64 +++++++++++++++++++++++++++++++++++------
 3 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e4be773..a0b1978 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -99,6 +99,19 @@ struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
 
 char *rmi_f01_get_product_ID(struct rmi_function *fn);
 
+#ifdef CONFIG_RMI4_F03
+int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
+			     int value);
+void rmi_f03_commit_buttons(struct rmi_function *fn);
+#else
+static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
+					   unsigned int button, int value)
+{
+	return 0;
+}
+static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
+#endif
+
 extern struct rmi_function_handler rmi_f01_handler;
 extern struct rmi_function_handler rmi_f03_handler;
 extern struct rmi_function_handler rmi_f11_handler;
diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
index daae1c95..535f426 100644
--- a/drivers/input/rmi4/rmi_f03.c
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -37,6 +37,34 @@ struct f03_data {
 	u8 rx_queue_length;
 };
 
+int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
+			     int value)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	unsigned int bit = BIT(button);
+
+	if (button > 2)
+		return -EINVAL;
+
+	if (value)
+		f03->overwrite_buttons |= bit;
+	else
+		f03->overwrite_buttons &= ~bit;
+
+	return 0;
+}
+
+void rmi_f03_commit_buttons(struct rmi_function *fn)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	int i;
+
+	f03->serio->extra_byte = f03->overwrite_buttons;
+
+	for (i = 0; i < 3; i++)
+		serio_interrupt(f03->serio, 0x00, 0x00);
+}
+
 static int rmi_f03_pt_write(struct serio *id, unsigned char val)
 {
 	struct f03_data *f03 = id->port_data;
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 7990bb0..14e3221 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -74,8 +74,11 @@ struct f30_data {
 
 	u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
 	u16 *gpioled_key_map;
+	u16 *gpio_passthrough_key_map;
 
 	struct input_dev *input;
+	bool trackstick_buttons;
+	struct rmi_function *f03;
 };
 
 static int rmi_f30_read_control_parameters(struct rmi_function *fn,
@@ -108,6 +111,13 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	if (!f30->input)
 		return 0;
 
+	if (f30->trackstick_buttons && !f30->f03) {
+		f30->f03 = rmi_find_function(rmi_dev, 3);
+
+		if (!f30->f03)
+			return -EBUSY;
+	}
+
 	/* Read the gpi led data. */
 	if (rmi_dev->xport->attn_data) {
 		memcpy(f30->data_regs, rmi_dev->xport->attn_data,
@@ -128,23 +138,29 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	for (reg_num = 0; reg_num < f30->register_count; ++reg_num) {
 		for (i = 0; gpiled < f30->gpioled_count && i < 8; ++i,
 			++gpiled) {
-			if (f30->gpioled_key_map[gpiled] != 0) {
-				/* buttons have pull up resistors */
-				value = (((f30->data_regs[reg_num] >> i) & 0x01)
-									== 0);
+			/* buttons have pull up resistors */
+			value = (((f30->data_regs[reg_num] >> i) & 0x01) == 0);
 
+			if (f30->gpioled_key_map[gpiled] != 0) {
 				rmi_dbg(RMI_DEBUG_FN, &fn->dev,
 					"%s: call input report key (0x%04x) value (0x%02x)",
 					__func__,
 					f30->gpioled_key_map[gpiled], value);
+
 				input_report_key(f30->input,
 						 f30->gpioled_key_map[gpiled],
 						 value);
+			} else if (f30->gpio_passthrough_key_map[gpiled]) {
+				rmi_f03_overwrite_button(f30->f03,
+						f30->gpio_passthrough_key_map[gpiled] - BTN_LEFT,
+						value);
 			}
-
 		}
 	}
 
+	if (f30->trackstick_buttons)
+		rmi_f03_commit_buttons(f30->f03);
+
 	return 0;
 }
 
@@ -242,10 +258,10 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
 	int retval = 0;
 	int control_address;
 	int i;
-	int button;
+	int button, extra_button;
 	u8 buf[RMI_F30_QUERY_SIZE];
 	u8 *ctrl_reg;
-	u8 *map_memory;
+	u8 *map_memory, *pt_memory;
 
 	f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
 			   GFP_KERNEL);
@@ -343,15 +359,47 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
 	map_memory = devm_kzalloc(&fn->dev,
 				  (f30->gpioled_count * (sizeof(u16))),
 				  GFP_KERNEL);
-	if (!map_memory) {
+	pt_memory = devm_kzalloc(&fn->dev,
+				 (f30->gpioled_count * (sizeof(u16))),
+				 GFP_KERNEL);
+	if (!map_memory || !pt_memory) {
 		dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
 		return -ENOMEM;
 	}
 
 	f30->gpioled_key_map = (u16 *)map_memory;
+	f30->gpio_passthrough_key_map = (u16 *)pt_memory;
 
 	pdata = rmi_get_platform_data(rmi_dev);
 	if (pdata && f30->has_gpio) {
+		/*
+		 * For touchpads the buttons are mapped as:
+		 * - bit 0 = Left, bit 1 = right, bit 2 = middle / clickbutton
+		 * - 3, 4, 5 are extended buttons and
+		 * - 6 and 7 are other sorts of GPIOs
+		 */
+		button = BTN_LEFT;
+		extra_button = BTN_LEFT;
+		for (i = 0; i < f30->gpioled_count - 2 && i < 3; i++) {
+			if (rmi_f30_is_valid_button(i, f30->ctrl))
+				f30->gpioled_key_map[i] = button++;
+		}
+
+		f30->trackstick_buttons = pdata &&
+				pdata->f30_data.trackstick_buttons;
+
+		if (f30->trackstick_buttons) {
+			for (i = 3; i < f30->gpioled_count - 2; i++) {
+				if (rmi_f30_is_valid_button(i, f30->ctrl))
+					f30->gpio_passthrough_key_map[i] = extra_button++;
+			}
+		} else {
+			for (i = 3; i < f30->gpioled_count - 2; i++) {
+				if (rmi_f30_is_valid_button(i, f30->ctrl))
+					f30->gpioled_key_map[i] = button++;
+			}
+		}
+	} else if (f30->has_gpio) {
 		button = BTN_LEFT;
 		for (i = 0; i < f30->gpioled_count; i++) {
 			if (rmi_f30_is_valid_button(i, f30->ctrl)) {
-- 
2.5.5

[PATCH 08/11] 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>
---
 drivers/input/mouse/synaptics.c | 99 +++++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics.h |  4 ++
 2 files changed, 103 insertions(+)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8781e23..eba6358 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,80 @@ 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 = {
+			.trackstick_buttons =
+			  !!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10),
+		},
+	};
+
+	if (priv->intertouch)
+		return -EINVAL;
+
+	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);
+
+	priv->intertouch = 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;
+
+	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
  ****************************************************************************/
@@ -1305,6 +1396,11 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 		device_remove_file(&psmouse->ps2dev.serio->dev,
 				   &psmouse_attr_disable_gesture.dattr);
 
+	if (priv->intertouch) {
+		platform_device_unregister(priv->intertouch);
+		priv->intertouch = NULL;
+	}
+
 	synaptics_reset(psmouse);
 	kfree(priv);
 	psmouse->private = NULL;
@@ -1547,6 +1643,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..b88755d 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.
@@ -196,6 +197,9 @@ struct synaptics_data {
 	bool					press;
 	bool					report_press;
 	bool					is_forcepad;
+
+	/* Intertouch handling */
+	struct platform_device *intertouch;
 };
 
 void synaptics_module_init(void);
-- 
2.5.5

[PATCH 09/11] Input: synaptics-rmi4 - add rmi_platform

[Benjamin Tissoires]

This driver is a glue between PS/2 devices that enumerate
the RMI4 device and the RMI4 SMBus driver.

We mostly 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>
---
 drivers/input/rmi4/Kconfig        |  12 ++
 drivers/input/rmi4/Makefile       |   1 +
 drivers/input/rmi4/rmi_platform.c | 235 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 248 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_platform.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index cfc14b3..2f11157 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -30,6 +30,7 @@ config RMI4_SPI
 config RMI4_SMB
 	tristate "RMI4 SMB Support"
 	depends on RMI4_CORE && I2C
+	select RMI4_PLATFORM
 	help
 	  Say Y here if you want to support RMI4 devices connected to an SMB
 	  bus.
@@ -39,6 +40,17 @@ config RMI4_SMB
 	  To compile this driver as a module, choose M here: the module will be
 	  called rmi_smbus.
 
+config RMI4_PLATFORM
+	tristate "RMI4 Platform Support"
+	depends on RMI4_CORE && RMI4_SMB && I2C
+	help
+	  Say Y here if you want to support RMI4 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 rmi_platform.
+
 config RMI4_F03
         bool "RMI4 Function 03 (PS2 Guest)"
         depends on RMI4_CORE
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 676e636..23af592 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -13,3 +13,4 @@ rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
 obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
 obj-$(CONFIG_RMI4_SPI) += rmi_spi.o
 obj-$(CONFIG_RMI4_SMB) += rmi_smbus.o
+obj-$(CONFIG_RMI4_PLATFORM) += rmi_platform.o
diff --git a/drivers/input/rmi4/rmi_platform.c b/drivers/input/rmi4/rmi_platform.c
new file mode 100644
index 0000000..8cd11d9
--- /dev/null
+++ b/drivers/input/rmi4/rmi_platform.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright (c) 2016 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/platform_device.h>
+#include <linux/rmi.h>
+#include <linux/slab.h>
+
+#define DRIVER_DESC	"RMI4 Platform PS/2 - SMBus bridge driver"
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@redhat.com>");
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+static struct workqueue_struct *krmi_wq;
+DEFINE_MUTEX(rmi_mutex);
+
+struct rmi_pltf {
+	struct i2c_client *smbus_client;
+	struct notifier_block i2c_notifier;
+	struct rmi_device_platform_data *pdata;
+};
+
+enum rmi_event_type {
+	RMI_REGISTER_DEVICE,
+	RMI_UNREGISTER_DEVICE,
+};
+
+struct rmi_work {
+	struct work_struct work;
+	enum rmi_event_type type;
+	struct rmi_pltf *rmi;
+	struct i2c_adapter *adap;
+};
+
+static void rmi_create_intertouch(struct rmi_pltf *rmi_pltf,
+				  struct i2c_adapter *adap)
+{
+	const struct i2c_board_info i2c_info = {
+		I2C_BOARD_INFO("rmi4_smbus", 0x2c),
+		.platform_data = rmi_pltf->pdata,
+	};
+
+	rmi_pltf->smbus_client = i2c_new_device(adap, &i2c_info);
+}
+
+static void rmi_worker(struct work_struct *work)
+{
+	struct rmi_work *rmi_work = container_of(work, struct rmi_work, work);
+
+	mutex_lock(&rmi_mutex);
+
+	switch (rmi_work->type) {
+	case RMI_REGISTER_DEVICE:
+		rmi_create_intertouch(rmi_work->rmi, rmi_work->adap);
+		break;
+	case RMI_UNREGISTER_DEVICE:
+		if (rmi_work->rmi->smbus_client)
+			i2c_unregister_device(rmi_work->rmi->smbus_client);
+		break;
+	};
+
+	kfree(rmi_work);
+
+	mutex_unlock(&rmi_mutex);
+}
+
+static int rmi_schedule_work(enum rmi_event_type type,
+			     struct rmi_pltf *rmi,
+			     struct i2c_adapter *adap)
+{
+	struct rmi_work *rmi_work = kzalloc(sizeof(*rmi_work), GFP_KERNEL);
+
+	if (!rmi_work)
+		return -ENOMEM;
+
+	rmi_work->type = type;
+	rmi_work->rmi = rmi;
+	rmi_work->adap = adap;
+
+	INIT_WORK(&rmi_work->work, rmi_worker);
+
+	queue_work(krmi_wq, &rmi_work->work);
+
+	return 0;
+}
+
+static int rmi_attach_i2c_device(struct device *dev, void *data)
+{
+	struct rmi_pltf *rmi_pltf = 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 (rmi_pltf->smbus_client)
+		return 0;
+
+	rmi_schedule_work(RMI_REGISTER_DEVICE, rmi_pltf, adap);
+
+	pr_debug("rmi_platform: adapter [%s] registered\n", adap->name);
+	return 0;
+}
+
+static int rmi_detach_i2c_device(struct device *dev, struct rmi_pltf *rmi_pltf)
+{
+	struct i2c_client *client;
+
+	if (dev->type == &i2c_adapter_type)
+		return 0;
+
+	mutex_lock(&rmi_mutex);
+
+	client = to_i2c_client(dev);
+	if (client == rmi_pltf->smbus_client)
+		rmi_pltf->smbus_client = NULL;
+
+	mutex_unlock(&rmi_mutex);
+
+	pr_debug("rmi_platform: client [%s] unregistered\n", client->name);
+	return 0;
+}
+
+static int rmi_notifier_call(struct notifier_block *nb,
+				   unsigned long action, void *data)
+{
+	struct device *dev = data;
+	struct rmi_pltf *rmi_pltf;
+
+	rmi_pltf = container_of(nb, struct rmi_pltf, i2c_notifier);
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		return rmi_attach_i2c_device(dev, rmi_pltf);
+	case BUS_NOTIFY_DEL_DEVICE:
+		return rmi_detach_i2c_device(dev, rmi_pltf);
+	}
+
+	return 0;
+}
+
+static int rmi_probe(struct platform_device *pdev)
+{
+	struct rmi_device_platform_data *pdata = pdev->dev.platform_data;
+	struct rmi_pltf *rmi_pltf;
+	int error;
+
+	rmi_pltf = devm_kzalloc(&pdev->dev, sizeof(struct rmi_pltf),
+				GFP_KERNEL);
+	if (!rmi_pltf)
+		return -ENOMEM;
+
+	rmi_pltf->i2c_notifier.notifier_call = rmi_notifier_call;
+
+	rmi_pltf->pdata = pdata;
+
+	/* Keep track of adapters which will be added or removed later */
+	error = bus_register_notifier(&i2c_bus_type, &rmi_pltf->i2c_notifier);
+	if (error)
+		return error;
+
+	/* Bind to already existing adapters right away */
+	i2c_for_each_dev(rmi_pltf, rmi_attach_i2c_device);
+
+	platform_set_drvdata(pdev, rmi_pltf);
+
+	return 0;
+}
+
+static int rmi_remove(struct platform_device *pdev)
+{
+	struct rmi_pltf *rmi_pltf = platform_get_drvdata(pdev);
+
+	bus_unregister_notifier(&i2c_bus_type, &rmi_pltf->i2c_notifier);
+
+	if (rmi_pltf->smbus_client)
+		rmi_schedule_work(RMI_UNREGISTER_DEVICE, rmi_pltf, NULL);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct platform_device_id rmi_id_table[] = {
+	{ .name = "rmi4" },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, rmi_id_table);
+
+static struct platform_driver rmi_drv = {
+	.driver		= {
+		.name	= "rmi4",
+	},
+	.probe		= rmi_probe,
+	.remove		= rmi_remove,
+	.id_table	= rmi_id_table,
+};
+
+static int __init rmi_init(void)
+{
+	int err;
+
+	krmi_wq = create_singlethread_workqueue("krmid");
+	if (!krmi_wq) {
+		pr_err("failed to create krmid workqueue\n");
+		return -ENOMEM;
+	}
+
+	err = platform_driver_register(&rmi_drv);
+	if (err)
+		destroy_workqueue(krmi_wq);
+
+	return err;
+}
+
+static void __exit rmi_exit(void)
+{
+	platform_driver_unregister(&rmi_drv);
+	destroy_workqueue(krmi_wq);
+}
+
+module_init(rmi_init);
+module_exit(rmi_exit);
-- 
2.5.5

[PATCH 10/11] Input: synaptics-rmi4 - smbus: call psmouse_deactivate before binding/resume

[Benjamin Tissoires]

The SMBus Synaptics devices enumerated as PS/2 devices have
the problem of being deaf to I2C if the touchpad has been
fully initialized over PS/2 (psmouse_activate being called).
A simple PS/2 deactivate command is enough to make it back alive.

To make sure the pass-through device does not interfere, we also
remove it from serio while using InterTouch.

Add a platform_data callback to reset the PS/2 device and
prevent it to be asynchronously re-enabled at resume.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/psmouse-base.c |  3 +++
 drivers/input/mouse/psmouse.h      |  1 +
 drivers/input/mouse/synaptics.c    | 32 ++++++++++++++++++++++++++++++++
 drivers/input/rmi4/rmi_smbus.c     | 30 ++++++++++++++++++++----------
 include/linux/rmi.h                | 13 +++++++++++++
 5 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index fa2d700..5d84e0c 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1607,6 +1607,9 @@ static int psmouse_reconnect(struct serio *serio)
 	unsigned char type;
 	int rc = -1;
 
+	if (psmouse->ignore_reconnect)
+		return 0;
+
 	mutex_lock(&psmouse_mutex);
 
 	if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index e0ca6cd..e9dc1a1 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -81,6 +81,7 @@ struct psmouse {
 
 	void (*pt_activate)(struct psmouse *psmouse);
 	void (*pt_deactivate)(struct psmouse *psmouse);
+	bool ignore_reconnect;
 };
 
 enum psmouse_type {
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index eba6358..e1a84a6 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -245,6 +245,30 @@ static const char * const smbus_pnp_ids[] = {
 
 static int rmi4_id;
 
+static void synaptics_pt_create(struct psmouse *psmouse);
+
+static int synaptics_intertouch_enable(void *data, bool value)
+{
+	struct psmouse *psmouse = data;
+	struct synaptics_data *priv = psmouse->private;
+
+	if (!psmouse)
+		return 0;
+
+	psmouse->ignore_reconnect = value;
+
+	if (value) {
+		serio_unregister_child_port(psmouse->ps2dev.serio);
+		psmouse_deactivate(psmouse);
+	} else {
+		psmouse_activate(psmouse);
+		if (SYN_CAP_PASS_THROUGH(priv->capabilities))
+			synaptics_pt_create(psmouse);
+	}
+
+	return 0;
+}
+
 static int synaptics_create_intertouch(struct psmouse *psmouse)
 {
 	struct synaptics_data *priv = psmouse->private;
@@ -261,6 +285,8 @@ static int synaptics_create_intertouch(struct psmouse *psmouse)
 			.trackstick_buttons =
 			  !!SYN_CAP_EXT_BUTTONS_STICK(priv->ext_cap_10),
 		},
+		.transport_enable = synaptics_intertouch_enable,
+		.transport_data = psmouse,
 	};
 
 	if (priv->intertouch)
@@ -1397,6 +1423,12 @@ static void synaptics_disconnect(struct psmouse *psmouse)
 				   &psmouse_attr_disable_gesture.dattr);
 
 	if (priv->intertouch) {
+		struct rmi_device_platform_data *pdata;
+
+		/* reset transport_data as it will get eventually freed */
+		pdata = priv->intertouch->dev.platform_data;
+		pdata->transport_data = NULL;
+
 		platform_device_unregister(priv->intertouch);
 		priv->intertouch = NULL;
 	}
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 4d6f228..023dbd5 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -244,8 +244,15 @@ static void rmi_smb_clear_state(struct rmi_smb_xport *rmi_smb)
 
 static int rmi_smb_enable_smbus_mode(struct rmi_smb_xport *rmi_smb)
 {
+	struct rmi_device_platform_data *pdata;
 	int retval;
 
+	pdata = dev_get_platdata(&rmi_smb->client->dev);
+
+	retval = rmi_transport_enable(pdata, true);
+	if (retval)
+		return retval;
+
 	/* we need to get the smbus version to activate the touchpad */
 	retval = rmi_smb_get_version(rmi_smb);
 	if (retval < 0)
@@ -261,13 +268,6 @@ static int rmi_smb_reset(struct rmi_transport_dev *xport, u16 reset_addr)
 
 	rmi_smb_clear_state(rmi_smb);
 
-	/*
-	 * we do not call the actual reset command, it has to be handled in
-	 * PS/2 or there will be races between PS/2 and SMBus.
-	 * PS/2 should ensure that a psmouse_reset is called before
-	 * intializing the device and after it has been removed to be in a known
-	 * state.
-	 */
 	return rmi_smb_enable_smbus_mode(rmi_smb);
 }
 
@@ -334,9 +334,13 @@ static int rmi_smb_probe(struct i2c_client *client,
 	rmi_smb->xport.proto_name = "smb2";
 	rmi_smb->xport.ops = &rmi_smb_ops;
 
+	retval = rmi_transport_enable(pdata, true);
+	if (retval)
+		return retval;
+
 	retval = rmi_smb_get_version(rmi_smb);
 	if (retval < 0)
-		return retval;
+		goto fail;
 
 	smbus_version = retval;
 	rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Smbus version is %d",
@@ -345,7 +349,8 @@ static int rmi_smb_probe(struct i2c_client *client,
 	if (smbus_version != 2) {
 		dev_err(&client->dev, "Unrecognized SMB version %d.\n",
 				smbus_version);
-		return -ENODEV;
+		retval = -ENODEV;
+		goto fail;
 	}
 
 	i2c_set_clientdata(client, rmi_smb);
@@ -355,20 +360,25 @@ static int rmi_smb_probe(struct i2c_client *client,
 		dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
 			client->addr);
 		i2c_set_clientdata(client, NULL);
-		return retval;
+		goto fail;
 	}
 
 	dev_info(&client->dev, "registered rmi smb driver at %#04x.\n",
 			client->addr);
 	return 0;
 
+fail:
+	rmi_transport_enable(pdata, false);
+	return retval;
 }
 
 static int rmi_smb_remove(struct i2c_client *client)
 {
+	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
 
 	rmi_unregister_transport_device(&rmi_smb->xport);
+	rmi_transport_enable(pdata, false);
 
 	return 0;
 }
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 4a071e8..02e1dae 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -214,6 +214,9 @@ struct rmi_device_platform_data {
 	struct rmi_2d_sensor_platform_data sensor_pdata;
 	struct rmi_f01_power_management power_management;
 	struct rmi_f30_data f30_data;
+
+	int (*transport_enable)(void*, bool);
+	void *transport_data;
 };
 
 /**
@@ -350,6 +353,16 @@ struct rmi_driver_data {
 	void *data;
 };
 
+static inline int rmi_transport_enable(struct rmi_device_platform_data *pdata,
+				       bool value)
+{
+	if (!pdata->transport_enable)
+		return 0;
+
+	return pdata->transport_enable(pdata->transport_data, value);
+}
+
+
 int rmi_register_transport_device(struct rmi_transport_dev *xport);
 void rmi_unregister_transport_device(struct rmi_transport_dev *xport);
 int rmi_process_interrupt_requests(struct rmi_device *rmi_dev);
-- 
2.5.5

[PATCH 11/11] Input: synaptics-rmi4 - smbus: on resume, try 3 times if init fails

[Benjamin Tissoires]

In some rare cases, we can't retrieve the SMBus version and so we fail
binding the touchpad back. Instead of leaving a touchpad dead, try again
to reinitialize it.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_smbus.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 023dbd5..a8e4af6 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -265,10 +265,29 @@ static int rmi_smb_reset(struct rmi_transport_dev *xport, u16 reset_addr)
 {
 	struct rmi_smb_xport *rmi_smb =
 		container_of(xport, struct rmi_smb_xport, xport);
+	struct i2c_client *client = rmi_smb->client;
+	struct rmi_device_platform_data *pdata;
+	int tries, ret;
 
 	rmi_smb_clear_state(rmi_smb);
 
-	return rmi_smb_enable_smbus_mode(rmi_smb);
+	for (tries = 3; tries > 0; tries--) {
+		ret = rmi_smb_enable_smbus_mode(rmi_smb);
+		if (!ret)
+			break;
+
+		/* we failed enabling SMBus, try again later */
+		msleep(500);
+	}
+
+	if (ret < 0) {
+		dev_warn(&client->dev,
+			 "failed to enable SMBus mode, giving up.\n");
+		pdata = dev_get_platdata(&rmi_smb->client->dev);
+		rmi_transport_enable(pdata, false);
+	}
+
+	return ret;
 }
 
 static const struct rmi_transport_ops rmi_smb_ops = {
-- 
2.5.5

[PATCH] Input: fix semicolon.cocci warnings

[kbuild test robot]

drivers/input/rmi4/rmi_platform.c:68:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

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

 rmi_platform.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/input/rmi4/rmi_platform.c
+++ b/drivers/input/rmi4/rmi_platform.c
@@ -65,7 +65,7 @@ static void rmi_worker(struct work_struc
 		if (rmi_work->rmi->smbus_client)
 			i2c_unregister_device(rmi_work->rmi->smbus_client);
 		break;
-	};
+	}
 
 	kfree(rmi_work);
 

Re: [PATCH 09/11] Input: synaptics-rmi4 - add rmi_platform

[kbuild test robot]

Hi Benjamin,

[auto build test WARNING on input/next]
[also build test WARNING on v4.8-rc3 next-20160823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/Synaptics-RMI4-over-SMBus/20160818-174713
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/input/rmi4/rmi_platform.c:68:2-3: Unneeded semicolon

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

Re: [PATCH 02/11] Input: serio - store the pt_buttons in the struct serio directly

[Andrew Duggan]

Resending as plain text

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> With RMI4 over SMBus, the pass-through device can be instantiated
> in a SMBus driver. However, compared to the psmouse-synaptics driver,
> this pass-through PS/2 driver has no clue whether the current
> serio_interrupt() is the beginning of the frame or not. Instead of
> adding a protocol analysis in RMI4 function F03, we can add an extra
> byte in struct serio to handle the extra data we want to append to the
> first byte.
>
> Convert the psmouse-synaptics device to use it too.
>
> Partially reverts cdd9dc1 ("Input: synaptics - re-route tracksticks
> buttons on the Lenovo 2015 series")
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Acked-by: Andrew Duggan <aduggan@synaptics.com>


> ---
>  drivers/input/mouse/psmouse-base.c |  3 +++
>  drivers/input/mouse/synaptics.c    | 19 ++++++++++---------
>  drivers/input/mouse/synaptics.h    |  1 -
>  include/linux/serio.h              |  8 ++++++++
>  4 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 5784e20..dbc002a 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -365,6 +365,9 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
>  		goto out;
>  	}
>
> +	if (psmouse->pktcnt == 1)
> +		psmouse->packet[0] |= serio->extra_byte;
> +
>  	psmouse->last = jiffies;
>  	psmouse_handle_byte(psmouse);
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index a41d832..8781e23 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -597,15 +597,13 @@ static int synaptics_is_pt_packet(unsigned char *buf)
>  	return (buf[0] & 0xFC) == 0x84 && (buf[3] & 0xCC) == 0xC4;
>  }
>
> -static void synaptics_pass_pt_packet(struct psmouse *psmouse,
> -				     struct serio *ptport,
> +static void synaptics_pass_pt_packet(struct serio *ptport,
>  				     unsigned char *packet)
>  {
> -	struct synaptics_data *priv = psmouse->private;
>  	struct psmouse *child = serio_get_drvdata(ptport);
>
>  	if (child && child->state == PSMOUSE_ACTIVATED) {
> -		serio_interrupt(ptport, packet[1] | priv->pt_buttons, 0);
> +		serio_interrupt(ptport, packet[1], 0);
>  		serio_interrupt(ptport, packet[4], 0);
>  		serio_interrupt(ptport, packet[5], 0);
>  		if (child->pktsize == 4)
> @@ -857,6 +855,7 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
>  	struct synaptics_data *priv = psmouse->private;
>  	int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1;
>  	char buf[6] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
> +	int pt_buttons;
>  	int i;
>
>  	if (!SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap))
> @@ -887,11 +886,13 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse,
>  		return;
>
>  	/* The trackstick expects at most 3 buttons */
> -	priv->pt_buttons = SYN_CAP_EXT_BUTTON_STICK_L(hw->ext_buttons)      |
> -			   SYN_CAP_EXT_BUTTON_STICK_R(hw->ext_buttons) << 1 |
> -			   SYN_CAP_EXT_BUTTON_STICK_M(hw->ext_buttons) << 2;
> +	pt_buttons = SYN_CAP_EXT_BUTTON_STICK_L(hw->ext_buttons)      |
> +		     SYN_CAP_EXT_BUTTON_STICK_R(hw->ext_buttons) << 1 |
> +		     SYN_CAP_EXT_BUTTON_STICK_M(hw->ext_buttons) << 2;
> +
> +	priv->pt_port->extra_byte = pt_buttons;
>
> -	synaptics_pass_pt_packet(psmouse, priv->pt_port, buf);
> +	synaptics_pass_pt_packet(priv->pt_port, buf);
>  }
>
>  static void synaptics_report_buttons(struct psmouse *psmouse,
> @@ -1132,7 +1133,7 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
>  		if (SYN_CAP_PASS_THROUGH(priv->capabilities) &&
>  		    synaptics_is_pt_packet(psmouse->packet)) {
>  			if (priv->pt_port)
> -				synaptics_pass_pt_packet(psmouse, priv->pt_port,
> +				synaptics_pass_pt_packet(priv->pt_port,
>  							 psmouse->packet);
>  		} else
>  			synaptics_process_packet(psmouse);
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index 56faa7e..116ae25 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -183,7 +183,6 @@ struct synaptics_data {
>  	bool disable_gesture;			/* disable gestures */
>
>  	struct serio *pt_port;			/* Pass-through serio port */
> -	unsigned char pt_buttons;		/* Pass-through buttons */
>
>  	/*
>  	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet
> diff --git a/include/linux/serio.h b/include/linux/serio.h
> index c733cff..f3b75c8 100644
> --- a/include/linux/serio.h
> +++ b/include/linux/serio.h
> @@ -64,6 +64,14 @@ struct serio {
>  	 * may get indigestion when exposed to concurrent access (i8042).
>  	 */
>  	struct mutex *ps2_cmd_mutex;
> +
> +	/*
> +	 * For use with Synaptics devices that have the trackstick buttons
> +	 * not actually wired to the trackstick PS/2 device.
> +	 * This byte will be OR-ed with the first byte of the incoming packet
> +	 * that contains actual data (not commands).
> +	 */
> +	u8 extra_byte;
>  };
>  #define to_serio_port(d)	container_of(d, struct serio, dev)
>
>

Re: [PATCH 03/11] Input: synaptics-rmi4 - have only one struct platform data

[Andrew Duggan]

Resending as plain text

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> If struct rmi_device_platform_data contains pointers to other struct,
> it gets difficult to allocate a fixed size struct and copy it over between
> drivers.
>
> Change the pointers into a struct and change the code in rmi4 accordingly.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> ---
>
> this patch will conflict with Andrew's patch to switch hid-rmi
> to use rmi4_core...
> ---
>  drivers/input/rmi4/rmi_f11.c | 4 ++--
>  drivers/input/rmi4/rmi_f12.c | 4 ++--
>  drivers/input/rmi4/rmi_f30.c | 7 +++----
>  include/linux/rmi.h          | 4 ++--
>  4 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 20c7134..b14a7b6 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1063,8 +1063,8 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>  		rc = rmi_2d_sensor_of_probe(&fn->dev, &f11->sensor_pdata);
>  		if (rc)
>  			return rc;
> -	} else if (pdata->sensor_pdata) {
> -		f11->sensor_pdata = *pdata->sensor_pdata;
> +	} else {
> +		f11->sensor_pdata = pdata->sensor_pdata;
>  	}
>
>  	f11->rezero_wait_ms = f11->sensor_pdata.rezero_wait;
> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 332c02f..a631bed 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -274,8 +274,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
>  		ret = rmi_2d_sensor_of_probe(&fn->dev, &f12->sensor_pdata);
>  		if (ret)
>  			return ret;
> -	} else if (pdata->sensor_pdata) {
> -		f12->sensor_pdata = *pdata->sensor_pdata;
> +	} else {
> +		f12->sensor_pdata = pdata->sensor_pdata;
>  	}
>
>  	ret = rmi_read_register_desc(rmi_dev, query_addr,
> diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
> index 760aff1..7990bb0 100644
> --- a/drivers/input/rmi4/rmi_f30.c
> +++ b/drivers/input/rmi4/rmi_f30.c
> @@ -192,7 +192,7 @@ static int rmi_f30_config(struct rmi_function *fn)
>  				rmi_get_platform_data(fn->rmi_dev);
>  	int error;
>
> -	if (pdata->f30_data && pdata->f30_data->disable) {
> +	if (pdata && pdata->f30_data.disable) {

My one comment is that pdata struct is embedded in the transport device 
so rmi_get_platform_data() will not return NULL. Making the check of 
pdata unnecessary.


>  		drv->clear_irq_bits(fn->rmi_dev, fn->irq_mask);
>  	} else {
>  		/* Write Control Register values back to device */
> @@ -362,8 +362,7 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
>  				 * f30->has_mech_mouse_btns, but I am
>  				 * not sure, so use only the pdata info
>  				 */
> -				if (pdata->f30_data &&
> -				    pdata->f30_data->buttonpad)
> +				if (pdata && pdata->f30_data.buttonpad)

Same with this check of pdata.

>  					break;
>  			}
>  		}
> @@ -378,7 +377,7 @@ static int rmi_f30_probe(struct rmi_function *fn)
>  	const struct rmi_device_platform_data *pdata =
>  				rmi_get_platform_data(fn->rmi_dev);
>
> -	if (pdata->f30_data && pdata->f30_data->disable)
> +	if (pdata && pdata->f30_data.disable)

And this one.

That's a fairly minor comment and I could see an argument for keeping 
the checks in the event that the implementation of 
rmi_get_platform_data() changes.

So:
Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

Andrew

>  		return 0;
>
>  	rc = rmi_f30_initialize(fn);
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index e0aca14..4a071e8 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -211,9 +211,9 @@ struct rmi_device_platform_data {
>  	struct rmi_device_platform_data_spi spi_data;
>
>  	/* function handler pdata */
> -	struct rmi_2d_sensor_platform_data *sensor_pdata;
> +	struct rmi_2d_sensor_platform_data sensor_pdata;
>  	struct rmi_f01_power_management power_management;
> -	struct rmi_f30_data *f30_data;
> +	struct rmi_f30_data f30_data;
>  };
>
>  /**
>

Re: [PATCH 04/11] Input: synaptics-rmi4 - add support for F03

[Andrew Duggan]

Resending as plain text

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> From: Lyude Paul <thatslyude@gmail.com>
>
> This adds basic functionality for PS/2 passthrough on Synaptics
> Touchpads using RMI4 through smbus.
>
> Signed-off-by: Lyude Paul <thatslyude@gmail.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

> ---
>  drivers/input/mouse/psmouse-base.c |   6 +
>  drivers/input/rmi4/Kconfig         |   9 ++
>  drivers/input/rmi4/Makefile        |   1 +
>  drivers/input/rmi4/rmi_bus.c       |   3 +
>  drivers/input/rmi4/rmi_driver.h    |   1 +
>  drivers/input/rmi4/rmi_f03.c       | 226 +++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serio.h         |   1 +
>  7 files changed, 247 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_f03.c
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index dbc002a..fa2d700 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -1666,6 +1666,12 @@ static struct serio_device_id psmouse_serio_ids[] = {
>  		.id	= SERIO_ANY,
>  		.extra	= SERIO_ANY,
>  	},
> +	{
> +		.type	= SERIO_RMI_PSTHRU,
> +		.proto	= SERIO_ANY,
> +		.id	= SERIO_ANY,
> +		.extra	= SERIO_ANY,
> +	},
>  	{ 0 }
>  };
>
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index 86a180b..cfc14b3 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -39,6 +39,15 @@ config RMI4_SMB
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called rmi_smbus.
>
> +config RMI4_F03
> +        bool "RMI4 Function 03 (PS2 Guest)"
> +        depends on RMI4_CORE
> +        help
> +          Say Y here if you want to add support for RMI4 function 03.
> +
> +          Function 03 provides PS2 guest support for RMI4 devices. This
> +          includes support for TrackPoints on TouchPads.
> +
>  config RMI4_2D_SENSOR
>  	bool
>  	depends on RMI4_CORE
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 3c8ebf2..676e636 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
>  rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
>
>  # Function drivers
> +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
>  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
>  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index a735806..e1e3b80 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -303,6 +303,9 @@ struct bus_type rmi_bus_type = {
>
>  static struct rmi_function_handler *fn_handlers[] = {
>  	&rmi_f01_handler,
> +#ifdef CONFIG_RMI4_F03
> +	&rmi_f03_handler,
> +#endif
>  #ifdef CONFIG_RMI4_F11
>  	&rmi_f11_handler,
>  #endif
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 6e140fa..a7cb383 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -99,6 +99,7 @@ void rmi_unregister_physical_driver(void);
>  char *rmi_f01_get_product_ID(struct rmi_function *fn);
>
>  extern struct rmi_function_handler rmi_f01_handler;
> +extern struct rmi_function_handler rmi_f03_handler;
>  extern struct rmi_function_handler rmi_f11_handler;
>  extern struct rmi_function_handler rmi_f12_handler;
>  extern struct rmi_function_handler rmi_f30_handler;
> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> new file mode 100644
> index 0000000..9945512
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f03.c
> @@ -0,0 +1,226 @@
> +/*
> + * Copyright (C) 2015-2016 Red Hat
> + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
> + *
> + * 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/slab.h>
> +#include <linux/serio.h>
> +#include <linux/notifier.h>
> +#include "rmi_driver.h"
> +
> +#define RMI_F03_RX_DATA_OFB		0x01
> +#define RMI_F03_OB_SIZE			2
> +
> +#define RMI_F03_OB_OFFSET		2
> +#define RMI_F03_OB_DATA_OFFSET		1
> +#define RMI_F03_OB_FLAG_TIMEOUT		(1 << 6)
> +#define RMI_F03_OB_FLAG_PARITY		(1 << 7)
> +
> +#define RMI_F03_DEVICE_COUNT		0x07
> +#define RMI_F03_BYTES_PER_DEVICE_MASK	0x70
> +#define RMI_F03_BYTES_PER_DEVICE_SHIFT	4
> +#define RMI_F03_QUEUE_LENGTH		0x0F
> +
> +struct f03_data {
> +	struct rmi_function *fn;
> +
> +	struct serio *serio;
> +
> +	unsigned int overwrite_buttons;
> +
> +	u8 device_count;
> +	u8 rx_queue_length;
> +};
> +
> +static int rmi_f03_pt_write(struct serio *id, unsigned char val)
> +{
> +	struct f03_data *f03 = id->port_data;
> +	int rc;
> +
> +	dev_dbg(&f03->fn->dev, "%s: Wrote %.2hhx to PS/2 passthrough address",
> +		__func__, val);

Note, at some point these dev_dbg messages should be switched to rmi_dbg 
to allow them to be turned on then the RMI_DEBUG_FN bit is set in the 
debug_flags.

> +
> +	rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);
> +	if (rc) {
> +		dev_err(&f03->fn->dev,
> +			"%s: Failed to write to F03 TX register.\n", __func__);
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static inline int rmi_f03_initialize(struct rmi_function *fn)
> +{
> +	struct f03_data *f03;
> +	struct device *dev = &fn->dev;
> +	int rc;
> +	u8 bytes_per_device;
> +	u8 query1;
> +	size_t query2_len;
> +
> +	rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1);
> +	if (rc) {
> +		dev_err(dev, "Failed to read query register.\n");
> +		return rc;
> +	}
> +
> +	f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL);
> +	if (!f03)
> +		return -ENOMEM;
> +
> +	f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
> +	bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >>
> +		RMI_F03_BYTES_PER_DEVICE_SHIFT;
> +
> +	query2_len = f03->device_count * bytes_per_device;
> +
> +	/*
> +	 * The first generation of image sensors don't have a second part to
> +	 * their f03 query, as such we have to set some of these values manually
> +	 */
> +	if (query2_len < 1) {
> +		f03->device_count = 1;
> +		f03->rx_queue_length = 7;
> +	} else {
> +		u8 query2[query2_len];
> +
> +		rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1,
> +				    query2, query2_len);
> +		if (rc) {
> +			dev_err(dev, "Failed to read second set of query registers.\n");
> +			return rc;
> +		}
> +
> +		f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH;
> +	}
> +
> +	f03->fn = fn;
> +
> +	dev_set_drvdata(dev, f03);
> +
> +	return f03->device_count;
> +}
> +
> +static inline int rmi_f03_register_pt(struct rmi_function *fn)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> +
> +	if (!serio)
> +		return -ENOMEM;
> +
> +	serio->id.type = SERIO_RMI_PSTHRU;
> +	serio->write = rmi_f03_pt_write;
> +	serio->port_data = f03;
> +
> +	strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through",
> +		sizeof(serio->name));
> +	strlcpy(serio->phys, "synaptics-rmi4-pt/serio1",
> +		sizeof(serio->phys));
> +	 serio->dev.parent = &fn->dev;
> +
> +	f03->serio = serio;
> +
> +	serio_register_port(serio);
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_probe(struct rmi_function *fn)
> +{
> +	int rc;
> +
> +	rc = rmi_f03_initialize(fn);
> +	if (rc < 0)
> +		return rc;
> +
> +	dev_dbg(&fn->dev, "%d devices on PS/2 passthrough", rc);
> +
> +	rc = rmi_f03_register_pt(fn);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_config(struct rmi_function *fn)
> +{
> +	fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask);
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	u16 data_addr = fn->fd.data_base_addr;
> +	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
> +	u8 obs[ob_len];
> +	u8 ob_status;
> +	u8 ob_data;
> +	unsigned int serio_flags;
> +	int i;
> +	int retval;
> +
> +	/* Grab all of the data registers, and check them for data */
> +	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
> +				&obs, ob_len);
> +	if (retval) {
> +		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
> +			__func__);
> +		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
> +		return retval;
> +	}
> +
> +	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
> +		ob_status = obs[i];
> +		ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
> +		serio_flags = 0;
> +
> +		if (!(ob_status & RMI_F03_RX_DATA_OFB))
> +			continue;
> +
> +		if (ob_status & RMI_F03_OB_FLAG_TIMEOUT)
> +			serio_flags |= SERIO_TIMEOUT;
> +		if (ob_status & RMI_F03_OB_FLAG_PARITY)
> +			serio_flags |= SERIO_PARITY;
> +
> +		dev_dbg(&fn->dev,
> +			"%s: Received %.2hhx from PS2 guest T: %c P: %c\n",
> +			__func__, ob_data,
> +			serio_flags & SERIO_TIMEOUT ?  'Y' : 'N',
> +			serio_flags & SERIO_PARITY ? 'Y' : 'N');
> +
> +		serio_interrupt(f03->serio, ob_data, serio_flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static void rmi_f03_remove(struct rmi_function *fn)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +
> +	serio_unregister_port(f03->serio);
> +}
> +
> +struct rmi_function_handler rmi_f03_handler = {
> +	.driver = {
> +		.name = "rmi4_f03",
> +	},
> +	.func = 0x03,
> +	.probe = rmi_f03_probe,
> +	.config = rmi_f03_config,
> +	.attention = rmi_f03_attention,
> +	.remove = rmi_f03_remove,
> +};
> +
> +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
> +MODULE_DESCRIPTION("RMI F03 module");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
> index f2447a8..7012178 100644
> --- a/include/uapi/linux/serio.h
> +++ b/include/uapi/linux/serio.h
> @@ -30,6 +30,7 @@
>  #define SERIO_HIL_MLC	0x03
>  #define SERIO_PS_PSTHRU	0x05
>  #define SERIO_8042_XL	0x06
> +#define SERIO_RMI_PSTHRU	0x07
>
>  /*
>   * Serio protocols
>

Re: [PATCH 05/11] Input: synaptics-rmi4 - f03: grab data passed by transport device

[Andrew Duggan]

Resending as plain text

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> From: Dennis Wassenberg <dennis.wassenberg@secunet.com>
>
> First check if there are data available passed by the transport device.
> If data available use these data. If there are no data available
> try to read the rmi block if dsata are passed this way.
>
> This is the way the other rmi function handlers will do this.
> That why apply this to f03 as well.
>
> This will fix corrupted or missing data issues.
>

This patch is needed on HID devices because the firmware reads F03 data 
registers and adds them to the HID attention report. Reading those 
registers from the driver after the firmware read them will result in 
invalid data. Which is exactly what Dennis is describing here.


> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

> ---
>  drivers/input/rmi4/rmi_f03.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> index 9945512..daae1c95 100644
> --- a/drivers/input/rmi4/rmi_f03.c
> +++ b/drivers/input/rmi4/rmi_f03.c
> @@ -158,6 +158,7 @@ static int rmi_f03_config(struct rmi_function *fn)
>
>  static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
>  {
> +	struct rmi_device *rmi_dev = fn->rmi_dev;
>  	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
>  	u16 data_addr = fn->fd.data_base_addr;
>  	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
> @@ -166,16 +167,32 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
>  	u8 ob_data;
>  	unsigned int serio_flags;
>  	int i;
> -	int retval;
> -
> -	/* Grab all of the data registers, and check them for data */
> -	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
> -				&obs, ob_len);
> -	if (retval) {
> -		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
> -			__func__);
> -		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
> -		return retval;
> +	int ret;
> +
> +	if (!rmi_dev || !rmi_dev->xport)
> +		return -ENODEV;
> +
> +	if (rmi_dev->xport->attn_data) {
> +		/* First grab the data passed by the transport device */
> +		if (rmi_dev->xport->attn_size < ob_len) {
> +			dev_warn(&fn->dev, "F03 interrupted, but data is missing!\n");
> +			return 0;
> +		}
> +
> +		memcpy(obs, rmi_dev->xport->attn_data, ob_len);
> +
> +		rmi_dev->xport->attn_data += ob_len;
> +		rmi_dev->xport->attn_size -= ob_len;
> +	} else {
> +		/* Grab all of the data registers, and check them for data */
> +		ret = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
> +				     &obs, ob_len);
> +		if (ret) {
> +			dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
> +				__func__);
> +			serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
> +			return ret;
> +		}
>  	}
>
>  	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
>

Re: [PATCH 06/11] Input: synaptics-rmi4 - Add rmi_find_function()

[Andrew Duggan]

Resending as plain text

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> If a function needs to communicate with an other, it's better to have
> a way to retrieve this other.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

> ---
>  drivers/input/rmi4/rmi_driver.c | 13 +++++++++++++
>  drivers/input/rmi4/rmi_driver.h |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index faa295e..304f142 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -181,6 +181,19 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>  }
>  EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
>
> +struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number)
> +{
> +	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> +	struct rmi_function *entry;
> +
> +	list_for_each_entry(entry, &data->function_list, node) {
> +		if (entry->fd.function_number == number)
> +			return entry;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int suspend_one_function(struct rmi_function *fn)
>  {
>  	struct rmi_function_handler *fh;
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index a7cb383..e4be773 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -95,6 +95,7 @@ bool rmi_register_desc_has_subpacket(const struct rmi_register_desc_item *item,
>  bool rmi_is_physical_driver(struct device_driver *);
>  int rmi_register_physical_driver(void);
>  void rmi_unregister_physical_driver(void);
> +struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
>
>  char *rmi_f01_get_product_ID(struct rmi_function *fn);
>
>

Re: [PATCH 07/11] Input: synaptics-rmi4 - f30/f03: Forward mechanical buttons on buttonpads to PS/2 guest

[Andrew Duggan]

Resending as plain text

Hi Benjamin,

This patch causes standard clickpads without extended buttons to not 
work. I'll explain some more below.

On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> From: Lyude Paul <thatslyude@gmail.com>
>
> On the latest series of ThinkPads, the button events for the TrackPoint
> are reported through the touchpad itself as opposed to the TrackPoint
> device. In order to report these buttons properly, we need to forward
> them to the TrackPoint device and send the button presses/releases
> through there instead.
>
> Signed-off-by: Lyude Paul <thatslyude@gmail.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/rmi4/rmi_driver.h | 13 +++++++++
>  drivers/input/rmi4/rmi_f03.c    | 28 ++++++++++++++++++
>  drivers/input/rmi4/rmi_f30.c    | 64 +++++++++++++++++++++++++++++++++++------
>  3 files changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index e4be773..a0b1978 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -99,6 +99,19 @@ struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
>
>  char *rmi_f01_get_product_ID(struct rmi_function *fn);
>
> +#ifdef CONFIG_RMI4_F03
> +int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
> +			     int value);
> +void rmi_f03_commit_buttons(struct rmi_function *fn);
> +#else
> +static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
> +					   unsigned int button, int value)
> +{
> +	return 0;
> +}
> +static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
> +#endif
> +
>  extern struct rmi_function_handler rmi_f01_handler;
>  extern struct rmi_function_handler rmi_f03_handler;
>  extern struct rmi_function_handler rmi_f11_handler;
> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> index daae1c95..535f426 100644
> --- a/drivers/input/rmi4/rmi_f03.c
> +++ b/drivers/input/rmi4/rmi_f03.c
> @@ -37,6 +37,34 @@ struct f03_data {
>  	u8 rx_queue_length;
>  };
>
> +int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
> +			     int value)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	unsigned int bit = BIT(button);
> +
> +	if (button > 2)
> +		return -EINVAL;
> +
> +	if (value)
> +		f03->overwrite_buttons |= bit;
> +	else
> +		f03->overwrite_buttons &= ~bit;
> +
> +	return 0;
> +}
> +
> +void rmi_f03_commit_buttons(struct rmi_function *fn)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	int i;
> +
> +	f03->serio->extra_byte = f03->overwrite_buttons;
> +
> +	for (i = 0; i < 3; i++)
> +		serio_interrupt(f03->serio, 0x00, 0x00);
> +}
> +
>  static int rmi_f03_pt_write(struct serio *id, unsigned char val)
>  {
>  	struct f03_data *f03 = id->port_data;
> diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
> index 7990bb0..14e3221 100644
> --- a/drivers/input/rmi4/rmi_f30.c
> +++ b/drivers/input/rmi4/rmi_f30.c
> @@ -74,8 +74,11 @@ struct f30_data {
>
>  	u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
>  	u16 *gpioled_key_map;
> +	u16 *gpio_passthrough_key_map;
>
>  	struct input_dev *input;
> +	bool trackstick_buttons;
> +	struct rmi_function *f03;
>  };
>
>  static int rmi_f30_read_control_parameters(struct rmi_function *fn,
> @@ -108,6 +111,13 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
>  	if (!f30->input)
>  		return 0;
>
> +	if (f30->trackstick_buttons && !f30->f03) {
> +		f30->f03 = rmi_find_function(rmi_dev, 3);
> +
> +		if (!f30->f03)
> +			return -EBUSY;
> +	}
> +
>  	/* Read the gpi led data. */
>  	if (rmi_dev->xport->attn_data) {
>  		memcpy(f30->data_regs, rmi_dev->xport->attn_data,
> @@ -128,23 +138,29 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
>  	for (reg_num = 0; reg_num < f30->register_count; ++reg_num) {
>  		for (i = 0; gpiled < f30->gpioled_count && i < 8; ++i,
>  			++gpiled) {
> -			if (f30->gpioled_key_map[gpiled] != 0) {
> -				/* buttons have pull up resistors */
> -				value = (((f30->data_regs[reg_num] >> i) & 0x01)
> -									== 0);
> +			/* buttons have pull up resistors */
> +			value = (((f30->data_regs[reg_num] >> i) & 0x01) == 0);
>
> +			if (f30->gpioled_key_map[gpiled] != 0) {
>  				rmi_dbg(RMI_DEBUG_FN, &fn->dev,
>  					"%s: call input report key (0x%04x) value (0x%02x)",
>  					__func__,
>  					f30->gpioled_key_map[gpiled], value);
> +
>  				input_report_key(f30->input,
>  						 f30->gpioled_key_map[gpiled],
>  						 value);
> +			} else if (f30->gpio_passthrough_key_map[gpiled]) {
> +				rmi_f03_overwrite_button(f30->f03,
> +						f30->gpio_passthrough_key_map[gpiled] - BTN_LEFT,
> +						value);
>  			}
> -
>  		}
>  	}
>
> +	if (f30->trackstick_buttons)
> +		rmi_f03_commit_buttons(f30->f03);
> +
>  	return 0;
>  }
>
> @@ -242,10 +258,10 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
>  	int retval = 0;
>  	int control_address;
>  	int i;
> -	int button;
> +	int button, extra_button;
>  	u8 buf[RMI_F30_QUERY_SIZE];
>  	u8 *ctrl_reg;
> -	u8 *map_memory;
> +	u8 *map_memory, *pt_memory;
>
>  	f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
>  			   GFP_KERNEL);
> @@ -343,15 +359,47 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
>  	map_memory = devm_kzalloc(&fn->dev,
>  				  (f30->gpioled_count * (sizeof(u16))),
>  				  GFP_KERNEL);
> -	if (!map_memory) {
> +	pt_memory = devm_kzalloc(&fn->dev,
> +				 (f30->gpioled_count * (sizeof(u16))),
> +				 GFP_KERNEL);
> +	if (!map_memory || !pt_memory) {
>  		dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
>  		return -ENOMEM;
>  	}
>
>  	f30->gpioled_key_map = (u16 *)map_memory;
> +	f30->gpio_passthrough_key_map = (u16 *)pt_memory;
>
>  	pdata = rmi_get_platform_data(rmi_dev);
>  	if (pdata && f30->has_gpio) {

This existing check of pdata is not needed because pdata is embedded in 
the transport device and will never be NULL. That means that in this 
patch the else case will never be called.

> +		/*
> +		 * For touchpads the buttons are mapped as:
> +		 * - bit 0 = Left, bit 1 = right, bit 2 = middle / clickbutton
> +		 * - 3, 4, 5 are extended buttons and
> +		 * - 6 and 7 are other sorts of GPIOs
> +		 */
> +		button = BTN_LEFT;
> +		extra_button = BTN_LEFT;
> +		for (i = 0; i < f30->gpioled_count - 2 && i < 3; i++) {

Subtracting 2 from gpioled_count does not have the intended affect. The 
name gpioled_count comes from the spec, but that byte is really a bit 
map. On a typical clickpad bit two is set as mentioned in the new 
comment. The result is that this for loop only runs once and it only 
checks the first bit of ctrl 2 and ctrl 3 for a valid button. Since bit 
0 is not set then no valid buttons are reported for the clickpad.

It looks like the Lenovo systems which this patch is targeting actually 
have 6 gpios defined (bits 2 - 7 are set) so this code works on those 
system since the "count" is sufficiently large. Also, maybe it's time to 
rename gpioled_count to something like gpioled_map.

> +			if (rmi_f30_is_valid_button(i, f30->ctrl))
> +				f30->gpioled_key_map[i] = button++;
> +		}
> +
> +		f30->trackstick_buttons = pdata &&
> +				pdata->f30_data.trackstick_buttons;
> +
> +		if (f30->trackstick_buttons) {
> +			for (i = 3; i < f30->gpioled_count - 2; i++) {
> +				if (rmi_f30_is_valid_button(i, f30->ctrl))
> +					f30->gpio_passthrough_key_map[i] = extra_button++;
> +			}
> +		} else {
> +			for (i = 3; i < f30->gpioled_count - 2; i++) {
> +				if (rmi_f30_is_valid_button(i, f30->ctrl))
> +					f30->gpioled_key_map[i] = button++;
> +			}
> +		}
> +	} else if (f30->has_gpio) {

As noted above this else block is never called.

Andrew

>  		button = BTN_LEFT;
>  		for (i = 0; i < f30->gpioled_count; i++) {
>  			if (rmi_f30_is_valid_button(i, f30->ctrl)) {
>

Re: [PATCH 07/11] Input: synaptics-rmi4 - f30/f03: Forward mechanical buttons on buttonpads to PS/2 guest

[Benjamin Tissoires]

Hi Andrew,

On Aug 26 2016 or thereabouts, Andrew Duggan wrote:
> Resending as plain text
> 
> Hi Benjamin,
> 
> This patch causes standard clickpads without extended buttons to not work.
> I'll explain some more below.
> 
> On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
> >From: Lyude Paul <thatslyude@gmail.com>
> >
> >On the latest series of ThinkPads, the button events for the TrackPoint
> >are reported through the touchpad itself as opposed to the TrackPoint
> >device. In order to report these buttons properly, we need to forward
> >them to the TrackPoint device and send the button presses/releases
> >through there instead.
> >
> >Signed-off-by: Lyude Paul <thatslyude@gmail.com>
> >Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >---
> > drivers/input/rmi4/rmi_driver.h | 13 +++++++++
> > drivers/input/rmi4/rmi_f03.c    | 28 ++++++++++++++++++
> > drivers/input/rmi4/rmi_f30.c    | 64 +++++++++++++++++++++++++++++++++++------
> > 3 files changed, 97 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> >index e4be773..a0b1978 100644
> >--- a/drivers/input/rmi4/rmi_driver.h
> >+++ b/drivers/input/rmi4/rmi_driver.h
> >@@ -99,6 +99,19 @@ struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
> >
> > char *rmi_f01_get_product_ID(struct rmi_function *fn);
> >
> >+#ifdef CONFIG_RMI4_F03
> >+int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
> >+			     int value);
> >+void rmi_f03_commit_buttons(struct rmi_function *fn);
> >+#else
> >+static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
> >+					   unsigned int button, int value)
> >+{
> >+	return 0;
> >+}
> >+static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
> >+#endif
> >+
> > extern struct rmi_function_handler rmi_f01_handler;
> > extern struct rmi_function_handler rmi_f03_handler;
> > extern struct rmi_function_handler rmi_f11_handler;
> >diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> >index daae1c95..535f426 100644
> >--- a/drivers/input/rmi4/rmi_f03.c
> >+++ b/drivers/input/rmi4/rmi_f03.c
> >@@ -37,6 +37,34 @@ struct f03_data {
> > 	u8 rx_queue_length;
> > };
> >
> >+int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
> >+			     int value)
> >+{
> >+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> >+	unsigned int bit = BIT(button);
> >+
> >+	if (button > 2)
> >+		return -EINVAL;
> >+
> >+	if (value)
> >+		f03->overwrite_buttons |= bit;
> >+	else
> >+		f03->overwrite_buttons &= ~bit;
> >+
> >+	return 0;
> >+}
> >+
> >+void rmi_f03_commit_buttons(struct rmi_function *fn)
> >+{
> >+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> >+	int i;
> >+
> >+	f03->serio->extra_byte = f03->overwrite_buttons;
> >+
> >+	for (i = 0; i < 3; i++)
> >+		serio_interrupt(f03->serio, 0x00, 0x00);
> >+}
> >+
> > static int rmi_f03_pt_write(struct serio *id, unsigned char val)
> > {
> > 	struct f03_data *f03 = id->port_data;
> >diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
> >index 7990bb0..14e3221 100644
> >--- a/drivers/input/rmi4/rmi_f30.c
> >+++ b/drivers/input/rmi4/rmi_f30.c
> >@@ -74,8 +74,11 @@ struct f30_data {
> >
> > 	u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
> > 	u16 *gpioled_key_map;
> >+	u16 *gpio_passthrough_key_map;
> >
> > 	struct input_dev *input;
> >+	bool trackstick_buttons;
> >+	struct rmi_function *f03;
> > };
> >
> > static int rmi_f30_read_control_parameters(struct rmi_function *fn,
> >@@ -108,6 +111,13 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
> > 	if (!f30->input)
> > 		return 0;
> >
> >+	if (f30->trackstick_buttons && !f30->f03) {
> >+		f30->f03 = rmi_find_function(rmi_dev, 3);
> >+
> >+		if (!f30->f03)
> >+			return -EBUSY;
> >+	}
> >+
> > 	/* Read the gpi led data. */
> > 	if (rmi_dev->xport->attn_data) {
> > 		memcpy(f30->data_regs, rmi_dev->xport->attn_data,
> >@@ -128,23 +138,29 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
> > 	for (reg_num = 0; reg_num < f30->register_count; ++reg_num) {
> > 		for (i = 0; gpiled < f30->gpioled_count && i < 8; ++i,
> > 			++gpiled) {
> >-			if (f30->gpioled_key_map[gpiled] != 0) {
> >-				/* buttons have pull up resistors */
> >-				value = (((f30->data_regs[reg_num] >> i) & 0x01)
> >-									== 0);
> >+			/* buttons have pull up resistors */
> >+			value = (((f30->data_regs[reg_num] >> i) & 0x01) == 0);
> >
> >+			if (f30->gpioled_key_map[gpiled] != 0) {
> > 				rmi_dbg(RMI_DEBUG_FN, &fn->dev,
> > 					"%s: call input report key (0x%04x) value (0x%02x)",
> > 					__func__,
> > 					f30->gpioled_key_map[gpiled], value);
> >+
> > 				input_report_key(f30->input,
> > 						 f30->gpioled_key_map[gpiled],
> > 						 value);
> >+			} else if (f30->gpio_passthrough_key_map[gpiled]) {
> >+				rmi_f03_overwrite_button(f30->f03,
> >+						f30->gpio_passthrough_key_map[gpiled] - BTN_LEFT,
> >+						value);
> > 			}
> >-
> > 		}
> > 	}
> >
> >+	if (f30->trackstick_buttons)
> >+		rmi_f03_commit_buttons(f30->f03);
> >+
> > 	return 0;
> > }
> >
> >@@ -242,10 +258,10 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
> > 	int retval = 0;
> > 	int control_address;
> > 	int i;
> >-	int button;
> >+	int button, extra_button;
> > 	u8 buf[RMI_F30_QUERY_SIZE];
> > 	u8 *ctrl_reg;
> >-	u8 *map_memory;
> >+	u8 *map_memory, *pt_memory;
> >
> > 	f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
> > 			   GFP_KERNEL);
> >@@ -343,15 +359,47 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
> > 	map_memory = devm_kzalloc(&fn->dev,
> > 				  (f30->gpioled_count * (sizeof(u16))),
> > 				  GFP_KERNEL);
> >-	if (!map_memory) {
> >+	pt_memory = devm_kzalloc(&fn->dev,
> >+				 (f30->gpioled_count * (sizeof(u16))),
> >+				 GFP_KERNEL);
> >+	if (!map_memory || !pt_memory) {
> > 		dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
> > 		return -ENOMEM;
> > 	}
> >
> > 	f30->gpioled_key_map = (u16 *)map_memory;
> >+	f30->gpio_passthrough_key_map = (u16 *)pt_memory;
> >
> > 	pdata = rmi_get_platform_data(rmi_dev);
> > 	if (pdata && f30->has_gpio) {
> 
> This existing check of pdata is not needed because pdata is embedded in the
> transport device and will never be NULL. That means that in this patch the
> else case will never be called.

oops. Good catch

> 
> >+		/*
> >+		 * For touchpads the buttons are mapped as:
> >+		 * - bit 0 = Left, bit 1 = right, bit 2 = middle / clickbutton
> >+		 * - 3, 4, 5 are extended buttons and
> >+		 * - 6 and 7 are other sorts of GPIOs
> >+		 */
> >+		button = BTN_LEFT;
> >+		extra_button = BTN_LEFT;
> >+		for (i = 0; i < f30->gpioled_count - 2 && i < 3; i++) {
> 
> Subtracting 2 from gpioled_count does not have the intended affect. The name
> gpioled_count comes from the spec, but that byte is really a bit map. On a
> typical clickpad bit two is set as mentioned in the new comment. The result

I really doubt this is a bit map. On the T450s, only bit 3 (the 4th bit
then) is set and this corresponds to the numerical value "8". If it were
a bit map, I would expect bits 0-7 to be set -> 0xFF.

Aren't you mixing the gpioled_count and the gpioled_key_map? Because bit
2 set on gpioled_count would be 4, and I can't figure out a proper use
of this number :)

> is that this for loop only runs once and it only checks the first bit of
> ctrl 2 and ctrl 3 for a valid button. Since bit 0 is not set then no valid
> buttons are reported for the clickpad.
> 
> It looks like the Lenovo systems which this patch is targeting actually have
> 6 gpios defined (bits 2 - 7 are set) so this code works on those system

Yes, the conditions in the for loops are wrong. I think they could
be fixed easily by having one case for (f30->has_gpio &&
f30->trackstick_buttons) and one other when f30->trackstick_buttons is
not set. In the trackstick button case, I should also only take into
account the first 6 buttons (it's a special case after all :-P ).

> since the "count" is sufficiently large. Also, maybe it's time to rename
> gpioled_count to something like gpioled_map.
> 
> >+			if (rmi_f30_is_valid_button(i, f30->ctrl))
> >+				f30->gpioled_key_map[i] = button++;
> >+		}
> >+
> >+		f30->trackstick_buttons = pdata &&
> >+				pdata->f30_data.trackstick_buttons;
> >+
> >+		if (f30->trackstick_buttons) {
> >+			for (i = 3; i < f30->gpioled_count - 2; i++) {
> >+				if (rmi_f30_is_valid_button(i, f30->ctrl))
> >+					f30->gpio_passthrough_key_map[i] = extra_button++;
> >+			}
> >+		} else {
> >+			for (i = 3; i < f30->gpioled_count - 2; i++) {
> >+				if (rmi_f30_is_valid_button(i, f30->ctrl))
> >+					f30->gpioled_key_map[i] = button++;
> >+			}
> >+		}
> >+	} else if (f30->has_gpio) {
> 
> As noted above this else block is never called.

Yep :(

Thanks for the other reviews BTW. I amended the patches locally and will
resubmit this week or the week after if there are more reviews coming
in.

Cheers,
Benjamin

> 
> Andrew
> 
> > 		button = BTN_LEFT;
> > 		for (i = 0; i < f30->gpioled_count; i++) {
> > 			if (rmi_f30_is_valid_button(i, f30->ctrl)) {
> >
> 

Re: [PATCH 07/11] Input: synaptics-rmi4 - f30/f03: Forward mechanical buttons on buttonpads to PS/2 guest

[Andrew Duggan]

On 08/30/2016 08:13 AM, Benjamin Tissoires wrote:
> Hi Andrew,
>
> On Aug 26 2016 or thereabouts, Andrew Duggan wrote:
>> Resending as plain text
>>
>> Hi Benjamin,
>>
>> This patch causes standard clickpads without extended buttons to not work.
>> I'll explain some more below.
>>
>> On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
>>> From: Lyude Paul <thatslyude@gmail.com>
>>>
>>> On the latest series of ThinkPads, the button events for the TrackPoint
>>> are reported through the touchpad itself as opposed to the TrackPoint
>>> device. In order to report these buttons properly, we need to forward
>>> them to the TrackPoint device and send the button presses/releases
>>> through there instead.
>>>
>>> Signed-off-by: Lyude Paul <thatslyude@gmail.com>
>>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>> ---
>>> drivers/input/rmi4/rmi_driver.h | 13 +++++++++
>>> drivers/input/rmi4/rmi_f03.c    | 28 ++++++++++++++++++
>>> drivers/input/rmi4/rmi_f30.c    | 64 +++++++++++++++++++++++++++++++++++------
>>> 3 files changed, 97 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
>>> index e4be773..a0b1978 100644
>>> --- a/drivers/input/rmi4/rmi_driver.h
>>> +++ b/drivers/input/rmi4/rmi_driver.h
>>> @@ -99,6 +99,19 @@ struct rmi_function *rmi_find_function(struct rmi_device *rmi_dev, u8 number);
>>>
>>> char *rmi_f01_get_product_ID(struct rmi_function *fn);
>>>
>>> +#ifdef CONFIG_RMI4_F03
>>> +int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
>>> +			     int value);
>>> +void rmi_f03_commit_buttons(struct rmi_function *fn);
>>> +#else
>>> +static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
>>> +					   unsigned int button, int value)
>>> +{
>>> +	return 0;
>>> +}
>>> +static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
>>> +#endif
>>> +
>>> extern struct rmi_function_handler rmi_f01_handler;
>>> extern struct rmi_function_handler rmi_f03_handler;
>>> extern struct rmi_function_handler rmi_f11_handler;
>>> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
>>> index daae1c95..535f426 100644
>>> --- a/drivers/input/rmi4/rmi_f03.c
>>> +++ b/drivers/input/rmi4/rmi_f03.c
>>> @@ -37,6 +37,34 @@ struct f03_data {
>>> 	u8 rx_queue_length;
>>> };
>>>
>>> +int rmi_f03_overwrite_button(struct rmi_function *fn, unsigned int button,
>>> +			     int value)
>>> +{
>>> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
>>> +	unsigned int bit = BIT(button);
>>> +
>>> +	if (button > 2)
>>> +		return -EINVAL;
>>> +
>>> +	if (value)
>>> +		f03->overwrite_buttons |= bit;
>>> +	else
>>> +		f03->overwrite_buttons &= ~bit;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +void rmi_f03_commit_buttons(struct rmi_function *fn)
>>> +{
>>> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
>>> +	int i;
>>> +
>>> +	f03->serio->extra_byte = f03->overwrite_buttons;
>>> +
>>> +	for (i = 0; i < 3; i++)
>>> +		serio_interrupt(f03->serio, 0x00, 0x00);
>>> +}
>>> +
>>> static int rmi_f03_pt_write(struct serio *id, unsigned char val)
>>> {
>>> 	struct f03_data *f03 = id->port_data;
>>> diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
>>> index 7990bb0..14e3221 100644
>>> --- a/drivers/input/rmi4/rmi_f30.c
>>> +++ b/drivers/input/rmi4/rmi_f30.c
>>> @@ -74,8 +74,11 @@ struct f30_data {
>>>
>>> 	u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
>>> 	u16 *gpioled_key_map;
>>> +	u16 *gpio_passthrough_key_map;
>>>
>>> 	struct input_dev *input;
>>> +	bool trackstick_buttons;
>>> +	struct rmi_function *f03;
>>> };
>>>
>>> static int rmi_f30_read_control_parameters(struct rmi_function *fn,
>>> @@ -108,6 +111,13 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
>>> 	if (!f30->input)
>>> 		return 0;
>>>
>>> +	if (f30->trackstick_buttons && !f30->f03) {
>>> +		f30->f03 = rmi_find_function(rmi_dev, 3);
>>> +
>>> +		if (!f30->f03)
>>> +			return -EBUSY;
>>> +	}
>>> +
>>> 	/* Read the gpi led data. */
>>> 	if (rmi_dev->xport->attn_data) {
>>> 		memcpy(f30->data_regs, rmi_dev->xport->attn_data,
>>> @@ -128,23 +138,29 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
>>> 	for (reg_num = 0; reg_num < f30->register_count; ++reg_num) {
>>> 		for (i = 0; gpiled < f30->gpioled_count && i < 8; ++i,
>>> 			++gpiled) {
>>> -			if (f30->gpioled_key_map[gpiled] != 0) {
>>> -				/* buttons have pull up resistors */
>>> -				value = (((f30->data_regs[reg_num] >> i) & 0x01)
>>> -									== 0);
>>> +			/* buttons have pull up resistors */
>>> +			value = (((f30->data_regs[reg_num] >> i) & 0x01) == 0);
>>>
>>> +			if (f30->gpioled_key_map[gpiled] != 0) {
>>> 				rmi_dbg(RMI_DEBUG_FN, &fn->dev,
>>> 					"%s: call input report key (0x%04x) value (0x%02x)",
>>> 					__func__,
>>> 					f30->gpioled_key_map[gpiled], value);
>>> +
>>> 				input_report_key(f30->input,
>>> 						 f30->gpioled_key_map[gpiled],
>>> 						 value);
>>> +			} else if (f30->gpio_passthrough_key_map[gpiled]) {
>>> +				rmi_f03_overwrite_button(f30->f03,
>>> +						f30->gpio_passthrough_key_map[gpiled] - BTN_LEFT,
>>> +						value);
>>> 			}
>>> -
>>> 		}
>>> 	}
>>>
>>> +	if (f30->trackstick_buttons)
>>> +		rmi_f03_commit_buttons(f30->f03);
>>> +
>>> 	return 0;
>>> }
>>>
>>> @@ -242,10 +258,10 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
>>> 	int retval = 0;
>>> 	int control_address;
>>> 	int i;
>>> -	int button;
>>> +	int button, extra_button;
>>> 	u8 buf[RMI_F30_QUERY_SIZE];
>>> 	u8 *ctrl_reg;
>>> -	u8 *map_memory;
>>> +	u8 *map_memory, *pt_memory;
>>>
>>> 	f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
>>> 			   GFP_KERNEL);
>>> @@ -343,15 +359,47 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
>>> 	map_memory = devm_kzalloc(&fn->dev,
>>> 				  (f30->gpioled_count * (sizeof(u16))),
>>> 				  GFP_KERNEL);
>>> -	if (!map_memory) {
>>> +	pt_memory = devm_kzalloc(&fn->dev,
>>> +				 (f30->gpioled_count * (sizeof(u16))),
>>> +				 GFP_KERNEL);
>>> +	if (!map_memory || !pt_memory) {
>>> 		dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
>>> 		return -ENOMEM;
>>> 	}
>>>
>>> 	f30->gpioled_key_map = (u16 *)map_memory;
>>> +	f30->gpio_passthrough_key_map = (u16 *)pt_memory;
>>>
>>> 	pdata = rmi_get_platform_data(rmi_dev);
>>> 	if (pdata && f30->has_gpio) {
>> This existing check of pdata is not needed because pdata is embedded in the
>> transport device and will never be NULL. That means that in this patch the
>> else case will never be called.
> oops. Good catch
>
>>> +		/*
>>> +		 * For touchpads the buttons are mapped as:
>>> +		 * - bit 0 = Left, bit 1 = right, bit 2 = middle / clickbutton
>>> +		 * - 3, 4, 5 are extended buttons and
>>> +		 * - 6 and 7 are other sorts of GPIOs
>>> +		 */
>>> +		button = BTN_LEFT;
>>> +		extra_button = BTN_LEFT;
>>> +		for (i = 0; i < f30->gpioled_count - 2 && i < 3; i++) {
>> Subtracting 2 from gpioled_count does not have the intended affect. The name
>> gpioled_count comes from the spec, but that byte is really a bit map. On a
>> typical clickpad bit two is set as mentioned in the new comment. The result
> I really doubt this is a bit map. On the T450s, only bit 3 (the 4th bit
> then) is set and this corresponds to the numerical value "8". If it were
> a bit map, I would expect bits 0-7 to be set -> 0xFF.
>
> Aren't you mixing the gpioled_count and the gpioled_key_map? Because bit
> 2 set on gpioled_count would be 4, and I can't figure out a proper use
> of this number :)

Yes, I was confused and that last email from me needs to be deleted from 
the internet. I remembered that gpioled_count doesn't work the way I 
expect it to, but I confused a few things when trying to explain why 
subtracting 2 from gpioled_count doesn't work. So let me try this again. 
The gpioled_count is the number of bits used to represent gpios or leds 
in ctrl 2 and ctrl 3. On a standard clickpad the gpioled_count is set to 
3 which means the first 3 bits of ctrl 2 and ctrl 3 need to be checked. 
This is because the click button is defined in bit 2 (like the comment 
above).

The Lenovos have more GPIOs so the count is 8. Which means all 8 bits 
need to be checked since gpios are defined up to bit 7.

>> is that this for loop only runs once and it only checks the first bit of
>> ctrl 2 and ctrl 3 for a valid button. Since bit 0 is not set then no valid
>> buttons are reported for the clickpad.
>>
>> It looks like the Lenovo systems which this patch is targeting actually have
>> 6 gpios defined (bits 2 - 7 are set) so this code works on those system
> Yes, the conditions in the for loops are wrong. I think they could
> be fixed easily by having one case for (f30->has_gpio &&
> f30->trackstick_buttons) and one other when f30->trackstick_buttons is
> not set. In the trackstick button case, I should also only take into
> account the first 6 buttons (it's a special case after all :-P ).
>
>> since the "count" is sufficiently large. Also, maybe it's time to rename
>> gpioled_count to something like gpioled_map.
>>
>>> +			if (rmi_f30_is_valid_button(i, f30->ctrl))
>>> +				f30->gpioled_key_map[i] = button++;
>>> +		}
>>> +
>>> +		f30->trackstick_buttons = pdata &&
>>> +				pdata->f30_data.trackstick_buttons;
>>> +
>>> +		if (f30->trackstick_buttons) {
>>> +			for (i = 3; i < f30->gpioled_count - 2; i++) {
>>> +				if (rmi_f30_is_valid_button(i, f30->ctrl))
>>> +					f30->gpio_passthrough_key_map[i] = extra_button++;
>>> +			}
>>> +		} else {
>>> +			for (i = 3; i < f30->gpioled_count - 2; i++) {
>>> +				if (rmi_f30_is_valid_button(i, f30->ctrl))
>>> +					f30->gpioled_key_map[i] = button++;
>>> +			}
>>> +		}
>>> +	} else if (f30->has_gpio) {
>> As noted above this else block is never called.
> Yep :(
>
> Thanks for the other reviews BTW. I amended the patches locally and will
> resubmit this week or the week after if there are more reviews coming
> in.

I'll take a look at the rest of the patches in the series when I have a 
chance. But, this was the only patch which caused an issue for me.

Thanks,
Andrew

> Cheers,
> Benjamin
>
>> Andrew
>>
>>> 		button = BTN_LEFT;
>>> 		for (i = 0; i < f30->gpioled_count; i++) {
>>> 			if (rmi_f30_is_valid_button(i, f30->ctrl)) {
>>>