Input: ili251x - add support for Ilitek ILI251x touchscreens
Subsystems (3)
PATCH 1/1 Input: ili251x - add support for Ilitek ILI251x touchscreens
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.
--- .../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
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +35 -0
@@ -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>; + };
drivers/input/touchscreen/Kconfig +12 -0
@@ -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
drivers/input/touchscreen/Makefile +1 -0
@@ -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
drivers/input/touchscreen/ili251x.c new file +350 -0
350 lines not on this page · raw message
Signature · 1 line
2.17.0
Rob Herring
maintainerQuoted 7 lines · Philipp Puschmann
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.
Quoted 6 lines
The usage of pressure is optional. Touchscreens may deliver constant and so useless pressure data.
Signed-off-by Philipp Puschmann <[email protected]>--- .../bindings/input/touchscreen/ili251x.txt | 35 ++
Please make the binding a separate patch.
Quoted 13 lines
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.cDocumentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +0
-0
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt line 1
Quoted 1 line
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +1
-0@@ -0,0 +1,35 @@ +Ilitek ili251x touchscreen driver
Bindings are not drivers.
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt line 7
Quoted 6 lines
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +6
-0+ +This driver uses protocol version 3 and should be compatible with other +Ilitek touch controllers that use protocol 3.x + +Required properties: + - compatible: "ili251x"
Needs a vendor prefix.
Don't use wildcards in compatible strings. Maybe you want to incorporate the protocol version into the compatible string.
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt line 20
Quoted 13 lines
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +13
-0+ - 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
We already have 'silead,max-fingers', so probably time for a common property.
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt line 21
Quoted 1 line
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +1
-0+ - pressure: support pressure data
This should be implied by the compatible string.
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt line 26
Quoted 5 lines
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +5
-0+ - generic options : See touchscreen.txt + +Example: + + ili251x@41 {
touchscreen@41
Quoted 9 lines
Documentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +9
-0+ 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>; + };
Hi Philipp,
I had a fast look to your driver and I have few comments.
Quoted 7 lines
.../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
Please split the patch, the bindig should be on a separate patch and must come before the driver.
drivers/input/touchscreen/ili251x.c line 22
Quoted 9 lines
drivers/input/touchscreen/ili251x.c new file +9
-0+#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
Can you please group and sort these definitions by type? REGs with REGs and others together?
Please start the defines names with a unique identifier, ILI251X_REG_* instead of just REG_*
Quoted 25 lines
drivers/input/touchscreen/ili251x.c new file +25
-0+struct finger { +u8 x_high:6 ; +u8 dummy:1 ; +u8 status:1 ; +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; +u8 y_low; +u8 y_high; +u8 xchannel_num; +u8 ychannel_num; +u8 max_fingers; +} __packed; +
drivers/input/touchscreen/ili251x.c line 58
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+struct firmware_version { +u8 id; +u8 major; +u8 minor; +} __packed; + +struct protocol_version { +u8 major; +u8 minor; +} __packed;
panel_info, firmware_version and protocol_version are used very little in the driver (just in probe). Is it really necessary to keep a definition?
Is there a way to get rid of them?
drivers/input/touchscreen/ili251x.c line 66
Quoted 7 lines
drivers/input/touchscreen/ili251x.c new file +7
-0+struct ili251x_data { +struct i2c_client *client; +struct input_dev *input; +unsigned int max_fingers; +bool use_pressure; +struct gpio_desc *reset_gpio; +};
Please start also the strct definitions with the unique identifier ili251x_* like you did with ili251x_data.
Quoted 16 lines
drivers/input/touchscreen/ili251x.c new file +16
-0+ +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,
drivers/input/touchscreen/ili251x.c line 92
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+ } + }; + +if (i2c_transfer (client->adapter, msg,2 ) !=2 ) { +dev_err (&client->dev,"i2c transfer failed\n" ); +return -EIO ; + } + +return 0 ; +}
I do not see the need for a ili251x_read_reg function. You are not reading more than 240 bytes per time, am I right?
In this case I would use the smbus functions (at least whenever possible in case I miscalculated the 240b), this is ju a duplicated code.
Quoted 21 lines
drivers/input/touchscreen/ili251x.c new file +21
-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 ;
drivers/input/touchscreen/ili251x.c line 124
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+ +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 );
the x and y calculation can go uinside the if() below, right?
drivers/input/touchscreen/ili251x.c line 133
Quoted 8 lines
drivers/input/touchscreen/ili251x.c new file +8
-0+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); + + }
just a small nitpick, that is more a matter of preference, with
if(!touch)
continue;
we save a level of indentation.
Dmitry Torokhov
maintainerHi Philipp,
Quoted 9 lines · Philipp Puschmann
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.
Quoted 110 lines
Signed-off-by Philipp Puschmann <[email protected]>--- .../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.cDocumentation/devicetree/bindings/input/touchscreen/ili251x.txt new file +35
-0@@ -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>; + };drivers/input/touchscreen/Kconfig +12
-0@@ -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_TESTdrivers/input/touchscreen/Makefile +1
-0@@ -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.odrivers/input/touchscreen/ili251x.c new file +17
-0@@ -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
drivers/input/touchscreen/ili251x.c line 27
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+#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.
Quoted 4 lines
drivers/input/touchscreen/ili251x.c new file +4
-0+u8 x_low; +u8 y_high; +u8 y_low; +u8 pressure;
drivers/input/touchscreen/ili251x.c line 41
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+} __packed; + +struct touchdata { +u8 status; +struct finger fingers[MAX_FINGERS ]; +} __packed; + +struct panel_info { +u8 x_low; +u8 x_high;
__le16?
drivers/input/touchscreen/ili251x.c line 43
Quoted 2 lines
drivers/input/touchscreen/ili251x.c new file +2
-0+u8 y_low; +u8 y_high;
__le16
drivers/input/touchscreen/ili251x.c line 47
Quoted 4 lines
drivers/input/touchscreen/ili251x.c new file +4
-0+u8 xchannel_num; +u8 ychannel_num; +u8 max_fingers; +} __packed;
This does not need to be packed.
Quoted 141 lines
drivers/input/touchscreen/ili251x.c new file +141
-0+ +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; +
drivers/input/touchscreen/ili251x.c line 198
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+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.
Quoted 55 lines
drivers/input/touchscreen/ili251x.c new file +55
-0+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" ))
drivers/input/touchscreen/ili251x.c line 263
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+ 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.
drivers/input/touchscreen/ili251x.c line 275
Quoted 12 lines
drivers/input/touchscreen/ili251x.c new file +12
-0+ +__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.
drivers/input/touchscreen/ili251x.c line 279
Quoted 4 lines
drivers/input/touchscreen/ili251x.c new file +4
-0+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.
Quoted 14 lines
drivers/input/touchscreen/ili251x.c new file +14
-0+ +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) {
drivers/input/touchscreen/ili251x.c line 303
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+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.
Quoted 16 lines
drivers/input/touchscreen/ili251x.c new file +16
-0+ +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) +{
drivers/input/touchscreen/ili251x.c line 329
Quoted 10 lines
drivers/input/touchscreen/ili251x.c new file +10
-0+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().
Quoted 24 lines
drivers/input/touchscreen/ili251x.c new file +21
-0+ +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 <[email protected]>" ); +MODULE_DESCRIPTION ("ili251x I2C Touchscreen Driver" ); +MODULE_LICENSE ("GPL" );Signature · 1 line
2.17.0
Thanks.
Signature · 1 line
Dmitry
Hi Dmitry,
hi Rob,
hi Andi,
thank you for your replies and your helpful hints. I will take care of them and send a V2 of this driver + dt-bindings soon. As i have a working version of this driver working on it has not a very priority so it may need some time.
Most of your points are correct and don't need further discussion. For the pressure data i'm unsure, see below.
Am 10.05.2018 um 00:41 schrieb Dmitry Torokhov:
Quoted 5 lines
Quoted 2 lines
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.
I don't know. My touchscreen here delivers a constant (useless) pressure of
10. So my chip delivers "something". I'm not sure if the chip always delivers no real pressure data or if this is dependent from the wiring. This was the reason i put it in the dt.
Thanks
Philipp
Signature · 11 lines
Bitte beachten Sie unsere neue Adresse am Göttinger Standort: emlix GmbH, Gothaer Platz 3, 37083 Göttingen Philipp Puschmann, emlix GmbH, http://www.emlix.com Fon +49 551 30664-0, Fax +49 551 30664-11 Gothaer Platz 3, 37083 Göttingen, Germany Sitz der Gesellschaft: Göttingen, Amtsgericht Goettingen HR B 3160 Geschaeftsführung: Heike Jordan, Dr. Uwe Kracke Ust-IdNr.: DE 205 198 055 emlix - smart embedded open source