Received: by 10.223.176.46 with SMTP id f43csp3701393wra; Mon, 22 Jan 2018 20:15:49 -0800 (PST) X-Google-Smtp-Source: AH8x225kTkVZykahiid6dEGa+KZo4x/ufBwD1W9mO2UpoP66YpuGmxSq7vUA7kbnQAmQ3jskhvWS X-Received: by 10.36.157.151 with SMTP id f145mr1928923itd.113.1516680949328; Mon, 22 Jan 2018 20:15:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1516680949; cv=none; d=google.com; s=arc-20160816; b=S8SkeyP29tYGJhe9pP41wo95FRgsdywXvS+K78YEkikvX3Gx8wEUYGUTXEbEqwhJT2 CX4vctMObdvXkvNLhmHrF333wu3rw5uf8sYN/H2cBCNHH3k56zHoyq484bvPN+EZkfjc E4Vz6MhKW9qEdHqB4+WsTZJcF0k7qWP0OzE5iUArOF0CTUkIuJEMdQugwxWmbDLPqhI9 d/xMCnJ5T9BMTKGCLA0muCKeaOqck5KF9FaO+NEW6Xa6Kfgvm2EGzs7uxK9sWxOdQGr1 mTnZ6jeK+tsqUnZpBTEje7ZjW8hvqNQKU6pIyllHcHXz5C0Yk3RIP4VwJODbhET6U5MI MMtQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=U9eqemiB5HySsL5Galco2ZqKPJiw4z0lP8N/mz3zY1o=; b=nW5x+dpdr7MJYiYHlMlpnnipC1a3DAoKwYUcaFj2O3fnn/JaYGccRAUBP57tW8l0OG GrPGvyho68tZ13+4pMQKAUcoYy/H4IuwVY0Tf7fqUOoFteGm6r6ULMyT8Rlkk2zv+R4e HueA/EDCKEEpSh0cNT0+XWHOe5/PRK2EB/FOBcSFCpc2+EW/6th8TeF1qXra2tdN0+p2 Ew1sdSbxwCuLZefCJ/GHblPFS98w/pJIfoCxdD8kphFRFyWWfiiaarYjnL14Ce9Gd9FM 7Zf2urBJEN7Pps0lyCyNpJc8F4vtm+d/kaIqPUS5a0+oUm3gXnjuNVGyxoOeg71ZYOA0 3V8g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Q2s2IOP6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f67si7587680itb.164.2018.01.22.20.15.36; Mon, 22 Jan 2018 20:15:49 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Q2s2IOP6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751421AbeAWEPK (ORCPT + 99 others); Mon, 22 Jan 2018 23:15:10 -0500 Received: from mail-pf0-f193.google.com ([209.85.192.193]:38859 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751293AbeAWEPI (ORCPT ); Mon, 22 Jan 2018 23:15:08 -0500 Received: by mail-pf0-f193.google.com with SMTP id k19so8805155pfj.5; Mon, 22 Jan 2018 20:15:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=U9eqemiB5HySsL5Galco2ZqKPJiw4z0lP8N/mz3zY1o=; b=Q2s2IOP6NFIKyCmUgMl9AKy8AuLtbIpDE3xrDjYZWf3aVm3nKYTEvHyOYZoMPM9DnG c1E/6ejvVPIzMsfCl3oXNICSMINEgwETLF+Oq15KaYOfvXlkV5gNkIbfvgcjV51gRFdC 2JfdOg3eLtSfBTMqXjKUUfkTa9fe5gQz4oJvVGPO4Xkv7fSvXJVSfYtaOXna9y/Mv2zn GNroFLGwwwmnl7P6avUSF1c6Evl0UuD1Aklt6/tpmS9OFMI+CIL6R7ahfTzZYVIaT+9c AINB9Rz/Eob5SiZfkNmGY4Mn70CrgHDFow8TaamHGmRSNMcbvU9vyg3Lye0VbDonPt7m HVDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=U9eqemiB5HySsL5Galco2ZqKPJiw4z0lP8N/mz3zY1o=; b=thuMSqau4B19u4lslVWE7g+gpz+aTbq6cRiOnGx68OMlylJ9IPKpAYyVdw7AIAHhcm m5H/or8AjTsLuvzFwFMOPMdaX9qEVTdjzGumkm/vaCizY0n6g/S26+YGmVGfXtFfDZvc R+YvH3FMGDFp1qv+Pd2qT67pOc/oJEQ1Mp4/nNi3I3lTCfDULDr3NtT1LezkOtYgh2hb UgX+W9eov0StWe0RGbL3wHZBaJM0a2SsjJXGmDdRYbu46otp+FIX3A5g+n5FWlD/4dAv RvYZj4abWYEMg/IYym4FCxFEWSDQTDta7LUUIEQCOzx0iJpaR3uJHpoTT14g3dWEH74o OQAQ== X-Gm-Message-State: AKwxytdBhGVXJ0jrlRZqH2OOv2IobFsPXieZO1u9KCOwvt3YK1ImJoHX S2OgDNMWREERQr8Hm2xteOY= X-Received: by 10.101.64.74 with SMTP id h10mr7766734pgp.200.1516680906589; Mon, 22 Jan 2018 20:15:06 -0800 (PST) Received: from dtor-ws ([2620:0:1000:1611:da80:8749:c06f:9515]) by smtp.gmail.com with ESMTPSA id l8sm29299114pgc.58.2018.01.22.20.15.05 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 22 Jan 2018 20:15:05 -0800 (PST) Date: Mon, 22 Jan 2018 20:15:03 -0800 From: Dmitry Torokhov To: =?iso-8859-1?Q?Myl=E8ne?= Josserand Cc: robh+dt@kernel.org, mark.rutland@arm.com, linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, devicetree@vger.kernel.org, thomas.petazzoni@free-electrons.com, maxime.ripard@free-electrons.com Subject: Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen Message-ID: <20180123041503.cezgsi7h2h4twu7n@dtor-ws> References: <20171201153957.13053-1-mylene.josserand@free-electrons.com> <20171201153957.13053-2-mylene.josserand@free-electrons.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171201153957.13053-2-mylene.josserand@free-electrons.com> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Myl?ne, On Fri, Dec 01, 2017 at 04:39:56PM +0100, Myl?ne Josserand wrote: > This is the basic driver for the Cypress TrueTouch Gen5 touchscreen > controllers. This driver supports only the I2C bus but it uses regmap > so SPI support could be added later. > The touchscreen can retrieve some defined zone that are handled as > buttons (according to the hardware). That is why it handles > button and multitouch events. > > Reviewed-by: Maxime Ripard > Signed-off-by: Myl?ne Josserand > --- > drivers/input/touchscreen/Kconfig | 16 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/cyttsp5.c | 1110 +++++++++++++++++++++++++++++++++++ > 3 files changed, 1127 insertions(+) > create mode 100644 drivers/input/touchscreen/cyttsp5.c > > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index 0cfdb7cb610e..28eea6d5f1bb 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -238,6 +238,22 @@ config TOUCHSCREEN_CYTTSP4_SPI > To compile this driver as a module, choose M here: the > module will be called cyttsp4_spi. > > +config TOUCHSCREEN_CYTTSP5 > + tristate "Cypress TrueTouch Gen5 Touchscreen Driver" > + depends on OF Does it have to be? Please use generic device properties (device_property_* API) and it can be used with ACPI and static board files, of needed. > + select REGMAP_I2C > + select CRC_ITU_T > + help > + Driver for Parade TrueTouch Standard Product > + Generation 5 touchscreen controllers. > + I2C bus interface support only. > + > + Say Y here if you have a Cypress Gen5 touchscreen. > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called cyttsp5. > + > config TOUCHSCREEN_DA9034 > tristate "Touchscreen support for Dialog Semiconductor DA9034" > depends on PMIC_DA903X > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index d2a2b3b7af27..e7d124901dd9 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI) += cyttsp_spi.o > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_CORE) += cyttsp4_core.o > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_I2C) += cyttsp4_i2c.o cyttsp_i2c_common.o > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_SPI) += cyttsp4_spi.o > +obj-$(CONFIG_TOUCHSCREEN_CYTTSP5) += cyttsp5.o > obj-$(CONFIG_TOUCHSCREEN_DA9034) += da9034-ts.o > obj-$(CONFIG_TOUCHSCREEN_DA9052) += da9052_tsi.o > obj-$(CONFIG_TOUCHSCREEN_DYNAPRO) += dynapro.o > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c > new file mode 100644 > index 000000000000..a41feea2cd5a > --- /dev/null > +++ b/drivers/input/touchscreen/cyttsp5.c > @@ -0,0 +1,1110 @@ > +/* > + * Parade TrueTouch(TM) Standard Product V5 Module. > + * For use with Parade touchscreen controllers. > + * > + * Copyright (C) 2015 Parade Technologies > + * Copyright (C) 2012-2015 Cypress Semiconductor > + * Copyright (C) 2017 Free Electrons > + * > + * Author: Myl?ne Josserand > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2, and only version 2, as published by the > + * Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CYTTSP5_NAME "cyttsp5" > +#define CY_I2C_DATA_SIZE (2 * 256) > +#define HID_VERSION 0x0100 > +#define CY_MAX_INPUT 512 > +#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32 > +#define CY_BITS_PER_BTN 1 > +#define CY_NUM_BTN_EVENT_ID ((1 << CY_BITS_PER_BTN) - 1) > + > +#define MAX_AREA 255 > +#define HID_OUTPUT_BL_SOP 0x1 > +#define HID_OUTPUT_BL_EOP 0x17 > +#define HID_OUTPUT_BL_LAUNCH_APP 0x3B > +#define HID_OUTPUT_BL_LAUNCH_APP_SIZE 11 > +#define HID_OUTPUT_GET_SYSINFO 0x2 > +#define HID_OUTPUT_GET_SYSINFO_SIZE 5 > + > +#define HID_DESC_REG 0x1 > +#define HID_INPUT_REG 0x3 > +#define HID_OUTPUT_REG 0x4 > + > +#define REPORT_ID_TOUCH 0x1 > +#define REPORT_ID_BTN 0x3 > +#define REPORT_SIZE_5 5 > +#define REPORT_SIZE_8 8 > +#define REPORT_SIZE_16 16 > + > +/* Touch reports offsets */ > +/* Header offsets */ > +#define TOUCH_REPORT_DESC_HDR_CONTACTCOUNT 16 > +/* Record offsets */ > +#define TOUCH_REPORT_DESC_CONTACTID 8 > +#define TOUCH_REPORT_DESC_X 16 > +#define TOUCH_REPORT_DESC_Y 32 > +#define TOUCH_REPORT_DESC_P 48 > +#define TOUCH_REPORT_DESC_MAJ 56 > +#define TOUCH_REPORT_DESC_MIN 64 > + > +/* HID */ > +#define HID_TOUCH_REPORT_ID 0x1 > +#define HID_BTN_REPORT_ID 0x3 > +#define HID_APP_RESPONSE_REPORT_ID 0x1F > +#define HID_APP_OUTPUT_REPORT_ID 0x2F > +#define HID_BL_RESPONSE_REPORT_ID 0x30 > +#define HID_BL_OUTPUT_REPORT_ID 0x40 > + > +#define HID_OUTPUT_RESPONSE_REPORT_OFFSET 2 > +#define HID_OUTPUT_RESPONSE_CMD_OFFSET 4 > +#define HID_OUTPUT_RESPONSE_CMD_MASK 0x7F > + > +#define HID_SYSINFO_SENSING_OFFSET 33 > +#define HID_SYSINFO_BTN_OFFSET 48 > +#define HID_SYSINFO_BTN_MASK 0xFF > +#define HID_SYSINFO_MAX_BTN 8 > + > +/* Timeout in ms */ > +#define CY_HID_OUTPUT_TIMEOUT 200 > +#define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT 3000 > +#define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT 4000 > + > +/* maximum number of concurrent tracks */ > +#define TOUCH_REPORT_SIZE 10 > +#define TOUCH_INPUT_HEADER_SIZE 7 > +#define BTN_REPORT_SIZE 9 > +#define BTN_INPUT_HEADER_SIZE 5 > + > +/* All usage pages for Touch Report */ > +#define TOUCH_REPORT_USAGE_PG_X 0x00010030 > +#define TOUCH_REPORT_USAGE_PG_Y 0x00010031 > +#define TOUCH_REPORT_USAGE_PG_P 0x000D0030 > +#define TOUCH_REPORT_USAGE_PG_CONTACTID 0x000D0051 > +#define TOUCH_REPORT_USAGE_PG_CONTACTCOUNT 0x000D0054 > +#define TOUCH_REPORT_USAGE_PG_MAJ 0xFF010062 > +#define TOUCH_REPORT_USAGE_PG_MIN 0xFF010063 > +#define TOUCH_COL_USAGE_PG 0x000D0022 > + > +/* helpers */ > +#define HI_BYTE(x) (u8)(((x) >> 8) & 0xFF) > +#define LOW_BYTE(x) (u8)((x) & 0xFF) > + > +/* System Information interface definitions */ > +struct cyttsp5_sensing_conf_data_dev { > + u8 electrodes_x; > + u8 electrodes_y; > + __le16 len_x; > + __le16 len_y; > + __le16 res_x; > + __le16 res_y; > + __le16 max_z; > + u8 origin_x; > + u8 origin_y; > + u8 btn; > + u8 scan_mode; > + u8 max_num_of_tch_per_refresh_cycle; > +} __packed; > + > +struct cyttsp5_sensing_conf_data { > + u16 res_x; > + u16 res_y; > + u16 max_z; > + u16 len_x; > + u16 len_y; > + u8 origin_x; > + u8 origin_y; > + u8 max_tch; > +}; > + > +enum { HID_CMD_DONE, HID_CMD_BUSY } hid_cmd_state; > + > +enum cyttsp5_tch_abs { /* for ordering within the extracted touch data array */ > + CY_TCH_X, /* X */ > + CY_TCH_Y, /* Y */ > + CY_TCH_P, /* P (Z) */ > + CY_TCH_T, /* TOUCH ID */ > + CY_TCH_MAJ, /* TOUCH_MAJOR */ > + CY_TCH_MIN, /* TOUCH_MINOR */ > + CY_TCH_NUM_ABS, > +}; > + > +struct cyttsp5_tch_abs_params { > + size_t ofs; /* abs byte offset */ > + size_t size; /* size in bits */ > + size_t min; /* min value */ > + size_t max; /* max value */ > + size_t bofs; /* bit offset */ > +}; > + > +struct cyttsp5_touch { > + int hdr; > + int abs[CY_TCH_NUM_ABS]; > +}; > + > +struct cyttsp5_sysinfo { > + struct cyttsp5_sensing_conf_data sensing_conf_data; > + int num_btns; > + struct cyttsp5_tch_abs_params tch_hdr; > + struct cyttsp5_tch_abs_params tch_abs[CY_TCH_NUM_ABS]; > + u32 key_code[HID_SYSINFO_MAX_BTN]; > + u8 *xy_mode; > + u8 *xy_data; > +}; > + > +struct cyttsp5_hid_desc { > + __le16 hid_desc_len; > + u8 packet_id; > + u8 reserved_byte; > + __le16 bcd_version; > + __le16 report_desc_len; > + __le16 report_desc_register; > + __le16 input_register; > + __le16 max_input_len; > + __le16 output_register; > + __le16 max_output_len; > + __le16 command_register; > + __le16 data_register; > + __le16 vendor_id; > + __le16 product_id; > + __le16 version_id; > + u8 reserved[4]; > +} __packed; > + > +struct cyttsp5 { > + struct device *dev; > + struct mutex system_lock; > + wait_queue_head_t wait_q; > + struct cyttsp5_sysinfo sysinfo; > + int hid_cmd_state; > + struct cyttsp5_hid_desc hid_desc; > + u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER]; > + u8 input_buf[CY_MAX_INPUT]; > + u8 response_buf[CY_MAX_INPUT]; > + struct gpio_desc *reset_gpio; > + struct input_dev *input; > + char phys[NAME_MAX]; > + int num_prv_rec; > + struct regmap *regmap; > +}; > + > +/* > + * For what understood in the datasheet, the register does not > + * matter. For consistency, used the Input Register address > + * but it does mean anything to the device. The important data > + * to send is the I2C address > + */ > +static int cyttsp5_read(struct cyttsp5 *ts, u8 *buf, u32 max) > +{ > + int rc; > + u32 size; > + u8 temp[2]; > + > + if (!buf) > + return -EINVAL; Is it really possible? This is not a publicly facing API but private driver data, do you ever happen to call it with NULL? > + > + /* Read the frame to retrieve the size */ > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, temp, 2); > + if (rc < 0) > + return rc; Can we call this kind of variables "error" pelase? > + > + size = get_unaligned_le16(temp); > + if (!size || size == 2) > + return 0; > + > + if (size > max) > + return -EINVAL; > + > + /* Get the real value */ > + return regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, size); > +} > + > +static int cyttsp5_write(struct cyttsp5 *ts, unsigned int reg, u8 *data, > + size_t size) > +{ > + u8 cmd[size + 1]; > + > + /* High bytes of register address needed as first byte of cmd */ > + cmd[0] = HI_BYTE(reg); > + > + /* Copy the rest of the data */ > + if (data) > + memcpy(&cmd[1], data, size); > + > + /* The hardware wants to receive a frame with the address register The multiline comment block should start with "/*" on a separate line please. > + * contains in the first two bytes. As the regmap_write function > + * add the register adresse in the frame, we use the LOW_BYTE as > + * first frame byte for the address register and the first > + * data byte is the high register + left of the cmd to send > + */ > + return regmap_bulk_write(ts->regmap, LOW_BYTE(reg), cmd, size + 1); > +} > + > +static void cyttsp5_final_sync(struct input_dev *input, int max_slots, > + unsigned long *ids) > +{ > + int t; > + > + for (t = 0; t < max_slots; t++) { for_each_set_bit() { } > + if (test_bit(t, ids)) > + continue; > + input_mt_slot(input, t); > + input_mt_report_slot_state(input, MT_TOOL_FINGER, false); > + } > + > + input_sync(input); > +} > + > +static void cyttsp5_report_slot_liftoff(struct cyttsp5 *ts, int max_slots) > +{ > + int t; > + > + if (ts->num_prv_rec == 0) > + return; Do you need this check? The caller checks the same condition. > + > + for (t = 0; t < max_slots; t++) { > + input_mt_slot(ts->input, t); > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, false); > + } > +} > + > +static void cyttsp5_mt_lift_all(struct cyttsp5 *ts) > +{ > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int max = si->tch_abs[CY_TCH_T].max; > + > + if (ts->num_prv_rec != 0) { > + cyttsp5_report_slot_liftoff(ts, max); > + input_sync(ts->input); > + ts->num_prv_rec = 0; > + } > +} > + > +static void cyttsp5_get_touch_axis(int *axis, int size, int max, u8 *xy_data, > + int bofs) > +{ > + int nbyte; > + int next; > + > + for (nbyte = 0, *axis = 0, next = 0; nbyte < size; nbyte++) > + *axis = *axis + ((xy_data[nbyte] >> bofs) << (nbyte * 8)); > + > + *axis &= max - 1; > +} > + > +static void cyttsp5_get_touch_record(struct cyttsp5 *ts, > + struct cyttsp5_touch *touch, u8 *xy_data) > +{ > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + enum cyttsp5_tch_abs abs; > + > + for (abs = CY_TCH_X; abs < CY_TCH_NUM_ABS; abs++) { > + cyttsp5_get_touch_axis(&touch->abs[abs], > + si->tch_abs[abs].size, > + si->tch_abs[abs].max, > + xy_data + si->tch_abs[abs].ofs, > + si->tch_abs[abs].bofs); > + } > +} > + > +static void cyttsp5_get_mt_touches(struct cyttsp5 *ts, > + struct cyttsp5_touch *tch, int num_cur_tch) > +{ > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int i, t = 0; > + DECLARE_BITMAP(ids, si->tch_abs[CY_TCH_T].max); > + u8 *tch_addr; > + int tmp; > + > + bitmap_zero(ids, si->tch_abs[CY_TCH_T].max); > + memset(tch->abs, 0, sizeof(tch->abs)); > + > + for (i = 0; i < num_cur_tch; i++) { > + tch_addr = si->xy_data + (i * TOUCH_REPORT_SIZE); > + cyttsp5_get_touch_record(ts, tch, tch_addr); > + > + /* Convert MAJOR/MINOR from mm to resolution */ > + tmp = tch->abs[CY_TCH_MAJ] * 100 * si->sensing_conf_data.res_x; > + tch->abs[CY_TCH_MAJ] = tmp / si->sensing_conf_data.len_x; > + tmp = tch->abs[CY_TCH_MIN] * 100 * si->sensing_conf_data.res_x; > + tch->abs[CY_TCH_MIN] = tmp / si->sensing_conf_data.len_x; > + > + t = tch->abs[CY_TCH_T]; > + input_mt_slot(ts->input, t); > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, true); > + __set_bit(t, ids); > + > + /* position and pressure fields */ > + input_report_abs(ts->input, ABS_MT_POSITION_X, > + tch->abs[CY_TCH_X]); > + input_report_abs(ts->input, ABS_MT_POSITION_Y, > + tch->abs[CY_TCH_Y]); > + input_report_abs(ts->input, ABS_MT_PRESSURE, > + tch->abs[CY_TCH_P]); > + > + /* Get the extended touch fields */ > + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR, > + tch->abs[CY_TCH_MAJ]); > + input_report_abs(ts->input, ABS_MT_TOUCH_MINOR, > + tch->abs[CY_TCH_MIN]); > + } > + > + cyttsp5_final_sync(ts->input, si->tch_abs[CY_TCH_T].max, ids); > + > + ts->num_prv_rec = num_cur_tch; > +} > + > +/* read xy_data for all current touches */ > +static int cyttsp5_xy_worker(struct cyttsp5 *ts) > +{ > + struct device *dev = ts->dev; > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int max_tch = si->sensing_conf_data.max_tch; > + struct cyttsp5_touch tch; > + u8 num_cur_tch; > + > + cyttsp5_get_touch_axis(&tch.hdr, si->tch_hdr.size, > + si->tch_hdr.max, > + si->xy_mode + 3 + si->tch_hdr.ofs, > + si->tch_hdr.bofs); > + > + num_cur_tch = tch.hdr; > + if (num_cur_tch > max_tch) { > + dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch); > + num_cur_tch = max_tch; > + } > + > + if (num_cur_tch == 0 && ts->num_prv_rec == 0) > + return 0; > + > + /* extract xy_data for all currently reported touches */ > + if (num_cur_tch) > + cyttsp5_get_mt_touches(ts, &tch, num_cur_tch); > + else > + cyttsp5_mt_lift_all(ts); > + > + return 0; > +} > + > +static int cyttsp5_mt_attention(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); Why don't you pass ts into cyttsp5_mt_attention? > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int rc; > + > + if (si->xy_mode[2] != HID_TOUCH_REPORT_ID) > + return 0; > + > + /* core handles handshake */ > + rc = cyttsp5_xy_worker(ts); > + if (rc < 0) > + dev_err(dev, "xy_worker error r=%d\n", rc); > + > + return rc; > +} > + > +static int cyttsp5_setup_input_device(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int max_x, max_y, max_p; > + int max_x_tmp, max_y_tmp; > + int rc; > + > + __set_bit(EV_ABS, ts->input->evbit); > + __set_bit(EV_REL, ts->input->evbit); > + __set_bit(EV_KEY, ts->input->evbit); > + > + max_x_tmp = si->sensing_conf_data.res_x; > + max_y_tmp = si->sensing_conf_data.res_y; > + max_x = max_y_tmp - 1; > + max_y = max_x_tmp - 1; > + max_p = si->sensing_conf_data.max_z; > + > + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0); Error handling. > + > + __set_bit(ABS_MT_POSITION_X, ts->input->absbit); > + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit); > + __set_bit(ABS_MT_PRESSURE, ts->input->absbit); > + > + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0); > + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0); > + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0); > + > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0); > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0); You might want to use the standard DT touchscreen properties and reporting functions to support axis inversion and swapping of axes. > + > + rc = input_register_device(ts->input); > + if (rc < 0) > + dev_err(dev, "Error, failed register input device r=%d\n", rc); > + > + return rc; > +} > + > +#ifdef CONFIG_OF > +static int cyttsp5_parse_dt_key_code(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + struct device_node *np; > + int i; > + > + np = dev->of_node; > + if (!np) > + return -EINVAL; > + > + if (!si->num_btns) > + return 0; > + > + /* Initialize the button to RESERVED */ > + for (i = 0; i < si->num_btns; i++) > + si->key_code[i] = KEY_RESERVED; > + > + return of_property_read_u32_array(np, "linux,keycodes", > + si->key_code, si->num_btns); > +} > +#else > +static int cyttsp5_parse_dt_key_code(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int i; > + > + if (!si->num_btns) > + return 0; > + > + /* Initialize the button to RESERVED */ > + for (i = 0; i < si->num_btns; i++) > + si->key_code[i] = KEY_RESERVED; > + > + return 0; > +} > +#endif > + > +static int cyttsp5_btn_attention(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int cur_btn; > + int cur_btn_state; > + > + if (si->xy_mode[2] != HID_BTN_REPORT_ID || !si->num_btns) > + return 0; > + > + /* extract button press/release touch information */ > + for (cur_btn = 0; cur_btn < si->num_btns; cur_btn++) { > + /* Get current button state */ > + cur_btn_state = (si->xy_data[0] >> (cur_btn * CY_BITS_PER_BTN)) > + & CY_NUM_BTN_EVENT_ID; > + > + input_report_key(ts->input, si->key_code[cur_btn], > + cur_btn_state); > + input_sync(ts->input); You do not need to sen sync after each button, send it once after all buttons sent. > + } > + > + return 0; > +} > + > +static u16 cyttsp5_compute_crc(u8 *buf, u32 size) > +{ > + u16 remainder = 0xFFFF; > + u16 xor_mask = 0x0000; > + u32 index; > + u32 byte_value; > + u32 table_index; > + u32 crc_bit_width = sizeof(u16) * 8; > + > + /* Divide the message by polynomial, via the table. */ > + for (index = 0; index < size; index++) { > + byte_value = buf[index]; > + table_index = ((byte_value >> 4) & 0x0F) > + ^ (remainder >> (crc_bit_width - 4)); > + remainder = crc_itu_t_table[table_index] > + ^ (remainder << 4); > + table_index = (byte_value & 0x0F) > + ^ (remainder >> (crc_bit_width - 4)); > + remainder = crc_itu_t_table[table_index] > + ^ (remainder << 4); > + } > + > + /* Perform the final remainder CRC. */ > + return remainder ^ xor_mask; > +} > + > +static int cyttsp5_validate_cmd_response(struct cyttsp5 *ts, u8 code) > +{ > + u16 size, crc; > + u8 status, offset; > + int command_code; > + > + size = get_unaligned_le16(&ts->response_buf[0]); > + > + if (!size) > + return 0; > + > + offset = ts->response_buf[HID_OUTPUT_RESPONSE_REPORT_OFFSET]; > + > + if (offset == HID_BL_RESPONSE_REPORT_ID) { > + if (ts->response_buf[4] != HID_OUTPUT_BL_SOP) { > + dev_err(ts->dev, "HID output response, wrong SOP\n"); > + return -EPROTO; > + } > + > + if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) { > + dev_err(ts->dev, "HID output response, wrong EOP\n"); > + return -EPROTO; > + } > + > + crc = cyttsp5_compute_crc(&ts->response_buf[4], size - 7); > + if (ts->response_buf[size - 3] != LOW_BYTE(crc) || > + ts->response_buf[size - 2] != HI_BYTE(crc)) { > + dev_err(ts->dev, "HID output response, wrong CRC 0x%X\n", > + crc); > + return -EPROTO; > + } > + > + status = ts->response_buf[5]; > + if (status) { > + dev_err(ts->dev, "HID output response, ERROR:%d\n", > + status); > + return -EPROTO; > + } > + } > + > + if (offset == HID_APP_RESPONSE_REPORT_ID) { > + command_code = ts->response_buf[HID_OUTPUT_RESPONSE_CMD_OFFSET] > + & HID_OUTPUT_RESPONSE_CMD_MASK; > + if (command_code != code) { > + dev_err(ts->dev, > + "HID output response, wrong command_code:%X\n", > + command_code); > + return -EPROTO; > + } > + } > + > + return 0; > +} > + > +static void cyttsp5_si_get_btn_data(struct cyttsp5 *ts) > +{ > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + int i; > + unsigned int btns = ts->response_buf[HID_SYSINFO_BTN_OFFSET] > + & HID_SYSINFO_BTN_MASK; > + > + si->num_btns = 0; > + for (i = 0; i < HID_SYSINFO_MAX_BTN; i++) { > + if (btns & BIT(i)) > + si->num_btns++; > + } si->num_btns = hweight8(ts->response_buf[HID_SYSINFO_BTN_OFFSET]); > +} > + > +static int cyttsp5_get_sysinfo_regs(struct cyttsp5 *ts) > +{ > + struct cyttsp5_sensing_conf_data *scd = &ts->sysinfo.sensing_conf_data; > + struct cyttsp5_sensing_conf_data_dev *scd_dev = > + (struct cyttsp5_sensing_conf_data_dev *) > + &ts->response_buf[HID_SYSINFO_SENSING_OFFSET]; > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + > + cyttsp5_si_get_btn_data(ts); > + > + scd->max_tch = scd_dev->max_num_of_tch_per_refresh_cycle; > + scd->res_x = get_unaligned_le16(&scd_dev->res_x); > + scd->res_y = get_unaligned_le16(&scd_dev->res_y); > + scd->max_z = get_unaligned_le16(&scd_dev->max_z); > + scd->len_x = get_unaligned_le16(&scd_dev->len_x); > + scd->len_y = get_unaligned_le16(&scd_dev->len_y); > + > + si->xy_data = devm_kzalloc(ts->dev, scd->max_tch * TOUCH_REPORT_SIZE, > + GFP_KERNEL); > + if (!si->xy_data) > + return -ENOMEM; > + > + si->xy_mode = devm_kzalloc(ts->dev, TOUCH_INPUT_HEADER_SIZE, > + GFP_KERNEL); > + if (!si->xy_mode) > + return -ENOMEM; Why do we need these 2 allocated separately form the driver data? > + > + return 0; > +} > + > +static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts) > +{ > + int rc; > + u8 cmd[HID_OUTPUT_GET_SYSINFO_SIZE]; > + > + ts->hid_cmd_state = HID_CMD_BUSY; > + > + /* HI bytes of Output register address */ > + cmd[0] = LOW_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE); > + cmd[1] = HI_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE); > + cmd[2] = HID_APP_OUTPUT_REPORT_ID; > + cmd[3] = 0x0; /* Reserved */ > + cmd[4] = HID_OUTPUT_GET_SYSINFO; > + > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd, > + HID_OUTPUT_GET_SYSINFO_SIZE); > + if (rc) { > + dev_err(ts->dev, "Failed to write command %d", rc); > + goto error; > + } > + > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE), > + msecs_to_jiffies(CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT)); > + if (!rc) { > + dev_err(ts->dev, "HID output cmd execution timed out\n"); > + rc = -ETIME; > + goto error; > + } > + > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_GET_SYSINFO); > + if (rc) { > + dev_err(ts->dev, "Validation of the response failed\n"); > + goto error; > + } > + > + return cyttsp5_get_sysinfo_regs(ts); > + > +error: > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_DONE; > + mutex_unlock(&ts->system_lock); > + return rc; > +} > + > +static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts) > +{ > + int rc; > + u8 cmd[HID_OUTPUT_BL_LAUNCH_APP]; > + u16 crc; > + > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_BUSY; > + mutex_unlock(&ts->system_lock); > + > + cmd[0] = LOW_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE); > + cmd[1] = HI_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE); > + cmd[2] = HID_BL_OUTPUT_REPORT_ID; > + cmd[3] = 0x0; /* Reserved */ > + cmd[4] = HID_OUTPUT_BL_SOP; > + cmd[5] = HID_OUTPUT_BL_LAUNCH_APP; > + cmd[6] = 0x0; /* Low bytes of data */ > + cmd[7] = 0x0; /* Hi bytes of data */ > + crc = cyttsp5_compute_crc(&cmd[4], 4); > + cmd[8] = LOW_BYTE(crc); > + cmd[9] = HI_BYTE(crc); > + cmd[10] = HID_OUTPUT_BL_EOP; > + > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd, > + HID_OUTPUT_BL_LAUNCH_APP_SIZE); > + if (rc) { > + dev_err(ts->dev, "Failed to write command %d", rc); > + goto error; > + } > + > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE), > + msecs_to_jiffies(CY_HID_OUTPUT_TIMEOUT)); > + if (!rc) { > + dev_err(ts->dev, "HID output cmd execution timed out\n"); > + rc = -ETIME; > + goto error; > + } > + > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_BL_LAUNCH_APP); > + if (rc) { > + dev_err(ts->dev, "Validation of the response failed\n"); > + goto error; > + } > + > + return rc; > + > +error: > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_DONE; > + mutex_unlock(&ts->system_lock); > + > + return rc; > +} > + > +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts, > + struct cyttsp5_hid_desc *desc) > +{ > + struct device *dev = ts->dev; > + __le16 hid_desc_register = HID_DESC_REG; > + int rc; > + u8 cmd[2]; > + > + /* Read HID descriptor length and version */ > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_BUSY; > + mutex_unlock(&ts->system_lock); > + > + /* Set HID descriptor register */ > + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register)); > + > + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0); > + if (rc < 0) { > + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc); > + goto error; > + } > + > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE), > + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT)); > + if (!rc) { > + dev_err(ts->dev, "HID get descriptor timed out\n"); > + rc = -ETIME; > + goto error; > + } > + > + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc)); > + > + /* Check HID descriptor length and version */ > + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) || > + le16_to_cpu(desc->bcd_version) != HID_VERSION) { > + dev_err(dev, "Unsupported HID version\n"); > + return -ENODEV; > + } > + > + goto exit; > + > +error: > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_DONE; > + mutex_unlock(&ts->system_lock); > +exit: > + return rc; > +} > + > +static int fill_tch_abs(struct cyttsp5_tch_abs_params *tch_abs, int report_size, > + int offset) > +{ > + tch_abs->ofs = offset / 8; > + tch_abs->size = report_size / 8; > + if (report_size % 8) > + tch_abs->size += 1; > + tch_abs->min = 0; > + tch_abs->max = 1 << report_size; > + tch_abs->bofs = offset - (tch_abs->ofs << 3); > + > + return 0; > +} > + > +static int move_button_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si) > +{ > + memcpy(si->xy_mode, ts->input_buf, BTN_INPUT_HEADER_SIZE); > + memcpy(si->xy_data, &ts->input_buf[BTN_INPUT_HEADER_SIZE], > + BTN_REPORT_SIZE); > + > + return 0; > +} > + > +static int move_touch_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si) > +{ > + int max_tch = si->sensing_conf_data.max_tch; > + int num_cur_tch; > + int length; > + struct cyttsp5_tch_abs_params *tch = &si->tch_hdr; > + > + memcpy(si->xy_mode, ts->input_buf, TOUCH_INPUT_HEADER_SIZE); > + > + cyttsp5_get_touch_axis(&num_cur_tch, tch->size, > + tch->max, si->xy_mode + 3 + tch->ofs, tch->bofs); > + if (unlikely(num_cur_tch > max_tch)) > + num_cur_tch = max_tch; > + > + length = num_cur_tch * TOUCH_REPORT_SIZE; > + > + memcpy(si->xy_data, &ts->input_buf[TOUCH_INPUT_HEADER_SIZE], length); > + > + return 0; > +} > + > +static irqreturn_t cyttsp5_handle_irq(int irq, void *handle) > +{ > + struct cyttsp5 *ts = handle; > + int report_id; > + int size; > + int rc; > + > + rc = cyttsp5_read(ts, ts->input_buf, CY_MAX_INPUT); > + if (rc) > + return IRQ_HANDLED; > + > + size = get_unaligned_le16(&ts->input_buf[0]); > + > + /* check reset */ > + if (size == 0) { > + memcpy(ts->response_buf, ts->input_buf, 2); > + > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_DONE; > + mutex_unlock(&ts->system_lock); > + wake_up(&ts->wait_q); I'd use spinlock in wait queue and wake_up_locked() instead of a separate mutex. > + return IRQ_HANDLED; > + } > + > + report_id = ts->input_buf[2]; > + switch(report_id) { } > + if (report_id == HID_TOUCH_REPORT_ID) { > + move_touch_data(ts, &ts->sysinfo); > + cyttsp5_mt_attention(ts->dev); > + } else if (report_id == HID_BTN_REPORT_ID) { > + move_button_data(ts, &ts->sysinfo); > + cyttsp5_btn_attention(ts->dev); > + } else { > + /* It is not an input but a command response */ > + memcpy(ts->response_buf, ts->input_buf, size); > + > + mutex_lock(&ts->system_lock); > + ts->hid_cmd_state = HID_CMD_DONE; > + mutex_unlock(&ts->system_lock); > + wake_up(&ts->wait_q); > + } > + > + return IRQ_HANDLED; > +} > + > +static int cyttsp5_deassert_int(struct cyttsp5 *ts) > +{ > + u16 size; > + u8 buf[2]; > + int rc; > + > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2); > + if (rc < 0) > + return rc; > + > + size = get_unaligned_le16(&buf[0]); > + if (size == 2 || size == 0) > + return 0; > + > + return -EINVAL; > +} > + > +static int cyttsp5_fill_all_touch(struct cyttsp5 *ts) > +{ > + struct cyttsp5_sysinfo *si = &ts->sysinfo; > + > + fill_tch_abs(&si->tch_abs[CY_TCH_X], REPORT_SIZE_16, > + TOUCH_REPORT_DESC_X); > + fill_tch_abs(&si->tch_abs[CY_TCH_Y], REPORT_SIZE_16, > + TOUCH_REPORT_DESC_Y); > + fill_tch_abs(&si->tch_abs[CY_TCH_P], REPORT_SIZE_8, > + TOUCH_REPORT_DESC_P); > + fill_tch_abs(&si->tch_abs[CY_TCH_T], REPORT_SIZE_5, > + TOUCH_REPORT_DESC_CONTACTID); > + fill_tch_abs(&si->tch_hdr, REPORT_SIZE_5, > + TOUCH_REPORT_DESC_HDR_CONTACTCOUNT); > + fill_tch_abs(&si->tch_abs[CY_TCH_MAJ], REPORT_SIZE_8, > + TOUCH_REPORT_DESC_MAJ); > + fill_tch_abs(&si->tch_abs[CY_TCH_MIN], REPORT_SIZE_8, > + TOUCH_REPORT_DESC_MIN); > + > + return 0; > +} > + > +static int cyttsp5_startup(struct cyttsp5 *ts) > +{ > + int rc; > + > + rc = cyttsp5_deassert_int(ts); > + if (rc) { > + dev_err(ts->dev, "Error on deassert int r=%d\n", rc); > + return -ENODEV; > + } > + > + /* > + * Launch the application as the device starts in bootloader mode > + * because of a power-on-reset > + */ > + rc = cyttsp5_hid_output_bl_launch_app(ts); > + if (rc < 0) { > + dev_err(ts->dev, "Error on launch app r=%d\n", rc); > + return rc; > + } > + > + rc = cyttsp5_get_hid_descriptor(ts, &ts->hid_desc); > + if (rc < 0) { > + dev_err(ts->dev, "Error on getting HID descriptor r=%d\n", rc); > + return rc; > + } > + > + rc = cyttsp5_fill_all_touch(ts); > + if (rc < 0) { > + dev_err(ts->dev, "Error on report descriptor r=%d\n", rc); > + return rc; > + } > + > + rc = cyttsp5_hid_output_get_sysinfo(ts); > + if (rc) { > + dev_err(ts->dev, "Error on getting sysinfo r=%d\n", rc); > + return rc; > + } > + > + return rc; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id cyttsp5_of_match[] = { > + { .compatible = "cypress,cyttsp5", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, cyttsp5_of_match); > +#endif > + > +static const struct i2c_device_id cyttsp5_i2c_id[] = { > + { CYTTSP5_NAME, 0, }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id); > + > +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq, > + const char *name) > +{ > + struct cyttsp5 *ts; > + struct cyttsp5_sysinfo *si; > + int rc = 0, i; > + > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL); > + if (!ts) > + return -ENOMEM; > + > + /* Initialize device info */ > + ts->regmap = regmap; > + ts->dev = dev; > + si = &ts->sysinfo; > + dev_set_drvdata(dev, ts); > + > + /* Initialize mutexes and spinlocks */ > + mutex_init(&ts->system_lock); > + > + /* Initialize wait queue */ > + init_waitqueue_head(&ts->wait_q); > + > + /* Reset the gpio to be in a reset state */ > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(ts->reset_gpio)) { > + rc = PTR_ERR(ts->reset_gpio); > + dev_err(dev, "Failed to request reset gpio, error %d\n", rc); > + return rc; > + } > + gpiod_set_value(ts->reset_gpio, 1); Why not request it as GPIOD_OUT_HIGH if you going to activate it immediately? However it sounds as if you have a reset line that is active low and you want to driver it low to reset, and then release. In this case you need to do ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); - chip usually needs to be some time in reset state gpiod_set_value(ts->reset_gpio, 0); and make sure that correct polarity is specified in device tree. > + > + /* Need a delay to have device up */ > + msleep(20); > + > + rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, Just use IRQF_ONESHOT and make sure DTS specifies interrupt properly. BTW,I'd recommend using level, not edge interrupts. It is so easy to miss an edge. > + name, ts); > + if (rc) { > + dev_err(dev, "unable to request IRQ\n"); > + return rc; > + } > + > + rc = cyttsp5_startup(ts); > + if (rc) { > + dev_err(ts->dev, "Fail initial startup r=%d\n", rc); > + return rc; > + } > + > + rc = cyttsp5_parse_dt_key_code(dev); > + if (rc < 0) { > + dev_err(ts->dev, "Error while parsing dts %d\n", rc); > + return rc; > + } > + > + ts->input = devm_input_allocate_device(dev); > + if (!ts->input) { > + dev_err(dev, "Error, failed to allocate input device\n"); > + return -ENODEV; > + } > + > + ts->input->name = "cyttsp5"; > + scnprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev)); > + ts->input->phys = ts->phys; > + ts->input->dev.parent = ts->dev; > + input_set_drvdata(ts->input, ts); > + > + touchscreen_parse_properties(ts->input, true, NULL); > + > + __set_bit(EV_KEY, ts->input->evbit); > + for (i = 0; i < si->num_btns; i++) > + __set_bit(si->key_code[i], ts->input->keybit); > + > + return cyttsp5_setup_input_device(dev); > +} > + > +static int cyttsp5_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct regmap *regmap; > + static const struct regmap_config config = { > + .reg_bits = 8, > + .val_bits = 8, > + }; > + > + regmap = devm_regmap_init_i2c(client, &config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "regmap allocation failed: %ld\n", > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + return cyttsp5_probe(&client->dev, regmap, client->irq, client->name); > +} > + > +static int cyttsp5_remove(struct device *dev) > +{ > + struct cyttsp5 *ts = dev_get_drvdata(dev); > + > + input_unregister_device(ts->input); Not needed: you are using managed input device. > + > + return 0; > +} > + > +static int cyttsp5_i2c_remove(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + > + return cyttsp5_remove(dev); Not needed if you remove cyttsp5_remove(). > +} > + > +static struct i2c_driver cyttsp5_i2c_driver = { > + .driver = { > + .name = CYTTSP5_NAME, > + .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(cyttsp5_of_match), > + }, > + .probe = cyttsp5_i2c_probe, > + .remove = cyttsp5_i2c_remove, > + .id_table = cyttsp5_i2c_id, > +}; > + > +module_i2c_driver(cyttsp5_i2c_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product"); > +MODULE_AUTHOR("Myl?ne Josserand "); > -- > 2.11.0 > Thanks. -- Dmitry