Hi Philipp, On Mon, May 07, 2018 at 03:18:23PM +0200, Philipp Puschmann wrote: > The driver supports at least the ili2511 chipset but may support other > Ilitek chipsets using Ilitek i2c protocol v3.x. > > The tested ili2511-based touchscreen delivers garbage for more than 6 > fingers while it should support up to 10 fingers. The reason is still > unclear and this remains a FIXME in the driver for now. > > The usage of pressure is optional. Touchscreens may deliver constant > and so useless pressure data. Is it dependent on model or what? I would much rather we did not have DT property for this. > > Signed-off-by: Philipp Puschmann <pp@xxxxxxxxx> > --- > .../bindings/input/touchscreen/ili251x.txt | 35 ++ > drivers/input/touchscreen/Kconfig | 12 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/ili251x.c | 350 ++++++++++++++++++ > 4 files changed, 398 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ili251x.txt > create mode 100644 drivers/input/touchscreen/ili251x.c > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/ili251x.txt b/Documentation/devicetree/bindings/input/touchscreen/ili251x.txt > new file mode 100644 > index 000000000000..f21ad93d3bdd > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/touchscreen/ili251x.txt > @@ -0,0 +1,35 @@ > +Ilitek ili251x touchscreen driver > + > +This driver uses protocol version 3 and should be compatible with other > +Ilitek touch controllers that use protocol 3.x > + > +Required properties: > + - compatible: "ili251x" > + - reg: I2C slave address of the chip (0x41) > + - interrupt-parent: a phandle pointing to the interrupt controller > + serving the interrupt for this chip > + - interrupts: interrupt specification for the touchdetect > + interrupt > + > +Optional properties: > + - reset-gpios: GPIO specification for the RESET input > + > + - pinctrl-names: should be "default" > + - pinctrl-0: a phandle pointing to the pin settings for the > + control gpios > + - max-fingers: the maximum number of fingers to handle > + - pressure: support pressure data > + - generic options : See touchscreen.txt > + > +Example: > + > + ili251x@41 { > + compatible = "ili251x"; > + reg = <0x41>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_touchpanel>; > + interrupt-parent = <&gpio5>; > + interrupts = <20 IRQ_TYPE_EDGE_FALLING>; > + reset-gpios = <&gpio5 18 GPIO_ACTIVE_HIGH>; > + max-fingers = <6>; > + }; > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index 4f15496fec8b..569528834d48 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -380,6 +380,18 @@ config TOUCHSCREEN_ILI210X > To compile this driver as a module, choose M here: the > module will be called ili210x. > > +config TOUCHSCREEN_ILI251X > + tristate "Ilitek ILI251X based touchscreen" > + depends on I2C > + help > + Say Y here if you have a ILI251X based touchscreen > + controller. This driver supports ILI2511. > + > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called ili251x. > + > config TOUCHSCREEN_IPROC > tristate "IPROC touch panel driver support" > depends on ARCH_BCM_IPROC || COMPILE_TEST > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index dddae7973436..e795b62e5f64 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -43,6 +43,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o > obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o > obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o > obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o > +obj-$(CONFIG_TOUCHSCREEN_ILI251X) += ili251x.o > obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o > obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o > obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o > diff --git a/drivers/input/touchscreen/ili251x.c b/drivers/input/touchscreen/ili251x.c > new file mode 100644 > index 000000000000..203367b59902 > --- /dev/null > +++ b/drivers/input/touchscreen/ili251x.c > @@ -0,0 +1,350 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2018, emlix GmbH. All rights reserved. */ > + > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/slab.h> > +#include <linux/input.h> > +#include <linux/input/mt.h> > +#include <linux/irq.h> > + > +#define MAX_FINGERS 10 > +#define REG_TOUCHDATA 0x10 > +#define TOUCHDATA_FINGERS 6 > +#define REG_TOUCHDATA2 0x14 > +#define TOUCHDATA2_FINGERS 4 > +#define REG_PANEL_INFO 0x20 > +#define REG_FIRMWARE_VERSION 0x40 > +#define REG_PROTO_VERSION 0x42 > +#define REG_CALIBRATE 0xcc > + > +struct finger { > + u8 x_high:6; > + u8 dummy:1; > + u8 status:1; This does not work on BE as the order of bits will be reversed. Please use masks, shifts, etc instead. > + u8 x_low; > + u8 y_high; > + u8 y_low; > + u8 pressure; > +} __packed; > + > +struct touchdata { > + u8 status; > + struct finger fingers[MAX_FINGERS]; > +} __packed; > + > +struct panel_info { > + u8 x_low; > + u8 x_high; __le16? > + u8 y_low; > + u8 y_high; __le16 > + u8 xchannel_num; > + u8 ychannel_num; > + u8 max_fingers; > +} __packed; This does not need to be packed. > + > +struct firmware_version { > + u8 id; > + u8 major; > + u8 minor; > +} __packed; > + > +struct protocol_version { > + u8 major; > + u8 minor; > +} __packed; > + > +struct ili251x_data { > + struct i2c_client *client; > + struct input_dev *input; > + unsigned int max_fingers; > + bool use_pressure; > + struct gpio_desc *reset_gpio; > +}; > + > +static int ili251x_read_reg(struct i2c_client *client, u8 reg, void *buf, > + size_t len) > +{ > + struct i2c_msg msg[2] = { > + { > + .addr = client->addr, > + .flags = 0, > + .len = 1, > + .buf = ®, > + }, > + { > + .addr = client->addr, > + .flags = I2C_M_RD, > + .len = len, > + .buf = buf, > + } > + }; > + > + if (i2c_transfer(client->adapter, msg, 2) != 2) { > + dev_err(&client->dev, "i2c transfer failed\n"); > + return -EIO; > + } > + > + return 0; > +} > + > +static void ili251x_report_events(struct ili251x_data *data, > + const struct touchdata *touchdata) > +{ > + struct input_dev *input = data->input; > + unsigned int i; > + bool touch; > + unsigned int x, y; > + const struct finger *finger; > + unsigned int reported_fingers = 0; > + > + /* the touch chip does not count the real fingers but switches between > + * 0, 6 and 10 reported fingers * > + * > + * FIXME: With a tested ili2511 we received only garbage for fingers > + * 6-9. As workaround we add a device tree option to limit the > + * handled number of fingers > + */ > + if (touchdata->status == 1) > + reported_fingers = 6; > + else if (touchdata->status == 2) > + reported_fingers = 10; > + > + for (i = 0; i < reported_fingers && i < data->max_fingers; i++) { > + input_mt_slot(input, i); > + > + finger = &touchdata->fingers[i]; > + > + touch = finger->status; > + input_mt_report_slot_state(input, MT_TOOL_FINGER, touch); > + x = finger->x_low | (finger->x_high << 8); > + y = finger->y_low | (finger->y_high << 8); > + > + if (touch) { > + input_report_abs(input, ABS_MT_POSITION_X, x); > + input_report_abs(input, ABS_MT_POSITION_Y, y); > + if (data->use_pressure) > + input_report_abs(input, ABS_MT_PRESSURE, > + finger->pressure); > + > + } > + } > + > + input_mt_report_pointer_emulation(input, false); > + input_sync(input); > +} > + > +static irqreturn_t ili251x_irq(int irq, void *irq_data) > +{ > + struct ili251x_data *data = irq_data; > + struct i2c_client *client = data->client; > + struct touchdata touchdata; > + int error; > + > + error = ili251x_read_reg(client, REG_TOUCHDATA, > + &touchdata, > + sizeof(touchdata) - > + sizeof(struct finger)*TOUCHDATA2_FINGERS); > + > + if (!error && touchdata.status == 2 && data->max_fingers > 6) > + error = ili251x_read_reg(client, REG_TOUCHDATA2, > + &touchdata.fingers[TOUCHDATA_FINGERS], > + sizeof(struct finger)*TOUCHDATA2_FINGERS); > + > + if (!error) > + ili251x_report_events(data, &touchdata); > + else > + dev_err(&client->dev, > + "Unable to get touchdata, err = %d\n", error); > + > + return IRQ_HANDLED; > +} > + > +static void ili251x_reset(struct ili251x_data *data) > +{ > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); > + usleep_range(50, 100); > + gpiod_set_value(data->reset_gpio, 0); > + msleep(100); > + } > +} > + > +static int ili251x_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct ili251x_data *data; > + struct input_dev *input; > + struct panel_info panel; > + struct device_node *np = dev->of_node; > + struct firmware_version firmware; > + struct protocol_version protocol; > + int xmax, ymax; > + int error; > + > + dev_dbg(dev, "Probing for ili251x I2C Touschreen driver"); > + > + if (client->irq <= 0) { > + dev_err(dev, "No IRQ!\n"); > + return -EINVAL; > + } > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + input = devm_input_allocate_device(dev); > + if (!data || !input) Please use separate tests here. > + return -ENOMEM; > + > + data->client = client; > + data->input = input; > + data->use_pressure = false; > + > + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(data->reset_gpio)) { > + error = PTR_ERR(data->reset_gpio); > + if (error != -EPROBE_DEFER) > + dev_err(dev, > + "Failed to get reset GPIO: %d\n", error); > + return error; > + } > + > + ili251x_reset(data); > + > + error = ili251x_read_reg(client, REG_FIRMWARE_VERSION, > + &firmware, sizeof(firmware)); > + if (error) { > + dev_err(dev, "Failed to get firmware version, err: %d\n", > + error); > + return error; > + } > + > + error = ili251x_read_reg(client, REG_PROTO_VERSION, > + &protocol, sizeof(protocol)); > + if (error) { > + dev_err(dev, "Failed to get protocol version, err: %d\n", > + error); > + return error; > + } > + if (protocol.major != 3) { > + dev_err(dev, "This driver expects protocol version 3.x, Chip uses: %d\n", > + protocol.major); > + return -EINVAL; > + } > + > + error = ili251x_read_reg(client, REG_PANEL_INFO, &panel, sizeof(panel)); > + if (error) { > + dev_err(dev, "Failed to get panel information, err: %d\n", > + error); > + return error; > + } > + > + data->max_fingers = panel.max_fingers; > + if (np) { > + int max_fingers; > + > + error = of_property_read_u32(np, "max-fingers", &max_fingers); > + if (!error && max_fingers < data->max_fingers) > + data->max_fingers = max_fingers; > + > + if (of_property_read_bool(np, "pressure")) > + data->use_pressure = true; > + } > + > + xmax = panel.x_low | (panel.x_high << 8); > + ymax = panel.y_low | (panel.y_high << 8); > + > + /* Setup input device */ > + input->name = "ili251x Touchscreen"; > + input->id.bustype = BUS_I2C; > + input->dev.parent = dev; This is done by devm_input_allocate_device(), please drop. > + > + __set_bit(EV_SYN, input->evbit); > + __set_bit(EV_KEY, input->evbit); > + __set_bit(EV_ABS, input->evbit); > + __set_bit(BTN_TOUCH, input->keybit); > + > + /* Single touch */ > + input_set_abs_params(input, ABS_X, 0, xmax, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, ymax, 0, 0); > + > + /* Multi touch */ > + input_mt_init_slots(input, data->max_fingers, 0); Error handling please. The touchscreens should be marked as INPUT_MT_DIRECT. > + input_set_abs_params(input, ABS_MT_POSITION_X, 0, xmax, 0, 0); > + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ymax, 0, 0); > + if (data->use_pressure) > + input_set_abs_params(input, ABS_MT_PRESSURE, 0, U8_MAX, 0, 0); You want to set up absolute axes before you call input_mt_init_slots() so that single-touch emulation is set up properly. > + > + i2c_set_clientdata(client, data); > + > + error = devm_request_threaded_irq(dev, client->irq, NULL, ili251x_irq, > + IRQF_ONESHOT, client->name, data); > + > + if (error) { > + dev_err(dev, "Unable to request touchscreen IRQ, err: %d\n", > + error); > + return error; > + } > + > + error = input_register_device(data->input); > + if (error) { > + dev_err(dev, "Cannot register input device, err: %d\n", error); > + return error; > + } > + > + device_init_wakeup(dev, 1); > + > + dev_info(dev, > + "ili251x initialized (IRQ: %d), firmware version %d.%d.%d fingers %d", > + client->irq, firmware.id, firmware.major, firmware.minor, > + data->max_fingers); dve_dbg() if you really need this, but I would rather drop. > + > + return 0; > +} > + > +static int __maybe_unused ili251x_i2c_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + > + if (device_may_wakeup(&client->dev)) > + enable_irq_wake(client->irq); > + > + return 0; > +} > + > +static int __maybe_unused ili251x_i2c_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + > + if (device_may_wakeup(&client->dev)) > + disable_irq_wake(client->irq); > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(ili251x_i2c_pm, > + ili251x_i2c_suspend, ili251x_i2c_resume); All this boilerplate can be removed if you use dev_pm_set_wake_irq() in probe(). > + > +static const struct i2c_device_id ili251x_i2c_id[] = { > + { "ili251x", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ili251x_i2c_id); > + > +static struct i2c_driver ili251x_ts_driver = { > + .driver = { > + .name = "ili251x_i2c", > + .pm = &ili251x_i2c_pm, > + }, > + .id_table = ili251x_i2c_id, > + .probe = ili251x_i2c_probe, > +}; > + > +module_i2c_driver(ili251x_ts_driver); > + > +MODULE_AUTHOR("Philipp Puschmann <pp@xxxxxxxxx>"); > +MODULE_DESCRIPTION("ili251x I2C Touchscreen Driver"); > +MODULE_LICENSE("GPL"); > -- > 2.17.0 > Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html