[PATCH v4] Input: synaptics-rmi4: Add F30 support

[PATCH v4] Input: synaptics-rmi4: Add F30 support

[Andrew Duggan]

RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
buttons.  In particular, the mechanical button support is used in
an increasing number of touchpads.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Allie Xiong <axiong@synaptics.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
---
I discovered that the v3 version of this patch from March wasn't compatible with the version
of the driver currently in the synaptics-rmi4 branch. This patch fixes that. The only difference
from v3 is adding a #define and fixing a spacing issue.

 drivers/input/rmi4/Kconfig   |  12 ++
 drivers/input/rmi4/Makefile  |   1 +
 drivers/input/rmi4/rmi_f30.c | 400 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/rmi.h          |   7 +-
 4 files changed, 419 insertions(+), 1 deletion(-)
 create mode 100644 drivers/input/rmi4/rmi_f30.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index d0c7b6e..14d5f49 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -63,3 +63,15 @@ config RMI4_F11_PEN
 	  If your system is not recognizing pen touches and you know your
 	  sensor supports pen input, you probably want to turn this feature
 	  off.
+
+config RMI4_F30
+        tristate "RMI4 Function 30 (GPIO LED)"
+        depends on RMI4_CORE
+        help
+          Say Y here if you want to add support for RMI4 function 30.
+
+          Function 30 provides GPIO and LED support for RMI4 devices. This
+	  includes support for buttons on TouchPads and ClickPads.
+
+          To compile this driver as a module, choose M here: the
+          module will be called rmi-f30.
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 5c6bad5..ecffd72 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -3,6 +3,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
 
 # Function drivers
 obj-$(CONFIG_RMI4_F11) += rmi_f11.o
+obj-$(CONFIG_RMI4_F30) += rmi_f30.o
 
 # Transports
 obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
new file mode 100644
index 0000000..05cc8e8
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -0,0 +1,400 @@
+/*
+ * Copyright (c) 2012 - 2014 Synaptics Incorporated
+ *
+ * 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/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
+#include "rmi_driver.h"
+
+#define NAME_BUFFER_SIZE			256
+#define RMI_F30_QUERY_SIZE			2
+
+/* Defs for Query 0 */
+#define RMI_F30_EXTENDED_PATTERNS		0x01
+#define RMI_F30_HAS_MAPPABLE_BUTTONS		(1 << 1)
+#define RMI_F30_HAS_LED			(1 << 2)
+#define RMI_F30_HAS_GPIO			(1 << 3)
+#define RMI_F30_HAS_HAPTIC			(1 << 4)
+#define RMI_F30_HAS_GPIO_DRV_CTL		(1 << 5)
+#define RMI_F30_HAS_MECH_MOUSE_BTNS		(1 << 6)
+
+/* Defs for Query 1 */
+#define RMI_F30_GPIO_LED_COUNT			0x1F
+
+/* Defs for Control Registers */
+#define RMI_F30_CTRL_1_GPIO_DEBOUNCE		0x01
+#define RMI_F30_CTRL_1_HALT			(1 << 4)
+#define RMI_F30_CTRL_1_HALTED			(1 << 5)
+#define RMI_F30_CTRL_10_NUM_MECH_MOUSE_BTNS	0x03
+
+struct rmi_f30_ctrl_data {
+	int address;
+	int length;
+	u8 *regs;
+};
+
+#define RMI_F30_CTRL_MAX_REGS		32
+#define RMI_F30_CTRL_MAX_BYTES		((RMI_F30_CTRL_MAX_REGS + 7) >> 3)
+#define RMI_F30_CTRL_MAX_REG_BLOCKS	11
+
+#define RMI_F30_CTRL_REGS_MAX_SIZE (RMI_F30_CTRL_MAX_BYTES		\
+					+ 1				\
+					+ RMI_F30_CTRL_MAX_BYTES	\
+					+ RMI_F30_CTRL_MAX_BYTES	\
+					+ RMI_F30_CTRL_MAX_BYTES	\
+					+ 6				\
+					+ RMI_F30_CTRL_MAX_REGS		\
+					+ RMI_F30_CTRL_MAX_REGS		\
+					+ RMI_F30_CTRL_MAX_BYTES	\
+					+ 1				\
+					+ 1)
+
+struct f30_data {
+	/* Query Data */
+	bool has_extended_pattern;
+	bool has_mappable_buttons;
+	bool has_led;
+	bool has_gpio;
+	bool has_haptic;
+	bool has_gpio_driver_control;
+	bool has_mech_mouse_btns;
+	u8 gpioled_count;
+
+	u8 register_count;
+
+	/* Control Register Data */
+	struct rmi_f30_ctrl_data ctrl[RMI_F30_CTRL_MAX_REG_BLOCKS];
+	u8 ctrl_regs[RMI_F30_CTRL_REGS_MAX_SIZE];
+	u32 ctrl_regs_size;
+
+	u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
+	u16 *gpioled_key_map;
+	u8 *gpioled_sense_map;
+
+	char input_phys[NAME_BUFFER_SIZE];
+	struct input_dev *input;
+};
+
+static int rmi_f30_read_control_parameters(struct rmi_function *fn,
+						struct f30_data *f30)
+{
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	int error = 0;
+
+	error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
+				f30->ctrl_regs, f30->ctrl_regs_size);
+	if (error) {
+		dev_err(&rmi_dev->dev, "%s : Could not read control registers at 0x%x error (%d)\n",
+			__func__, fn->fd.control_base_addr, error);
+		return error;
+	}
+
+	return 0;
+}
+
+static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
+{
+	struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+	int retval;
+	int gpiled = 0;
+	int value = 0;
+	int i;
+	int reg_num;
+
+	/* Read the gpi led data. */
+	retval = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr,
+		f30->data_regs, f30->register_count);
+
+	if (retval) {
+		dev_err(&fn->dev, "%s: Failed to read F30 data registers.\n",
+			__func__);
+		return retval;
+	}
+
+	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) {
+				value = (((f30->data_regs[reg_num] >> i) & 0x01)
+					 == f30->gpioled_sense_map[gpiled]);
+
+				dev_dbg(&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);
+			}
+
+		}
+	}
+
+	input_sync(f30->input); /* sync after groups of events */
+	return 0;
+}
+
+static int rmi_f30_register_device(struct rmi_function *fn)
+{
+	int i;
+	int rc;
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+	struct rmi_driver *driver = fn->rmi_dev->driver;
+	struct input_dev *input_dev;
+
+	input_dev = devm_input_allocate_device(&fn->dev);
+	if (!input_dev) {
+		dev_err(&fn->dev, "Failed to allocate input device.\n");
+		return -ENOMEM;
+	}
+
+	f30->input = input_dev;
+
+	if (driver->set_input_params) {
+		rc = driver->set_input_params(rmi_dev, input_dev);
+		if (rc < 0) {
+			dev_err(&fn->dev, "%s: Error in setting input device.\n",
+				__func__);
+			return rc;
+		}
+	}
+	sprintf(f30->input_phys, "%s/input0", dev_name(&fn->dev));
+	input_dev->phys = f30->input_phys;
+	input_dev->dev.parent = &rmi_dev->dev;
+	input_set_drvdata(input_dev, f30);
+
+	set_bit(EV_SYN, input_dev->evbit);
+	set_bit(EV_KEY, input_dev->evbit);
+
+	input_dev->keycode = f30->gpioled_key_map;
+	input_dev->keycodesize = sizeof(u16);
+	input_dev->keycodemax = f30->gpioled_count;
+
+	for (i = 0; i < f30->gpioled_count; i++) {
+		if (f30->gpioled_key_map[i] != 0)
+			input_set_capability(input_dev, EV_KEY,
+						f30->gpioled_key_map[i]);
+	}
+
+	rc = input_register_device(input_dev);
+	if (rc < 0) {
+		dev_err(&fn->dev, "Failed to register input device.\n");
+		return rc;
+	}
+	return 0;
+}
+
+static int rmi_f30_config(struct rmi_function *fn)
+{
+	struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+	int error;
+
+	/* Write Control Register values back to device */
+	error = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
+				f30->ctrl_regs, f30->ctrl_regs_size);
+	if (error) {
+		dev_err(&fn->rmi_dev->dev,
+			"%s : Could not write control registers at 0x%x error (%d)\n",
+			__func__, fn->fd.control_base_addr, error);
+		return error;
+	}
+
+	return 0;
+}
+
+static inline void rmi_f30_set_ctrl_data(struct rmi_f30_ctrl_data *ctrl,
+					int *ctrl_addr, int len, u8 **reg)
+{
+	ctrl->address = *ctrl_addr;
+	ctrl->length = len;
+	ctrl->regs = *reg;
+	*ctrl_addr += len;
+	*reg += len;
+}
+
+static inline int rmi_f30_initialize(struct rmi_function *fn)
+{
+	struct f30_data *f30;
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	const struct rmi_device_platform_data *pdata;
+	int retval = 0;
+	int control_address;
+	u8 buf[RMI_F30_QUERY_SIZE];
+	u8 *ctrl_reg;
+	u8 *map_memory;
+
+	f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
+			   GFP_KERNEL);
+	if (!f30) {
+		dev_err(&fn->dev, "Failed to allocate f30_data.\n");
+		return -ENOMEM;
+	}
+	dev_set_drvdata(&fn->dev, f30);
+
+	retval = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr, buf,
+				RMI_F30_QUERY_SIZE);
+
+	if (retval) {
+		dev_err(&fn->dev, "Failed to read query register.\n");
+		return retval;
+	}
+
+	f30->has_extended_pattern = buf[0] & RMI_F30_EXTENDED_PATTERNS;
+	f30->has_mappable_buttons = buf[0] & RMI_F30_HAS_MAPPABLE_BUTTONS;
+	f30->has_led = buf[0] & RMI_F30_HAS_LED;
+	f30->has_gpio = buf[0] & RMI_F30_HAS_GPIO;
+	f30->has_haptic = buf[0] & RMI_F30_HAS_HAPTIC;
+	f30->has_gpio_driver_control = buf[0] & RMI_F30_HAS_GPIO_DRV_CTL;
+	f30->has_mech_mouse_btns = buf[0] & RMI_F30_HAS_MECH_MOUSE_BTNS;
+	f30->gpioled_count = buf[1] & RMI_F30_GPIO_LED_COUNT;
+
+	f30->register_count = (f30->gpioled_count + 7) >> 3;
+
+	control_address = fn->fd.control_base_addr;
+	ctrl_reg = f30->ctrl_regs;
+
+	/* Allocate buffers for the control registers */
+	if (f30->has_led && f30->has_led)
+		rmi_f30_set_ctrl_data(&f30->ctrl[0], &control_address,
+					f30->register_count, &ctrl_reg);
+
+	rmi_f30_set_ctrl_data(&f30->ctrl[1], &control_address, sizeof(u8),
+				&ctrl_reg);
+
+	if (f30->has_gpio) {
+		rmi_f30_set_ctrl_data(&f30->ctrl[2], &control_address,
+					f30->register_count, &ctrl_reg);
+
+		rmi_f30_set_ctrl_data(&f30->ctrl[3], &control_address,
+					f30->register_count, &ctrl_reg);
+	}
+
+	if (f30->has_led) {
+		int ctrl5_len;
+
+		rmi_f30_set_ctrl_data(&f30->ctrl[4], &control_address,
+					f30->register_count, &ctrl_reg);
+
+		if (f30->has_extended_pattern)
+			ctrl5_len = 6;
+		else
+			ctrl5_len = 2;
+
+		rmi_f30_set_ctrl_data(&f30->ctrl[5], &control_address,
+					ctrl5_len, &ctrl_reg);
+	}
+
+	if (f30->has_led || f30->has_gpio_driver_control) {
+		/* control 6 uses a byte per gpio/led */
+		rmi_f30_set_ctrl_data(&f30->ctrl[6], &control_address,
+					f30->gpioled_count, &ctrl_reg);
+	}
+
+	if (f30->has_mappable_buttons) {
+		/* control 7 uses a byte per gpio/led */
+		rmi_f30_set_ctrl_data(&f30->ctrl[7], &control_address,
+					f30->gpioled_count, &ctrl_reg);
+	}
+
+	if (f30->has_haptic) {
+		rmi_f30_set_ctrl_data(&f30->ctrl[8], &control_address,
+					f30->register_count, &ctrl_reg);
+
+		rmi_f30_set_ctrl_data(&f30->ctrl[9], &control_address,
+					sizeof(u8), &ctrl_reg);
+	}
+
+	if (f30->has_mech_mouse_btns)
+		rmi_f30_set_ctrl_data(&f30->ctrl[10], &control_address,
+					sizeof(u8), &ctrl_reg);
+
+	f30->ctrl_regs_size = ctrl_reg - f30->ctrl_regs
+				?: RMI_F30_CTRL_REGS_MAX_SIZE;
+
+	map_memory = devm_kzalloc(&fn->dev,
+				(f30->gpioled_count
+				* (sizeof(u16) + sizeof(u8))),
+				GFP_KERNEL);
+	if (!map_memory) {
+		dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
+		return -ENOMEM;
+	}
+
+	f30->gpioled_key_map = (u16 *)map_memory;
+	f30->gpioled_sense_map = map_memory + sizeof(u16)
+					* f30->gpioled_count;
+
+	pdata = rmi_get_platform_data(rmi_dev);
+	if (pdata) {
+		if (!pdata->gpioled_map) {
+			dev_warn(&fn->dev,
+				"%s - gpioled_map is NULL", __func__);
+		} else if (!pdata->gpioled_map->map) {
+			dev_warn(&fn->dev,
+				 "Platform Data button map is missing!\n");
+		} else {
+			int i;
+			for (i = 0; i < f30->gpioled_count
+				|| i < pdata->gpioled_map->ngpioleds; i++) {
+				f30->gpioled_key_map[i] =
+					pdata->gpioled_map->map[i].button;
+				f30->gpioled_sense_map[i] =
+					pdata->gpioled_map->map[i].sense;
+			}
+		}
+	}
+
+	retval = rmi_f30_read_control_parameters(fn, f30);
+	if (retval < 0) {
+		dev_err(&fn->dev,
+			"Failed to initialize F19 control params.\n");
+		return retval;
+	}
+
+	return 0;
+}
+
+static int rmi_f30_probe(struct rmi_function *fn)
+{
+	int rc;
+
+	rc = rmi_f30_initialize(fn);
+	if (rc < 0)
+		goto error_exit;
+
+	rc = rmi_f30_register_device(fn);
+	if (rc < 0)
+		goto error_exit;
+
+	return 0;
+
+error_exit:
+	return rc;
+
+}
+
+static struct rmi_function_handler rmi_f30_handler = {
+	.driver = {
+		.name = "rmi_f30",
+	},
+	.func = 0x30,
+	.probe = rmi_f30_probe,
+	.config = rmi_f30_config,
+	.attention = rmi_f30_attention,
+};
+
+module_rmi_driver(rmi_f30_handler);
+
+MODULE_AUTHOR("Allie Xiong <axiong@synaptics.com>");
+MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com>");
+MODULE_DESCRIPTION("RMI F30 module");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ca35b2f..5150eb9 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -146,9 +146,14 @@ struct rmi_button_map {
 	u8 *map;
 };
 
+struct rmi_f30_button {
+	u16 button;
+	int sense;
+};
+
 struct rmi_f30_gpioled_map {
 	u8 ngpioleds;
-	u8 *map;
+	struct rmi_f30_button *map;
 };
 
 /**
-- 
1.9.1

Re: [PATCH v4] Input: synaptics-rmi4: Add F30 support

[Dmitry Torokhov]

Hi Andrew,


On Thu, Jul 24, 2014 at 04:47:21PM -0700, Andrew Duggan wrote:
> RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
> buttons.  In particular, the mechanical button support is used in
> an increasing number of touchpads.
> 

I was reading the spec for F30 and it looks to me that instead instead of basic
keymap it should actually implement a gpio chip plus LED devices (probably
later). Then, if GPIOs are indeed attached to buttons one can use standard
gpio_keys driver to create input device.

Thanks.

-- 
Dmitry

Re: [PATCH v4] Input: synaptics-rmi4: Add F30 support

[Andrew Duggan]

Hi Dmitry,

With the renewed interest in supporting SMBus with the Synaptics RMI4 
driver I thought I would look into this again. I'm sorry for not 
responding to your comments sooner, especially since you took the time 
to read the F30 spec. Now that it has been over a year later we may need 
to essentially start over. But, this time I will make sure to follow up 
and hopefully not waste your time in the future.

On 07/27/2014 12:26 AM, Dmitry Torokhov wrote:
> Hi Andrew,
>
>
> On Thu, Jul 24, 2014 at 04:47:21PM -0700, Andrew Duggan wrote:
>> RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
>> buttons.  In particular, the mechanical button support is used in
>> an increasing number of touchpads.
>>
> I was reading the spec for F30 and it looks to me that instead instead of basic
> keymap it should actually implement a gpio chip plus LED devices (probably
> later). Then, if GPIOs are indeed attached to buttons one can use standard
> gpio_keys driver to create input device.

The most common use of F30 is handling input from the tact switch on 
clickpads. Since the tact switch is wired up to the touch controller, 
it's the touch controller that handles the GPIO. The touch controller 
then asserts attention to the host, which then issues reads to see what 
interrupted, and then reads the F30 data register to get the state of 
the GPIO. Would gpio chip work with this setup? After looking at gpio 
chip a little bit I'm not really seeing how that would work. It looks 
like to work with gpio_keys each button would need it's own irq? I 
couldn't find an example of a driver doing something similar, but if 
there is one could you point me to it?

For touchpad buttons we know based on convention which GPIOs represent 
which buttons. In the case of touchpads we can avoid creating a button 
map in the platform data. For touchpads we also want to report the input 
events for F30 from the same input device which is reporting finger 
data. Benjamin's patchset contains a patch which allows for a unified 
input device which will allow more then one function to use the same 
input device.

The F30 spec is fairly generic so it could be used for things besides 
the touchpad buttons and we might want to report events independently of 
finger position. However, besides touchpad LEDs I'm not sure F30 has 
ever been used for other applications in production so I'm not sure its 
worth implementing something more generic at this time.

> Thanks.
>

Also, the fact that it took me over a year to reply to these comments 
highlights that we at Synaptics have been inconsistent in our effort to 
get the synaptics-rmi4 driver upstreamed. I think it's about time we 
have a single upstreamed driver which can support all of our RMI devices 
so I'm willing do more on behalf of Synaptics to help get it upstreamed.

Thanks,
Andrew

Re: [PATCH v4] Input: synaptics-rmi4: Add F30 support

[Dmitry Torokhov]

Hi Andrew,

On Fri, Aug 07, 2015 at 05:36:51PM -0700, Andrew Duggan wrote:
> Hi Dmitry,
> 
> With the renewed interest in supporting SMBus with the Synaptics
> RMI4 driver I thought I would look into this again. I'm sorry for
> not responding to your comments sooner, especially since you took
> the time to read the F30 spec. Now that it has been over a year
> later we may need to essentially start over. But, this time I will
> make sure to follow up and hopefully not waste your time in the
> future.

This is great and I agree that we need to get RMI4 into kernel proper.
Unfortunately it is quite large body of code and I am afraid that I wont
be able to give it enough attention until mid-September.

In the meantime, Benjamin, maybe you could work with Andrew? You have
done a lot of work on input side.

Thanks.

> 
> On 07/27/2014 12:26 AM, Dmitry Torokhov wrote:
> >Hi Andrew,
> >
> >
> >On Thu, Jul 24, 2014 at 04:47:21PM -0700, Andrew Duggan wrote:
> >>RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
> >>buttons.  In particular, the mechanical button support is used in
> >>an increasing number of touchpads.
> >>
> >I was reading the spec for F30 and it looks to me that instead instead of basic
> >keymap it should actually implement a gpio chip plus LED devices (probably
> >later). Then, if GPIOs are indeed attached to buttons one can use standard
> >gpio_keys driver to create input device.
> 
> The most common use of F30 is handling input from the tact switch on
> clickpads. Since the tact switch is wired up to the touch
> controller, it's the touch controller that handles the GPIO. The
> touch controller then asserts attention to the host, which then
> issues reads to see what interrupted, and then reads the F30 data
> register to get the state of the GPIO. Would gpio chip work with
> this setup? After looking at gpio chip a little bit I'm not really
> seeing how that would work. It looks like to work with gpio_keys
> each button would need it's own irq? I couldn't find an example of a
> driver doing something similar, but if there is one could you point
> me to it?
> 
> For touchpad buttons we know based on convention which GPIOs
> represent which buttons. In the case of touchpads we can avoid
> creating a button map in the platform data. For touchpads we also
> want to report the input events for F30 from the same input device
> which is reporting finger data. Benjamin's patchset contains a patch
> which allows for a unified input device which will allow more then
> one function to use the same input device.
> 
> The F30 spec is fairly generic so it could be used for things
> besides the touchpad buttons and we might want to report events
> independently of finger position. However, besides touchpad LEDs I'm
> not sure F30 has ever been used for other applications in production
> so I'm not sure its worth implementing something more generic at
> this time.

OK, fair enough. I'll have to take another look then - the details are
quite hazy ;)

> 
> >Thanks.
> >
> 
> Also, the fact that it took me over a year to reply to these
> comments highlights that we at Synaptics have been inconsistent in
> our effort to get the synaptics-rmi4 driver upstreamed. I think it's
> about time we have a single upstreamed driver which can support all
> of our RMI devices so I'm willing do more on behalf of Synaptics to
> help get it upstreamed.
> 
> Thanks,
> Andrew

Thanks.

-- 
Dmitry

Re: [PATCH v4] Input: synaptics-rmi4: Add F30 support

[Andrew Duggan]

Hi Dmitry,

On 08/13/2015 09:38 AM, Dmitry Torokhov wrote:
> Hi Andrew,
>
> On Fri, Aug 07, 2015 at 05:36:51PM -0700, Andrew Duggan wrote:
>> Hi Dmitry,
>>
>> With the renewed interest in supporting SMBus with the Synaptics
>> RMI4 driver I thought I would look into this again. I'm sorry for
>> not responding to your comments sooner, especially since you took
>> the time to read the F30 spec. Now that it has been over a year
>> later we may need to essentially start over. But, this time I will
>> make sure to follow up and hopefully not waste your time in the
>> future.
> This is great and I agree that we need to get RMI4 into kernel proper.
> Unfortunately it is quite large body of code and I am afraid that I wont
> be able to give it enough attention until mid-September.
>
> In the meantime, Benjamin, maybe you could work with Andrew? You have
> done a lot of work on input side.

I understand, it is a lot of code. If you have time later this year to 
work on it then that would be great! In the meantime, I'll post the 
device tree related patches I've been working on the last couple of days 
and then we can go from there when you have time.

Thanks,
Andrew

> Thanks.
>
>> On 07/27/2014 12:26 AM, Dmitry Torokhov wrote:
>>> Hi Andrew,
>>>
>>>
>>> On Thu, Jul 24, 2014 at 04:47:21PM -0700, Andrew Duggan wrote:
>>>> RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
>>>> buttons.  In particular, the mechanical button support is used in
>>>> an increasing number of touchpads.
>>>>
>>> I was reading the spec for F30 and it looks to me that instead instead of basic
>>> keymap it should actually implement a gpio chip plus LED devices (probably
>>> later). Then, if GPIOs are indeed attached to buttons one can use standard
>>> gpio_keys driver to create input device.
>> The most common use of F30 is handling input from the tact switch on
>> clickpads. Since the tact switch is wired up to the touch
>> controller, it's the touch controller that handles the GPIO. The
>> touch controller then asserts attention to the host, which then
>> issues reads to see what interrupted, and then reads the F30 data
>> register to get the state of the GPIO. Would gpio chip work with
>> this setup? After looking at gpio chip a little bit I'm not really
>> seeing how that would work. It looks like to work with gpio_keys
>> each button would need it's own irq? I couldn't find an example of a
>> driver doing something similar, but if there is one could you point
>> me to it?
>>
>> For touchpad buttons we know based on convention which GPIOs
>> represent which buttons. In the case of touchpads we can avoid
>> creating a button map in the platform data. For touchpads we also
>> want to report the input events for F30 from the same input device
>> which is reporting finger data. Benjamin's patchset contains a patch
>> which allows for a unified input device which will allow more then
>> one function to use the same input device.
>>
>> The F30 spec is fairly generic so it could be used for things
>> besides the touchpad buttons and we might want to report events
>> independently of finger position. However, besides touchpad LEDs I'm
>> not sure F30 has ever been used for other applications in production
>> so I'm not sure its worth implementing something more generic at
>> this time.
> OK, fair enough. I'll have to take another look then - the details are
> quite hazy ;)
>
>>> Thanks.
>>>
>> Also, the fact that it took me over a year to reply to these
>> comments highlights that we at Synaptics have been inconsistent in
>> our effort to get the synaptics-rmi4 driver upstreamed. I think it's
>> about time we have a single upstreamed driver which can support all
>> of our RMI devices so I'm willing do more on behalf of Synaptics to
>> help get it upstreamed.
>>
>> Thanks,
>> Andrew
> Thanks.
>