From: Benjamin Tissoires <[email protected]>
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.
[BT] cured the code to rely only on the unified input node created
by rmi_driver.
Signed-off-by: Andrew Duggan <[email protected]>
Signed-off-by: Allie Xiong <[email protected]>
Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/input/rmi4/Kconfig | 12 ++
drivers/input/rmi4/Makefile | 1 +
drivers/input/rmi4/rmi_bus.c | 12 +-
drivers/input/rmi4/rmi_driver.h | 8 +
drivers/input/rmi4/rmi_f30.c | 403 ++++++++++++++++++++++++++++++++++++++++
include/linux/rmi.h | 19 +-
6 files changed, 448 insertions(+), 7 deletions(-)
create mode 100644 drivers/input/rmi4/rmi_f30.c
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 2fce69f..02dc641 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -90,3 +90,15 @@ config RMI4_F12
Function 12 provides 2D multifinger pointing for touchscreens and
touchpads. For sensors that support relative pointing, F12 also
provides mouse input.
+
+config RMI4_F30
+ bool "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 4ea8ef5..766957a 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -6,6 +6,7 @@ rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
# Function drivers
rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
+rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
# Transports
obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 0ea7dfc..c50ddfc 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -367,15 +367,24 @@ static int __init rmi_bus_init(void)
goto err_unregister_f11;
}
+ error = rmi_register_f30_handler();
+ if (error) {
+ pr_err("%s: error registering the RMI F30 handler: %d\n",
+ __func__, error);
+ goto err_unregister_f12;
+ }
+
error = rmi_register_physical_driver();
if (error) {
pr_err("%s: error registering the RMI physical driver: %d\n",
__func__, error);
- goto err_unregister_f12;
+ goto err_unregister_f30;
}
return 0;
+err_unregister_f30:
+ rmi_unregister_f30_handler();
err_unregister_f12:
rmi_unregister_f12_handler();
err_unregister_f11:
@@ -396,6 +405,7 @@ static void __exit rmi_bus_exit(void)
*/
rmi_unregister_physical_driver();
+ rmi_unregister_f30_handler();
rmi_unregister_f12_handler();
rmi_unregister_f11_handler();
rmi_unregister_f01_handler();
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 4464236..e710aee 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -163,4 +163,12 @@ static inline int rmi_register_f12_handler(void) { return 0; }
static inline void rmi_unregister_f12_handler(void) {}
#endif
+#ifdef CONFIG_RMI4_F30
+int rmi_register_f30_handler(void);
+void rmi_unregister_f30_handler(void);
+#else
+static inline int rmi_register_f30_handler(void) { return 0; }
+static inline void rmi_unregister_f30_handler(void) {}
+#endif
+
#endif
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
new file mode 100644
index 0000000..89c2a10
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -0,0 +1,403 @@
+/*
+ * 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 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;
+
+ 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;
+
+ if (!f30->input)
+ return 0;
+
+ /* 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) {
+ /* buttons have pull up resistors */
+ value = (((f30->data_regs[reg_num] >> i) & 0x01)
+ == 0);
+
+ 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);
+ }
+
+ }
+ }
+
+ return 0;
+}
+
+static int rmi_f30_register_device(struct rmi_function *fn)
+{
+ int i;
+ struct rmi_device *rmi_dev = fn->rmi_dev;
+ struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
+ struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+ struct input_dev *input_dev;
+ int button_count = 0;
+
+ input_dev = drv_data->input;
+ if (!input_dev) {
+ dev_info(&fn->dev, "F30: no input device found, ignoring.\n");
+ return -EINVAL;
+ }
+
+ f30->input = input_dev;
+
+ 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]);
+ button_count++;
+ }
+ }
+
+ if (button_count == 1)
+ __set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
+ return 0;
+}
+
+static int rmi_f30_config(struct rmi_function *fn)
+{
+ struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+ struct rmi_driver *drv = fn->rmi_dev->driver;
+ 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;
+ }
+
+ drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+ 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 bool rmi_f30_is_valid_button(int button,
+ struct rmi_f30_ctrl_data *ctrl)
+{
+ int byte_position = button >> 3;
+ int bit_position = button & 0x07;
+
+ /*
+ * ctrl2 -> dir == 0 -> input mode
+ * ctrl3 -> data == 1 -> actual button
+ */
+ return !(ctrl[2].regs[byte_position] & BIT(bit_position)) &&
+ (ctrl[3].regs[byte_position] & BIT(bit_position));
+}
+
+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;
+ int i;
+ int button;
+ 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)
+ 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;
+
+ retval = rmi_f30_read_control_parameters(fn, f30);
+ if (retval < 0) {
+ dev_err(&fn->dev,
+ "Failed to initialize F19 control params.\n");
+ return retval;
+ }
+
+ map_memory = devm_kzalloc(&fn->dev,
+ (f30->gpioled_count * (sizeof(u16))),
+ 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;
+
+ pdata = rmi_get_platform_data(rmi_dev);
+ if (pdata && f30->has_gpio) {
+ button = BTN_LEFT;
+ for (i = 0; i < f30->gpioled_count; i++) {
+ if (rmi_f30_is_valid_button(i, f30->ctrl)) {
+ f30->gpioled_key_map[i] = button++;
+
+ /*
+ * buttonpad might be given by
+ * f30->has_mech_mouse_btns, but I am
+ * not sure, so use only the pdata info
+ */
+ if (pdata->f30_data &&
+ pdata->f30_data->buttonpad)
+ break;
+ }
+ }
+ }
+
+ 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,
+};
+
+int __init rmi_register_f30_handler(void)
+{
+ return rmi_register_function_handler(&rmi_f30_handler);
+}
+
+void rmi_unregister_f30_handler(void)
+{
+ rmi_unregister_function_handler(&rmi_f30_handler);
+}
+
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 0092787..8de6581 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -114,6 +114,18 @@ struct rmi_2d_sensor_platform_data {
};
/**
+ * struct rmi_f30_data - overrides defaults for a single F30 GPIOs/LED chip.
+ * @buttonpad - the touchpad is a buttonpad, so enable only the first actual
+ * button that is found.
+ * @trackstick_buttons - Set when the function 30 is handling the physical
+ * buttons of the trackstick (as a PD/2 passthrough device.
+ */
+struct rmi_f30_data {
+ bool buttonpad;
+ bool trackstick_buttons;
+};
+
+/**
* struct rmi_f01_power - override default power management settings.
*
*/
@@ -159,11 +171,6 @@ struct rmi_button_map {
u8 *map;
};
-struct rmi_f30_gpioled_map {
- u8 ngpioleds;
- u8 *map;
-};
-
/**
* struct rmi_device_platform_data_spi - provides parameters used in SPI
* communications. All Synaptics SPI products support a standard SPI
@@ -273,7 +280,7 @@ struct rmi_device_platform_data {
struct rmi_f01_power_management power_management;
struct rmi_button_map *f19_button_map;
struct rmi_button_map *f1a_button_map;
- struct rmi_f30_gpioled_map *gpioled_map;
+ struct rmi_f30_data *f30_data;
struct rmi_button_map *f41_button_map;
bool unified_input;
--
2.1.4
On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <[email protected]> wrote:
> From: Benjamin Tissoires <[email protected]>
>
> 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.
>
> [BT] cured the code to rely only on the unified input node created
> by rmi_driver.
>
> Signed-off-by: Andrew Duggan <[email protected]>
> Signed-off-by: Allie Xiong <[email protected]>
> Signed-off-by: Benjamin Tissoires <[email protected]>
I see this function driver is not yet adding any gpio_chip or
LEDs class devices, which is fine, we can add that later when
we have something to test. Or is iit using that LED feature
in the input layer that corresponds to caps lock etc?
I don't quite get it I guess :/
But I guess it should also be squashed into the original F30 driver.
Yours,
Linus Waleij
Hey Linus,
first thanks for the reviews. Much appreciated.
On Mon, Nov 9, 2015 at 2:32 PM, Linus Walleij <[email protected]> wrote:
> On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <[email protected]> wrote:
>
>> From: Benjamin Tissoires <[email protected]>
>>
>> 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.
>>
>> [BT] cured the code to rely only on the unified input node created
>> by rmi_driver.
>>
>> Signed-off-by: Andrew Duggan <[email protected]>
>> Signed-off-by: Allie Xiong <[email protected]>
>> Signed-off-by: Benjamin Tissoires <[email protected]>
>
> I see this function driver is not yet adding any gpio_chip or
> LEDs class devices, which is fine, we can add that later when
> we have something to test. Or is iit using that LED feature
> in the input layer that corresponds to caps lock etc?
Do not take my words as the official ones, but when we discussed with
Synaptics about F30 (and unified input), they told us that they
designed the driver based on the phone use case. In such use case, the
power (and maybe LEDs) are handled through F30, and the touchscreen
through F11/12. Problem is, I am not even sure there are phones around
with such F30/F11 combination.
So in the end, from what I can see, F30 is used for buttons on
touchpads/clickpads, and LEDs when there are some on these touchpads.
I don't know if the keyboards would use F30 for their LEDs though.
That being said. Unless Synaptics tells us that there are uses of a
non "unified" input device somewhere, I would also agree to only keep
the "unified" input node, which would simplify both F11/12 and F30.
>
> I don't quite get it I guess :/
>
> But I guess it should also be squashed into the original F30 driver.
I think this is the original F30 driver :-)
Cheers,
Benjamin
On 11/09/2015 06:06 AM, Benjamin Tissoires wrote:
> Hey Linus,
>
> first thanks for the reviews. Much appreciated.
>
> On Mon, Nov 9, 2015 at 2:32 PM, Linus Walleij <[email protected]> wrote:
>> On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <[email protected]> wrote:
>>
>>> From: Benjamin Tissoires <[email protected]>
>>>
>>> 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.
>>>
>>> [BT] cured the code to rely only on the unified input node created
>>> by rmi_driver.
>>>
>>> Signed-off-by: Andrew Duggan <[email protected]>
>>> Signed-off-by: Allie Xiong <[email protected]>
>>> Signed-off-by: Benjamin Tissoires <[email protected]>
>> I see this function driver is not yet adding any gpio_chip or
>> LEDs class devices, which is fine, we can add that later when
>> we have something to test. Or is iit using that LED feature
>> in the input layer that corresponds to caps lock etc?
F30 is currently only used in touchpads and mostly used to report the
the click of the tact switch on clickpads. When the GPIO interrupts we
report the appropriate button event to the host. Because the GPIOs are
wired to our ASIC and not to the host I'm not sure there is any benefit
to exposing it to the host using gpio_chip.
Dmitry asked a similar question and here is my response to him:
http://www.spinics.net/lists/linux-input/msg40458.html
There are a few rmi touchpads which have LEDs in the top left corner to
indicate if the touchpad has been enabled / disabled. Writing to a
register in F30 would turn on and off the LED. I don't think they are
being made anymore so I haven't really looked into how we would
implement this. If there is something in the input layer for controlling
LEDs that might be the way to do it.
> Do not take my words as the official ones, but when we discussed with
> Synaptics about F30 (and unified input), they told us that they
> designed the driver based on the phone use case. In such use case, the
> power (and maybe LEDs) are handled through F30, and the touchscreen
> through F11/12. Problem is, I am not even sure there are phones around
> with such F30/F11 combination.
>
> So in the end, from what I can see, F30 is used for buttons on
> touchpads/clickpads, and LEDs when there are some on these touchpads.
>
> I don't know if the keyboards would use F30 for their LEDs though.
>
> That being said. Unless Synaptics tells us that there are uses of a
> non "unified" input device somewhere, I would also agree to only keep
> the "unified" input node, which would simplify both F11/12 and F30.
The original architecture of the driver was to have each function have
it's own input device. The reasoning was that you would have a phone
with a touchscreen (F11) and maybe some capacitive buttons (F19 or F1A
or something) and that the system should see these as two separate
devices. I'm wondering if this is needed though. Would userspace care if
a touchscreen also reported KEY_HOME, KEY_BACK, or KEY_MENU?
At this point I agree, it's probably time to just have F11, F12, and F30
all report from the input device created in rmi_driver and remove the
non unified option. If someday there is a function which needs to report
data from a different input device it can just create it in that function.
Andrew
>> I don't quite get it I guess :/
>>
>> But I guess it should also be squashed into the original F30 driver.
> I think this is the original F30 driver :-)
>
> Cheers,
> Benjamin
On Mon, Nov 09, 2015 at 02:54:08PM -0800, Andrew Duggan wrote:
> On 11/09/2015 06:06 AM, Benjamin Tissoires wrote:
> >Hey Linus,
> >
> >first thanks for the reviews. Much appreciated.
> >
> >On Mon, Nov 9, 2015 at 2:32 PM, Linus Walleij <[email protected]> wrote:
> >>On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <[email protected]> wrote:
> >>
> >>>From: Benjamin Tissoires <[email protected]>
> >>>
> >>>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.
> >>>
> >>>[BT] cured the code to rely only on the unified input node created
> >>>by rmi_driver.
> >>>
> >>>Signed-off-by: Andrew Duggan <[email protected]>
> >>>Signed-off-by: Allie Xiong <[email protected]>
> >>>Signed-off-by: Benjamin Tissoires <[email protected]>
> >>I see this function driver is not yet adding any gpio_chip or
> >>LEDs class devices, which is fine, we can add that later when
> >>we have something to test. Or is iit using that LED feature
> >>in the input layer that corresponds to caps lock etc?
>
> F30 is currently only used in touchpads and mostly used to report
> the the click of the tact switch on clickpads. When the GPIO
> interrupts we report the appropriate button event to the host.
> Because the GPIOs are wired to our ASIC and not to the host I'm not
> sure there is any benefit to exposing it to the host using
> gpio_chip.
>
> Dmitry asked a similar question and here is my response to him:
> http://www.spinics.net/lists/linux-input/msg40458.html
>
> There are a few rmi touchpads which have LEDs in the top left corner
> to indicate if the touchpad has been enabled / disabled. Writing to
> a register in F30 would turn on and off the LED. I don't think they
> are being made anymore so I haven't really looked into how we would
> implement this. If there is something in the input layer for
> controlling LEDs that might be the way to do it.
No, it should be eventually exported as a generic LED (input core itself
now registers its capslock, etc leds with LED core and I do not plan on
adding new input LEDs).
>
> >Do not take my words as the official ones, but when we discussed with
> >Synaptics about F30 (and unified input), they told us that they
> >designed the driver based on the phone use case. In such use case, the
> >power (and maybe LEDs) are handled through F30, and the touchscreen
> >through F11/12. Problem is, I am not even sure there are phones around
> >with such F30/F11 combination.
> >
> >So in the end, from what I can see, F30 is used for buttons on
> >touchpads/clickpads, and LEDs when there are some on these touchpads.
> >
> >I don't know if the keyboards would use F30 for their LEDs though.
> >
> >That being said. Unless Synaptics tells us that there are uses of a
> >non "unified" input device somewhere, I would also agree to only keep
> >the "unified" input node, which would simplify both F11/12 and F30.
>
> The original architecture of the driver was to have each function
> have it's own input device. The reasoning was that you would have a
> phone with a touchscreen (F11) and maybe some capacitive buttons
> (F19 or F1A or something) and that the system should see these as
> two separate devices. I'm wondering if this is needed though. Would
> userspace care if a touchscreen also reported KEY_HOME, KEY_BACK, or
> KEY_MENU?
>
> At this point I agree, it's probably time to just have F11, F12, and
> F30 all report from the input device created in rmi_driver and
> remove the non unified option. If someday there is a function which
> needs to report data from a different input device it can just
> create it in that function.
Hmm.. if F30 is used for clickpads I agree that we'd want to mix it in
with the F11 data. But for generic capacitive buttons I think it would
be better if they are reported separately. Whether we want to do that
form F30 or have F30 actually export gpios and use gpio-keys to actually
create input device - I am open to discussing.
Thanks.
--
Dmitry
On Mon, Nov 9, 2015 at 11:54 PM, Andrew Duggan <[email protected]> wrote:
> F30 is currently only used in touchpads and mostly used to report the the
> click of the tact switch on clickpads. When the GPIO interrupts we report
> the appropriate button event to the host. Because the GPIOs are wired to our
> ASIC and not to the host I'm not sure there is any benefit to exposing it to
> the host using gpio_chip.
>
> Dmitry asked a similar question and here is my response to him:
> http://www.spinics.net/lists/linux-input/msg40458.html
So I think this is basically a terminology problem. There are some lines
here that the firmware calls "GPIO" (GENERAL PURPOSE input/output)
but in practice it really doesn't mean that, right?
In practice this thing is not randomly connected to doorbells, relays,
I2C bitbanged buses or whatnot. It is connected to certain keys,
and it is used for input. So it is not GPIO at all, but special purpose
input. No output.
Then it should be contained totally inside the RMI4 driver and connect
directly to the input layer.
It doesn't matter if the Synaptics firmware and/or internal documentation
calls it GPIO, if it walks and talks and acts like a duck it is a duck.
So register an input device for it and dispatch input events and be
happy.
For LEDs it makes more sense to actually register with the LEDs
subsystem as it is clearly output-only.
Yours,
Linus Walleij
On 11/09/2015 03:25 PM, Dmitry Torokhov wrote:
> On Mon, Nov 09, 2015 at 02:54:08PM -0800, Andrew Duggan wrote:
>> On 11/09/2015 06:06 AM, Benjamin Tissoires wrote:
>>> Hey Linus,
>>>
>>> first thanks for the reviews. Much appreciated.
>>>
>>> On Mon, Nov 9, 2015 at 2:32 PM, Linus Walleij <[email protected]> wrote:
>>>> On Fri, Nov 6, 2015 at 12:42 AM, Andrew Duggan <[email protected]> wrote:
>>>>
>>>>> From: Benjamin Tissoires <[email protected]>
>>>>>
>>>>> 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.
>>>>>
>>>>> [BT] cured the code to rely only on the unified input node created
>>>>> by rmi_driver.
>>>>>
>>>>> Signed-off-by: Andrew Duggan <[email protected]>
>>>>> Signed-off-by: Allie Xiong <[email protected]>
>>>>> Signed-off-by: Benjamin Tissoires <[email protected]>
>>>> I see this function driver is not yet adding any gpio_chip or
>>>> LEDs class devices, which is fine, we can add that later when
>>>> we have something to test. Or is iit using that LED feature
>>>> in the input layer that corresponds to caps lock etc?
>> F30 is currently only used in touchpads and mostly used to report
>> the the click of the tact switch on clickpads. When the GPIO
>> interrupts we report the appropriate button event to the host.
>> Because the GPIOs are wired to our ASIC and not to the host I'm not
>> sure there is any benefit to exposing it to the host using
>> gpio_chip.
>>
>> Dmitry asked a similar question and here is my response to him:
>> http://www.spinics.net/lists/linux-input/msg40458.html
>>
>> There are a few rmi touchpads which have LEDs in the top left corner
>> to indicate if the touchpad has been enabled / disabled. Writing to
>> a register in F30 would turn on and off the LED. I don't think they
>> are being made anymore so I haven't really looked into how we would
>> implement this. If there is something in the input layer for
>> controlling LEDs that might be the way to do it.
> No, it should be eventually exported as a generic LED (input core itself
> now registers its capslock, etc leds with LED core and I do not plan on
> adding new input LEDs).
>
>>> Do not take my words as the official ones, but when we discussed with
>>> Synaptics about F30 (and unified input), they told us that they
>>> designed the driver based on the phone use case. In such use case, the
>>> power (and maybe LEDs) are handled through F30, and the touchscreen
>>> through F11/12. Problem is, I am not even sure there are phones around
>>> with such F30/F11 combination.
>>>
>>> So in the end, from what I can see, F30 is used for buttons on
>>> touchpads/clickpads, and LEDs when there are some on these touchpads.
>>>
>>> I don't know if the keyboards would use F30 for their LEDs though.
>>>
>>> That being said. Unless Synaptics tells us that there are uses of a
>>> non "unified" input device somewhere, I would also agree to only keep
>>> the "unified" input node, which would simplify both F11/12 and F30.
>> The original architecture of the driver was to have each function
>> have it's own input device. The reasoning was that you would have a
>> phone with a touchscreen (F11) and maybe some capacitive buttons
>> (F19 or F1A or something) and that the system should see these as
>> two separate devices. I'm wondering if this is needed though. Would
>> userspace care if a touchscreen also reported KEY_HOME, KEY_BACK, or
>> KEY_MENU?
>>
>> At this point I agree, it's probably time to just have F11, F12, and
>> F30 all report from the input device created in rmi_driver and
>> remove the non unified option. If someday there is a function which
>> needs to report data from a different input device it can just
>> create it in that function.
> Hmm.. if F30 is used for clickpads I agree that we'd want to mix it in
> with the F11 data. But for generic capacitive buttons I think it would
> be better if they are reported separately. Whether we want to do that
> form F30 or have F30 actually export gpios and use gpio-keys to actually
> create input device - I am open to discussing.
>
> Thanks.
>
I would suggest that we get rid of the unified input option and always
create an input device in rmi_driver and that can be the default input
device which F11 and F12 can use to report events. Then in the case of
capacitive buttons we can create an input device internally in that
function driver specificity for those events so they can be reported
separately. F30 will also use the default input device in the case of
touchpads.
I don't know of any non touchpad devices which use F30 and I think it's
unlikely that there would be any. I think it's really meant for
controlling the buttons and LEDs which are on the touchpad PCB. There
probably isn't much reason to wire external GPIOs and LEDs to be
controlled by our ASIC*. If such a device existed then exporting GPIOs
with gpio_chip and using gpio-keys seems like a reasonable way to do it.
I'm just not sure we will have devices which need to use that
implementation.
* There is the one exception which is the Lenovo systems which have the
external physical stick buttons are wired to our ASIC, but that is a
special case and it's events should be forwarded to the stick's input
device instead of being reported separately using gpio-keys.
Andrew
On Wed, Nov 11, 2015 at 8:10 PM, Andrew Duggan <[email protected]> wrote:
> I don't know of any non touchpad devices which use F30 and I think it's
> unlikely that there would be any. I think it's really meant for controlling
> the buttons and LEDs which are on the touchpad PCB. There probably isn't
> much reason to wire external GPIOs and LEDs to be controlled by our ASIC*.
> If such a device existed then exporting GPIOs with gpio_chip and using
> gpio-keys seems like a reasonable way to do it. I'm just not sure we will
> have devices which need to use that implementation.
>
> * There is the one exception which is the Lenovo systems which have the
> external physical stick buttons are wired to our ASIC, but that is a special
> case and it's events should be forwarded to the stick's input device instead
> of being reported separately using gpio-keys.
We are aligned here.
Yours,
Linus Walleij