Hi Caleb, On Mon, Jun 24, 2024 at 03:30:30AM +0200, Caleb Connolly wrote: > The TCM oncell is the next generation of Synaptics touchscreen ICs. > These run a very featured firmware with a reasonably well defined API. > It is however entirely incompatible with the existing RMI4 interface. > > Unfortunately, no public datasheet for the interface seems to be > available, instead this driver was created through a combination of > vendor drivers and trial and error. > > The firmware interface implies support for defining the exact bit > encoding of the touch reports, however on the S3908 chip + firmware > found in the OnePlus 8t the TCM_SET_TOUCH_REPORT_CONFIG command appears > to be unsupported. > > Co-developed-by: Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx> > Signed-off-by: Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx> > Signed-off-by: Caleb Connolly <caleb@xxxxxxxxxxxxxxxx> > --- > MAINTAINERS | 7 + > drivers/input/touchscreen/Kconfig | 11 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/synaptics_tcm_oncell.c | 617 +++++++++++++++++++++++ > 4 files changed, 636 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2b9cfbf92d7a..db589c841d8c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -21826,8 +21826,15 @@ M: Icenowy Zheng <icenowy@xxxxxxx> > S: Maintained > F: Documentation/devicetree/bindings/regulator/silergy,sy8106a.yaml > F: drivers/regulator/sy8106a-regulator.c > > +SYNAPTICS TCM ONCELL TOUCHSCREEN DRIVER > +M: Caleb Connolly <caleb@xxxxxxxxxxxxxxxx> > +L: linux-input@xxxxxxxxxxxxxxx > +S: Maintained > +F: Documentation/devicetree/bindings/input/touchscreen/syna,tcm-oncell.yaml > +F: drivers/input/touchscreen/synaptics_tcm_oncell.c > + > SYNC FILE FRAMEWORK > M: Sumit Semwal <sumit.semwal@xxxxxxxxxx> > R: Gustavo Padovan <gustavo@xxxxxxxxxxx> > L: linux-media@xxxxxxxxxxxxxxx > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index c821fe3ee794..43c4fd80601c 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -531,8 +531,19 @@ config TOUCHSCREEN_S6SY761 > > To compile this driver as module, choose M here: the > module will be called s6sy761. > > +config TOUCHSCREEN_SYNAPTICS_TCM_ONCELL > + tristate "Synaptics TCM Oncell Touchscreen driver" > + depends on I2C > + help > + Say Y if you have the Synaptics S3908 TCM Oncell > + > + If unsure, say N > + > + To compile this driver as module, choose M here: the > + module will be called synaptics_tcm_oncell. > + > config TOUCHSCREEN_GUNZE > tristate "Gunze AHL-51S touchscreen" > select SERIO > help > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index a81cb5aa21a5..6a2b85050c3a 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -88,8 +88,9 @@ obj-$(CONFIG_TOUCHSCREEN_STMFTS) += stmfts.o > obj-$(CONFIG_TOUCHSCREEN_STMPE) += stmpe-ts.o > obj-$(CONFIG_TOUCHSCREEN_SUN4I) += sun4i-ts.o > obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o > obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o > +obj-$(CONFIG_TOUCHSCREEN_SYNAPTICS_TCM_ONCELL) += synaptics_tcm_oncell.o > obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o > obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o > obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o > obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o > diff --git a/drivers/input/touchscreen/synaptics_tcm_oncell.c b/drivers/input/touchscreen/synaptics_tcm_oncell.c > new file mode 100644 > index 000000000000..c1ba9a3a93c0 > --- /dev/null > +++ b/drivers/input/touchscreen/synaptics_tcm_oncell.c > @@ -0,0 +1,617 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Driver for Synaptics TCM Oncell Touchscreens > + * > + * Copyright (c) 2024 Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx> > + * Copyright (c) 2024 Caleb Connolly <caleb@xxxxxxxxxxxxxxxx> > + */ > + > +#include <linux/i2c.h> > +#include <linux/input.h> > +#include <linux/input/touchscreen.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/property.h> > +#include <asm/unaligned.h> > +#include <linux/delay.h> > +#include <linux/input/mt.h> > +#include <linux/input/touchscreen.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/of_gpio.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > + > +/* > + * The TCM oncell interface uses a command byte, which may be followed by additional > + * data. The packet format is defined in the tcm_cmd struct. > + * > + * The following list only defines commands that are used in this driver (and their > + * counterparts for context). Vendor reference implementations can be found at > + * https://github.com/LineageOS/android_kernel_oneplus_sm8250/tree/ee0a7ee1939ffd53000e42051caf8f0800defb27/drivers/input/touchscreen/synaptics_tcm > + */ > + > +/* > + * Request information about the chip. We don't send this command explicitly as > + * the controller automatically sends this information when starting up. > + */ > +#define TCM_IDENTIFY 0x02 > + > +/* Enable/disable reporting touch inputs */ > +#define TCM_ENABLE_REPORT 0x05 > +#define TCM_DISABLE_REPORT 0x06 > + > +/* > + * After powering on, we send this to exit the bootloader mode and run the main > + * firmware. > + */ > +#define TCM_RUN_APPLICATION_FIRMWARE 0x14 > + > +/* > + * Reports information about the vendor provided application firmware. This is > + * also used to determine when the firmware has finished booting. > + */ > +#define TCM_GET_APPLICATION_INFO 0x20 > + > +#define MODE_APPLICATION 0x01 > + > +#define APP_STATUS_OK 0x00 > +#define APP_STATUS_BOOTING 0x01 > +#define APP_STATUS_UPDATING 0x02 > +#define APP_STATUS_BAD_APP_CONFIG 0xff > + > +/* status codes */ > +#define REPORT_IDLE 0x00 > +#define REPORT_OK 0x01 > +#define REPORT_BUSY 0x02 > +#define REPORT_CONTINUED_READ 0x03 > +#define REPORT_RECEIVE_BUFFER_OVERFLOW 0x0c > +#define REPORT_PREVIOUS_COMMAND_PENDING 0x0d > +#define REPORT_NOT_IMPLEMENTED 0x0e > +#define REPORT_ERROR 0x0f > + > +/* report types */ > +#define REPORT_IDENTIFY 0x10 > +#define REPORT_TOUCH 0x11 > +#define REPORT_DELTA 0x12 > +#define REPORT_RAW 0x13 > +#define REPORT_DEBUG 0x14 > +#define REPORT_LOG 0x1d > +#define REPORT_TOUCH_HOLD 0x20 > +#define REPORT_INVALID 0xff > + > +/* Touch report codes */ > +#define TOUCH_END 0 > +#define TOUCH_FOREACH_ACTIVE_OBJECT 1 > +#define TOUCH_FOREACH_OBJECT 2 > +#define TOUCH_FOREACH_END 3 > +#define TOUCH_PAD_TO_NEXT_BYTE 4 > +#define TOUCH_TIMESTAMP 5 > +#define TOUCH_OBJECT_N_INDEX 6 > +#define TOUCH_OBJECT_N_CLASSIFICATION 7 > +#define TOUCH_OBJECT_N_X_POSITION 8 > +#define TOUCH_OBJECT_N_Y_POSITION 9 > +#define TOUCH_OBJECT_N_Z 10 > +#define TOUCH_OBJECT_N_X_WIDTH 11 > +#define TOUCH_OBJECT_N_Y_WIDTH 12 > +#define TOUCH_OBJECT_N_TX_POSITION_TIXELS 13 > +#define TOUCH_OBJECT_N_RX_POSITION_TIXELS 14 > +#define TOUCH_0D_BUTTONS_STATE 15 > +#define TOUCH_GESTURE_DOUBLE_TAP 16 > +#define TOUCH_FRAME_RATE 17 /* Normally 80hz */ > +#define TOUCH_POWER_IM 18 > +#define TOUCH_CID_IM 19 > +#define TOUCH_RAIL_IM 20 > +#define TOUCH_CID_VARIANCE_IM 21 > +#define TOUCH_NSM_FREQUENCY 22 > +#define TOUCH_NSM_STATE 23 > +#define TOUCH_NUM_OF_ACTIVE_OBJECTS 23 > +#define TOUCH_NUM_OF_CPU_CYCLES_USED_SINCE_LAST_FRAME 24 > +#define TOUCH_TUNING_GAUSSIAN_WIDTHS 0x80 > +#define TOUCH_TUNING_SMALL_OBJECT_PARAMS 0x81 > +#define TOUCH_TUNING_0D_BUTTONS_VARIANCE 0x82 > +#define TOUCH_REPORT_GESTURE_SWIPE 193 > +#define TOUCH_REPORT_GESTURE_CIRCLE 194 > +#define TOUCH_REPORT_GESTURE_UNICODE 195 > +#define TOUCH_REPORT_GESTURE_VEE 196 > +#define TOUCH_REPORT_GESTURE_TRIANGLE 197 > +#define TOUCH_REPORT_GESTURE_INFO 198 > +#define TOUCH_REPORT_GESTURE_COORDINATE 199 > +#define TOUCH_REPORT_CUSTOMER_GRIP_INFO 203 I don't think all these are used. Also, could we align values on the same column please? > + > +struct tcm_message_header { > + u8 marker; > + u8 code; > +} __packed; I don't think this needs to be packed. > + > +/* header + 2 bytes (which are length of data depending on report code) */ > +#define REPORT_PEAK_LEN (sizeof(struct tcm_message_header) + 2) > + > +struct tcm_cmd { > + u8 cmd; > + u16 length; > + u8 data[]; > +}; > + > +struct tcm_identification { > + struct tcm_message_header header; > + u16 length; > + u8 version; > + u8 mode; > + char part_number[16]; > + u8 build_id[4]; > + u8 max_write_size[2]; > +} __packed; > + > +struct tcm_app_info { > + struct tcm_message_header header; > + u16 length; On-wire data needs to be annotated with proper endiannes (__le16 or __be16) and converted to CPU-endianness before use. > + u8 version[2]; > + u16 status; > + u8 static_config_size[2]; Should this be __le16 or __be16. > + u8 dynamic_config_size[2]; > + u8 app_config_start_write_block[2]; > + u8 app_config_size[2]; > + u8 max_touch_report_config_size[2]; > + u8 max_touch_report_payload_size[2]; > + char customer_config_id[16]; > + u16 max_x; > + u16 max_y; > + u8 max_objects[2]; > + u8 num_of_buttons[2]; > + u8 num_of_image_rows[2]; > + u8 num_of_image_cols[2]; > + u8 has_hybrid_data[2]; > +} __packed; > + > +struct tcm_report_config_prop { > + u8 id; /* TOUCH_OBJECT_* */ > + u8 bits; /* Size of the field in bits */ > +}; > + > +struct tcm_report_config_entry { > + u8 foreach; /* TOUCH_FOREACH_* (and maybe other things?) */ > + int n_props; > + const struct tcm_report_config_prop *props; > +}; > + > +struct tcm_report_config { > + int n_entries; > + const struct tcm_report_config_entry *entries; > +}; > + > +struct tcm_data { > + struct i2c_client *client; > + struct regmap *regmap; > + struct input_dev *input; > + struct touchscreen_properties prop; > + struct gpio_desc *reset_gpio; > + struct completion response; > + struct regulator_bulk_data supplies[2]; > + > + /* annoying state */ > + u16 buf_size; > + char buf[256]; > +}; > + > +static int tcm_send_cmd(struct tcm_data *tcm, struct tcm_cmd *cmd) > +{ > + struct i2c_client *client = tcm->client; > + struct i2c_msg msg; > + int ret; > + > + dev_dbg(&client->dev, "sending command %#x\n", cmd->cmd); > + > + msg.addr = client->addr; > + msg.flags = 0; > + msg.len = 1 + cmd->length; > + msg.buf = (u8 *)cmd; > + > + ret = i2c_transfer(client->adapter, &msg, 1); > + if (ret == 1) > + return 0; > + else if (ret < 0) > + return ret; > + else > + return -EIO; > +} > + > +static int tcm_send_cmd_noargs(struct tcm_data *tcm, u8 cmd) > +{ > + struct tcm_cmd c = { > + .cmd = cmd, > + .length = 0, > + }; > + > + return tcm_send_cmd(tcm, &c); > +} > + > +static int tcm_recv_report(struct tcm_data *tcm, > + void *buf, size_t length) > +{ > + struct i2c_client *client = tcm->client; > + struct i2c_msg msg; > + int ret; > + > + msg.addr = client->addr; > + msg.flags = I2C_M_RD; > + msg.len = length; > + msg.buf = buf; > + > + ret = i2c_transfer(client->adapter, &msg, 1); > + if (ret == 1) > + return 0; > + else if (ret < 0) > + return ret; > + else > + return -EIO; > +} > + > +static int tcm_read_message(struct tcm_data *tcm, u8 cmd, void *buf, size_t length) > +{ > + int ret; > + > + reinit_completion(&tcm->response); > + ret = tcm_send_cmd_noargs(tcm, cmd); > + if (ret) > + return ret; > + > + ret = wait_for_completion_timeout(&tcm->response, msecs_to_jiffies(1000)); > + if (ret == 0) > + return -ETIMEDOUT; > + > + if (buf) { > + if (length > tcm->buf_size) { > + dev_warn(&tcm->client->dev, "expected %zu bytes, got %u\n", > + length, tcm->buf_size); > + } > + length = min(tcm->buf_size, length); > + memcpy(buf, tcm->buf, length); > + } > + > + return 0; > +} > + > +static void tcm_power_off(void *data) > +{ > + struct tcm_data *tcm = data; > + > + disable_irq(tcm->client->irq); > + regulator_bulk_disable(ARRAY_SIZE(tcm->supplies), tcm->supplies); > +} > + > +static int tcm_input_open(struct input_dev *dev) > +{ > + struct tcm_data *tcm = input_get_drvdata(dev); > + > + return i2c_smbus_write_byte(tcm->client, TCM_ENABLE_REPORT); > +} > + > +static void tcm_input_close(struct input_dev *dev) > +{ > + struct tcm_data *tcm = input_get_drvdata(dev); > + int ret; > + > + ret = i2c_smbus_write_byte(tcm->client, TCM_DISABLE_REPORT); > + if (ret) > + dev_err(&tcm->client->dev, "failed to turn off sensing\n"); > +} > + > +/* > + * The default report config looks like this: > + * > + * a5 01 80 00 11 08 1e 08 0f 01 04 01 06 04 07 04 > + * 08 0c 09 0c 0a 08 0b 08 0c 08 0d 10 0e 10 03 00 > + * 00 00 > + * > + * a5 01 80 00 - HEADER + length > + * > + * 11 08 - TOUCH_FRAME_RATE (8 bits) > + * 30 08 - UNKNOWN (8 bits) > + * 0f 01 - TOUCH_0D_BUTTONS_STATE (1 bit) > + * 04 01 - TOUCH_PAD_TO_NEXT_BYTE (7 bits - padding) > + * 06 04 - TOUCH_OBJECT_N_INDEX (4 bits) > + * 07 04 - TOUCH_OBJECT_N_CLASSIFICATION (4 bits) > + * 08 0c - TOUCH_OBJECT_N_X_POSITION (12 bits) > + * 09 0c - TOUCH_OBJECT_N_Y_POSITION (12 bits) > + * 0a 08 - TOUCH_OBJECT_N_Z (8 bits) > + * 0b 08 - TOUCH_OBJECT_N_X_WIDTH (8 bits) > + * 0c 08 - TOUCH_OBJECT_N_Y_WIDTH (8 bits) > + * 0d 10 - TOUCH_OBJECT_N_TX_POSITION_TIXELS (16 bits) ?? > + * 0e 10 - TOUCH_OBJECT_N_RX_POSITION_TIXELS (16 bits) ?? > + * 03 00 - TOUCH_FOREACH_END (0 bits) > + * 00 00 - TOUCH_END (0 bits) > + * > + * Since we only support this report config, we just hardcode the format below. > + * To support additional report configs, we would need to parse the config and > + * use it to parse the reports dynamically. > + */ > + > +struct tcm_default_report_data { > + u8 fps; > + struct { > + u8 unknown; > + u8 buttons; > + u8 idx : 4; > + u8 classification : 4; > + u16 x : 12; > + u16 y : 12; This will be a mess on big endian. Please no bitfields. Use normally sized fields (u8, u16, etc) and masks/shifts to get data. > + u8 z; > + u8 width_x; > + u8 width_y; > + u8 tx; > + u8 rx; > + } __packed points[]; > +} __packed; > + > +static int tcm_handle_touch_report(struct tcm_data *tcm, char *buf, size_t len) > +{ > + struct tcm_default_report_data *data; > + > + buf += REPORT_PEAK_LEN; > + len -= REPORT_PEAK_LEN; > + > + dev_dbg(&tcm->client->dev, "touch report len %zu\n", len); > + if ((len - 1) % 11) > + dev_err(&tcm->client->dev, "invalid touch report length\n"); > + > + data = (struct tcm_default_report_data *)buf; > + > + /* We don't need to report releases because we have INPUT_MT_DROP_UNUSED */ > + for (int i = 0; i < (len - 1) / 11; i++) { > + u8 major_width, minor_width; > + > + minor_width = data->points[i].width_x; > + major_width = data->points[i].width_y; > + > + if (minor_width > major_width) > + swap(major_width, minor_width); > + > + dev_dbg(&tcm->client->dev, "touch report: idx %u x %u y %u\n", > + data->points[i].idx, data->points[i].x, data->points[i].y); > + input_mt_slot(tcm->input, data->points[i].idx); > + input_mt_report_slot_state(tcm->input, MT_TOOL_FINGER, true); > + > + input_report_abs(tcm->input, ABS_MT_POSITION_X, data->points[i].x); > + input_report_abs(tcm->input, ABS_MT_POSITION_Y, data->points[i].y); touchscreen_report_pos(...) instead of the 2 above so that rotation/axis swap will be handled properly. > + input_report_abs(tcm->input, ABS_MT_TOUCH_MAJOR, major_width); > + input_report_abs(tcm->input, ABS_MT_TOUCH_MINOR, minor_width); > + input_report_abs(tcm->input, ABS_MT_PRESSURE, data->points[i].z); > + } > + > + input_mt_sync_frame(tcm->input); > + input_sync(tcm->input); > + > + return 0; > +} > + > +static irqreturn_t tcm_report_irq(int irq, void *data) > +{ > + struct tcm_data *tcm = data; > + struct tcm_message_header *header; > + char buf[256]; > + u16 len; > + int ret; > + > + header = (struct tcm_message_header *)buf; > + ret = tcm_recv_report(tcm, buf, sizeof(buf)); > + if (ret) { > + dev_err(&tcm->client->dev, "failed to read report: %d\n", ret); > + return IRQ_HANDLED; > + } > + > + switch (header->code) { > + case REPORT_OK: > + case REPORT_IDENTIFY: > + case REPORT_TOUCH: > + case REPORT_DELTA: > + case REPORT_RAW: > + case REPORT_DEBUG: > + case REPORT_TOUCH_HOLD: > + break; > + default: > + dev_dbg(&tcm->client->dev, "Ignoring report %#x\n", header->code); > + return IRQ_HANDLED; > + } > + > + /* Not present for REPORT_CONTINUED_READ */ > + len = get_unaligned_le16(buf + sizeof(*header)); > + > + dev_dbg(&tcm->client->dev, "report %#x len %u\n", header->code, len); > + print_hex_dump_bytes("report: ", DUMP_PREFIX_OFFSET, buf, > + min(sizeof(buf), len + sizeof(*header))); > + > + if (len > sizeof(buf) - sizeof(*header)) { > + dev_err(&tcm->client->dev, "report too long\n"); > + return IRQ_HANDLED; > + } > + > + /* Check if this is a read response or an indication. For indications > + * (user touched the screen) we just parse the report directly. > + */ > + if (completion_done(&tcm->response) && header->code == REPORT_TOUCH) { > + tcm_handle_touch_report(tcm, buf, len + sizeof(*header)); > + return IRQ_HANDLED; > + } > + > + tcm->buf_size = len + sizeof(*header); > + memcpy(tcm->buf, buf, len + sizeof(*header)); > + complete(&tcm->response); > + > + return IRQ_HANDLED; > +} > + > +static int tcm_hw_init(struct tcm_data *tcm, u16 *max_x, u16 *max_y) > +{ > + int ret; > + struct tcm_identification id = { 0 }; > + struct tcm_app_info app_info = { 0 }; > + > + /* > + * Tell the firmware to start up. After starting it sends an IDENTIFY report, which > + * we treat like a response to this message even though it's technically a new report. > + */ > + ret = tcm_read_message(tcm, TCM_RUN_APPLICATION_FIRMWARE, &id, sizeof(id)); > + if (ret) { > + dev_err(&tcm->client->dev, "failed to identify device: %d\n", ret); > + return ret; > + } > + > + dev_dbg(&tcm->client->dev, "Synaptics TCM %s v%d mode %d\n", > + id.part_number, id.version, id.mode); > + if (id.mode != MODE_APPLICATION) { > + /* We don't support firmware updates or anything else */ > + dev_err(&tcm->client->dev, "Device is not in application mode\n"); > + return -ENODEV; > + } > + > + do { > + msleep(20); > + ret = tcm_read_message(tcm, TCM_GET_APPLICATION_INFO, &app_info, sizeof(app_info)); > + if (ret) { > + dev_err(&tcm->client->dev, "failed to get application info: %d\n", ret); > + return ret; > + } > + } while (app_info.status == APP_STATUS_BOOTING || app_info.status == APP_STATUS_UPDATING); > + > + dev_dbg(&tcm->client->dev, "Application firmware v%d.%d (customer '%s') status %d\n", > + app_info.version[0], app_info.version[1], app_info.customer_config_id, > + app_info.status); > + > + *max_x = app_info.max_x; > + *max_y = app_info.max_y; > + > + return 0; > +} > + > +static int tcm_power_on(struct tcm_data *tcm) > +{ > + int ret; > + > + ret = regulator_bulk_enable(ARRAY_SIZE(tcm->supplies), > + tcm->supplies); > + if (ret) > + return ret; > + > + gpiod_set_value(tcm->reset_gpio, false); > + usleep_range(10000, 11000); > + gpiod_set_value(tcm->reset_gpio, true); Here you have reset GPIO asserted, which means that the controller will stay in reset state indefinitely. gpiod API operates on logical states (active/inactive) and performs conversion to physical state (LOW/HIGH) internally. Also we should use gpiod_set_value_cansleep() unless we are in atomic context. > + usleep_range(80000, 81000); > + > + return 0; > +} > + > +static int tcm_probe(struct i2c_client *client) > +{ > + struct tcm_data *tcm; > + struct tcm_report_config __maybe_unused report_config; It is definitely not used, there no "maybe" about it. > + u16 max_x, max_y; > + int ret; Call this and similar variables holding error code "error" please. > + > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C | > + I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_I2C_BLOCK)) > + return -ENODEV; > + > + tcm = devm_kzalloc(&client->dev, sizeof(struct tcm_data), GFP_KERNEL); sizeof(*tcm) > + if (!tcm) > + return -ENOMEM; > + > + i2c_set_clientdata(client, tcm); > + tcm->client = client; > + > + init_completion(&tcm->response); > + > + tcm->supplies[0].supply = "vdd"; > + tcm->supplies[1].supply = "vcc"; > + ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(tcm->supplies), > + tcm->supplies); > + if (ret) > + return ret; > + > + tcm->reset_gpio = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH); Do all designs require GPIO line be specified? Can it be made optional? > + > + ret = devm_add_action_or_reset(&client->dev, tcm_power_off, > + tcm); > + if (ret) > + return ret; > + > + ret = tcm_power_on(tcm); > + if (ret) > + return ret; > + > + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL, > + tcm_report_irq, > + IRQF_TRIGGER_LOW | IRQF_ONESHOT, Please do not hardcode trigger (besides IRQF_ONESHOT), let platform decide. Also interrupt is hot there, are you sure we will not try to report input events for not-yet-allocated input device if someone is thouching the device at this point? > + "synaptics_tcm_report", tcm); > + if (ret < 0) > + return ret; > + > + ret = tcm_hw_init(tcm, &max_x, &max_y); > + if (ret) { > + dev_err(&client->dev, "failed to initialize hardware\n"); > + return ret; > + } > + > + tcm->input = devm_input_allocate_device(&client->dev); > + if (!tcm->input) > + return -ENOMEM; > + > + tcm->input->name = "Synaptics TCM Oncell Touchscreen"; > + tcm->input->id.bustype = BUS_I2C; > + tcm->input->open = tcm_input_open; > + tcm->input->close = tcm_input_close; > + > + input_set_abs_params(tcm->input, ABS_MT_POSITION_X, 0, max_x, 0, 0); > + input_set_abs_params(tcm->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0); > + input_set_abs_params(tcm->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0); > + input_set_abs_params(tcm->input, ABS_MT_TOUCH_MINOR, 0, 255, 0, 0); > + input_set_abs_params(tcm->input, ABS_MT_PRESSURE, 0, 255, 0, 0); > + > + touchscreen_parse_properties(tcm->input, true, &tcm->prop); > + > + ret = input_mt_init_slots(tcm->input, 10, INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); > + if (ret) > + return ret; > + > + input_set_drvdata(tcm->input, tcm); > + > + ret = input_register_device(tcm->input); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static const struct of_device_id syna_driver_ids[] = { > + { > + .compatible = "syna,s3908", > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, syna_driver_ids); > + > +static const struct i2c_device_id syna_i2c_ids[] = { > + { "synaptics-tcm", 0 }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, syna_i2c_ids); > + > +static struct i2c_driver syna_i2c_driver = { > + .probe = tcm_probe, > + .id_table = syna_i2c_ids, > + .driver = { > + .name = "synaptics-tcm", > + .of_match_table = syna_driver_ids, > + }, > +}; > + > +module_i2c_driver(syna_i2c_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx>"); > +MODULE_AUTHOR("Caleb Connolly <caleb@xxxxxxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("A driver for Synaptics TCM Oncell Touchpanels"); > + > > -- > 2.45.0 > -- Dmitry