* Re: [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver
2020-09-27 12:35 ` [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
@ 2020-09-28 9:48 ` Amit Pundir
2020-09-28 21:51 ` Dmitry Torokhov
2020-09-28 22:16 ` Dmitry Torokhov
2020-10-06 18:34 ` Dan Carpenter
2 siblings, 1 reply; 9+ messages in thread
From: Amit Pundir @ 2020-09-28 9:48 UTC (permalink / raw)
To: kholk11
Cc: Dmitry Torokhov, Rob Herring, rydberg, priv.luk, linux-input,
lkml, marijns95, Konrad Dybcio, martin.botka1, phone-devel
On Sun, 27 Sep 2020 at 18:06, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> This is a driver for the Novatek in-cell touch controller and
> supports various chips from the NT36xxx family, currently
> including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
>
> Functionality like wake gestures and firmware flashing is not
> included: I am not aware of any of these DrIC+Touch combo
> chips not including a non-volatile memory and it should be
> highly unlikely to find one, since the touch firmware is
> embedded into the DriverIC one, which is obviously necessary
> to drive the display unit.
>
> However, the necessary address for the firmware update
> procedure was included into the address table in this driver
> so, in the event that someone finds the need to implement it
> for a reason or another, it will be pretty straightforward to.
>
> This driver is lightly based on the downstream implementation [1].
> [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
>
This is so cool!
Xiaomi PocoF1 phone uses same family touchscreen IC and I could
test this series on that phone as well.
However I do have to make a minor change to get it working. The
downstream driver and even the github code you linked above use
GPIO mapped IRQ, and I had to switch to that to get touchscreen
working with your driver
https://github.com/pundiramit/linux/commit/0a73eb656c1e80787dc195641ce7b0076fddb38e.
I wonder if I'm missing any devicetree property other than interrupts?
Regards,
Amit Pundir
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/nt36xxx.c | 742 ++++++++++++++++++++++++++++
> drivers/input/touchscreen/nt36xxx.h | 122 +++++
> 4 files changed, 877 insertions(+)
> create mode 100644 drivers/input/touchscreen/nt36xxx.c
> create mode 100644 drivers/input/touchscreen/nt36xxx.h
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 35c867b2d9a7..6d118b967021 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -605,6 +605,18 @@ config TOUCHSCREEN_MTOUCH
> To compile this driver as a module, choose M here: the
> module will be called mtouch.
>
> +config TOUCHSCREEN_NT36XXX
> + tristate "Novatek NT36XXX In-Cell I2C touchscreen controller"
> + depends on I2C
> + help
> + Say Y here if you have a Novatek NT36xxx series In-Cell
> + touchscreen connected to your system over I2C.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called nt36xxx.
> +
> config TOUCHSCREEN_IMX6UL_TSC
> tristate "Freescale i.MX6UL touchscreen controller"
> depends on (OF && GPIOLIB) || COMPILE_TEST
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 30d1e1b42492..424a555e03d5 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_TOUCHSCREEN_MIGOR) += migor_ts.o
> obj-$(CONFIG_TOUCHSCREEN_MMS114) += mms114.o
> obj-$(CONFIG_TOUCHSCREEN_MTOUCH) += mtouch.o
> obj-$(CONFIG_TOUCHSCREEN_MK712) += mk712.o
> +obj-$(CONFIG_TOUCHSCREEN_NT36XXX) += nt36xxx.o
> obj-$(CONFIG_TOUCHSCREEN_HP600) += hp680_ts_input.o
> obj-$(CONFIG_TOUCHSCREEN_HP7XX) += jornada720_ts.o
> obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO) += ipaq-micro-ts.o
> diff --git a/drivers/input/touchscreen/nt36xxx.c b/drivers/input/touchscreen/nt36xxx.c
> new file mode 100644
> index 000000000000..57e189938d12
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.c
> @@ -0,0 +1,742 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/input/mt.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "nt36xxx.h"
> +
> +static const struct nt36xxx_mem_map nt36xxx_memory_maps[] = {
> + [NT36525_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36672A_IC] = { 0x21c00, 0x20000, 0x23000, 0x24000, 0x24002 },
> + [NT36676F_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36772_IC] = { 0x11e00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36870_IC] = { 0x25000, 0x20000, 0x23000, 0x24000, 0x24002 },
> +};
> +
> +static const struct nt36xxx_trim_table trim_id_table[] = {
> + {
> + .id = { 0x0A, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 1, 0, 0, 1, 1, 1 },
> + .mapid = NT36672A_IC,
> + },
> + {
> + .id = { 0x55, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0x55, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x25, 0x65, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x68, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x76, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36676F_IC,
> + },
> +};
> +
> +static int nt36xxx_read(struct i2c_client *client, uint16_t address,
> + u8 *buf, uint16_t len)
> +{
> + int ret, retry = NT36XXX_MAX_RETRIES;
> + struct i2c_msg msg[] = {
> + /* Write slave position to i2c devices */
> + {
> + .addr = address,
> + .len = 1,
> + .buf = &buf[0]
> + },
> + /* Read data from position */
> + {
> + .addr = address,
> + .flags = I2C_M_RD,
> + .len = len - 1,
> + .buf = &buf[1]
> + }
> + };
> +
> + do {
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (likely(ret == ARRAY_SIZE(msg)))
> + return 0;
> + } while (--retry);
> +
> + return ret < 0 ? ret : -EIO;
> +}
> +
> +static int nt36xxx_write(struct i2c_client *client, uint16_t address,
> + u8 *buf, uint16_t len)
> +{
> + int ret, retry = NT36XXX_MAX_RETRIES;
> + struct i2c_msg msg[] = {
> + {
> + .addr = address,
> + .flags = 0,
> + .len = len,
> + .buf = buf,
> + },
> + };
> +
> + do {
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (likely(ret == ARRAY_SIZE(msg)))
> + return 0;
> +
> + usleep_range(100, 200);
> + } while (--retry);
> +
> + return ret < 0 ? ret : -EIO;
> +}
> +
> +static int nt36xxx_set_page(struct nt36xxx_i2c *ts, uint32_t pageaddr)
> +{
> + u8 buf[3];
> +
> + buf[0] = NT36XXX_CMD_SET_PAGE;
> + buf[1] = (pageaddr >> 16) & 0xFF;
> + buf[2] = (pageaddr >> 8) & 0xFF;
> +
> + return nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR, buf, 3);
> +}
> +
> +static int nt36xxx_sw_reset_idle(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { 0x00, NT36XXX_CMD_SW_RESET };
> + int ret;
> +
> + ret = nt36xxx_write(ts->client, ts->client->addr,
> + buf, ARRAY_SIZE(buf));
> +
> + usleep_range(15000, 16000);
> + return ret;
> +}
> +
> +static int nt36xxx_bootloader_reset(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { 0x00, NT36XXX_CMD_BOOTLOADER_RESET };
> + int ret;
> +
> + ret = nt36xxx_write(ts->client, ts->client->addr,
> + buf, ARRAY_SIZE(buf));
> +
> + msleep(35);
> + return ret;
> +}
> +
> +static int nt36xxx_check_reset_state(struct nt36xxx_i2c *ts,
> + enum nt36xxx_fw_state fw_state)
> +{
> + u8 buf[6] = { NT36XXX_EVT_RESET_COMPLETE, 0x00, 0x00,
> + 0x00, 0x00, 0x00 };
> + int ret, retry = NT36XXX_MAX_FW_RST_RETRY;
> +
> + do {
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 6);
> + if (likely(ret != -EIO) &&
> + (buf[1] >= fw_state) &&
> + (buf[1] <= NT36XXX_STATE_MAX)) {
> + ret = 0;
> + break;
> + }
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + if (!retry) {
> + dev_err(&ts->client->dev, "Firmware reset failed.\n");
> + ret = -EBUSY;
> + }
> +
> + return ret;
> +}
> +
> +static int nt36xxx_read_pid(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { NT36XXX_EVT_PROJECTID, 0x00, 0x00 };
> + int ret = 0;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + /* Read project id */
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR,
> + buf, ARRAY_SIZE(buf));
> + if (unlikely(ret < 0))
> + return ret;
> +
> + ts->fw_info.nvt_pid = (buf[2] << 8) + buf[1];
> +
> + return 0;
> +}
> +
> +static int __nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + u8 buf[17] = { 0 };
> + int ret = 0;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + /* Read fw info */
> + buf[0] = NT36XXX_EVT_FWINFO;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 17);
> + fwi->fw_ver = buf[1];
> + fwi->x_num = buf[3];
> + fwi->y_num = buf[4];
> + fwi->abs_x_max = (uint16_t)((buf[5] << 8) | buf[6]);
> + fwi->abs_y_max = (uint16_t)((buf[7] << 8) | buf[8]);
> + fwi->max_buttons = buf[11];
> +
> + /* Check fw info integrity and clear x_num, y_num if broken */
> + if ((buf[1] + buf[2]) != 0xFF) {
> + dev_dbg(&ts->client->dev,
> + "FW info is broken! fw_ver=0x%02X, ~fw_ver=0x%02X\n",
> + buf[1], buf[2]);
> + fwi->fw_ver = 0;
> + fwi->x_num = 18;
> + fwi->y_num = 32;
> + fwi->abs_x_max = TOUCH_DEFAULT_MAX_WIDTH;
> + fwi->abs_y_max = TOUCH_DEFAULT_MAX_HEIGHT;
> + fwi->max_buttons = 0;
> + return -EINVAL;
> + }
> +
> + /* Get Novatek ProjectID */
> + return nt36xxx_read_pid(ts);
> +}
> +
> +static int nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + int i, ret = 0;
> +
> + for (i = 0; i < NT36XXX_MAX_RETRIES; i++) {
> + ret = __nt36xxx_get_fw_info(ts);
> + if (ret == 0)
> + break;
> + }
> +
> + dev_dbg(&ts->client->dev,
> + "Firmware Info: ver=0x%x res=%ux%u max=%ux%u buttons=%u",
> + fwi->fw_ver, fwi->x_num, fwi->y_num,
> + fwi->abs_x_max, fwi->abs_y_max, fwi->max_buttons);
> +
> + return ret;
> +}
> +
> +static void nt36xxx_i2c_work_func(struct work_struct *work)
> +{
> + struct nt36xxx_i2c *ts = container_of(work, struct nt36xxx_i2c, ts_work);
> + struct nt36xxx_abs_object *obj = &ts->abs_obj;
> + struct input_dev *input = ts->input;
> + u8 input_id = 0;
> + u8 press_id[TOUCH_MAX_FINGER_NUM] = { 0 };
> + u8 point[POINT_DATA_LEN + 1] = { 0 };
> + unsigned int ppos = 0;
> + int i, ret, finger_cnt = 0;
> +
> + mutex_lock(&ts->lock);
> +
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, point,
> + POINT_DATA_LEN + 1);
> + if (unlikely(ret < 0)) {
> + dev_dbg(&ts->client->dev,
> + "Cannot read touch point data: %d\n", ret);
> + goto xfer_error;
> + }
> +
> + for (i = 0; i < ts->max_fingers; i++) {
> + ppos = 1 + 6 * i;
> + input_id = point[ppos + 0] >> 3;
> + if ((input_id == 0) || (input_id > ts->max_fingers))
> + continue;
> +
> + if (((point[ppos] & 0x07) == 0x01) ||
> + ((point[ppos] & 0x07) == 0x02)) {
> + obj->x = (point[ppos + 1] << 4) +
> + (point[ppos + 3] >> 4);
> + obj->y = (point[ppos + 2] << 4) +
> + (point[ppos + 3] & 0xf);
> + if ((obj->x > ts->fw_info.abs_x_max) ||
> + (obj->y > ts->fw_info.abs_y_max))
> + continue;
> +
> + obj->tm = point[ppos + 4];
> + if (obj->tm == 0)
> + obj->tm = 1;
> +
> + obj->z = point[ppos + 5];
> + if (i < 2) {
> + obj->z += point[i + 63] << 8;
> + if (obj->z > TOUCH_MAX_PRESSURE)
> + obj->z = TOUCH_MAX_PRESSURE;
> + }
> +
> + if (obj->z == 0)
> + obj->z = 1;
> +
> + press_id[input_id - 1] = 1;
> + input_mt_slot(input, input_id - 1);
> + input_mt_report_slot_state(input,
> + MT_TOOL_FINGER, true);
> +
> + input_report_abs(input, ABS_MT_POSITION_X, obj->x);
> + input_report_abs(input, ABS_MT_POSITION_Y, obj->y);
> + input_report_abs(input, ABS_MT_TOUCH_MAJOR, obj->tm);
> + input_report_abs(input, ABS_MT_PRESSURE, obj->z);
> +
> + finger_cnt++;
> + }
> + }
> +
> + for (i = 0; i < ts->max_fingers; i++) {
> + if (press_id[i] != 1) {
> + input_mt_slot(input, i);
> + input_report_abs(input, ABS_MT_TOUCH_MAJOR, 0);
> + input_report_abs(input, ABS_MT_PRESSURE, 0);
> + input_mt_report_slot_state(input,
> + MT_TOOL_FINGER, false);
> + }
> + }
> +
> + input_report_key(input, BTN_TOUCH, (finger_cnt > 0));
> +
> + input_sync(input);
> +
> +xfer_error:
> + enable_irq(ts->client->irq);
> +
> + mutex_unlock(&ts->lock);
> +}
> +
> +static irqreturn_t nt36xxx_i2c_irq_handler(int irq, void *dev_id)
> +{
> + struct nt36xxx_i2c *ts = dev_id;
> +
> + disable_irq_nosync(ts->client->irq);
> + queue_work(ts->ts_workq, &ts->ts_work);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int nvt_stop_crc_reboot(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[4] = { 0 };
> + int ret, retry = NT36XXX_MAX_RETRIES;
> +
> + /* Read dummy buffer to check CRC fail reboot is happening or not */
> +
> + /* Change I2C index to prevent geting 0xFF, but not 0xFC */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret < 0) {
> + dev_dbg(&ts->client->dev,
> + "CRC reset failed: Cannot select page.\n");
> + return ret;
> + }
> +
> + /* Read to check if buf is 0xFC which means IC is in CRC reboot */
> + buf[0] = 0x4E;
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 3);
> + if (ret < 0)
> + return ret;
> +
> + if (((buf[1] == 0xFC) && (buf[2] == 0xFC) && (buf[3] == 0xFC)) ||
> + ((buf[1] == 0xFF) && (buf[2] == 0xFF) && (buf[3] == 0xFF))) {
> + /* IC is in CRC fail reboot loop, needs to be stopped! */
> + do {
> + /* Write i2c cmds to reset idle - part #1 */
> + buf[0] = 0x00;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, ts->client->addr,
> + buf, 2);
> +
> + /* Write i2c cmds to reset idle - part #2 */
> + buf[0] = 0x00;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, ts->client->addr,
> + buf, 2);
> + usleep_range(1000, 1100);
> +
> + /* Clear CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x35;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR, buf, 2);
> +
> + /* Check CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x35;
> + buf[1] = 0x00;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 2);
> +
> + if (buf[1] == 0xA5)
> + break;
> + } while (--retry);
> +
> + if (retry == 0) {
> + dev_err(&ts->client->dev,
> + "CRC reset failed: buf[1]=0x%02X\n", buf[1]);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int nt36xxx_i2c_chip_version_init(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[7] = { 0 };
> + int retry = NT36XXX_MAX_RETRIES;
> + int sz = sizeof(trim_id_table) / sizeof(struct nt36xxx_trim_table);
> + int i, list, mapid, ret;
> +
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0) {
> + dev_err(&ts->client->dev, "Can't reset the nvt IC\n");
> + return ret;
> + }
> +
> + do {
> + ret = nt36xxx_sw_reset_idle(ts);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x00;
> + buf[1] = NT36XXX_CMD_UNLOCK;
> + nt36xxx_write(ts->client, ts->client->addr, buf, 2);
> + usleep_range(10000, 11000);
> +
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret < 0)
> + continue;
> +
> + memset(buf, 0, ARRAY_SIZE(buf));
> + buf[0] = NT36XXX_EVT_CHIPID;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 7);
> +
> + /* Compare read chip id with trim list */
> + for (list = 0; list < sz; list++) {
> + /* Compare each not masked byte */
> + for (i = 0; i < NT36XXX_ID_LEN_MAX; i++) {
> + if (trim_id_table[list].mask[i] &&
> + buf[i + 1] != trim_id_table[list].id[i])
> + break;
> + }
> +
> + if (i == NT36XXX_ID_LEN_MAX) {
> + mapid = trim_id_table[list].mapid;
> + ts->mmap = &nt36xxx_memory_maps[mapid];
> + return 0;
> + }
> +
> + ts->mmap = NULL;
> + ret = -ENOENT;
> + }
> +
> + /* Stop CRC check to prevent IC auto reboot */
> + if (((buf[1] == 0xFC) && (buf[2] == 0xFC) &&
> + (buf[3] == 0xFC)) ||
> + ((buf[1] == 0xFF) && (buf[2] == 0xFF) &&
> + (buf[3] == 0xFF))) {
> + ret = nvt_stop_crc_reboot(ts);
> + if (ret < 0)
> + continue;
> + }
> +
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + return ret;
> +}
> +
> +static int nt36xxx_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct nt36xxx_i2c *ts;
> + struct input_dev *input;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + dev_err(&client->dev, "i2c_check_functionality error\n");
> + return -EIO;
> + }
> +
> + if (!client->irq) {
> + dev_err(&client->dev, "No irq specified\n");
> + return -EINVAL;
> + }
> +
> + ts = devm_kzalloc(&client->dev, sizeof(struct nt36xxx_i2c), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->supplies = devm_kcalloc(&client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + sizeof(struct regulator_bulk_data),
> + GFP_KERNEL);
> + if (!ts->supplies)
> + return -ENOMEM;
> +
> + input = devm_input_allocate_device(&client->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + ts->client = client;
> + ts->input = input;
> + i2c_set_clientdata(client, ts);
> +
> + ts->ts_workq = alloc_ordered_workqueue("nt36xxx", 0);
> + if (!ts->ts_workq)
> + return -EINVAL;
> +
> + ts->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio))
> + return PTR_ERR(ts->reset_gpio);
> + if (ts->reset_gpio)
> + gpiod_set_consumer_name(ts->reset_gpio, "nt36xxx reset");
> +
> + /* These supplies are optional */
> + ts->supplies[0].supply = "vdd";
> + ts->supplies[1].supply = "vio";
> + ret = devm_regulator_bulk_get(&client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + ts->supplies);
> + if (ret != 0) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(&client->dev, "Cannot get supplies: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_bulk_enable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + if (ret)
> + return ret;
> +
> + usleep_range(10000, 11000);
> +
> + mutex_init(&ts->lock);
> +
> + ret = nt36xxx_i2c_chip_version_init(ts);
> + if (ret) {
> + dev_err(&client->dev, "Failed to check chip version\n");
> + goto error;
> + }
> +
> + mutex_lock(&ts->lock);
> + ret = nt36xxx_bootloader_reset(ts);
> + ret += nt36xxx_check_reset_state(ts, NT36XXX_STATE_INIT);
> + ret += nt36xxx_get_fw_info(ts);
> + mutex_unlock(&ts->lock);
> + if (unlikely(ret < 0))
> + goto error;
> +
> + ts->max_fingers = TOUCH_MAX_FINGER_NUM;
> + INIT_WORK(&ts->ts_work, nt36xxx_i2c_work_func);
> +
> + input->phys = devm_kasprintf(&client->dev, GFP_KERNEL,
> + "%s/input0", dev_name(&client->dev));
> +
> + input->name = NT36XXX_INPUT_DEVICE_NAME;
> + input->id.bustype = BUS_I2C;
> + input->dev.parent = &client->dev;
> +
> + input->evbit[0] =
> + BIT_MASK(EV_SYN) | BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> + input->propbit[0] = BIT(INPUT_PROP_DIRECT);
> +
> + input_mt_init_slots(input, ts->max_fingers, 0);
> + input_set_abs_params(input, ABS_MT_PRESSURE, 0,
> + TOUCH_MAX_PRESSURE, 0, 0);
> +
> + if (ts->max_fingers > 1) {
> + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> + ts->fw_info.abs_x_max - 1, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> + ts->fw_info.abs_y_max - 1, 0, 0);
> + }
> +
> + input_set_drvdata(input, ts);
> +
> + ret = input_register_device(ts->input);
> + if (ret) {
> + dev_err(&client->dev, "Failed to register input device: %d\n",
> + ret);
> + goto error;
> + }
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + nt36xxx_i2c_irq_handler, IRQF_ONESHOT,
> + client->name, ts);
> + if (ret != 0) {
> + dev_err(&client->dev, "request irq failed: %d\n", ret);
> + goto intr_error;
> + }
> +
> + disable_irq(client->irq);
> + device_init_wakeup(&client->dev, 1);
> + enable_irq(client->irq);
> +
> + return 0;
> +
> +intr_error:
> + input_unregister_device(ts->input);
> +error:
> + regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + return ret;
> +}
> +
> +static int nt36xxx_i2c_remove(struct i2c_client *client)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(client);
> +
> + free_irq(client->irq, ts);
> + regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + mutex_destroy(&ts->lock);
> + input_unregister_device(ts->input);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_suspend(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + u8 buf[] = { NT36XXX_EVT_HOST_CMD, NT36XXX_CMD_ENTER_SLEEP };
> + int i, ret;
> +
> + disable_irq(ts->client->irq);
> +
> + ret = nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR,
> + buf, ARRAY_SIZE(buf));
> + if (unlikely(ret < 0)) {
> + dev_err(&ts->client->dev, "Cannot enter suspend!!\n");
> + return ret;
> + }
> +
> + /* Release all touches */
> + for (i = 0; i < ts->max_fingers; i++) {
> + input_mt_slot(ts->input, i);
> + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR, 0);
> + input_report_abs(ts->input, ABS_MT_PRESSURE, 0);
> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, 0);
> + }
> + input_sync(ts->input);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_resume(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + mutex_lock(&ts->lock);
> +
> + if (ts->reset_gpio)
> + gpiod_set_value(ts->reset_gpio, 1);
> +
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0)
> + goto end;
> +
> + ret = nt36xxx_check_reset_state(ts, NT36XXX_STATE_REK);
> + if (ret < 0)
> + goto end;
> +
> + enable_irq(ts->client->irq);
> +end:
> + mutex_unlock(&ts->lock);
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(nt36xxx_i2c_pm,
> + nt36xxx_i2c_suspend, nt36xxx_i2c_resume);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id nt36xxx_of_match[] = {
> + { .compatible = "novatek,nt36xxx" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, nt36xxx_of_match);
> +#endif
> +
> +static const struct i2c_device_id nt36xxx_i2c_ts_id[] = {
> + { "NVT-ts", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, nt36xxx_i2c_ts_id);
> +
> +static struct i2c_driver nt36xxx_i2c_ts_driver = {
> + .driver = {
> + .name = "nt36xxx_ts",
> + .of_match_table = of_match_ptr(nt36xxx_of_match),
> + .pm = &nt36xxx_i2c_pm,
> + },
> + .probe = nt36xxx_i2c_probe,
> + .remove = nt36xxx_i2c_remove,
> + .id_table = nt36xxx_i2c_ts_id,
> +};
> +module_i2c_driver(nt36xxx_i2c_ts_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Novatek NT36XXX Touchscreen Driver");
> +
> diff --git a/drivers/input/touchscreen/nt36xxx.h b/drivers/input/touchscreen/nt36xxx.h
> new file mode 100644
> index 000000000000..6f03dfb45656
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.h
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
> +
> +#ifndef NT36XXX_H
> +#define NT36XXX_H
> +
> +#define NT36XXX_INPUT_DEVICE_NAME "Novatek NT36XXX Touch Sensor"
> +
> +/* These chips have this fixed address when in bootloader :( */
> +#define NT36XXX_BLDR_ADDR 0x01
> +
> +/* Input device info */
> +#define NVT_TS_NAME "NVTCapacitiveTouchScreen"
> +
> +/* Number of bytes for chip identification */
> +#define NT36XXX_ID_LEN_MAX 6
> +
> +/* Touch info */
> +#define TOUCH_DEFAULT_MAX_WIDTH 1080
> +#define TOUCH_DEFAULT_MAX_HEIGHT 2246
> +#define TOUCH_MAX_FINGER_NUM 10
> +#define TOUCH_MAX_PRESSURE 1000
> +
> +/* Point data length */
> +#define POINT_DATA_LEN 65
> +
> +/* Global pages */
> +#define NT36XXX_PAGE_CHIP_INFO 0x0001f64e
> +#define NT36XXX_PAGE_CRC 0x0003f135
> +
> +/* Misc */
> +#define NT36XXX_NUM_SUPPLIES 2
> +#define NT36XXX_MAX_RETRIES 5
> +#define NT36XXX_MAX_FW_RST_RETRY 50
> +
> +struct nt36xxx_abs_object {
> + u16 x;
> + u16 y;
> + u16 z;
> + u8 tm;
> +};
> +
> +struct nt36xxx_fw_info {
> + u8 fw_ver;
> + u8 x_num;
> + u8 y_num;
> + u8 max_buttons;
> + u16 abs_x_max;
> + u16 abs_y_max;
> + u16 nvt_pid;
> +};
> +
> +struct nt36xxx_mem_map {
> + u32 evtbuf_addr;
> + u32 pipe0_addr;
> + u32 pipe1_addr;
> + u32 flash_csum_addr;
> + u32 flash_data_addr;
> +};
> +
> +struct nt36xxx_i2c {
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct regulator_bulk_data *supplies;
> + struct gpio_desc *reset_gpio;
> +
> + struct work_struct ts_work;
> + struct workqueue_struct *ts_workq;
> + struct mutex lock;
> +
> + struct nt36xxx_fw_info fw_info;
> + struct nt36xxx_abs_object abs_obj;
> +
> + const struct nt36xxx_mem_map *mmap;
> + u8 max_fingers;
> +};
> +
> +enum nt36xxx_chips {
> + NT36525_IC = 0,
> + NT36672A_IC,
> + NT36676F_IC,
> + NT36772_IC,
> + NT36870_IC,
> + NTMAX_IC,
> +};
> +
> +struct nt36xxx_trim_table {
> + u8 id[NT36XXX_ID_LEN_MAX];
> + u8 mask[NT36XXX_ID_LEN_MAX];
> + enum nt36xxx_chips mapid;
> +};
> +
> +enum nt36xxx_cmds {
> + NT36XXX_CMD_ENTER_SLEEP = 0x11,
> + NT36XXX_CMD_ENTER_WKUP_GESTURE = 0x13,
> + NT36XXX_CMD_UNLOCK = 0x35,
> + NT36XXX_CMD_BOOTLOADER_RESET = 0x69,
> + NT36XXX_CMD_SW_RESET = 0xa5,
> + NT36XXX_CMD_SET_PAGE = 0xff,
> +};
> +
> +enum nt36xxx_fw_state {
> + NT36XXX_STATE_INIT = 0xa0, /* IC reset */
> + NT36XXX_STATE_REK, /* ReK baseline */
> + NT36XXX_STATE_REK_FINISH, /* Baseline is ready */
> + NT36XXX_STATE_NORMAL_RUN, /* Normal run */
> + NT36XXX_STATE_MAX = 0xaf
> +};
> +
> +enum nt36xxx_i2c_events {
> + NT36XXX_EVT_CHIPID = 0x4e,
> + NT36XXX_EVT_HOST_CMD = 0x50,
> + NT36XXX_EVT_HS_OR_SUBCMD = 0x51, /* Handshake or subcommand byte */
> + NT36XXX_EVT_RESET_COMPLETE = 0x60,
> + NT36XXX_EVT_FWINFO = 0x78,
> + NT36XXX_EVT_PROJECTID = 0x9a,
> +};
> +
> +#endif
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver
2020-09-28 9:48 ` Amit Pundir
@ 2020-09-28 21:51 ` Dmitry Torokhov
2020-09-29 7:58 ` Amit Pundir
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2020-09-28 21:51 UTC (permalink / raw)
To: Amit Pundir
Cc: kholk11, Rob Herring, rydberg, priv.luk, linux-input, lkml,
marijns95, Konrad Dybcio, martin.botka1, phone-devel
On Mon, Sep 28, 2020 at 03:18:18PM +0530, Amit Pundir wrote:
> On Sun, 27 Sep 2020 at 18:06, <kholk11@gmail.com> wrote:
> >
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> >
> > This is a driver for the Novatek in-cell touch controller and
> > supports various chips from the NT36xxx family, currently
> > including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
> >
> > Functionality like wake gestures and firmware flashing is not
> > included: I am not aware of any of these DrIC+Touch combo
> > chips not including a non-volatile memory and it should be
> > highly unlikely to find one, since the touch firmware is
> > embedded into the DriverIC one, which is obviously necessary
> > to drive the display unit.
> >
> > However, the necessary address for the firmware update
> > procedure was included into the address table in this driver
> > so, in the event that someone finds the need to implement it
> > for a reason or another, it will be pretty straightforward to.
> >
> > This driver is lightly based on the downstream implementation [1].
> > [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
> >
>
> This is so cool!
>
> Xiaomi PocoF1 phone uses same family touchscreen IC and I could
> test this series on that phone as well.
>
> However I do have to make a minor change to get it working. The
> downstream driver and even the github code you linked above use
> GPIO mapped IRQ, and I had to switch to that to get touchscreen
> working with your driver
> https://github.com/pundiramit/linux/commit/0a73eb656c1e80787dc195641ce7b0076fddb38e.
> I wonder if I'm missing any devicetree property other than interrupts?
Simply specifying GPIO controller/pin as interrupt property should work,
there should be no need to parse custom GPIO property and convert it to
IRQ in the driver.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver
2020-09-28 21:51 ` Dmitry Torokhov
@ 2020-09-29 7:58 ` Amit Pundir
0 siblings, 0 replies; 9+ messages in thread
From: Amit Pundir @ 2020-09-29 7:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: kholk11, Rob Herring, rydberg, priv.luk, linux-input, lkml,
marijns95, Konrad Dybcio, martin.botka1, phone-devel
On Tue, 29 Sep 2020 at 03:21, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Mon, Sep 28, 2020 at 03:18:18PM +0530, Amit Pundir wrote:
> > On Sun, 27 Sep 2020 at 18:06, <kholk11@gmail.com> wrote:
> > >
> > > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > >
> > > This is a driver for the Novatek in-cell touch controller and
> > > supports various chips from the NT36xxx family, currently
> > > including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
> > >
> > > Functionality like wake gestures and firmware flashing is not
> > > included: I am not aware of any of these DrIC+Touch combo
> > > chips not including a non-volatile memory and it should be
> > > highly unlikely to find one, since the touch firmware is
> > > embedded into the DriverIC one, which is obviously necessary
> > > to drive the display unit.
> > >
> > > However, the necessary address for the firmware update
> > > procedure was included into the address table in this driver
> > > so, in the event that someone finds the need to implement it
> > > for a reason or another, it will be pretty straightforward to.
> > >
> > > This driver is lightly based on the downstream implementation [1].
> > > [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
> > >
> >
> > This is so cool!
> >
> > Xiaomi PocoF1 phone uses same family touchscreen IC and I could
> > test this series on that phone as well.
> >
> > However I do have to make a minor change to get it working. The
> > downstream driver and even the github code you linked above use
> > GPIO mapped IRQ, and I had to switch to that to get touchscreen
> > working with your driver
> > https://github.com/pundiramit/linux/commit/0a73eb656c1e80787dc195641ce7b0076fddb38e.
> > I wonder if I'm missing any devicetree property other than interrupts?
>
> Simply specifying GPIO controller/pin as interrupt property should work,
> there should be no need to parse custom GPIO property and convert it to
> IRQ in the driver.
>
Indeed. I don't know what was tripping me off yesterday, but I got it
working today with interrupt properties. Thank you.
Regards,
Amit Pundir
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver
2020-09-27 12:35 ` [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
2020-09-28 9:48 ` Amit Pundir
@ 2020-09-28 22:16 ` Dmitry Torokhov
2020-10-06 18:34 ` Dan Carpenter
2 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2020-09-28 22:16 UTC (permalink / raw)
To: kholk11
Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, marijns95,
konradybcio, martin.botka1, phone-devel
Hi AngeloGioacchino,
On Sun, Sep 27, 2020 at 02:35:41PM +0200, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> This is a driver for the Novatek in-cell touch controller and
> supports various chips from the NT36xxx family, currently
> including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
>
> Functionality like wake gestures and firmware flashing is not
> included: I am not aware of any of these DrIC+Touch combo
> chips not including a non-volatile memory and it should be
> highly unlikely to find one, since the touch firmware is
> embedded into the DriverIC one, which is obviously necessary
> to drive the display unit.
>
> However, the necessary address for the firmware update
> procedure was included into the address table in this driver
> so, in the event that someone finds the need to implement it
> for a reason or another, it will be pretty straightforward to.
>
> This driver is lightly based on the downstream implementation [1].
> [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/nt36xxx.c | 742 ++++++++++++++++++++++++++++
> drivers/input/touchscreen/nt36xxx.h | 122 +++++
> 4 files changed, 877 insertions(+)
> create mode 100644 drivers/input/touchscreen/nt36xxx.c
> create mode 100644 drivers/input/touchscreen/nt36xxx.h
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 35c867b2d9a7..6d118b967021 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -605,6 +605,18 @@ config TOUCHSCREEN_MTOUCH
> To compile this driver as a module, choose M here: the
> module will be called mtouch.
>
> +config TOUCHSCREEN_NT36XXX
> + tristate "Novatek NT36XXX In-Cell I2C touchscreen controller"
> + depends on I2C
> + help
> + Say Y here if you have a Novatek NT36xxx series In-Cell
> + touchscreen connected to your system over I2C.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called nt36xxx.
> +
> config TOUCHSCREEN_IMX6UL_TSC
> tristate "Freescale i.MX6UL touchscreen controller"
> depends on (OF && GPIOLIB) || COMPILE_TEST
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 30d1e1b42492..424a555e03d5 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_TOUCHSCREEN_MIGOR) += migor_ts.o
> obj-$(CONFIG_TOUCHSCREEN_MMS114) += mms114.o
> obj-$(CONFIG_TOUCHSCREEN_MTOUCH) += mtouch.o
> obj-$(CONFIG_TOUCHSCREEN_MK712) += mk712.o
> +obj-$(CONFIG_TOUCHSCREEN_NT36XXX) += nt36xxx.o
> obj-$(CONFIG_TOUCHSCREEN_HP600) += hp680_ts_input.o
> obj-$(CONFIG_TOUCHSCREEN_HP7XX) += jornada720_ts.o
> obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO) += ipaq-micro-ts.o
> diff --git a/drivers/input/touchscreen/nt36xxx.c b/drivers/input/touchscreen/nt36xxx.c
> new file mode 100644
> index 000000000000..57e189938d12
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.c
> @@ -0,0 +1,742 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
Is this needed?
> +#include <linux/input/mt.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "nt36xxx.h"
> +
> +static const struct nt36xxx_mem_map nt36xxx_memory_maps[] = {
> + [NT36525_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36672A_IC] = { 0x21c00, 0x20000, 0x23000, 0x24000, 0x24002 },
> + [NT36676F_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36772_IC] = { 0x11e00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36870_IC] = { 0x25000, 0x20000, 0x23000, 0x24000, 0x24002 },
> +};
> +
> +static const struct nt36xxx_trim_table trim_id_table[] = {
> + {
> + .id = { 0x0A, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 1, 0, 0, 1, 1, 1 },
> + .mapid = NT36672A_IC,
> + },
> + {
> + .id = { 0x55, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0x55, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x25, 0x65, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x68, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x76, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36676F_IC,
> + },
> +};
> +
> +static int nt36xxx_read(struct i2c_client *client, uint16_t address,
Please use shorthand - u16 - in kernel.
> + u8 *buf, uint16_t len)
> +{
> + int ret, retry = NT36XXX_MAX_RETRIES;
> + struct i2c_msg msg[] = {
> + /* Write slave position to i2c devices */
> + {
> + .addr = address,
> + .len = 1,
> + .buf = &buf[0]
> + },
> + /* Read data from position */
> + {
> + .addr = address,
> + .flags = I2C_M_RD,
> + .len = len - 1,
> + .buf = &buf[1]
> + }
> + };
> +
> + do {
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (likely(ret == ARRAY_SIZE(msg)))
> + return 0;
> + } while (--retry);
> +
> + return ret < 0 ? ret : -EIO;
> +}
> +
> +static int nt36xxx_write(struct i2c_client *client, uint16_t address,
> + u8 *buf, uint16_t len)
> +{
> + int ret, retry = NT36XXX_MAX_RETRIES;
> + struct i2c_msg msg[] = {
> + {
> + .addr = address,
> + .flags = 0,
> + .len = len,
> + .buf = buf,
> + },
> + };
> +
> + do {
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (likely(ret == ARRAY_SIZE(msg)))
> + return 0;
> +
> + usleep_range(100, 200);
> + } while (--retry);
> +
> + return ret < 0 ? ret : -EIO;
> +}
> +
> +static int nt36xxx_set_page(struct nt36xxx_i2c *ts, uint32_t pageaddr)
> +{
> + u8 buf[3];
> +
> + buf[0] = NT36XXX_CMD_SET_PAGE;
> + buf[1] = (pageaddr >> 16) & 0xFF;
> + buf[2] = (pageaddr >> 8) & 0xFF;
> +
> + return nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR, buf, 3);
Sounds like regmap is a good option to be used here.
> +}
> +
> +static int nt36xxx_sw_reset_idle(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { 0x00, NT36XXX_CMD_SW_RESET };
> + int ret;
> +
> + ret = nt36xxx_write(ts->client, ts->client->addr,
> + buf, ARRAY_SIZE(buf));
> +
> + usleep_range(15000, 16000);
> + return ret;
> +}
> +
> +static int nt36xxx_bootloader_reset(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { 0x00, NT36XXX_CMD_BOOTLOADER_RESET };
> + int ret;
> +
> + ret = nt36xxx_write(ts->client, ts->client->addr,
> + buf, ARRAY_SIZE(buf));
> +
> + msleep(35);
> + return ret;
> +}
> +
> +static int nt36xxx_check_reset_state(struct nt36xxx_i2c *ts,
> + enum nt36xxx_fw_state fw_state)
> +{
> + u8 buf[6] = { NT36XXX_EVT_RESET_COMPLETE, 0x00, 0x00,
> + 0x00, 0x00, 0x00 };
> + int ret, retry = NT36XXX_MAX_FW_RST_RETRY;
> +
> + do {
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 6);
> + if (likely(ret != -EIO) &&
> + (buf[1] >= fw_state) &&
> + (buf[1] <= NT36XXX_STATE_MAX)) {
> + ret = 0;
> + break;
> + }
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + if (!retry) {
> + dev_err(&ts->client->dev, "Firmware reset failed.\n");
> + ret = -EBUSY;
> + }
> +
> + return ret;
> +}
> +
> +static int nt36xxx_read_pid(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[] = { NT36XXX_EVT_PROJECTID, 0x00, 0x00 };
> + int ret = 0;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + /* Read project id */
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR,
> + buf, ARRAY_SIZE(buf));
> + if (unlikely(ret < 0))
> + return ret;
> +
> + ts->fw_info.nvt_pid = (buf[2] << 8) + buf[1];
> +
> + return 0;
> +}
> +
> +static int __nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + u8 buf[17] = { 0 };
> + int ret = 0;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + /* Read fw info */
> + buf[0] = NT36XXX_EVT_FWINFO;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 17);
> + fwi->fw_ver = buf[1];
> + fwi->x_num = buf[3];
> + fwi->y_num = buf[4];
> + fwi->abs_x_max = (uint16_t)((buf[5] << 8) | buf[6]);
> + fwi->abs_y_max = (uint16_t)((buf[7] << 8) | buf[8]);
> + fwi->max_buttons = buf[11];
> +
> + /* Check fw info integrity and clear x_num, y_num if broken */
> + if ((buf[1] + buf[2]) != 0xFF) {
> + dev_dbg(&ts->client->dev,
> + "FW info is broken! fw_ver=0x%02X, ~fw_ver=0x%02X\n",
> + buf[1], buf[2]);
> + fwi->fw_ver = 0;
> + fwi->x_num = 18;
> + fwi->y_num = 32;
> + fwi->abs_x_max = TOUCH_DEFAULT_MAX_WIDTH;
> + fwi->abs_y_max = TOUCH_DEFAULT_MAX_HEIGHT;
> + fwi->max_buttons = 0;
> + return -EINVAL;
> + }
> +
> + /* Get Novatek ProjectID */
> + return nt36xxx_read_pid(ts);
> +}
> +
> +static int nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + int i, ret = 0;
> +
> + for (i = 0; i < NT36XXX_MAX_RETRIES; i++) {
> + ret = __nt36xxx_get_fw_info(ts);
> + if (ret == 0)
> + break;
> + }
> +
> + dev_dbg(&ts->client->dev,
> + "Firmware Info: ver=0x%x res=%ux%u max=%ux%u buttons=%u",
> + fwi->fw_ver, fwi->x_num, fwi->y_num,
> + fwi->abs_x_max, fwi->abs_y_max, fwi->max_buttons);
> +
> + return ret;
> +}
> +
> +static void nt36xxx_i2c_work_func(struct work_struct *work)
> +{
> + struct nt36xxx_i2c *ts = container_of(work, struct nt36xxx_i2c, ts_work);
> + struct nt36xxx_abs_object *obj = &ts->abs_obj;
> + struct input_dev *input = ts->input;
> + u8 input_id = 0;
> + u8 press_id[TOUCH_MAX_FINGER_NUM] = { 0 };
> + u8 point[POINT_DATA_LEN + 1] = { 0 };
> + unsigned int ppos = 0;
> + int i, ret, finger_cnt = 0;
> +
> + mutex_lock(&ts->lock);
> +
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, point,
> + POINT_DATA_LEN + 1);
Is the chip even no in bootloader mode? I only see transfers to
NT36XXX_BLDR_ADDR...
> + if (unlikely(ret < 0)) {
> + dev_dbg(&ts->client->dev,
> + "Cannot read touch point data: %d\n", ret);
> + goto xfer_error;
> + }
> +
> + for (i = 0; i < ts->max_fingers; i++) {
> + ppos = 1 + 6 * i;
> + input_id = point[ppos + 0] >> 3;
> + if ((input_id == 0) || (input_id > ts->max_fingers))
> + continue;
> +
> + if (((point[ppos] & 0x07) == 0x01) ||
> + ((point[ppos] & 0x07) == 0x02)) {
> + obj->x = (point[ppos + 1] << 4) +
> + (point[ppos + 3] >> 4);
> + obj->y = (point[ppos + 2] << 4) +
> + (point[ppos + 3] & 0xf);
> + if ((obj->x > ts->fw_info.abs_x_max) ||
> + (obj->y > ts->fw_info.abs_y_max))
> + continue;
> +
> + obj->tm = point[ppos + 4];
> + if (obj->tm == 0)
> + obj->tm = 1;
> +
> + obj->z = point[ppos + 5];
> + if (i < 2) {
> + obj->z += point[i + 63] << 8;
> + if (obj->z > TOUCH_MAX_PRESSURE)
> + obj->z = TOUCH_MAX_PRESSURE;
> + }
> +
> + if (obj->z == 0)
> + obj->z = 1;
> +
> + press_id[input_id - 1] = 1;
> + input_mt_slot(input, input_id - 1);
> + input_mt_report_slot_state(input,
> + MT_TOOL_FINGER, true);
> +
> + input_report_abs(input, ABS_MT_POSITION_X, obj->x);
> + input_report_abs(input, ABS_MT_POSITION_Y, obj->y);
> + input_report_abs(input, ABS_MT_TOUCH_MAJOR, obj->tm);
> + input_report_abs(input, ABS_MT_PRESSURE, obj->z);
> +
> + finger_cnt++;
> + }
> + }
> +
> + for (i = 0; i < ts->max_fingers; i++) {
> + if (press_id[i] != 1) {
> + input_mt_slot(input, i);
> + input_report_abs(input, ABS_MT_TOUCH_MAJOR, 0);
> + input_report_abs(input, ABS_MT_PRESSURE, 0);
> + input_mt_report_slot_state(input,
> + MT_TOOL_FINGER, false);
I think INPUT_MT_TRACK_UNUSED will help here.
> + }
> + }
> +
> + input_report_key(input, BTN_TOUCH, (finger_cnt > 0));
> +
> + input_sync(input);
> +
> +xfer_error:
> + enable_irq(ts->client->irq);
> +
> + mutex_unlock(&ts->lock);
> +}
> +
> +static irqreturn_t nt36xxx_i2c_irq_handler(int irq, void *dev_id)
> +{
> + struct nt36xxx_i2c *ts = dev_id;
> +
> + disable_irq_nosync(ts->client->irq);
> + queue_work(ts->ts_workq, &ts->ts_work);
You are using threaded interrupt, there is no need to involve workqueue
here.
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int nvt_stop_crc_reboot(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[4] = { 0 };
> + int ret, retry = NT36XXX_MAX_RETRIES;
> +
> + /* Read dummy buffer to check CRC fail reboot is happening or not */
> +
> + /* Change I2C index to prevent geting 0xFF, but not 0xFC */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret < 0) {
> + dev_dbg(&ts->client->dev,
> + "CRC reset failed: Cannot select page.\n");
> + return ret;
> + }
> +
> + /* Read to check if buf is 0xFC which means IC is in CRC reboot */
> + buf[0] = 0x4E;
> + ret = nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 3);
> + if (ret < 0)
> + return ret;
> +
> + if (((buf[1] == 0xFC) && (buf[2] == 0xFC) && (buf[3] == 0xFC)) ||
> + ((buf[1] == 0xFF) && (buf[2] == 0xFF) && (buf[3] == 0xFF))) {
> + /* IC is in CRC fail reboot loop, needs to be stopped! */
> + do {
> + /* Write i2c cmds to reset idle - part #1 */
> + buf[0] = 0x00;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, ts->client->addr,
> + buf, 2);
> +
> + /* Write i2c cmds to reset idle - part #2 */
> + buf[0] = 0x00;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, ts->client->addr,
> + buf, 2);
> + usleep_range(1000, 1100);
> +
> + /* Clear CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x35;
> + buf[1] = 0xA5;
> + nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR, buf, 2);
> +
> + /* Check CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x35;
> + buf[1] = 0x00;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 2);
> +
> + if (buf[1] == 0xA5)
> + break;
> + } while (--retry);
> +
> + if (retry == 0) {
> + dev_err(&ts->client->dev,
> + "CRC reset failed: buf[1]=0x%02X\n", buf[1]);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int nt36xxx_i2c_chip_version_init(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[7] = { 0 };
> + int retry = NT36XXX_MAX_RETRIES;
> + int sz = sizeof(trim_id_table) / sizeof(struct nt36xxx_trim_table);
> + int i, list, mapid, ret;
> +
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0) {
> + dev_err(&ts->client->dev, "Can't reset the nvt IC\n");
> + return ret;
> + }
> +
> + do {
> + ret = nt36xxx_sw_reset_idle(ts);
> + if (ret < 0)
> + continue;
> +
> + buf[0] = 0x00;
> + buf[1] = NT36XXX_CMD_UNLOCK;
> + nt36xxx_write(ts->client, ts->client->addr, buf, 2);
> + usleep_range(10000, 11000);
> +
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret < 0)
> + continue;
> +
> + memset(buf, 0, ARRAY_SIZE(buf));
> + buf[0] = NT36XXX_EVT_CHIPID;
> + nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 7);
> +
> + /* Compare read chip id with trim list */
> + for (list = 0; list < sz; list++) {
> + /* Compare each not masked byte */
> + for (i = 0; i < NT36XXX_ID_LEN_MAX; i++) {
> + if (trim_id_table[list].mask[i] &&
> + buf[i + 1] != trim_id_table[list].id[i])
> + break;
> + }
> +
> + if (i == NT36XXX_ID_LEN_MAX) {
> + mapid = trim_id_table[list].mapid;
> + ts->mmap = &nt36xxx_memory_maps[mapid];
> + return 0;
> + }
> +
> + ts->mmap = NULL;
> + ret = -ENOENT;
> + }
> +
> + /* Stop CRC check to prevent IC auto reboot */
> + if (((buf[1] == 0xFC) && (buf[2] == 0xFC) &&
> + (buf[3] == 0xFC)) ||
> + ((buf[1] == 0xFF) && (buf[2] == 0xFF) &&
> + (buf[3] == 0xFF))) {
> + ret = nvt_stop_crc_reboot(ts);
> + if (ret < 0)
> + continue;
> + }
> +
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + return ret;
> +}
> +
> +static int nt36xxx_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct nt36xxx_i2c *ts;
> + struct input_dev *input;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + dev_err(&client->dev, "i2c_check_functionality error\n");
> + return -EIO;
> + }
> +
> + if (!client->irq) {
> + dev_err(&client->dev, "No irq specified\n");
> + return -EINVAL;
> + }
> +
> + ts = devm_kzalloc(&client->dev, sizeof(struct nt36xxx_i2c), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->supplies = devm_kcalloc(&client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + sizeof(struct regulator_bulk_data),
> + GFP_KERNEL);
> + if (!ts->supplies)
> + return -ENOMEM;
> +
> + input = devm_input_allocate_device(&client->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + ts->client = client;
> + ts->input = input;
> + i2c_set_clientdata(client, ts);
> +
> + ts->ts_workq = alloc_ordered_workqueue("nt36xxx", 0);
> + if (!ts->ts_workq)
> + return -EINVAL;
> +
> + ts->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio))
> + return PTR_ERR(ts->reset_gpio);
> + if (ts->reset_gpio)
> + gpiod_set_consumer_name(ts->reset_gpio, "nt36xxx reset");
> +
> + /* These supplies are optional */
> + ts->supplies[0].supply = "vdd";
> + ts->supplies[1].supply = "vio";
> + ret = devm_regulator_bulk_get(&client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + ts->supplies);
> + if (ret != 0) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(&client->dev, "Cannot get supplies: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_bulk_enable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + if (ret)
> + return ret;
> +
> + usleep_range(10000, 11000);
> +
> + mutex_init(&ts->lock);
> +
> + ret = nt36xxx_i2c_chip_version_init(ts);
> + if (ret) {
> + dev_err(&client->dev, "Failed to check chip version\n");
> + goto error;
> + }
> +
> + mutex_lock(&ts->lock);
> + ret = nt36xxx_bootloader_reset(ts);
> + ret += nt36xxx_check_reset_state(ts, NT36XXX_STATE_INIT);
> + ret += nt36xxx_get_fw_info(ts);
> + mutex_unlock(&ts->lock);
What are you protecting here with this mutex?
> + if (unlikely(ret < 0))
> + goto error;
> +
> + ts->max_fingers = TOUCH_MAX_FINGER_NUM;
> + INIT_WORK(&ts->ts_work, nt36xxx_i2c_work_func);
> +
> + input->phys = devm_kasprintf(&client->dev, GFP_KERNEL,
> + "%s/input0", dev_name(&client->dev));
> +
> + input->name = NT36XXX_INPUT_DEVICE_NAME;
> + input->id.bustype = BUS_I2C;
> + input->dev.parent = &client->dev;
> +
> + input->evbit[0] =
> + BIT_MASK(EV_SYN) | BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
No need to set EV_SYN.
> + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
input_set_capability(input, EV_KEY, BTN_TOUCH);
> + input->propbit[0] = BIT(INPUT_PROP_DIRECT);
Please use __set_bit().
> +
> + input_mt_init_slots(input, ts->max_fingers, 0);
You need to add error handling and some flags here.
> + input_set_abs_params(input, ABS_MT_PRESSURE, 0,
> + TOUCH_MAX_PRESSURE, 0, 0);
> +
> + if (ts->max_fingers > 1) {
> + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> + ts->fw_info.abs_x_max - 1, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> + ts->fw_info.abs_y_max - 1, 0, 0);
> + }
Please use API from include/linux/input/touchscreen.h to enable basic
transformations.
> +
> + input_set_drvdata(input, ts);
> +
> + ret = input_register_device(ts->input);
> + if (ret) {
> + dev_err(&client->dev, "Failed to register input device: %d\n",
> + ret);
> + goto error;
> + }
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + nt36xxx_i2c_irq_handler, IRQF_ONESHOT,
> + client->name, ts);
> + if (ret != 0) {
> + dev_err(&client->dev, "request irq failed: %d\n", ret);
> + goto intr_error;
> + }
> +
> + disable_irq(client->irq);
> + device_init_wakeup(&client->dev, 1);
Rely on DT and I2C core to set up device for wakeup.
> + enable_irq(client->irq);
Why?
> +
> + return 0;
> +
> +intr_error:
> + input_unregister_device(ts->input);
You are using managed input device, no need to unregister explicitly.
> +error:
> + regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
Please install as custom devm action (devm_add_action_or_reset).
> + return ret;
> +}
> +
> +static int nt36xxx_i2c_remove(struct i2c_client *client)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(client);
> +
> + free_irq(client->irq, ts);
> + regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + mutex_destroy(&ts->lock);
> + input_unregister_device(ts->input);
All of this can be removed I believe.
> +
> + return 0;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_suspend(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + u8 buf[] = { NT36XXX_EVT_HOST_CMD, NT36XXX_CMD_ENTER_SLEEP };
> + int i, ret;
> +
> + disable_irq(ts->client->irq);
> +
> + ret = nt36xxx_write(ts->client, NT36XXX_BLDR_ADDR,
> + buf, ARRAY_SIZE(buf));
> + if (unlikely(ret < 0)) {
> + dev_err(&ts->client->dev, "Cannot enter suspend!!\n");
> + return ret;
> + }
> +
> + /* Release all touches */
> + for (i = 0; i < ts->max_fingers; i++) {
> + input_mt_slot(ts->input, i);
> + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR, 0);
> + input_report_abs(ts->input, ABS_MT_PRESSURE, 0);
> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, 0);
> + }
> + input_sync(ts->input);
If this is needed it needs to be in input core.
> +
> + return 0;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_resume(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + mutex_lock(&ts->lock);
> +
> + if (ts->reset_gpio)
> + gpiod_set_value(ts->reset_gpio, 1);
You are asserting reset and keep it asserted?
> +
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0)
> + goto end;
> +
> + ret = nt36xxx_check_reset_state(ts, NT36XXX_STATE_REK);
> + if (ret < 0)
> + goto end;
Is this all needed if the device is configured for wakeup and power to
the device was not cut?
> +
> + enable_irq(ts->client->irq);
> +end:
> + mutex_unlock(&ts->lock);
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(nt36xxx_i2c_pm,
> + nt36xxx_i2c_suspend, nt36xxx_i2c_resume);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id nt36xxx_of_match[] = {
> + { .compatible = "novatek,nt36xxx" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, nt36xxx_of_match);
> +#endif
> +
> +static const struct i2c_device_id nt36xxx_i2c_ts_id[] = {
> + { "NVT-ts", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, nt36xxx_i2c_ts_id);
> +
> +static struct i2c_driver nt36xxx_i2c_ts_driver = {
> + .driver = {
> + .name = "nt36xxx_ts",
> + .of_match_table = of_match_ptr(nt36xxx_of_match),
> + .pm = &nt36xxx_i2c_pm,
> + },
> + .probe = nt36xxx_i2c_probe,
> + .remove = nt36xxx_i2c_remove,
> + .id_table = nt36xxx_i2c_ts_id,
> +};
> +module_i2c_driver(nt36xxx_i2c_ts_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Novatek NT36XXX Touchscreen Driver");
> +
> diff --git a/drivers/input/touchscreen/nt36xxx.h b/drivers/input/touchscreen/nt36xxx.h
> new file mode 100644
> index 000000000000..6f03dfb45656
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.h
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
I'd keep all of this in .c file.
> +
> +#ifndef NT36XXX_H
> +#define NT36XXX_H
> +
> +#define NT36XXX_INPUT_DEVICE_NAME "Novatek NT36XXX Touch Sensor"
> +
> +/* These chips have this fixed address when in bootloader :( */
> +#define NT36XXX_BLDR_ADDR 0x01
> +
> +/* Input device info */
> +#define NVT_TS_NAME "NVTCapacitiveTouchScreen"
> +
> +/* Number of bytes for chip identification */
> +#define NT36XXX_ID_LEN_MAX 6
> +
> +/* Touch info */
> +#define TOUCH_DEFAULT_MAX_WIDTH 1080
> +#define TOUCH_DEFAULT_MAX_HEIGHT 2246
> +#define TOUCH_MAX_FINGER_NUM 10
> +#define TOUCH_MAX_PRESSURE 1000
> +
> +/* Point data length */
> +#define POINT_DATA_LEN 65
> +
> +/* Global pages */
> +#define NT36XXX_PAGE_CHIP_INFO 0x0001f64e
> +#define NT36XXX_PAGE_CRC 0x0003f135
> +
> +/* Misc */
> +#define NT36XXX_NUM_SUPPLIES 2
> +#define NT36XXX_MAX_RETRIES 5
> +#define NT36XXX_MAX_FW_RST_RETRY 50
> +
> +struct nt36xxx_abs_object {
> + u16 x;
> + u16 y;
> + u16 z;
> + u8 tm;
> +};
> +
> +struct nt36xxx_fw_info {
> + u8 fw_ver;
> + u8 x_num;
> + u8 y_num;
> + u8 max_buttons;
> + u16 abs_x_max;
> + u16 abs_y_max;
> + u16 nvt_pid;
> +};
> +
> +struct nt36xxx_mem_map {
> + u32 evtbuf_addr;
> + u32 pipe0_addr;
> + u32 pipe1_addr;
> + u32 flash_csum_addr;
> + u32 flash_data_addr;
> +};
> +
> +struct nt36xxx_i2c {
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct regulator_bulk_data *supplies;
> + struct gpio_desc *reset_gpio;
> +
> + struct work_struct ts_work;
> + struct workqueue_struct *ts_workq;
> + struct mutex lock;
> +
> + struct nt36xxx_fw_info fw_info;
> + struct nt36xxx_abs_object abs_obj;
> +
> + const struct nt36xxx_mem_map *mmap;
> + u8 max_fingers;
> +};
> +
> +enum nt36xxx_chips {
> + NT36525_IC = 0,
> + NT36672A_IC,
> + NT36676F_IC,
> + NT36772_IC,
> + NT36870_IC,
> + NTMAX_IC,
> +};
> +
> +struct nt36xxx_trim_table {
> + u8 id[NT36XXX_ID_LEN_MAX];
> + u8 mask[NT36XXX_ID_LEN_MAX];
> + enum nt36xxx_chips mapid;
> +};
> +
> +enum nt36xxx_cmds {
> + NT36XXX_CMD_ENTER_SLEEP = 0x11,
> + NT36XXX_CMD_ENTER_WKUP_GESTURE = 0x13,
> + NT36XXX_CMD_UNLOCK = 0x35,
> + NT36XXX_CMD_BOOTLOADER_RESET = 0x69,
> + NT36XXX_CMD_SW_RESET = 0xa5,
> + NT36XXX_CMD_SET_PAGE = 0xff,
> +};
> +
> +enum nt36xxx_fw_state {
> + NT36XXX_STATE_INIT = 0xa0, /* IC reset */
> + NT36XXX_STATE_REK, /* ReK baseline */
> + NT36XXX_STATE_REK_FINISH, /* Baseline is ready */
> + NT36XXX_STATE_NORMAL_RUN, /* Normal run */
> + NT36XXX_STATE_MAX = 0xaf
> +};
> +
> +enum nt36xxx_i2c_events {
> + NT36XXX_EVT_CHIPID = 0x4e,
> + NT36XXX_EVT_HOST_CMD = 0x50,
> + NT36XXX_EVT_HS_OR_SUBCMD = 0x51, /* Handshake or subcommand byte */
> + NT36XXX_EVT_RESET_COMPLETE = 0x60,
> + NT36XXX_EVT_FWINFO = 0x78,
> + NT36XXX_EVT_PROJECTID = 0x9a,
> +};
> +
> +#endif
> --
> 2.28.0
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver
2020-09-27 12:35 ` [PATCH v2 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
2020-09-28 9:48 ` Amit Pundir
2020-09-28 22:16 ` Dmitry Torokhov
@ 2020-10-06 18:34 ` Dan Carpenter
2 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2020-10-06 18:34 UTC (permalink / raw)
To: kbuild, kholk11, dmitry.torokhov
Cc: lkp, kbuild-all, robh+dt, rydberg, priv.luk, linux-input,
linux-kernel, kholk11, marijns95, konradybcio, martin.botka1
[-- Attachment #1: Type: text/plain, Size: 3921 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/kholk11-gmail-com/Add-Novatek-NT36xxx-touchscreen-driver/20200927-203756
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-m021-20201003 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/input/touchscreen/nt36xxx.c:470 nt36xxx_i2c_chip_version_init() warn: calling memset(x, y, ARRAY_SIZE());
vim +470 drivers/input/touchscreen/nt36xxx.c
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 443 static int nt36xxx_i2c_chip_version_init(struct nt36xxx_i2c *ts)
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 444 {
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 445 u8 buf[7] = { 0 };
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 446 int retry = NT36XXX_MAX_RETRIES;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 447 int sz = sizeof(trim_id_table) / sizeof(struct nt36xxx_trim_table);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 448 int i, list, mapid, ret;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 449
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 450 ret = nt36xxx_bootloader_reset(ts);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 451 if (ret < 0) {
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 452 dev_err(&ts->client->dev, "Can't reset the nvt IC\n");
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 453 return ret;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 454 }
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 455
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 456 do {
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 457 ret = nt36xxx_sw_reset_idle(ts);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 458 if (ret < 0)
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 459 continue;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 460
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 461 buf[0] = 0x00;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 462 buf[1] = NT36XXX_CMD_UNLOCK;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 463 nt36xxx_write(ts->client, ts->client->addr, buf, 2);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 464 usleep_range(10000, 11000);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 465
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 466 ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 467 if (ret < 0)
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 468 continue;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 469
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 @470 memset(buf, 0, ARRAY_SIZE(buf));
memset() should always zero the number of bytes (sizeof(buf)), not the
number of elements (ARRAY_SIZE(buf)). For u8, of course, the elements
are byte size so it's the same. But if someone uses
`grep -w memset $(git ls-files) | grep ARRAY_SIZE` then this will show
up as improper code.
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 471 buf[0] = NT36XXX_EVT_CHIPID;
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 472 nt36xxx_read(ts->client, NT36XXX_BLDR_ADDR, buf, 7);
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 473
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 474 /* Compare read chip id with trim list */
6375d650f6f53aa AngeloGioacchino Del Regno 2020-09-27 475 for (list = 0; list < sz; list++) {
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31853 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread