| Message ID | 20180403180348.18444-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, On Tue, Apr 03, 2018 at 08:03:48PM +0200, Hans de Goede wrote: > The ChipOne icn8505 is an i2c capacitive touchscreen controller typically > used in cheap x86 tablets, this commit adds a driver for it. > > Note the icn8505 is somewhat similar to the icn8318 and I started with > modifying that driver to support both, but in the end the differences were > too large and I decided to write a new driver instead. > > Cc: Antonio Davide Trogu <trogu.davide@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > MAINTAINERS | 6 + > drivers/input/touchscreen/Kconfig | 11 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/chipone_icn8505.c | 512 ++++++++++++++++++++ > 4 files changed, 530 insertions(+) > create mode 100644 drivers/input/touchscreen/chipone_icn8505.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index fe2574f68c34..563f04b40981 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3436,6 +3436,12 @@ S: Maintained > F: Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt > F: drivers/input/touchscreen/chipone_icn8318.c > > +CHIPONE ICN8505 I2C TOUCHSCREEN DRIVER > +M: Hans de Goede <hdegoede@redhat.com> > +L: linux-input@vger.kernel.org > +S: Maintained > +F: drivers/input/touchscreen/chipone_icn8505.c > + > CHROME HARDWARE PLATFORM SUPPORT > M: Benson Leung <bleung@chromium.org> > M: Olof Johansson <olof@lixom.net> > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index 4f15496fec8b..94cc740a4203 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -164,6 +164,17 @@ config TOUCHSCREEN_CHIPONE_ICN8318 > To compile this driver as a module, choose M here: the > module will be called chipone_icn8318. > > +config TOUCHSCREEN_CHIPONE_ICN8505 > + tristate "chipone icn8505 touchscreen controller" > + depends on I2C && ACPI > + help > + Say Y here if you have a ChipOne icn8505 based I2C touchscreen. > + > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called chipone_icn8505. > + > config TOUCHSCREEN_CY8CTMG110 > tristate "cy8ctmg110 touchscreen" > depends on I2C > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index dddae7973436..fd4fd32fb73f 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o > obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o > obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o > obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o > +obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8505) += chipone_icn8505.o > obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o > obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o > obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C) += cyttsp_i2c.o cyttsp_i2c_common.o > diff --git a/drivers/input/touchscreen/chipone_icn8505.c b/drivers/input/touchscreen/chipone_icn8505.c > new file mode 100644 > index 000000000000..b524fd6a13a3 > --- /dev/null > +++ b/drivers/input/touchscreen/chipone_icn8505.c > @@ -0,0 +1,512 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Driver for ChipOne icn8505 i2c touchscreen controller > + * > + * Copyright (c) 2015-2018 Red Hat Inc. > + * > + * Red Hat authors: > + * Hans de Goede <hdegoede@redhat.com> > + */ > + > +#include <asm/unaligned.h> > +#include <linux/acpi.h> > +#include <linux/crc32.h> > +#include <linux/delay.h> > +#include <linux/firmware.h> > +#include <linux/interrupt.h> > +#include <linux/i2c.h> > +#include <linux/input.h> > +#include <linux/input/mt.h> > +#include <linux/input/touchscreen.h> > +#include <linux/module.h> > + > +/* Normal operation mode defines */ > +#define ICN8505_REG_ADDR_WIDTH 16 > + > +#define ICN8505_REG_POWER 0x0004 > +#define ICN8505_REG_TOUCHDATA 0x1000 > +#define ICN8505_REG_CONFIGDATA 0x8000 > + > +/* ICN8505_REG_POWER commands */ > +#define ICN8505_POWER_ACTIVE 0x00 > +#define ICN8505_POWER_MONITOR 0x01 > +#define ICN8505_POWER_HIBERNATE 0x02 > +/* > + * The Android driver uses these to turn on/off the charger filter, but the > + * filter is way too aggressive making e.g. onscreen keyboards unusable. > + */ > +#define ICN8505_POWER_ENA_CHARGER_MODE 0x55 > +#define ICN8505_POWER_DIS_CHARGER_MODE 0x66 > + > +#define ICN8505_MAX_TOUCHES 10 > + > +/* Programming mode defines */ > +#define ICN8505_PROG_I2C_ADDR 0x30 > +#define ICN8505_PROG_REG_ADDR_WIDTH 24 > + > +#define MAX_FW_UPLOAD_TRIES 3 > + > +struct icn8505_touch { > + __u8 slot; > + __u8 x[2]; > + __u8 y[2]; > + __u8 pressure; /* Seems more like finger width then pressure really */ > + __u8 event; > +/* The difference between 2 and 3 is unclear */ > +#define ICN8505_EVENT_NO_DATA 1 /* No finger seen yet since wakeup */ > +#define ICN8505_EVENT_UPDATE1 2 /* New or updated coordinates */ > +#define ICN8505_EVENT_UPDATE2 3 /* New or updated coordinates */ > +#define ICN8505_EVENT_END 4 /* Finger lifted */ > +} __packed; > + > +struct icn8505_touch_data { > + __u8 softbutton; > + __u8 touch_count; > + struct icn8505_touch touches[ICN8505_MAX_TOUCHES]; > +} __packed; > + > +struct icn8505_data { > + struct i2c_client *client; > + struct input_dev *input; > + struct gpio_desc *wake_gpio; > + struct touchscreen_properties prop; > + char firmware_name[32]; > +}; > + > +static int icn8505_read_xfer(struct i2c_client *client, u16 i2c_addr, > + int reg_addr, int reg_addr_width, > + void *data, int len, bool silent) > +{ > + u8 buf[3]; > + int i, ret; > + struct i2c_msg msg[2] = { > + { > + .addr = i2c_addr, > + .buf = buf, > + .len = reg_addr_width / 8, > + }, > + { > + .addr = i2c_addr, > + .flags = I2C_M_RD, > + .buf = data, > + .len = len, > + } > + }; > + > + for (i = 0; i < (reg_addr_width / 8); i++) > + buf[i] = (reg_addr >> (reg_addr_width - (i + 1) * 8)) & 0xff; > + > + ret = i2c_transfer(client->adapter, msg, 2); > + if (ret < 0 && !silent) > + dev_err(&client->dev, "Error reading addr %#x reg %#x: %d\n", > + i2c_addr, reg_addr, ret); I'd rather we did: if (ret != ARRAY_SIZE(msg)) { if (ret >= 0) ret = -EIO; if (!silent) dev_err(...); return ret; } return 0; > + > + return ret; > +} > + > +static int icn8505_write_xfer(struct i2c_client *client, u16 i2c_addr, > + int reg_addr, int reg_addr_width, > + const void *data, int len, bool silent) > +{ > + u8 buf[3 + 32]; /* 3 bytes for 24 bit reg-addr + 32 bytes max len */ > + int i, ret; > + struct i2c_msg msg = { > + .addr = i2c_addr, > + .buf = buf, > + .len = reg_addr_width / 8 + len, > + }; > + > + if (WARN_ON(len > 32)) > + return -EINVAL; > + > + for (i = 0; i < (reg_addr_width / 8); i++) > + buf[i] = (reg_addr >> (reg_addr_width - (i + 1) * 8)) & 0xff; > + > + memcpy(buf + reg_addr_width / 8, data, len); > + > + ret = i2c_transfer(client->adapter, &msg, 1); > + if (ret < 0 && !silent) > + dev_err(&client->dev, "Error writing addr %#x reg %#x: %d\n", > + i2c_addr, reg_addr, ret); Same here. > + > + return ret; > +} > + > +static int icn8505_read_data(struct icn8505_data *icn8505, int reg, > + void *buf, int len) > +{ > + return icn8505_read_xfer(icn8505->client, icn8505->client->addr, reg, > + ICN8505_REG_ADDR_WIDTH, buf, len, false); > +} > + > +static int icn8505_read_reg_silent(struct icn8505_data *icn8505, int reg) > +{ > + u8 buf; > + int ret; > + > + ret = icn8505_read_xfer(icn8505->client, icn8505->client->addr, reg, > + ICN8505_REG_ADDR_WIDTH, &buf, 1, true); > + if (ret < 0) > + return ret; > + > + return buf; > +} > + > +static int icn8505_write_reg(struct icn8505_data *icn8505, int reg, u8 val) > +{ > + return icn8505_write_xfer(icn8505->client, icn8505->client->addr, reg, > + ICN8505_REG_ADDR_WIDTH, &val, 1, false); > +} > + > +static int icn8505_read_prog_data(struct icn8505_data *icn8505, int reg, > + void *buf, int len) > +{ > + return icn8505_read_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, > + ICN8505_PROG_REG_ADDR_WIDTH, buf, len, false); > +} > + > +static int icn8505_write_prog_data(struct icn8505_data *icn8505, int reg, > + const void *buf, int len) > +{ > + return icn8505_write_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, > + ICN8505_PROG_REG_ADDR_WIDTH, buf, len, false); > +} > + > +static int icn8505_write_prog_reg(struct icn8505_data *icn8505, int reg, u8 val) > +{ > + return icn8505_write_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, > + ICN8505_PROG_REG_ADDR_WIDTH, &val, 1, false); > +} > + > +/* > + * Note this function uses a number of magic register addresses and values, > + * there are deliberately no defines for these because the algorithm is taken > + * from the icn85xx Android driver and I do not want to make up possibly wrong > + * names for the addresses and/or values. > + */ > +static int icn8505_try_fw_upload(struct icn8505_data *icn8505, > + const struct firmware *fw, int try) "Try" is used only in error messages, I'd rather reported that in icn8505_upload_fw(): dev_err(dev, "Failed to upload firmware: %d (attempt %d of %d)\n", error, try, MAX_FW_UPLOAD_TRIES); > +{ > + struct device *dev = &icn8505->client->dev; > + size_t offset, count; > + u8 buf[4]; > + u32 crc; > + int ret; > + > + /* Put the controller in programming mode */ > + ret = icn8505_write_prog_reg(icn8505, 0xcc3355, 0x5a); > + if (ret < 0) > + return ret; Please call this "error". > + > + usleep_range(2000, 5000); > + > + ret = icn8505_write_prog_reg(icn8505, 0x040400, 0x01); > + if (ret < 0) > + return ret; > + > + usleep_range(2000, 5000); > + > + ret = icn8505_read_prog_data(icn8505, 0x040002, buf, 1); > + if (ret < 0) > + return ret; > + > + if (buf[0] != 0x85) { > + dev_err(dev, "Failed to enter programming mode try %d/%d\n", > + try, MAX_FW_UPLOAD_TRIES); > + return -ENODEV; > + } > + > + usleep_range(1000, 5000); > + > + /* Enable CRC mode */ > + ret = icn8505_write_prog_reg(icn8505, 0x40028, 1); > + if (ret < 0) > + return ret; > + > + /* Send the firmware to SRAM */ > + for (offset = 0; offset < fw->size; offset += count) { > + count = min_t(size_t, fw->size - offset, 32); > + ret = icn8505_write_prog_data(icn8505, offset, > + fw->data + offset, count); > + if (ret < 0) > + return ret; > + } > + > + /* Disable CRC mode */ > + ret = icn8505_write_prog_reg(icn8505, 0x40028, 0); > + if (ret < 0) > + return ret; > + > + /* Get and check length and CRC */ > + ret = icn8505_read_prog_data(icn8505, 0x40034, buf, 2); > + if (ret < 0) > + return ret; > + > + if (get_unaligned_le16(buf) != fw->size) { > + dev_warn(dev, "Length mismatch after uploading fw try %d/%d\n", > + try, MAX_FW_UPLOAD_TRIES); > + return -EIO; > + } > + > + ret = icn8505_read_prog_data(icn8505, 0x4002c, buf, 4); > + if (ret < 0) > + return ret; > + > + crc = crc32_be(0, fw->data, fw->size); > + if (get_unaligned_le32(buf) != crc) { > + dev_warn(dev, "CRC mismatch after uploading fw try %d/%d\n", > + try, MAX_FW_UPLOAD_TRIES); > + return -EIO; > + } > + > + /* Boot controller from SRAM */ > + ret = icn8505_write_prog_reg(icn8505, 0x40400, 0x03); > + if (ret < 0) > + return ret; > + > + usleep_range(2000, 5000); > + return 0; > +} > + > +static int icn8505_upload_fw(struct icn8505_data *icn8505) > +{ > + struct device *dev = &icn8505->client->dev; > + const struct firmware *fw; > + int i, ret; > + > + /* > + * Always load the firmware, even if we don't need it at boot, we > + * we may need it at resume. Having loaded it once will make the > + * firmware class code cache it at suspend/resume. > + */ > + ret = request_firmware(&fw, icn8505->firmware_name, dev); > + if (ret) { > + dev_err(dev, "Firmware request error %d\n", ret); > + return ret; > + } > + > + /* Check if the controller is not already up and running */ > + if (icn8505_read_reg_silent(icn8505, 0x000a) == 0x85) > + goto success; > + > + for (i = 1; i <= MAX_FW_UPLOAD_TRIES; i++) { > + ret = icn8505_try_fw_upload(icn8505, fw, i); > + if (ret >= 0) > + goto success; > + > + usleep_range(2000, 5000); > + } > + > + dev_err(dev, "Error uploading fw: %d\n", ret); > + > +success: > + release_firmware(fw); > + return ret; > +} > + > +static inline bool icn8505_touch_active(u8 event) No need to mark inline explicitly, let compiler decide. > +{ > + return (event == ICN8505_EVENT_UPDATE1) || > + (event == ICN8505_EVENT_UPDATE2); > +} > + > +static irqreturn_t icn8505_irq(int irq, void *dev_id) > +{ > + struct icn8505_data *icn8505 = dev_id; > + struct device *dev = &icn8505->client->dev; > + struct icn8505_touch_data touch_data; > + int i, ret; > + > + ret = icn8505_read_data(icn8505, ICN8505_REG_TOUCHDATA, > + &touch_data, sizeof(touch_data)); > + if (ret < 0) { > + dev_err(dev, "Error reading touch data: %d\n", ret); > + return IRQ_HANDLED; > + } > + > + if (touch_data.touch_count > ICN8505_MAX_TOUCHES) { > + dev_warn(dev, "Too much touches %d > %d\n", many. > + touch_data.touch_count, ICN8505_MAX_TOUCHES); > + touch_data.touch_count = ICN8505_MAX_TOUCHES; > + } > + > + for (i = 0; i < touch_data.touch_count; i++) { > + struct icn8505_touch *touch = &touch_data.touches[i]; > + bool act = icn8505_touch_active(touch->event); > + > + input_mt_slot(icn8505->input, touch->slot); > + input_mt_report_slot_state(icn8505->input, MT_TOOL_FINGER, act); > + if (!act) > + continue; > + > + touchscreen_report_pos(icn8505->input, &icn8505->prop, > + get_unaligned_le16(touch->x), > + get_unaligned_le16(touch->y), > + true); > + } > + > + input_mt_sync_frame(icn8505->input); > + input_report_key(icn8505->input, KEY_LEFTMETA, > + touch_data.softbutton == 1); > + input_sync(icn8505->input); > + > + return IRQ_HANDLED; > +} > + > +static int icn8505_probe_acpi(struct icn8505_data *icn8505, struct device *dev) > +{ > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > + const char *subsys = "unknown"; > + struct acpi_device *adev; > + union acpi_object *obj; > + acpi_status status; > + > + adev = ACPI_COMPANION(dev); > + if (!adev) > + return -ENODEV; > + > + status = acpi_evaluate_object(adev->handle, "_SUB", NULL, &buffer); > + if (ACPI_SUCCESS(status)) { > + obj = buffer.pointer; > + if (obj->type == ACPI_TYPE_STRING) > + subsys = obj->string.pointer; > + else > + dev_warn(dev, "Warning ACPI _SUB did not return a string\n"); > + } else { > + dev_warn(dev, "Warning ACPI _SUB failed: %#x\n", status); > + buffer.pointer = NULL; > + } > + > + snprintf(icn8505->firmware_name, sizeof(icn8505->firmware_name), > + "chipone/icn8505-%s.fw", subsys); What if string is too long? How about using devm_kasprintf()? > + > + kfree(buffer.pointer); > + return 0; > +} > + > +static int icn8505_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct icn8505_data *icn8505; > + struct input_dev *input; > + __le16 resolution[2]; > + int ret; > + > + if (!client->irq) { > + dev_err(dev, "Error no irq specified\n"); We know it is error from dev_err, please drop "Error" prefix. > + return -EINVAL; > + } > + > + icn8505 = devm_kzalloc(dev, sizeof(*icn8505), GFP_KERNEL); > + if (!icn8505) > + return -ENOMEM; > + > + input = devm_input_allocate_device(dev); > + if (!input) > + return -ENOMEM; > + > + input->name = client->name; > + input->id.bustype = BUS_I2C; > + input->dev.parent = dev; Done by devm_input_allocate_device(). > + > + input_set_capability(input, EV_ABS, ABS_MT_POSITION_X); > + input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y); > + input_set_capability(input, EV_KEY, KEY_LEFTMETA); > + > + icn8505->client = client; > + icn8505->input = input; > + input_set_drvdata(input, icn8505); > + > + ret = icn8505_probe_acpi(icn8505, dev); > + if (ret) > + return ret; Can we please call this "error"? > + > + ret = icn8505_upload_fw(icn8505); > + if (ret < 0) > + return ret; It does not look like we need firmware to determine device capabilities, consider moving it into ->open()? > + > + ret = icn8505_read_data(icn8505, ICN8505_REG_CONFIGDATA, > + resolution, sizeof(resolution)); > + if (ret < 0) { > + dev_err(dev, "Error reading resolution: %d\n", ret); > + return ret; > + } > + > + input_set_abs_params(input, ABS_MT_POSITION_X, 0, > + le16_to_cpu(resolution[0]) - 1, 0, 0); > + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, > + le16_to_cpu(resolution[1]) - 1, 0, 0); > + > + touchscreen_parse_properties(input, true, &icn8505->prop); > + if (!input_abs_get_max(input, ABS_MT_POSITION_X) || > + !input_abs_get_max(input, ABS_MT_POSITION_Y)) { > + dev_err(dev, "Error touchscreen-size-x and/or -y missing\n"); > + return -EINVAL; > + } > + > + ret = input_mt_init_slots(input, ICN8505_MAX_TOUCHES, > + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); > + if (ret) > + return ret; > + > + ret = devm_request_threaded_irq(dev, client->irq, NULL, icn8505_irq, > + IRQF_ONESHOT, client->name, icn8505); > + if (ret) { > + dev_err(dev, "Error requesting irq: %d\n", ret); > + return ret; > + } > + > + ret = input_register_device(input); > + if (ret) > + return ret; > + > + i2c_set_clientdata(client, icn8505); > + return 0; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int icn8505_suspend(struct device *dev) __maybe_unused instead of #ifdef please. > +{ > + struct icn8505_data *icn8505 = i2c_get_clientdata(to_i2c_client(dev)); > + > + disable_irq(icn8505->client->irq); > + icn8505_write_reg(icn8505, ICN8505_REG_POWER, ICN8505_POWER_HIBERNATE); > + return 0; > +} > + > +static int icn8505_resume(struct device *dev) > +{ > + struct icn8505_data *icn8505 = i2c_get_clientdata(to_i2c_client(dev)); > + int ret; > + > + ret = icn8505_upload_fw(icn8505); > + if (ret < 0) > + return ret; > + > + enable_irq(icn8505->client->irq); > + return 0; > +} > +#endif > + > +static SIMPLE_DEV_PM_OPS(icn8505_pm_ops, icn8505_suspend, icn8505_resume); > + > +static const struct acpi_device_id icn8505_acpi_match[] = { > + { "CHPN0001" }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, icn8505_acpi_match); > + > +static struct i2c_driver icn8505_driver = { > + .driver = { > + .name = "chipone_icn8505", > + .pm = &icn8505_pm_ops, > + .acpi_match_table = icn8505_acpi_match, > + }, > + .probe_new = icn8505_probe, > +}; > + > +module_i2c_driver(icn8505_driver); > + > +MODULE_DESCRIPTION("ChipOne icn8505 I2C Touchscreen Driver"); > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > +MODULE_LICENSE("GPL"); > -- > 2.17.0.rc2 > Thanks.
Hi Dmitry, Thank you for the quick review. I've some comments to your comments on my driver in 2 cases inline below. For the rest I agree and will fix things for v2 of this patch. On 07-04-18 00:35, Dmitry Torokhov wrote: > Hi Hans, > > On Tue, Apr 03, 2018 at 08:03:48PM +0200, Hans de Goede wrote: >> The ChipOne icn8505 is an i2c capacitive touchscreen controller typically >> used in cheap x86 tablets, this commit adds a driver for it. >> >> Note the icn8505 is somewhat similar to the icn8318 and I started with >> modifying that driver to support both, but in the end the differences were >> too large and I decided to write a new driver instead. >> >> Cc: Antonio Davide Trogu <trogu.davide@gmail.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- <snip> >> +static int icn8505_probe_acpi(struct icn8505_data *icn8505, struct device *dev) >> +{ >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; >> + const char *subsys = "unknown"; >> + struct acpi_device *adev; >> + union acpi_object *obj; >> + acpi_status status; >> + >> + adev = ACPI_COMPANION(dev); >> + if (!adev) >> + return -ENODEV; >> + >> + status = acpi_evaluate_object(adev->handle, "_SUB", NULL, &buffer); >> + if (ACPI_SUCCESS(status)) { >> + obj = buffer.pointer; >> + if (obj->type == ACPI_TYPE_STRING) >> + subsys = obj->string.pointer; >> + else >> + dev_warn(dev, "Warning ACPI _SUB did not return a string\n"); >> + } else { >> + dev_warn(dev, "Warning ACPI _SUB failed: %#x\n", status); >> + buffer.pointer = NULL; >> + } >> + >> + snprintf(icn8505->firmware_name, sizeof(icn8505->firmware_name), >> + "chipone/icn8505-%s.fw", subsys); > > What if string is too long? How about using devm_kasprintf()? The _SUB method returns an ACPI hwid type string which is always 8 chars, so no need for kasprintf() here. <snip> >> + >> + ret = icn8505_upload_fw(icn8505); >> + if (ret < 0) >> + return ret; > > It does not look like we need firmware to determine device capabilities, > consider moving it into ->open()? Without firmware loaded the device nacks any i2c transfers on its regular (non firmware upload) i2c address, so: >> + >> + ret = icn8505_read_data(icn8505, ICN8505_REG_CONFIGDATA, >> + resolution, sizeof(resolution)); >> + if (ret < 0) { >> + dev_err(dev, "Error reading resolution: %d\n", ret); >> + return ret; >> + } This will fail and we cannot determine the resolution. Also at least on the hardware I've when booted normally the EFI code has already uploaded the firmware, so icn8505_upload_fw() hits the path where it exits early because there is already firmware loaded and this is (almost) free. >> + >> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, >> + le16_to_cpu(resolution[0]) - 1, 0, 0); >> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, >> + le16_to_cpu(resolution[1]) - 1, 0, 0); Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/MAINTAINERS b/MAINTAINERS index fe2574f68c34..563f04b40981 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3436,6 +3436,12 @@ S: Maintained F: Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt F: drivers/input/touchscreen/chipone_icn8318.c +CHIPONE ICN8505 I2C TOUCHSCREEN DRIVER +M: Hans de Goede <hdegoede@redhat.com> +L: linux-input@vger.kernel.org +S: Maintained +F: drivers/input/touchscreen/chipone_icn8505.c + CHROME HARDWARE PLATFORM SUPPORT M: Benson Leung <bleung@chromium.org> M: Olof Johansson <olof@lixom.net> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 4f15496fec8b..94cc740a4203 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -164,6 +164,17 @@ config TOUCHSCREEN_CHIPONE_ICN8318 To compile this driver as a module, choose M here: the module will be called chipone_icn8318. +config TOUCHSCREEN_CHIPONE_ICN8505 + tristate "chipone icn8505 touchscreen controller" + depends on I2C && ACPI + help + Say Y here if you have a ChipOne icn8505 based I2C touchscreen. + + If unsure, say N. + + To compile this driver as a module, choose M here: the + module will be called chipone_icn8505. + config TOUCHSCREEN_CY8CTMG110 tristate "cy8ctmg110 touchscreen" depends on I2C diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index dddae7973436..fd4fd32fb73f 100644 --- a/drivers/input/touchscreen/Makefile +++ b/drivers/input/touchscreen/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o +obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8505) += chipone_icn8505.o obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C) += cyttsp_i2c.o cyttsp_i2c_common.o diff --git a/drivers/input/touchscreen/chipone_icn8505.c b/drivers/input/touchscreen/chipone_icn8505.c new file mode 100644 index 000000000000..b524fd6a13a3 --- /dev/null +++ b/drivers/input/touchscreen/chipone_icn8505.c @@ -0,0 +1,512 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Driver for ChipOne icn8505 i2c touchscreen controller + * + * Copyright (c) 2015-2018 Red Hat Inc. + * + * Red Hat authors: + * Hans de Goede <hdegoede@redhat.com> + */ + +#include <asm/unaligned.h> +#include <linux/acpi.h> +#include <linux/crc32.h> +#include <linux/delay.h> +#include <linux/firmware.h> +#include <linux/interrupt.h> +#include <linux/i2c.h> +#include <linux/input.h> +#include <linux/input/mt.h> +#include <linux/input/touchscreen.h> +#include <linux/module.h> + +/* Normal operation mode defines */ +#define ICN8505_REG_ADDR_WIDTH 16 + +#define ICN8505_REG_POWER 0x0004 +#define ICN8505_REG_TOUCHDATA 0x1000 +#define ICN8505_REG_CONFIGDATA 0x8000 + +/* ICN8505_REG_POWER commands */ +#define ICN8505_POWER_ACTIVE 0x00 +#define ICN8505_POWER_MONITOR 0x01 +#define ICN8505_POWER_HIBERNATE 0x02 +/* + * The Android driver uses these to turn on/off the charger filter, but the + * filter is way too aggressive making e.g. onscreen keyboards unusable. + */ +#define ICN8505_POWER_ENA_CHARGER_MODE 0x55 +#define ICN8505_POWER_DIS_CHARGER_MODE 0x66 + +#define ICN8505_MAX_TOUCHES 10 + +/* Programming mode defines */ +#define ICN8505_PROG_I2C_ADDR 0x30 +#define ICN8505_PROG_REG_ADDR_WIDTH 24 + +#define MAX_FW_UPLOAD_TRIES 3 + +struct icn8505_touch { + __u8 slot; + __u8 x[2]; + __u8 y[2]; + __u8 pressure; /* Seems more like finger width then pressure really */ + __u8 event; +/* The difference between 2 and 3 is unclear */ +#define ICN8505_EVENT_NO_DATA 1 /* No finger seen yet since wakeup */ +#define ICN8505_EVENT_UPDATE1 2 /* New or updated coordinates */ +#define ICN8505_EVENT_UPDATE2 3 /* New or updated coordinates */ +#define ICN8505_EVENT_END 4 /* Finger lifted */ +} __packed; + +struct icn8505_touch_data { + __u8 softbutton; + __u8 touch_count; + struct icn8505_touch touches[ICN8505_MAX_TOUCHES]; +} __packed; + +struct icn8505_data { + struct i2c_client *client; + struct input_dev *input; + struct gpio_desc *wake_gpio; + struct touchscreen_properties prop; + char firmware_name[32]; +}; + +static int icn8505_read_xfer(struct i2c_client *client, u16 i2c_addr, + int reg_addr, int reg_addr_width, + void *data, int len, bool silent) +{ + u8 buf[3]; + int i, ret; + struct i2c_msg msg[2] = { + { + .addr = i2c_addr, + .buf = buf, + .len = reg_addr_width / 8, + }, + { + .addr = i2c_addr, + .flags = I2C_M_RD, + .buf = data, + .len = len, + } + }; + + for (i = 0; i < (reg_addr_width / 8); i++) + buf[i] = (reg_addr >> (reg_addr_width - (i + 1) * 8)) & 0xff; + + ret = i2c_transfer(client->adapter, msg, 2); + if (ret < 0 && !silent) + dev_err(&client->dev, "Error reading addr %#x reg %#x: %d\n", + i2c_addr, reg_addr, ret); + + return ret; +} + +static int icn8505_write_xfer(struct i2c_client *client, u16 i2c_addr, + int reg_addr, int reg_addr_width, + const void *data, int len, bool silent) +{ + u8 buf[3 + 32]; /* 3 bytes for 24 bit reg-addr + 32 bytes max len */ + int i, ret; + struct i2c_msg msg = { + .addr = i2c_addr, + .buf = buf, + .len = reg_addr_width / 8 + len, + }; + + if (WARN_ON(len > 32)) + return -EINVAL; + + for (i = 0; i < (reg_addr_width / 8); i++) + buf[i] = (reg_addr >> (reg_addr_width - (i + 1) * 8)) & 0xff; + + memcpy(buf + reg_addr_width / 8, data, len); + + ret = i2c_transfer(client->adapter, &msg, 1); + if (ret < 0 && !silent) + dev_err(&client->dev, "Error writing addr %#x reg %#x: %d\n", + i2c_addr, reg_addr, ret); + + return ret; +} + +static int icn8505_read_data(struct icn8505_data *icn8505, int reg, + void *buf, int len) +{ + return icn8505_read_xfer(icn8505->client, icn8505->client->addr, reg, + ICN8505_REG_ADDR_WIDTH, buf, len, false); +} + +static int icn8505_read_reg_silent(struct icn8505_data *icn8505, int reg) +{ + u8 buf; + int ret; + + ret = icn8505_read_xfer(icn8505->client, icn8505->client->addr, reg, + ICN8505_REG_ADDR_WIDTH, &buf, 1, true); + if (ret < 0) + return ret; + + return buf; +} + +static int icn8505_write_reg(struct icn8505_data *icn8505, int reg, u8 val) +{ + return icn8505_write_xfer(icn8505->client, icn8505->client->addr, reg, + ICN8505_REG_ADDR_WIDTH, &val, 1, false); +} + +static int icn8505_read_prog_data(struct icn8505_data *icn8505, int reg, + void *buf, int len) +{ + return icn8505_read_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, + ICN8505_PROG_REG_ADDR_WIDTH, buf, len, false); +} + +static int icn8505_write_prog_data(struct icn8505_data *icn8505, int reg, + const void *buf, int len) +{ + return icn8505_write_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, + ICN8505_PROG_REG_ADDR_WIDTH, buf, len, false); +} + +static int icn8505_write_prog_reg(struct icn8505_data *icn8505, int reg, u8 val) +{ + return icn8505_write_xfer(icn8505->client, ICN8505_PROG_I2C_ADDR, reg, + ICN8505_PROG_REG_ADDR_WIDTH, &val, 1, false); +} + +/* + * Note this function uses a number of magic register addresses and values, + * there are deliberately no defines for these because the algorithm is taken + * from the icn85xx Android driver and I do not want to make up possibly wrong + * names for the addresses and/or values. + */ +static int icn8505_try_fw_upload(struct icn8505_data *icn8505, + const struct firmware *fw, int try) +{ + struct device *dev = &icn8505->client->dev; + size_t offset, count; + u8 buf[4]; + u32 crc; + int ret; + + /* Put the controller in programming mode */ + ret = icn8505_write_prog_reg(icn8505, 0xcc3355, 0x5a); + if (ret < 0) + return ret; + + usleep_range(2000, 5000); + + ret = icn8505_write_prog_reg(icn8505, 0x040400, 0x01); + if (ret < 0) + return ret; + + usleep_range(2000, 5000); + + ret = icn8505_read_prog_data(icn8505, 0x040002, buf, 1); + if (ret < 0) + return ret; + + if (buf[0] != 0x85) { + dev_err(dev, "Failed to enter programming mode try %d/%d\n", + try, MAX_FW_UPLOAD_TRIES); + return -ENODEV; + } + + usleep_range(1000, 5000); + + /* Enable CRC mode */ + ret = icn8505_write_prog_reg(icn8505, 0x40028, 1); + if (ret < 0) + return ret; + + /* Send the firmware to SRAM */ + for (offset = 0; offset < fw->size; offset += count) { + count = min_t(size_t, fw->size - offset, 32); + ret = icn8505_write_prog_data(icn8505, offset, + fw->data + offset, count); + if (ret < 0) + return ret; + } + + /* Disable CRC mode */ + ret = icn8505_write_prog_reg(icn8505, 0x40028, 0); + if (ret < 0) + return ret; + + /* Get and check length and CRC */ + ret = icn8505_read_prog_data(icn8505, 0x40034, buf, 2); + if (ret < 0) + return ret; + + if (get_unaligned_le16(buf) != fw->size) { + dev_warn(dev, "Length mismatch after uploading fw try %d/%d\n", + try, MAX_FW_UPLOAD_TRIES); + return -EIO; + } + + ret = icn8505_read_prog_data(icn8505, 0x4002c, buf, 4); + if (ret < 0) + return ret; + + crc = crc32_be(0, fw->data, fw->size); + if (get_unaligned_le32(buf) != crc) { + dev_warn(dev, "CRC mismatch after uploading fw try %d/%d\n", + try, MAX_FW_UPLOAD_TRIES); + return -EIO; + } + + /* Boot controller from SRAM */ + ret = icn8505_write_prog_reg(icn8505, 0x40400, 0x03); + if (ret < 0) + return ret; + + usleep_range(2000, 5000); + return 0; +} + +static int icn8505_upload_fw(struct icn8505_data *icn8505) +{ + struct device *dev = &icn8505->client->dev; + const struct firmware *fw; + int i, ret; + + /* + * Always load the firmware, even if we don't need it at boot, we + * we may need it at resume. Having loaded it once will make the + * firmware class code cache it at suspend/resume. + */ + ret = request_firmware(&fw, icn8505->firmware_name, dev); + if (ret) { + dev_err(dev, "Firmware request error %d\n", ret); + return ret; + } + + /* Check if the controller is not already up and running */ + if (icn8505_read_reg_silent(icn8505, 0x000a) == 0x85) + goto success; + + for (i = 1; i <= MAX_FW_UPLOAD_TRIES; i++) { + ret = icn8505_try_fw_upload(icn8505, fw, i); + if (ret >= 0) + goto success; + + usleep_range(2000, 5000); + } + + dev_err(dev, "Error uploading fw: %d\n", ret); + +success: + release_firmware(fw); + return ret; +} + +static inline bool icn8505_touch_active(u8 event) +{ + return (event == ICN8505_EVENT_UPDATE1) || + (event == ICN8505_EVENT_UPDATE2); +} + +static irqreturn_t icn8505_irq(int irq, void *dev_id) +{ + struct icn8505_data *icn8505 = dev_id; + struct device *dev = &icn8505->client->dev; + struct icn8505_touch_data touch_data; + int i, ret; + + ret = icn8505_read_data(icn8505, ICN8505_REG_TOUCHDATA, + &touch_data, sizeof(touch_data)); + if (ret < 0) { + dev_err(dev, "Error reading touch data: %d\n", ret); + return IRQ_HANDLED; + } + + if (touch_data.touch_count > ICN8505_MAX_TOUCHES) { + dev_warn(dev, "Too much touches %d > %d\n", + touch_data.touch_count, ICN8505_MAX_TOUCHES); + touch_data.touch_count = ICN8505_MAX_TOUCHES; + } + + for (i = 0; i < touch_data.touch_count; i++) { + struct icn8505_touch *touch = &touch_data.touches[i]; + bool act = icn8505_touch_active(touch->event); + + input_mt_slot(icn8505->input, touch->slot); + input_mt_report_slot_state(icn8505->input, MT_TOOL_FINGER, act); + if (!act) + continue; + + touchscreen_report_pos(icn8505->input, &icn8505->prop, + get_unaligned_le16(touch->x), + get_unaligned_le16(touch->y), + true); + } + + input_mt_sync_frame(icn8505->input); + input_report_key(icn8505->input, KEY_LEFTMETA, + touch_data.softbutton == 1); + input_sync(icn8505->input); + + return IRQ_HANDLED; +} + +static int icn8505_probe_acpi(struct icn8505_data *icn8505, struct device *dev) +{ + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + const char *subsys = "unknown"; + struct acpi_device *adev; + union acpi_object *obj; + acpi_status status; + + adev = ACPI_COMPANION(dev); + if (!adev) + return -ENODEV; + + status = acpi_evaluate_object(adev->handle, "_SUB", NULL, &buffer); + if (ACPI_SUCCESS(status)) { + obj = buffer.pointer; + if (obj->type == ACPI_TYPE_STRING) + subsys = obj->string.pointer; + else + dev_warn(dev, "Warning ACPI _SUB did not return a string\n"); + } else { + dev_warn(dev, "Warning ACPI _SUB failed: %#x\n", status); + buffer.pointer = NULL; + } + + snprintf(icn8505->firmware_name, sizeof(icn8505->firmware_name), + "chipone/icn8505-%s.fw", subsys); + + kfree(buffer.pointer); + return 0; +} + +static int icn8505_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct icn8505_data *icn8505; + struct input_dev *input; + __le16 resolution[2]; + int ret; + + if (!client->irq) { + dev_err(dev, "Error no irq specified\n"); + return -EINVAL; + } + + icn8505 = devm_kzalloc(dev, sizeof(*icn8505), GFP_KERNEL); + if (!icn8505) + return -ENOMEM; + + input = devm_input_allocate_device(dev); + if (!input) + return -ENOMEM; + + input->name = client->name; + input->id.bustype = BUS_I2C; + input->dev.parent = dev; + + input_set_capability(input, EV_ABS, ABS_MT_POSITION_X); + input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y); + input_set_capability(input, EV_KEY, KEY_LEFTMETA); + + icn8505->client = client; + icn8505->input = input; + input_set_drvdata(input, icn8505); + + ret = icn8505_probe_acpi(icn8505, dev); + if (ret) + return ret; + + ret = icn8505_upload_fw(icn8505); + if (ret < 0) + return ret; + + ret = icn8505_read_data(icn8505, ICN8505_REG_CONFIGDATA, + resolution, sizeof(resolution)); + if (ret < 0) { + dev_err(dev, "Error reading resolution: %d\n", ret); + return ret; + } + + input_set_abs_params(input, ABS_MT_POSITION_X, 0, + le16_to_cpu(resolution[0]) - 1, 0, 0); + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, + le16_to_cpu(resolution[1]) - 1, 0, 0); + + touchscreen_parse_properties(input, true, &icn8505->prop); + if (!input_abs_get_max(input, ABS_MT_POSITION_X) || + !input_abs_get_max(input, ABS_MT_POSITION_Y)) { + dev_err(dev, "Error touchscreen-size-x and/or -y missing\n"); + return -EINVAL; + } + + ret = input_mt_init_slots(input, ICN8505_MAX_TOUCHES, + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); + if (ret) + return ret; + + ret = devm_request_threaded_irq(dev, client->irq, NULL, icn8505_irq, + IRQF_ONESHOT, client->name, icn8505); + if (ret) { + dev_err(dev, "Error requesting irq: %d\n", ret); + return ret; + } + + ret = input_register_device(input); + if (ret) + return ret; + + i2c_set_clientdata(client, icn8505); + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int icn8505_suspend(struct device *dev) +{ + struct icn8505_data *icn8505 = i2c_get_clientdata(to_i2c_client(dev)); + + disable_irq(icn8505->client->irq); + icn8505_write_reg(icn8505, ICN8505_REG_POWER, ICN8505_POWER_HIBERNATE); + return 0; +} + +static int icn8505_resume(struct device *dev) +{ + struct icn8505_data *icn8505 = i2c_get_clientdata(to_i2c_client(dev)); + int ret; + + ret = icn8505_upload_fw(icn8505); + if (ret < 0) + return ret; + + enable_irq(icn8505->client->irq); + return 0; +} +#endif + +static SIMPLE_DEV_PM_OPS(icn8505_pm_ops, icn8505_suspend, icn8505_resume); + +static const struct acpi_device_id icn8505_acpi_match[] = { + { "CHPN0001" }, + { } +}; +MODULE_DEVICE_TABLE(acpi, icn8505_acpi_match); + +static struct i2c_driver icn8505_driver = { + .driver = { + .name = "chipone_icn8505", + .pm = &icn8505_pm_ops, + .acpi_match_table = icn8505_acpi_match, + }, + .probe_new = icn8505_probe, +}; + +module_i2c_driver(icn8505_driver); + +MODULE_DESCRIPTION("ChipOne icn8505 I2C Touchscreen Driver"); +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); +MODULE_LICENSE("GPL");
The ChipOne icn8505 is an i2c capacitive touchscreen controller typically used in cheap x86 tablets, this commit adds a driver for it. Note the icn8505 is somewhat similar to the icn8318 and I started with modifying that driver to support both, but in the end the differences were too large and I decided to write a new driver instead. Cc: Antonio Davide Trogu <trogu.davide@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- MAINTAINERS | 6 + drivers/input/touchscreen/Kconfig | 11 + drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/chipone_icn8505.c | 512 ++++++++++++++++++++ 4 files changed, 530 insertions(+) create mode 100644 drivers/input/touchscreen/chipone_icn8505.c