linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver
@ 2020-10-28 22:12 kholk11
  2020-10-28 22:13 ` [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp kholk11
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: kholk11 @ 2020-10-28 22:12 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, kholk11,
	marijns95, konradybcio, martin.botka1, phone-devel, devicetree,
	krzk, andy.shevchenko

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

This patch series adds support for the Novatek NT36xxx Series' In-Cell
touchscreen (integrated into the DriverIC).

This patch series has been tested against the following devices:
 - Sony Xperia 10        (SDM630 Ganges Kirin)
 - Sony Xperia 10 Plus   (SDM636 Ganges Mermaid)

Changes in v2:
- Fixed sparse warnings from lkp kernel test robot

Changes in v3 (as requested by Dmitry Torokhov):
- Using shorthand u16/u32 (sorry for the overlook!)
- Now using more input and touchscreen APIs
- Fixed useless workqueue involvements
- Removed useless locking
- Switched reads and writes to use regmap
- Moved header contents to nt36xxx.c
- Fixed reset gpio handling
- Other cleanups
- P.S.: Thanks, Dmitry!

Changes in v4:
- Fixed regmap read length for CRC_ERR_FLAG final check
- Fixed YAML binding, as requested by Krzysztof Kozlowski

Changes in v5:
- Replaced subsystem maintainer's name with .. mine,
  usage of additionalProperties to unevaluatedProperties
  and a typo fix for reset-gpios as per Rob Herring's review
- Changed compatible string as per Krzysztof K. request
- Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml
  in order to now reflect the driver name instead of the DT
  compatible
- Fixed blank line at EOF

Changes in v6:
- Removed include of_gpio.h, added mod_devicetable.h and
  gpio/consumer.h
- Added kerneldoc to relevant functions/enum
- Used traditional patterns for error checking where possible
- Documented calls to usleep/msleep
- Using be16_to_cpu / get_unaligned_be16 where possible
- Added helper for CRC error check on retrieved buffer
- Decreased indentation in the CRC reboot recovery function
- Removed instances of error code sum
- Dropped all likely/unlikely optimization as per request
- Removed redundant reset_gpio checks
- Dropped of_match_ptr and ifdefs for CONFIG_OF

Changes in v7:
- Fixed typo in nt36xxx.c

Changes in v8:
- Fixed typo reset-gpio -> reset-gpios in dt-bindings

Changes in v9:
- Includes are now sorted
- Used proposed sizeof variable instead of sizeof type
- Fixed a return value check for common pattern
- Added NULL check to devm_kasprintf call
- Returning ret on probe function to be consistent

AngeloGioacchino Del Regno (3):
  dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
  Input: Add Novatek NT36xxx touchscreen driver
  dt-bindings: touchscreen: Add binding for Novatek NT36xxx series
    driver

 .../bindings/input/touchscreen/nt36xxx.yaml   |  59 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/input/touchscreen/Kconfig             |  12 +
 drivers/input/touchscreen/Makefile            |   1 +
 drivers/input/touchscreen/nt36xxx.c           | 894 ++++++++++++++++++
 drivers/input/touchscreen/nt36xxx.h           | 122 +++
 6 files changed, 1090 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
 create mode 100644 drivers/input/touchscreen/nt36xxx.c
 create mode 100644 drivers/input/touchscreen/nt36xxx.h

-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
@ 2020-10-28 22:13 ` kholk11
  2020-10-29 15:50   ` Rob Herring
  2020-10-28 22:13 ` [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: kholk11 @ 2020-10-28 22:13 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, kholk11,
	marijns95, konradybcio, martin.botka1, phone-devel, devicetree,
	krzk, andy.shevchenko

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Add prefix for Novatek Microelectronics Corp.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 66e45112a8d7..f98ea0af487d 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -740,6 +740,8 @@ patternProperties:
     description: Nokia
   "^nordic,.*":
     description: Nordic Semiconductor
+  "^novatek,.*":
+    description: Novatek Microelectronics Corp.
   "^novtech,.*":
     description: NovTech, Inc.
   "^nutsboard,.*":
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
  2020-10-28 22:13 ` [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp kholk11
@ 2020-10-28 22:13 ` kholk11
  2020-12-07  6:43   ` Dmitry Torokhov
  2020-10-28 22:13 ` [PATCH v9 3/3] dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver kholk11
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: kholk11 @ 2020-10-28 22:13 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, kholk11,
	marijns95, konradybcio, martin.botka1, phone-devel, devicetree,
	krzk, andy.shevchenko

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 | 894 ++++++++++++++++++++++++++++
 drivers/input/touchscreen/nt36xxx.h | 122 ++++
 4 files changed, 1029 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..a572d2b87464
--- /dev/null
+++ b/drivers/input/touchscreen/nt36xxx.c
@@ -0,0 +1,894 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Novatek NT36xxx series touchscreens
+ *
+ * Copyright (C) 2010 - 2017 Novatek, Inc.
+ * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <asm/unaligned.h>
+
+/* FW Param address */
+#define NT36XXX_FW_ADDR 0x01
+
+/* 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 *hw_client;
+	struct i2c_client *fw_client;
+	struct regmap *regmap;
+	struct regmap *fw_regmap;
+	struct input_dev *input;
+	struct regulator_bulk_data *supplies;
+	struct gpio_desc *reset_gpio;
+
+	struct mutex lock;
+
+	struct touchscreen_properties prop;
+	struct nt36xxx_fw_info fw_info;
+	struct nt36xxx_abs_object abs_obj;
+
+	const struct nt36xxx_mem_map *mmap;
+};
+
+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 - Firmware state
+ * @NT36XXX_STATE_INIT: IC Reset
+ * @NT36XXX_STATE_REK: ReK baseline
+ * @NT36XXX_STATE_REK_FINISH: Baseline is ready
+ * @NT36XXX_STATE_NORMAL_RUN: Firmware is running
+ */
+enum nt36xxx_fw_state {
+	NT36XXX_STATE_INIT = 0xa0,
+	NT36XXX_STATE_REK,
+	NT36XXX_STATE_REK_FINISH,
+	NT36XXX_STATE_NORMAL_RUN,
+	NT36XXX_STATE_MAX = 0xaf
+};
+
+enum nt36xxx_i2c_events {
+	NT36XXX_EVT_REPORT = 0x00,
+	NT36XXX_EVT_CRC = 0x35,
+	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,
+};
+
+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,
+	},
+};
+
+/**
+ * nt36xxx_set_page - Set page number for read/write
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_set_page(struct nt36xxx_i2c *ts, u32 pageaddr)
+{
+	u32 data = cpu_to_be32(pageaddr) >> 8;
+	int ret;
+
+	ret = regmap_noinc_write(ts->fw_regmap, NT36XXX_CMD_SET_PAGE,
+				 &data, sizeof(data));
+	if (ret)
+		return ret;
+
+	usleep_range(100, 200);
+	return ret;
+}
+
+/**
+ * nt36xxx_sw_reset_idle - Warm restart the firmware
+ * @ts: Main driver structure
+ *
+ * This function restarts the running firmware without rebooting to
+ * the bootloader (warm restart)
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_sw_reset_idle(struct nt36xxx_i2c *ts)
+{
+	int ret;
+
+	ret = regmap_write(ts->regmap, ts->hw_client->addr,
+			   NT36XXX_CMD_SW_RESET);
+	if (ret)
+		return ret;
+
+	/* Wait until the MCU resets the fw state */
+	usleep_range(15000, 16000);
+	return ret;
+}
+
+/**
+ * nt36xxx_bootloader_reset - Reset MCU to bootloader
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_bootloader_reset(struct nt36xxx_i2c *ts)
+{
+	int ret;
+
+	ret = regmap_write(ts->regmap, ts->hw_client->addr,
+			   NT36XXX_CMD_BOOTLOADER_RESET);
+	if (ret)
+		return ret;
+
+	/* MCU has to reboot from bootloader: this is the typical boot time */
+	msleep(35);
+	return ret;
+}
+
+/**
+ * nt36xxx_check_reset_state - Check the boot state during reset
+ * @ts: Main driver structure
+ * @fw_state: Enumeration containing firmware states
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_check_reset_state(struct nt36xxx_i2c *ts,
+				     enum nt36xxx_fw_state fw_state)
+{
+	u8 buf[2] = { 0 };
+	int ret, retry = NT36XXX_MAX_FW_RST_RETRY;
+
+	do {
+		ret = regmap_noinc_read(ts->fw_regmap,
+					NT36XXX_EVT_RESET_COMPLETE,
+					buf, sizeof(buf));
+		if (likely(ret == 0) &&
+		    (buf[0] >= fw_state) &&
+		    (buf[0] <= NT36XXX_STATE_MAX)) {
+			ret = 0;
+			break;
+		}
+		usleep_range(10000, 11000);
+	} while (--retry);
+
+	if (!retry) {
+		dev_err(&ts->hw_client->dev, "Firmware reset failed.\n");
+		ret = -EBUSY;
+	}
+
+	return ret;
+}
+
+/**
+ * nt36xxx_read_pid - Read Novatek Project ID
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_read_pid(struct nt36xxx_i2c *ts)
+{
+	__be16 pid;
+	int ret;
+
+	ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
+	if (ret)
+		return ret;
+
+	ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_PROJECTID,
+				&pid, sizeof(pid));
+	if (ret < 0)
+		return ret;
+
+	ts->fw_info.nvt_pid = be16_to_cpu(pid);
+	return 0;
+}
+
+/**
+ * __nt36xxx_get_fw_info - Get working params from firmware
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int __nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
+{
+	struct nt36xxx_fw_info *fwi = &ts->fw_info;
+	u8 buf[11] = { 0 };
+	int ret = 0;
+
+	ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
+	if (ret)
+		return ret;
+
+	ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_FWINFO,
+				buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	fwi->fw_ver = buf[0];
+	fwi->x_num = buf[2];
+	fwi->y_num = buf[3];
+	fwi->abs_x_max = get_unaligned_be16(&buf[4]);
+	fwi->abs_y_max = get_unaligned_be16(&buf[6]);
+	fwi->max_buttons = buf[10];
+
+	/* Check fw info integrity and clear x_num, y_num if broken */
+	if ((buf[0] + buf[1]) != 0xFF) {
+		dev_err(&ts->hw_client->dev,
+			"FW info is broken! fw_ver=0x%02X, ~fw_ver=0x%02X\n",
+			buf[0], buf[1]);
+		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->hw_client->dev,
+		"FW Info: PID=0x%x, ver=0x%x res=%ux%u max=%ux%u buttons=%u",
+		fwi->nvt_pid, fwi->fw_ver, fwi->x_num, fwi->y_num,
+		fwi->abs_x_max, fwi->abs_y_max, fwi->max_buttons);
+
+	return ret;
+}
+
+/**
+ * nt36xxx_report - Report touch events
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static void nt36xxx_report(struct nt36xxx_i2c *ts)
+{
+	struct nt36xxx_abs_object *obj = &ts->abs_obj;
+	struct input_dev *input = ts->input;
+	u8 input_id = 0;
+	u8 point[POINT_DATA_LEN + 1] = { 0 };
+	unsigned int ppos = 0;
+	int i, ret, finger_cnt = 0;
+
+	mutex_lock(&ts->lock);
+
+	ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_REPORT,
+				point, sizeof(point));
+	if (ret < 0) {
+		dev_err(&ts->hw_client->dev,
+			"Cannot read touch point data: %d\n", ret);
+		goto xfer_error;
+	}
+
+	for (i = 0; i < TOUCH_MAX_FINGER_NUM; i++) {
+		ppos = 6 * i;
+		input_id = point[ppos + 0] >> 3;
+		if ((input_id == 0) || (input_id > TOUCH_MAX_FINGER_NUM))
+			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->prop.max_x) ||
+			    (obj->y > ts->prop.max_y))
+				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;
+
+			input_mt_slot(input, input_id - 1);
+			input_mt_report_slot_state(input,
+						   MT_TOOL_FINGER, true);
+			touchscreen_report_pos(input, &ts->prop, obj->x,
+					       obj->y, true);
+
+			input_report_abs(input, ABS_MT_TOUCH_MAJOR, obj->tm);
+			input_report_abs(input, ABS_MT_PRESSURE, obj->z);
+
+			finger_cnt++;
+		}
+	}
+	input_mt_sync_frame(input);
+	input_sync(input);
+
+xfer_error:
+	enable_irq(ts->hw_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->hw_client->irq);
+	nt36xxx_report(ts);
+
+	return IRQ_HANDLED;
+}
+
+static bool nt36xxx_in_crc_reboot_loop(u8 *buf)
+{
+	return ((buf[0] == 0xFC) && (buf[1] == 0xFC) && (buf[2] == 0xFC)) ||
+	       ((buf[0] == 0xFF) && (buf[1] == 0xFF) && (buf[2] == 0xFF));
+}
+
+/**
+ * nt36xxx_stop_crc_reboot - Stop CRC reboot loop and warm-reboot the firmware
+ * @ts: Main driver structure
+ *
+ * Return: Always zero for success, negative number for error
+ */
+static int nt36xxx_stop_crc_reboot(struct nt36xxx_i2c *ts)
+{
+	u8 buf[3] = { 0 };
+	u8 val;
+	int ret, retry = NT36XXX_MAX_RETRIES;
+
+	/* Read dummy buffer to check CRC fail reboot is happening or not */
+
+	/* Change I2C index to prevent getting 0xFF, but not 0xFC */
+	ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
+	if (ret) {
+		dev_dbg(&ts->hw_client->dev,
+			"CRC reset failed: Cannot select page.\n");
+		return ret;
+	}
+
+	/* If ChipID command returns 0xFC or 0xFF, the MCU is in CRC reboot */
+	ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CHIPID,
+				buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	if (!nt36xxx_in_crc_reboot_loop(buf))
+		return 0;
+
+	/* IC is in CRC fail reboot loop, needs to be stopped! */
+	do {
+		/* Special reset-idle sequence for CRC failure */
+		ret = regmap_write(ts->regmap, ts->hw_client->addr,
+				   NT36XXX_CMD_SW_RESET);
+		if (ret)
+			dev_dbg(&ts->hw_client->dev,
+				"SW Reset 1 failed: may not recover\n");
+
+		ret = regmap_write(ts->regmap, ts->hw_client->addr,
+				   NT36XXX_CMD_SW_RESET);
+		if (ret)
+			dev_dbg(&ts->hw_client->dev,
+				"SW Reset 2 failed: may not recover\n");
+		usleep_range(1000, 1100);
+
+		/* Clear CRC_ERR_FLAG */
+		ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
+		if (ret)
+			continue;
+
+		val = 0xA5;
+		ret = regmap_raw_write(ts->fw_regmap, NT36XXX_EVT_CRC,
+				       &val, sizeof(val));
+		if (ret)
+			continue;
+
+		/* Check CRC_ERR_FLAG */
+		ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
+		if (ret)
+			continue;
+
+		ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CRC,
+					&buf, sizeof(buf));
+		if (ret)
+			continue;
+
+		if (buf[0] == 0xA5)
+			break;
+	} while (--retry);
+
+	if (retry == 0) {
+		dev_err(&ts->hw_client->dev,
+			"CRC reset failed: buf=0x%2ph\n", buf);
+	}
+
+	return ret;
+}
+
+/**
+ * nt36xxx_i2c_chip_version_init - Detect Novatek NT36xxx family IC
+ * @ts: Main driver structure
+ *
+ * This function reads the ChipID from the IC and sets the right
+ * memory map for the detected chip.
+ *
+ * Return: Always zero for success, negative number for error
+ */
+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) {
+		dev_err(&ts->hw_client->dev, "Can't reset the nvt IC\n");
+		return ret;
+	}
+
+	do {
+		ret = nt36xxx_sw_reset_idle(ts);
+		if (ret)
+			continue;
+
+		ret = regmap_write(ts->regmap, ts->hw_client->addr, NT36XXX_CMD_UNLOCK);
+		if (ret)
+			continue;
+		usleep_range(10000, 11000);
+
+		ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
+		if (ret)
+			continue;
+
+		memset(buf, 0, ARRAY_SIZE(buf));
+		ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CHIPID,
+					buf, sizeof(buf));
+		if (ret)
+			continue;
+
+		/* 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] != 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 (nt36xxx_in_crc_reboot_loop(buf)) {
+			ret = nt36xxx_stop_crc_reboot(ts);
+			if (ret)
+				continue;
+		}
+
+		usleep_range(10000, 11000);
+	} while (--retry);
+
+	return ret;
+}
+
+static const struct regmap_config nt36xxx_i2c_regmap_hw_config = {
+	.name = "nt36xxx_i2c_hw",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+};
+
+static const struct regmap_config nt36xxx_i2c_regmap_fw_config = {
+	.name = "nt36xxx_i2c_fw",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+};
+
+static void nt36xxx_disable_regulators(void *data)
+{
+	struct nt36xxx_i2c *ts = data;
+
+	regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
+}
+
+static int nt36xxx_i2c_probe(struct i2c_client *hw_client,
+			     const struct i2c_device_id *id)
+{
+	struct nt36xxx_i2c *ts;
+	struct input_dev *input;
+	int ret;
+
+	if (!i2c_check_functionality(hw_client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&hw_client->dev, "i2c_check_functionality error\n");
+		return -EIO;
+	}
+
+	if (!hw_client->irq) {
+		dev_err(&hw_client->dev, "No irq specified\n");
+		return -EINVAL;
+	}
+
+	ts = devm_kzalloc(&hw_client->dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	ts->supplies = devm_kcalloc(&hw_client->dev,
+				    NT36XXX_NUM_SUPPLIES,
+				    sizeof(*ts->supplies),
+				    GFP_KERNEL);
+	if (!ts->supplies)
+		return -ENOMEM;
+
+	input = devm_input_allocate_device(&hw_client->dev);
+	if (!input)
+		return -ENOMEM;
+
+	ts->fw_client = i2c_new_dummy_device(hw_client->adapter,
+					     NT36XXX_FW_ADDR);
+	if (IS_ERR(ts->fw_client)) {
+		dev_err(&hw_client->dev, "Cannot add FW I2C device\n");
+		return PTR_ERR(ts->fw_client);
+	}
+
+	ts->hw_client = hw_client;
+	ts->input = input;
+	i2c_set_clientdata(ts->hw_client, ts);
+	i2c_set_clientdata(ts->fw_client, ts);
+
+	ts->reset_gpio = devm_gpiod_get_optional(&hw_client->dev, "reset",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->reset_gpio))
+		return PTR_ERR(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(&hw_client->dev,
+				      NT36XXX_NUM_SUPPLIES,
+				      ts->supplies);
+	if (ret)
+		return dev_err_probe(&hw_client->dev, ret,
+				     "Cannot get supplies: %d\n", ret);
+
+	ts->regmap = devm_regmap_init_i2c(ts->hw_client,
+					  &nt36xxx_i2c_regmap_hw_config);
+	if (IS_ERR(ts->regmap)) {
+		dev_err(&hw_client->dev, "regmap (hw-addr) init failed\n");
+		return PTR_ERR(ts->regmap);
+	}
+
+	ts->fw_regmap = devm_regmap_init_i2c(ts->fw_client,
+					     &nt36xxx_i2c_regmap_fw_config);
+	if (IS_ERR(ts->fw_regmap)) {
+		dev_err(&hw_client->dev, "regmap (fw-addr) init failed\n");
+		return PTR_ERR(ts->fw_regmap);
+	}
+
+	ret = regulator_bulk_enable(NT36XXX_NUM_SUPPLIES, ts->supplies);
+	if (ret)
+		return ret;
+
+	usleep_range(10000, 11000);
+
+	ret = devm_add_action_or_reset(&hw_client->dev,
+				       nt36xxx_disable_regulators, ts);
+	if (ret)
+		return ret;
+
+	mutex_init(&ts->lock);
+
+	/* Set memory maps for the specific chip version */
+	ret = nt36xxx_i2c_chip_version_init(ts);
+	if (ret) {
+		dev_err(&hw_client->dev, "Failed to check chip version\n");
+		return ret;
+	}
+
+	/* Reset the MCU */
+	ret = nt36xxx_bootloader_reset(ts);
+	if (ret < 0)
+		return ret;
+
+	/* Check and eventually wait until the MCU goes in reset state */
+	ret = nt36xxx_check_reset_state(ts, NT36XXX_STATE_INIT);
+	if (ret < 0)
+		return ret;
+
+	/* Get informations from the TS firmware */
+	ret = nt36xxx_get_fw_info(ts);
+	if (ret < 0)
+		return ret;
+
+	input->phys = devm_kasprintf(&hw_client->dev, GFP_KERNEL,
+				     "%s/input0", dev_name(&hw_client->dev));
+	if (!input->phys)
+		return -ENOMEM;
+
+	input->name = "Novatek NT36XXX Touchscreen";
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &hw_client->dev;
+
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_ABS, input->evbit);
+	input_set_capability(input, EV_KEY, BTN_TOUCH);
+
+	ret = input_mt_init_slots(input, TOUCH_MAX_FINGER_NUM,
+				  INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+	if (ret) {
+		dev_err(&hw_client->dev, "Cannot init MT slots (%d)\n", ret);
+		return ret;
+	}
+
+	input_set_abs_params(input, ABS_MT_PRESSURE, 0,
+			     TOUCH_MAX_PRESSURE, 0, 0);
+	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);
+
+	/* Override the firmware defaults, if needed */
+	touchscreen_parse_properties(input, true, &ts->prop);
+
+	input_set_drvdata(input, ts);
+
+	ret = input_register_device(ts->input);
+	if (ret) {
+		dev_err(&hw_client->dev, "Failed to register input device: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&hw_client->dev, hw_client->irq, NULL,
+					nt36xxx_i2c_irq_handler, IRQF_ONESHOT,
+					hw_client->name, ts);
+	if (ret) {
+		dev_err(&hw_client->dev, "request irq failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int __maybe_unused nt36xxx_i2c_suspend(struct device *dev)
+{
+	struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	disable_irq(ts->hw_client->irq);
+
+	ret = regmap_write(ts->fw_regmap, NT36XXX_EVT_HOST_CMD,
+			   NT36XXX_CMD_ENTER_SLEEP);
+	if (ret) {
+		dev_err(&ts->hw_client->dev, "Cannot enter suspend!!\n");
+		return ret;
+	}
+
+	gpiod_set_value(ts->reset_gpio, 1);
+
+	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);
+
+	gpiod_set_value(ts->reset_gpio, 0);
+
+	/* Reboot the MCU (also recalibrates the TS) */
+	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->hw_client->irq);
+end:
+	mutex_unlock(&ts->lock);
+	return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(nt36xxx_i2c_pm,
+			 nt36xxx_i2c_suspend, nt36xxx_i2c_resume);
+
+static const struct of_device_id nt36xxx_of_match[] = {
+	{ .compatible = "novatek,nt36525" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, nt36xxx_of_match);
+
+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",
+		.pm	= &nt36xxx_i2c_pm,
+		.of_match_table = nt36xxx_of_match,
+	},
+	.id_table	= nt36xxx_i2c_ts_id,
+	.probe		= nt36xxx_i2c_probe,
+};
+module_i2c_driver(nt36xxx_i2c_ts_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Novatek NT36XXX Touchscreen Driver");
+MODULE_AUTHOR("AngeloGioacchino Del Regno <kholk11@gmail.com>");
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] 12+ messages in thread

* [PATCH v9 3/3] dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
  2020-10-28 22:13 ` [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp kholk11
  2020-10-28 22:13 ` [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
@ 2020-10-28 22:13 ` kholk11
  2020-11-23 10:12 ` [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver Amit Pundir
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: kholk11 @ 2020-10-28 22:13 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, kholk11,
	marijns95, konradybcio, martin.botka1, phone-devel, devicetree,
	krzk, andy.shevchenko, Rob Herring

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Add binding for the Novatek NT36xxx series touchscreen driver.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/input/touchscreen/nt36xxx.yaml   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml b/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
new file mode 100644
index 000000000000..a360a9f5d43b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/nt36xxx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Novatek NT36xxx series touchscreen controller Bindings
+
+maintainers:
+  - AngeloGioacchino Del Regno <kholk11@gmail.com>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    const: novatek,nt36525
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vdd-supply:
+    description: Power supply regulator for VDD pin
+
+  vio-supply:
+    description: Power supply regulator on VDD-IO pin
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@62 {
+        compatible = "novatek,nt36525";
+        reg = <0x62>;
+        interrupt-parent = <&tlmm>;
+        interrupts = <45 IRQ_TYPE_EDGE_RISING>;
+        reset-gpios = <&tlmm 102 GPIO_ACTIVE_HIGH>;
+      };
+    };
+
+...
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
  2020-10-28 22:13 ` [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp kholk11
@ 2020-10-29 15:50   ` Rob Herring
  2020-10-29 17:22     ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-10-29 15:50 UTC (permalink / raw)
  To: kholk11
  Cc: robh+dt, konradybcio, linux-kernel, dmitry.torokhov, priv.luk,
	marijns95, phone-devel, devicetree, andy.shevchenko, rydberg,
	martin.botka1, krzk, linux-input

On Wed, 28 Oct 2020 23:13:00 +0100, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> 
> Add prefix for Novatek Microelectronics Corp.
> 
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
  2020-10-29 15:50   ` Rob Herring
@ 2020-10-29 17:22     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2020-10-29 17:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rob Herring, konradybcio, linux-kernel, dmitry.torokhov,
	priv.luk, marijns95, phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	andy.shevchenko, rydberg, martin.botka1, Krzysztof Kozlowski,
	linux-input

Il giorno gio 29 ott 2020 alle ore 16:50 Rob Herring <robh@kernel.org>
ha scritto:
>
> On Wed, 28 Oct 2020 23:13:00 +0100, kholk11@gmail.com wrote:
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> >
> > Add prefix for Novatek Microelectronics Corp.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.
>
The intention was to add the tag and I even recall adding it... probably
my finger slipped and the changes didn't get saved, my bad disattention.
I should probably stop developing when I'm overtired.

I'm sorry for that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
                   ` (2 preceding siblings ...)
  2020-10-28 22:13 ` [PATCH v9 3/3] dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver kholk11
@ 2020-11-23 10:12 ` Amit Pundir
  2020-11-26 17:09   ` AngeloGioacchino Del Regno
  2020-11-27  6:23 ` Amit Pundir
  2022-04-26  7:55 ` Danct12
  5 siblings, 1 reply; 12+ messages in thread
From: Amit Pundir @ 2020-11-23 10:12 UTC (permalink / raw)
  To: kholk11
  Cc: Dmitry Torokhov, Rob Herring, rydberg, priv.luk, linux-input,
	lkml, marijns95, Konrad Dybcio, martin.botka1, phone-devel, dt,
	krzk, andy.shevchenko

Hi,

On Thu, 29 Oct 2020 at 06:32, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> This patch series adds support for the Novatek NT36xxx Series' In-Cell
> touchscreen (integrated into the DriverIC).
>
> This patch series has been tested against the following devices:
>  - Sony Xperia 10        (SDM630 Ganges Kirin)
>  - Sony Xperia 10 Plus   (SDM636 Ganges Mermaid)

Tested the patch series on Xiaomi Poco F1 (SDM845 Beryllium) using
Novatek NT36672A IC. May I suggest adding "novatek,nt36672a" in the
list of compatible of_device_id{} as well.

Regards,
Amit Pundir


>
> Changes in v2:
> - Fixed sparse warnings from lkp kernel test robot
>
> Changes in v3 (as requested by Dmitry Torokhov):
> - Using shorthand u16/u32 (sorry for the overlook!)
> - Now using more input and touchscreen APIs
> - Fixed useless workqueue involvements
> - Removed useless locking
> - Switched reads and writes to use regmap
> - Moved header contents to nt36xxx.c
> - Fixed reset gpio handling
> - Other cleanups
> - P.S.: Thanks, Dmitry!
>
> Changes in v4:
> - Fixed regmap read length for CRC_ERR_FLAG final check
> - Fixed YAML binding, as requested by Krzysztof Kozlowski
>
> Changes in v5:
> - Replaced subsystem maintainer's name with .. mine,
>   usage of additionalProperties to unevaluatedProperties
>   and a typo fix for reset-gpios as per Rob Herring's review
> - Changed compatible string as per Krzysztof K. request
> - Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml
>   in order to now reflect the driver name instead of the DT
>   compatible
> - Fixed blank line at EOF
>
> Changes in v6:
> - Removed include of_gpio.h, added mod_devicetable.h and
>   gpio/consumer.h
> - Added kerneldoc to relevant functions/enum
> - Used traditional patterns for error checking where possible
> - Documented calls to usleep/msleep
> - Using be16_to_cpu / get_unaligned_be16 where possible
> - Added helper for CRC error check on retrieved buffer
> - Decreased indentation in the CRC reboot recovery function
> - Removed instances of error code sum
> - Dropped all likely/unlikely optimization as per request
> - Removed redundant reset_gpio checks
> - Dropped of_match_ptr and ifdefs for CONFIG_OF
>
> Changes in v7:
> - Fixed typo in nt36xxx.c
>
> Changes in v8:
> - Fixed typo reset-gpio -> reset-gpios in dt-bindings
>
> Changes in v9:
> - Includes are now sorted
> - Used proposed sizeof variable instead of sizeof type
> - Fixed a return value check for common pattern
> - Added NULL check to devm_kasprintf call
> - Returning ret on probe function to be consistent
>
> AngeloGioacchino Del Regno (3):
>   dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
>   Input: Add Novatek NT36xxx touchscreen driver
>   dt-bindings: touchscreen: Add binding for Novatek NT36xxx series
>     driver
>
>  .../bindings/input/touchscreen/nt36xxx.yaml   |  59 ++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/touchscreen/Kconfig             |  12 +
>  drivers/input/touchscreen/Makefile            |   1 +
>  drivers/input/touchscreen/nt36xxx.c           | 894 ++++++++++++++++++
>  drivers/input/touchscreen/nt36xxx.h           | 122 +++
>  6 files changed, 1090 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
>  create mode 100644 drivers/input/touchscreen/nt36xxx.c
>  create mode 100644 drivers/input/touchscreen/nt36xxx.h
>
> --
> 2.28.0
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver
  2020-11-23 10:12 ` [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver Amit Pundir
@ 2020-11-26 17:09   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2020-11-26 17:09 UTC (permalink / raw)
  To: Amit Pundir
  Cc: Dmitry Torokhov, Rob Herring, rydberg, priv.luk, linux-input,
	lkml, marijns95, Konrad Dybcio, martin.botka1, phone-devel, dt,
	Krzysztof Kozlowski, andy.shevchenko

Il giorno lun 23 nov 2020 alle ore 11:13 Amit Pundir
<amit.pundir@linaro.org> ha scritto:
>
> Hi,
>
> On Thu, 29 Oct 2020 at 06:32, <kholk11@gmail.com> wrote:
> >
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> >
> > This patch series adds support for the Novatek NT36xxx Series' In-Cell
> > touchscreen (integrated into the DriverIC).
> >
> > This patch series has been tested against the following devices:
> >  - Sony Xperia 10        (SDM630 Ganges Kirin)
> >  - Sony Xperia 10 Plus   (SDM636 Ganges Mermaid)
>
> Tested the patch series on Xiaomi Poco F1 (SDM845 Beryllium) using
> Novatek NT36672A IC. May I suggest adding "novatek,nt36672a" in the
> list of compatible of_device_id{} as well.
>
> Regards,
> Amit Pundir
>
>
> >
> > Changes in v2:
> > - Fixed sparse warnings from lkp kernel test robot
> >
> > Changes in v3 (as requested by Dmitry Torokhov):
> > - Using shorthand u16/u32 (sorry for the overlook!)
> > - Now using more input and touchscreen APIs
> > - Fixed useless workqueue involvements
> > - Removed useless locking
> > - Switched reads and writes to use regmap
> > - Moved header contents to nt36xxx.c
> > - Fixed reset gpio handling
> > - Other cleanups
> > - P.S.: Thanks, Dmitry!
> >
> > Changes in v4:
> > - Fixed regmap read length for CRC_ERR_FLAG final check
> > - Fixed YAML binding, as requested by Krzysztof Kozlowski
> >
> > Changes in v5:
> > - Replaced subsystem maintainer's name with .. mine,
> >   usage of additionalProperties to unevaluatedProperties
> >   and a typo fix for reset-gpios as per Rob Herring's review
> > - Changed compatible string as per Krzysztof K. request
> > - Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml
> >   in order to now reflect the driver name instead of the DT
> >   compatible
> > - Fixed blank line at EOF
> >
> > Changes in v6:
> > - Removed include of_gpio.h, added mod_devicetable.h and
> >   gpio/consumer.h
> > - Added kerneldoc to relevant functions/enum
> > - Used traditional patterns for error checking where possible
> > - Documented calls to usleep/msleep
> > - Using be16_to_cpu / get_unaligned_be16 where possible
> > - Added helper for CRC error check on retrieved buffer
> > - Decreased indentation in the CRC reboot recovery function
> > - Removed instances of error code sum
> > - Dropped all likely/unlikely optimization as per request
> > - Removed redundant reset_gpio checks
> > - Dropped of_match_ptr and ifdefs for CONFIG_OF
> >
> > Changes in v7:
> > - Fixed typo in nt36xxx.c
> >
> > Changes in v8:
> > - Fixed typo reset-gpio -> reset-gpios in dt-bindings
> >
> > Changes in v9:
> > - Includes are now sorted
> > - Used proposed sizeof variable instead of sizeof type
> > - Fixed a return value check for common pattern
> > - Added NULL check to devm_kasprintf call
> > - Returning ret on probe function to be consistent
> >
> > AngeloGioacchino Del Regno (3):
> >   dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
> >   Input: Add Novatek NT36xxx touchscreen driver
> >   dt-bindings: touchscreen: Add binding for Novatek NT36xxx series
> >     driver
> >
> >  .../bindings/input/touchscreen/nt36xxx.yaml   |  59 ++
> >  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
> >  drivers/input/touchscreen/Kconfig             |  12 +
> >  drivers/input/touchscreen/Makefile            |   1 +
> >  drivers/input/touchscreen/nt36xxx.c           | 894 ++++++++++++++++++
> >  drivers/input/touchscreen/nt36xxx.h           | 122 +++
> >  6 files changed, 1090 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
> >  create mode 100644 drivers/input/touchscreen/nt36xxx.c
> >  create mode 100644 drivers/input/touchscreen/nt36xxx.h
> >
> > --
> > 2.28.0
> >
Mind releasing a Tested-By tag for this?

Anyway, I was suggested to add a compatible only for the "oldest" IC that
is supported in this driver, since there is autodetection, that's why you see
only the 36525 compatible here!

Yours,
Angelo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
                   ` (3 preceding siblings ...)
  2020-11-23 10:12 ` [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver Amit Pundir
@ 2020-11-27  6:23 ` Amit Pundir
  2022-04-26  7:55 ` Danct12
  5 siblings, 0 replies; 12+ messages in thread
From: Amit Pundir @ 2020-11-27  6:23 UTC (permalink / raw)
  To: kholk11
  Cc: Dmitry Torokhov, Rob Herring, rydberg, priv.luk, linux-input,
	lkml, marijns95, Konrad Dybcio, martin.botka1, phone-devel, dt,
	krzk, andy.shevchenko

On Thu, 29 Oct 2020 at 06:32, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> This patch series adds support for the Novatek NT36xxx Series' In-Cell
> touchscreen (integrated into the DriverIC).
>
> This patch series has been tested against the following devices:
>  - Sony Xperia 10        (SDM630 Ganges Kirin)
>  - Sony Xperia 10 Plus   (SDM636 Ganges Mermaid)
>

Tested the patch series on Xiaomi Poco F1 (SDM845 Beryllium, Novatek
NT36672A IC).

For the whole series:
Tested-by: Amit Pundir <amit.pundir@linaro.org>

Regards,
Amit Pundir


> Changes in v2:
> - Fixed sparse warnings from lkp kernel test robot
>
> Changes in v3 (as requested by Dmitry Torokhov):
> - Using shorthand u16/u32 (sorry for the overlook!)
> - Now using more input and touchscreen APIs
> - Fixed useless workqueue involvements
> - Removed useless locking
> - Switched reads and writes to use regmap
> - Moved header contents to nt36xxx.c
> - Fixed reset gpio handling
> - Other cleanups
> - P.S.: Thanks, Dmitry!
>
> Changes in v4:
> - Fixed regmap read length for CRC_ERR_FLAG final check
> - Fixed YAML binding, as requested by Krzysztof Kozlowski
>
> Changes in v5:
> - Replaced subsystem maintainer's name with .. mine,
>   usage of additionalProperties to unevaluatedProperties
>   and a typo fix for reset-gpios as per Rob Herring's review
> - Changed compatible string as per Krzysztof K. request
> - Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml
>   in order to now reflect the driver name instead of the DT
>   compatible
> - Fixed blank line at EOF
>
> Changes in v6:
> - Removed include of_gpio.h, added mod_devicetable.h and
>   gpio/consumer.h
> - Added kerneldoc to relevant functions/enum
> - Used traditional patterns for error checking where possible
> - Documented calls to usleep/msleep
> - Using be16_to_cpu / get_unaligned_be16 where possible
> - Added helper for CRC error check on retrieved buffer
> - Decreased indentation in the CRC reboot recovery function
> - Removed instances of error code sum
> - Dropped all likely/unlikely optimization as per request
> - Removed redundant reset_gpio checks
> - Dropped of_match_ptr and ifdefs for CONFIG_OF
>
> Changes in v7:
> - Fixed typo in nt36xxx.c
>
> Changes in v8:
> - Fixed typo reset-gpio -> reset-gpios in dt-bindings
>
> Changes in v9:
> - Includes are now sorted
> - Used proposed sizeof variable instead of sizeof type
> - Fixed a return value check for common pattern
> - Added NULL check to devm_kasprintf call
> - Returning ret on probe function to be consistent
>
> AngeloGioacchino Del Regno (3):
>   dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
>   Input: Add Novatek NT36xxx touchscreen driver
>   dt-bindings: touchscreen: Add binding for Novatek NT36xxx series
>     driver
>
>  .../bindings/input/touchscreen/nt36xxx.yaml   |  59 ++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/touchscreen/Kconfig             |  12 +
>  drivers/input/touchscreen/Makefile            |   1 +
>  drivers/input/touchscreen/nt36xxx.c           | 894 ++++++++++++++++++
>  drivers/input/touchscreen/nt36xxx.h           | 122 +++
>  6 files changed, 1090 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
>  create mode 100644 drivers/input/touchscreen/nt36xxx.c
>  create mode 100644 drivers/input/touchscreen/nt36xxx.h
>
> --
> 2.28.0
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver
  2020-10-28 22:13 ` [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
@ 2020-12-07  6:43   ` Dmitry Torokhov
  2021-01-13 13:03     ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2020-12-07  6:43 UTC (permalink / raw)
  To: kholk11
  Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, marijns95,
	konradybcio, martin.botka1, phone-devel, devicetree, krzk,
	andy.shevchenko

Hi AngeloGioacchino,

On Wed, Oct 28, 2020 at 11:13:01PM +0100, kholk11@gmail.com wrote:
> +/**
> + * nt36xxx_set_page - Set page number for read/write
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_set_page(struct nt36xxx_i2c *ts, u32 pageaddr)
> +{
> +	u32 data = cpu_to_be32(pageaddr) >> 8;
> +	int ret;
> +
> +	ret = regmap_noinc_write(ts->fw_regmap, NT36XXX_CMD_SET_PAGE,
> +				 &data, sizeof(data));
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(100, 200);
> +	return ret;
> +}

Regmap is supposed to handle paged access for you as long as you set it
up for paged access. Why do you need custom page handling here?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver
  2020-12-07  6:43   ` Dmitry Torokhov
@ 2021-01-13 13:03     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-01-13 13:03 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, rydberg, priv.luk, linux-input, linux-kernel,
	marijns95, Konrad Dybcio, martin.botka1, phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Krzysztof Kozlowski, andy.shevchenko

Il giorno lun 7 dic 2020 alle ore 07:43 Dmitry Torokhov
<dmitry.torokhov@gmail.com> ha scritto:
>
> Hi AngeloGioacchino,
>
> On Wed, Oct 28, 2020 at 11:13:01PM +0100, kholk11@gmail.com wrote:
> > +/**
> > + * nt36xxx_set_page - Set page number for read/write
> > + * @ts: Main driver structure
> > + *
> > + * Return: Always zero for success, negative number for error
> > + */
> > +static int nt36xxx_set_page(struct nt36xxx_i2c *ts, u32 pageaddr)
> > +{
> > +     u32 data = cpu_to_be32(pageaddr) >> 8;
> > +     int ret;
> > +
> > +     ret = regmap_noinc_write(ts->fw_regmap, NT36XXX_CMD_SET_PAGE,
> > +                              &data, sizeof(data));
> > +     if (ret)
> > +             return ret;
> > +
> > +     usleep_range(100, 200);
> > +     return ret;
> > +}
>
> Regmap is supposed to handle paged access for you as long as you set it
> up for paged access. Why do you need custom page handling here?
>
> Thanks.
>
> --
> Dmitry

Regmap's page handling is using regmap_update_bits, hence calling a
regmap_read for each page switch and this is not always possible on
this MCU: especially, but not only, in the CRC reboot case, calling
a regmap_read before the page-switch will result in invalid data.

Hacking through the invalid data, we would still be able to set the
page at this point, but then in the reset-idle sequence handling the
CRC failure, we are setting page again: keeping in mind that this is
a i2c connected MCU, calling a regmap_read while sending the "special"
reset sequence is not in the likes of this MCU SW design, as it is
expecting a specific, precise command sequence.

If that happens, the MCU won't recognize the CRC reset-idle sequence
and will never recover from the error.

This can surely be solved by setting up a FLAT regcache and resetting
the register cache in (many) strategic places but, in my opinion,
that's going to be seriously messy, as I would have to do that in
basically every place - but the CRC reboot loop function, so we'd have
a register cache for *only* one special case in one function (which is
not even supposed to be ever called during regular functionality,
unless the MCU firmware panics somehow).

There is also another reason why I dislike using the paged access that
comes from the regmap API here: as you see, the event buffer address
is different for some ICs (probably for MCU FW differences) and this
would require me to dynamically set the regmap_range_cfg structure in
the probe function, then casting it to a const and setting it into the
regmap_config before registering regmap (which, by the way, is another
const struct).

Ah, also, as you can see the set_page function is doing:
        u32 data = cpu_to_be32(pageaddr) >> 8;
clearly, using the regmap page switching, I would have to change the
pages definitions *and* the nt36xxx_mem_map at least partially (because
"win_page << range->selector_shift"), and all of these definitions,
right now, are representing the same addresses that are referenced into
the MCU firmware, other than being basically 1:1 with what the downstream
driver provides.
Changing them around would not only, in some way, "hide" precious infos
on a series of MCUs that are not publicly documented, but would also make
the eventual porting of new ICs/MCUs that would be compatible with this
driver harder for who knows what's going on and *way harder* for other
"casual" developers.

So, for the aforementioned reasons, all summed together, I chose to not
use the regmap paged access.

Yours,
-- Angelo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver
  2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
                   ` (4 preceding siblings ...)
  2020-11-27  6:23 ` Amit Pundir
@ 2022-04-26  7:55 ` Danct12
  5 siblings, 0 replies; 12+ messages in thread
From: Danct12 @ 2022-04-26  7:55 UTC (permalink / raw)
  To: kholk11
  Cc: dmitry.torokhov, robh+dt, rydberg, priv.luk, linux-input,
	linux-kernel, marijns95, konradybcio, martin.botka1, phone-devel,
	devicetree, krzk, andy.shevchenko

Patch series tested, works fine on the Xiaomi Redmi Note 7 (SDM660, NT36672A IC)

To the whole patch series:
Tested-by: Dang Huynh <danct12@riseup.net>

On Wed, 28 Oct 2020 23:12:59 +0100
kholk11@gmail.com wrote:

> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> 
> This patch series adds support for the Novatek NT36xxx Series' In-Cell
> touchscreen (integrated into the DriverIC).
> 
> This patch series has been tested against the following devices:
>  - Sony Xperia 10        (SDM630 Ganges Kirin)
>  - Sony Xperia 10 Plus   (SDM636 Ganges Mermaid)
> 
> Changes in v2:
> - Fixed sparse warnings from lkp kernel test robot
> 
> Changes in v3 (as requested by Dmitry Torokhov):
> - Using shorthand u16/u32 (sorry for the overlook!)
> - Now using more input and touchscreen APIs
> - Fixed useless workqueue involvements
> - Removed useless locking
> - Switched reads and writes to use regmap
> - Moved header contents to nt36xxx.c
> - Fixed reset gpio handling
> - Other cleanups
> - P.S.: Thanks, Dmitry!
> 
> Changes in v4:
> - Fixed regmap read length for CRC_ERR_FLAG final check
> - Fixed YAML binding, as requested by Krzysztof Kozlowski
> 
> Changes in v5:
> - Replaced subsystem maintainer's name with .. mine,
>   usage of additionalProperties to unevaluatedProperties
>   and a typo fix for reset-gpios as per Rob Herring's review
> - Changed compatible string as per Krzysztof K. request
> - Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml
>   in order to now reflect the driver name instead of the DT
>   compatible
> - Fixed blank line at EOF
> 
> Changes in v6:
> - Removed include of_gpio.h, added mod_devicetable.h and
>   gpio/consumer.h
> - Added kerneldoc to relevant functions/enum
> - Used traditional patterns for error checking where possible
> - Documented calls to usleep/msleep
> - Using be16_to_cpu / get_unaligned_be16 where possible
> - Added helper for CRC error check on retrieved buffer
> - Decreased indentation in the CRC reboot recovery function
> - Removed instances of error code sum
> - Dropped all likely/unlikely optimization as per request
> - Removed redundant reset_gpio checks
> - Dropped of_match_ptr and ifdefs for CONFIG_OF
> 
> Changes in v7:
> - Fixed typo in nt36xxx.c
> 
> Changes in v8:
> - Fixed typo reset-gpio -> reset-gpios in dt-bindings
> 
> Changes in v9:
> - Includes are now sorted
> - Used proposed sizeof variable instead of sizeof type
> - Fixed a return value check for common pattern
> - Added NULL check to devm_kasprintf call
> - Returning ret on probe function to be consistent
> 
> AngeloGioacchino Del Regno (3):
>   dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
>   Input: Add Novatek NT36xxx touchscreen driver
>   dt-bindings: touchscreen: Add binding for Novatek NT36xxx series
>     driver
> 
>  .../bindings/input/touchscreen/nt36xxx.yaml   |  59 ++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/touchscreen/Kconfig             |  12 +
>  drivers/input/touchscreen/Makefile            |   1 +
>  drivers/input/touchscreen/nt36xxx.c           | 894 ++++++++++++++++++
>  drivers/input/touchscreen/nt36xxx.h           | 122 +++
>  6 files changed, 1090 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
>  create mode 100644 drivers/input/touchscreen/nt36xxx.c
>  create mode 100644 drivers/input/touchscreen/nt36xxx.h
> 
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-26  7:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 22:12 [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver kholk11
2020-10-28 22:13 ` [PATCH v9 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp kholk11
2020-10-29 15:50   ` Rob Herring
2020-10-29 17:22     ` AngeloGioacchino Del Regno
2020-10-28 22:13 ` [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver kholk11
2020-12-07  6:43   ` Dmitry Torokhov
2021-01-13 13:03     ` AngeloGioacchino Del Regno
2020-10-28 22:13 ` [PATCH v9 3/3] dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver kholk11
2020-11-23 10:12 ` [PATCH v9 0/3] Add Novatek NT36xxx touchscreen driver Amit Pundir
2020-11-26 17:09   ` AngeloGioacchino Del Regno
2020-11-27  6:23 ` Amit Pundir
2022-04-26  7:55 ` Danct12

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).