Hi Joe,

On Mon, Mar 22, 2021 at 06:41:31PM +0800, Joe Hung wrote:

Add support for ILITEK Lego series of touch devices.
Lego series includes ILITEK 213X/23XX/25XX.

Tested/passed with evaluation board with ILI2520/2322 IC.

The driver looks pretty good, a few more comments though.

Are these needed? Do you plan bringing in V3 protocol support?

+
+#define MOD_AP						0x5A
+#define MOD_BL						0x55

These are not used?

+
+struct ilitek_protocol_info {
+	u16 ver;
+	u8 ver_major;
+	u8 flag;
+};
+
+struct ilitek_touch_info {
+	u16 id;
+	u16 x;
+	u16 y;
+	u16 p;
+	u16 w;
+	u16 h;
+	u16 status;
+};
+
+struct ilitek_ts_data {
+	struct i2c_client		*client;
+	struct gpio_desc		*reset_gpio;
+	struct input_dev		*input_dev;
+	struct touchscreen_properties	prop;
+
+	int (*process_and_report)(struct ilitek_ts_data *ts);

Until the driver supports multiple protocol versions there is no point
in having this pointer.

+
+	struct PROTOCOL_MAP		*ptl_cb_func;
+	struct ilitek_protocol_info	ptl;
+
+	struct ilitek_touch_info	tpinfo[ILITEK_SUPPORT_MAX_POINT];
+
+	char				product_id[30];
+	u16				mcu_ver;
+	u8				ic_mode;
+	u8				firmware_ver[8];
+
+	s32				reset_time;
+	s32				screen_max_x;
+	s32				screen_max_y;
+	s32				screen_min_x;
+	s32				screen_min_y;
+	s32				max_tp;
+};
+
+enum ilitek_cmds {
+	/* common cmds */
+	GET_PTL_VER = 0,
+	GET_FW_VER,
+	GET_SCRN_RES,
+	GET_TP_RES,
+	GET_IC_MODE,
+	GET_MCU_VER,
+	SET_IC_SLEEP,
+	SET_IC_WAKE,
+
+	/* ALWAYS keep at the end */
+	MAX_CMD_CNT
+};
+
+typedef int protocol_func(struct ilitek_ts_data *ts,
+			  u16 cmd, u8 *inbuf, u8 *outbuf);
+
+struct PROTOCOL_MAP {
+	u16 cmd;
+	const char *name;
+	protocol_func *func;
+	u8 flag;
+};
+
+/* ILITEK I2C R/W APIs */
+static int ilitek_i2c_write_and_read(struct ilitek_ts_data *ts,
+				     u8 *cmd, int write_len, int delay,
+				     u8 *data, int read_len)
+{
+	int ret = 0;
+	struct i2c_client *client = ts->client;
+	struct i2c_msg msgs[2] = {
+		{.addr = client->addr, .flags = 0,
+		 .len = write_len, .buf = cmd,},
+		{.addr = client->addr, .flags = I2C_M_RD,
+		 .len = read_len, .buf = data,}
+	};
+
+	if (delay == 0 && write_len > 0 && read_len > 0) {
+		ret = i2c_transfer(client->adapter, msgs, 2);
+		if (ret < 0)
+			return ret;
+	} else {
+		if (write_len > 0) {
+			ret = i2c_transfer(client->adapter, msgs, 1);
+			if (ret < 0)
+				return ret;
+		}
+		if (delay > 0)
+			mdelay(delay);
+		if (read_len > 0) {
+			ret = i2c_transfer(client->adapter, msgs + 1, 1);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+/* ILITEK ISR APIs */
+static void ilitek_touch_down(struct ilitek_ts_data *ts,
+			      int id, int x, int y, int p, int h, int w)
+{
+	struct input_dev *input = ts->input_dev;
+
+	input_report_key(input, BTN_TOUCH, 1);
+
+	input_mt_slot(input, id);
+	input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+
+	touchscreen_report_pos(input, &ts->prop, x, y, true);
+
+	ts->tpinfo[id].status = 1;
+	ts->tpinfo[id].id = id;
+	ts->tpinfo[id].x = x;
+	ts->tpinfo[id].y = y;
+	ts->tpinfo[id].p = p;
+	ts->tpinfo[id].h = h;
+	ts->tpinfo[id].w = w;
+}
+
+static void ilitek_touch_release(struct ilitek_ts_data *ts, int id)
+{
+	struct input_dev *input = ts->input_dev;
+
+	if (ts->tpinfo[id].status == 1) {
+		input_mt_slot(input, id);
+		input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
+		ts->tpinfo[id].status = 0;
+	}
+}
+
+static void ilitek_touch_release_all(struct ilitek_ts_data *ts)
+{
+	struct input_dev *input = ts->input_dev;
+	int i = 0;
+
+	for (i = 0; i < ts->max_tp; i++)
+		ilitek_touch_release(ts, i);
+	input_report_key(input, BTN_TOUCH, 0);
+}
+
+static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
+{
+	int ret = 0;
+	u8 buf[512] = {0};
+	int packet_len = 5;
+	int packet_max_point = 10;
+	int report_max_point = 6;
+	int release_point = 0;
+	int i = 0, count = 0;
+	struct input_dev *input = ts->input_dev;
+	struct device *dev = &ts->client->dev;
+	u16 x, y, p = 10, w = 10, h = 10, status, id;
+
+	ret = ilitek_i2c_write_and_read(ts, NULL, 0, 0, buf, 64);
+	if (ret < 0) {
+		dev_err(dev, "get touch info failed, err:%d\n", ret);
+		goto err_release_touch_and_key;
+	}
+
+	report_max_point = buf[REPORT_ADDRESS_COUNT];
+	if (report_max_point > ts->max_tp) {
+		dev_err(dev, "FW report max point:%d > panel info. max:%d\n",
+			report_max_point, ts->max_tp);
+		ret = -EINVAL;
+		goto err_release_touch_and_key;
+	}
+
+	count = DIV_ROUND_UP(report_max_point, packet_max_point);
+	for (i = 1; i < count; i++) {
+		ret = ilitek_i2c_write_and_read(ts, NULL, 0, 0,
+			buf + i * 64, 64);
+		if (ret < 0) {
+			dev_err(dev, "get touch info. err, count=%d\n", count);
+			goto err_release_touch_and_key;
+		}
+	}
+
+	for (i = 0; i < report_max_point; i++) {
+		status = buf[i * packet_len + 1] & 0x40;
+		id = buf[i * packet_len + 1] & 0x3F;
+
+		if (!status) {
+			release_point++;
+			ilitek_touch_release(ts, id);
+			continue;
+		}
+
+		x = get_unaligned_le16(buf + i * packet_len + 2);
+		y = get_unaligned_le16(buf + i * packet_len + 4);
+
+		if (x > ts->screen_max_x || x < ts->screen_min_x ||
+		    y > ts->screen_max_y || y < ts->screen_min_y) {
+			dev_warn(dev, "invalid position, X[%d,%d,%d], Y[%d,%d,%d]\n",
+				 ts->screen_min_x, x, ts->screen_max_x,
+				 ts->screen_min_y, y, ts->screen_max_y);
+			continue;
+		}
+		ilitek_touch_down(ts, id, x, y, p, h, w);
+	}
+
+err_release_touch_and_key:
+	if (ret < 0 || release_point == report_max_point)
+		ilitek_touch_release_all(ts);

If you use input_mt_sync_frame() here you will not need to are about
releasing BTN_TOUCH or doing single-touch emulation.

+	input_sync(input);
+	return (ret < 0) ? ret : 0;
+}
+
+
+/* APIs of cmds for ILITEK Touch IC */
+static int api_protocol_set_cmd(struct ilitek_ts_data *ts,
+				u16 idx, u8 *inbuf, u8 *outbuf)
+{
+	u16 cmd;
+	int ret = 0;
+
+	if (idx >= MAX_CMD_CNT)
+		return -EINVAL;
+
+	/* check if cmd is supported by its protocol version */
+	if (!(ts->ptl.flag & ts->ptl_cb_func[idx].flag))
+		return -EINVAL;
+
+	cmd = ts->ptl_cb_func[idx].cmd;
+	ret = ts->ptl_cb_func[idx].func(ts, cmd, inbuf, outbuf);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int api_protocol_get_ptl_ver(struct ilitek_ts_data *ts,
+				    u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 3);
+	if (ret < 0)
+		return ret;
+
+	ts->ptl.ver = get_unaligned_be16(outbuf);
+	ts->ptl.ver_major = outbuf[0];
+
+	return 0;
+}
+
+static int api_protocol_get_mcu_ver(struct ilitek_ts_data *ts,
+				    u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 32);
+	if (ret < 0)
+		return ret;
+
+	ts->mcu_ver = get_unaligned_le16(outbuf);
+	memset(ts->product_id, 0, sizeof(ts->product_id));
+	memcpy(ts->product_id, outbuf+6, 26);
+
+	return 0;
+}
+
+static int api_protocol_get_fw_ver(struct ilitek_ts_data *ts,
+				   u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 8);
+	if (ret < 0)
+		return ret;
+
+	memcpy(ts->firmware_ver, outbuf, 8);
+
+	return 0;
+}
+
+static int api_protocol_get_scrn_res(struct ilitek_ts_data *ts,
+				     u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 8);
+	if (ret < 0)
+		return ret;
+
+	ts->screen_min_x = get_unaligned_le16(outbuf);
+	ts->screen_min_y = get_unaligned_le16(outbuf + 2);
+	ts->screen_max_x = get_unaligned_le16(outbuf + 4);
+	ts->screen_max_y = get_unaligned_le16(outbuf + 6);
+
+	return 0;
+}
+
+static int api_protocol_get_tp_res(struct ilitek_ts_data *ts,
+				   u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 15);
+	if (ret < 0)
+		return ret;
+
+	ts->max_tp = outbuf[8];
+
+	if (ts->max_tp > ILITEK_SUPPORT_MAX_POINT) {
+		dev_err(&ts->client->dev, "Invalid MAX_TP:%d from FW\n",
+			ts->max_tp);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int api_protocol_get_ic_mode(struct ilitek_ts_data *ts,
+				    u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	int ret = 0;
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	ret = ilitek_i2c_write_and_read(ts, buf, 1, 5, outbuf, 2);
+	if (ret < 0)
+		return ret;
+
+	ts->ic_mode = outbuf[0];
+	return 0;
+}
+
+static int api_protocol_set_ic_sleep(struct ilitek_ts_data *ts,
+				     u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	return ilitek_i2c_write_and_read(ts, buf, 1, 0, NULL, 0);
+}
+
+static int api_protocol_set_ic_wake(struct ilitek_ts_data *ts,
+				    u16 cmd, u8 *inbuf, u8 *outbuf)
+{
+	u8 buf[64] = {0};
+
+	buf[0] = cmd;
+	return ilitek_i2c_write_and_read(ts, buf, 1, 0, NULL, 0);
+}
+
+struct PROTOCOL_MAP ptl_func_map[] = {
+	/* common cmds */
+	[GET_PTL_VER]	= {ILITEK_TP_CMD_GET_PRL_VER, "GET_PTL_VER",
+			   api_protocol_get_ptl_ver, PTL_V3 | PTL_V6},
+	[GET_FW_VER]	= {ILITEK_TP_CMD_GET_FW_VER, "GET_FW_VER",
+			   api_protocol_get_fw_ver, PTL_V3 | PTL_V6},
+	[GET_SCRN_RES]	= {ILITEK_TP_CMD_GET_SCRN_RES, "GET_SCRN_RES",
+			   api_protocol_get_scrn_res, PTL_V3 | PTL_V6},
+	[GET_TP_RES]	= {ILITEK_TP_CMD_GET_TP_RES, "GET_TP_RES",
+			   api_protocol_get_tp_res, PTL_V3 | PTL_V6},
+	[GET_IC_MODE]	= {ILITEK_TP_CMD_GET_IC_MODE, "GET_IC_MODE",
+			   api_protocol_get_ic_mode, PTL_V3 | PTL_V6},
+	[GET_MCU_VER]	= {ILITEK_TP_CMD_GET_MCU_VER, "GET_MOD_VER",
+			   api_protocol_get_mcu_ver, PTL_V3 | PTL_V6},
+	[SET_IC_SLEEP]	= {ILITEK_TP_CMD_SET_IC_SLEEP, "SET_IC_SLEEP",
+			   api_protocol_set_ic_sleep, PTL_V3 | PTL_V6},
+	[SET_IC_WAKE]	= {ILITEK_TP_CMD_SET_IC_WAKE, "SET_IC_WAKE",
+			   api_protocol_set_ic_wake, PTL_V3 | PTL_V6},
+};
+
+/* Probe APIs */
+static void ilitek_reset(struct ilitek_ts_data *ts, int delay, bool boot)
+{
+	if (!boot)
+		disable_irq_nosync(ts->client->irq);

I am confused about this flag. ilitek_reset() is called from probe and
resume handlers, and in resume interrupts are already disabled, so why
are you trying to disable them again here?

+
+	if (ts->reset_gpio) {
+		gpiod_set_value(ts->reset_gpio, 1);
+		mdelay(10);
+		gpiod_set_value(ts->reset_gpio, 0);
+		mdelay(delay);
+	}
+
+	if (!boot)
+		enable_irq(ts->client->irq);
+}
+
+static int ilitek_protocol_init(struct ilitek_ts_data *ts)
+{
+	int ret = 0;
+	u8 outbuf[64] = {0};
+
+	ts->ptl.flag = PTL_V6;
+	ts->ptl_cb_func = ptl_func_map;
+	ts->process_and_report = ilitek_process_and_report_v6;
+	ts->reset_time = 600;
+
+	ret = api_protocol_set_cmd(ts, GET_PTL_VER, NULL, outbuf);
+	if (ret < 0)
+		return ret;
+
+	/* Protocol v3 is not support currently */
+	if (ts->ptl.ver_major == 0x3 ||
+	    ts->ptl.ver == BL_V1_6 ||
+	    ts->ptl.ver == BL_V1_7)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ilitek_read_tp_info(struct ilitek_ts_data *ts, bool boot)
+{
+	u8 outbuf[256] = {0};
+	int error = 0;
+
+	error = api_protocol_set_cmd(ts, GET_PTL_VER, NULL, outbuf);
+	if (error < 0)
+		return error;

api_protocol_set_cmd() returns normalized (-<rc>, 0], i.e. it is never
positive, so simply

	if (error)
		return error;

+
+	error = api_protocol_set_cmd(ts, GET_MCU_VER, NULL, outbuf);
+	if (error < 0)
+		return error;
+
+	error = api_protocol_set_cmd(ts, GET_FW_VER, NULL, outbuf);
+	if (error < 0)
+		return error;
+
+	if (boot) {
+		error = api_protocol_set_cmd(ts, GET_SCRN_RES, NULL,
+					     outbuf);
+		if (error < 0)
+			return error;
+	}
+
+	error = api_protocol_set_cmd(ts, GET_TP_RES, NULL, outbuf);
+	if (error < 0)
+		return error;
+
+	error = api_protocol_set_cmd(ts, GET_IC_MODE, NULL, outbuf);
+	if (error < 0)
+		return error;
+
+	return 0;
+}
+
+static int ilitek_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
+{
+	int ret = 0;
+	struct input_dev *input = NULL;

No need to initialize variables like this, it only hurts, as otherwise
compiler can warn you if you forger to assign a value to a variable and
try to use it.

+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	ts->input_dev = input;
+	input->name = ILITEK_TS_NAME;
+	input->id.bustype = BUS_I2C;
+
+	__set_bit(INPUT_PROP_DIRECT, input->propbit);
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);

I do not think anything of this besides INPUT_PROP_DIRECT is needed, as
input_set_abs_params, and input_mt_init_slots will set needed
capabilities.

+
+	/* Single touch input setup */
+	input_set_abs_params(input, ABS_X, ts->screen_min_x,
+			     ts->screen_max_x, 0, 0);
+	input_set_abs_params(input, ABS_Y, ts->screen_min_y,
+			     ts->screen_max_y, 0, 0);
+
+	/* Multi-touch input setup */
+	input_set_abs_params(input, ABS_MT_POSITION_X,
+			     ts->screen_min_x,
+			     ts->screen_max_x, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_Y,
+			     ts->screen_min_y,
+			     ts->screen_max_y, 0, 0);
+
+	touchscreen_parse_properties(input, true, &ts->prop);
+
+	ret = input_mt_init_slots(input, ts->max_tp, INPUT_MT_DIRECT);

I prefer variables that hold error codes to be called "error".

Also, if you specify INPUT_MT_DROP_UNUSED as an additional flag for
input_mt_init_slots() and use input_mt_sync_frame() when reporting
events you would not need to track the state of contacts in your driver
and explicitly release them.

+	if (ret) {
+		dev_err(dev, "failed to initialize MT slots, err:%d\n", ret);
+		return ret;
+	}
+
+	ret = input_register_device(input);
+	if (ret) {
+		dev_err(dev, "failed to register input device, err:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static irqreturn_t ilitek_i2c_isr(int irq, void *dev_id)
+{
+	struct ilitek_ts_data *ts = dev_id;
+	int ret = 0;
+
+	ret = ts->process_and_report(ts);
+
+	if (ret < 0) {
+		dev_err(&ts->client->dev, "[%s] err:%d\n", __func__, ret);
+		return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t firmware_version_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ilitek_ts_data *ts = i2c_get_clientdata(client);
+
+	return scnprintf(buf, PAGE_SIZE,
+			 "fw version: [%02X%02X.%02X%02X.%02X%02X.%02X%02X]\n",
+			 ts->firmware_ver[0], ts->firmware_ver[1],
+			 ts->firmware_ver[2], ts->firmware_ver[3],
+			 ts->firmware_ver[4], ts->firmware_ver[5],
+			 ts->firmware_ver[6], ts->firmware_ver[7]);
+}
+static DEVICE_ATTR_RO(firmware_version);
+
+static ssize_t product_id_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ilitek_ts_data *ts = i2c_get_clientdata(client);
+
+	return scnprintf(buf, PAGE_SIZE, "product id: [%04X], module: [%s]\n",
+			 ts->mcu_ver, ts->product_id);
+}
+static DEVICE_ATTR_RO(product_id);
+
+static struct attribute *ilitek_sysfs_attrs[] = {
+	&dev_attr_firmware_version.attr,
+	&dev_attr_product_id.attr,
+	NULL
+};
+
+static struct attribute_group ilitek_attrs_group[] = {
+	{.attrs = ilitek_sysfs_attrs},
+};
+
+static int ilitek_ts_i2c_probe(struct i2c_client *client,
+			       const struct i2c_device_id *id)
+{
+	struct ilitek_ts_data *ts = NULL;
+	struct device *dev = &client->dev;
+	int ret = 0;

Please drop unneeded initializations, s/ret/error/

+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(dev, "%s: i2c check functionality failed\n",
+			ILITEK_TS_NAME);

I do not think you need to have ILITEK_TS_NAME as device and driver name
are already printed by dev_err.

+		return -ENXIO;
+	}
+
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	ts->client = client;
+	i2c_set_clientdata(client, ts);
+
+	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);

Make it devm_gpiod_get_optional() as you already handle it being not
present in ilitek_reset().

+	if (IS_ERR(ts->reset_gpio)) {
+		ret = PTR_ERR(ts->reset_gpio);
+		dev_err(dev, "request gpiod failed, err:%d", ret);
+		return ret;
+	}
+
+	ilitek_reset(ts, 1000, true);
+	ret = ilitek_protocol_init(ts);
+	if (ret < 0) {
+		dev_err(dev, "protocol init failed, err:%d", ret);
+		return ret;
+	}
+
+	ret = ilitek_read_tp_info(ts, true);
+	if (ret < 0) {
+		dev_err(dev, "read tp info failed, err:%d", ret);
+		return ret;
+	}
+
+	ret = ilitek_input_dev_init(dev, ts);
+	if (ret < 0) {
+		dev_err(dev, "input dev init failed, err:%d", ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(dev, ts->client->irq, NULL,
+					ilitek_i2c_isr, IRQF_ONESHOT,
+					"ilitek_touch_irq", ts);
+	if (ret) {
+		dev_err(dev, "request threaded irq failed, err:%d\n", ret);
+		return ret;
+	}
+
+	ret = devm_device_add_group(dev, ilitek_attrs_group);
+	if (ret) {
+		dev_err(dev, "sysfs create group failed, err:%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __maybe_unused ilitek_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ilitek_ts_data *ts = i2c_get_clientdata(client);
+	int error = 0;
+
+	disable_irq(client->irq);
+
+	if (device_may_wakeup(dev)) {
+		enable_irq_wake(client->irq);

You do not need to enable irq wake here, i2c core will do it for you.

+	} else {
+		error = api_protocol_set_cmd(ts, SET_IC_SLEEP, NULL, NULL);
+		if (error < 0)
+			return error;
+	}
+
+	return 0;
+}
+
+static int __maybe_unused ilitek_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ilitek_ts_data *ts = i2c_get_clientdata(client);
+	int error = 0;
+
+	if (device_may_wakeup(dev)) {
+		disable_irq_wake(client->irq);
+	} else {
+		error = api_protocol_set_cmd(ts, SET_IC_WAKE, NULL, NULL);
+		if (error < 0)
+			return error;
+	}
+
+	ilitek_reset(ts, ts->reset_time, false);

Are you sure you need to perform reset when device is a wakeup source?
I'd expect in that case it would stay operational.

+	enable_irq(client->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ilitek_pm_ops, ilitek_suspend, ilitek_resume);
+
+static const struct i2c_device_id ilitek_ts_i2c_id[] = {
+	{ILITEK_TS_NAME, 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, ilitek_ts_i2c_id);
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id ilitekts_acpi_id[] = {
+	{ "ILTK0001", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, ilitekts_acpi_id);
+#endif
+
+#ifdef CONFIG_OF
+static const struct of_device_id ilitek_ts_i2c_match[] = {
+	{.compatible = "ilitek,ili2130",},
+	{.compatible = "ilitek,ili2131",},
+	{.compatible = "ilitek,ili2132",},
+	{.compatible = "ilitek,ili2316",},
+	{.compatible = "ilitek,ili2322",},
+	{.compatible = "ilitek,ili2323",},
+	{.compatible = "ilitek,ili2326",},
+	{.compatible = "ilitek,ili2520",},
+	{.compatible = "ilitek,ili2521",},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ilitek_ts_i2c_match);
+#endif
+
+static struct i2c_driver ilitek_ts_i2c_driver = {
+	.driver = {
+		.name = ILITEK_TS_NAME,
+		.pm = &ilitek_pm_ops,
+		.of_match_table = of_match_ptr(ilitek_ts_i2c_match),
+		.acpi_match_table = ACPI_PTR(ilitekts_acpi_id),
+	},
+	.probe = ilitek_ts_i2c_probe,
+	.id_table = ilitek_ts_i2c_id,
+};
+
+module_i2c_driver(ilitek_ts_i2c_driver);
+
+MODULE_AUTHOR("ILITEK");
+MODULE_DESCRIPTION("ILITEK I2C Touchscreen Driver");
+MODULE_LICENSE("GPL");
--
2.25.1

Thanks.

-- 
Dmitry