[PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver.

[PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver.

[SP_ISW1_AT]

[-- Attachment #1: Type: text/html, Size: 1596 bytes --]

[-- Attachment #2: Type: text/plain, Size: 881 bytes --]

From: Ben Huang <Ben_Huang@novatek.com.tw>

Add entry for maintenance of Novatek NT726xx SoC i2c driver.

Signed-off-by: Ben Huang <Ben_Huang@novatek.com.tw>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ec290e38b44..7a77a1690f15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19014,6 +19014,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/nolibc/linux-nolibc.git
 F:	tools/include/nolibc/
 F:	tools/testing/selftests/nolibc/
 
+NOVATEK NT726XX I2C CONTROLLER DRIVER
+M:	Ben Huang <ben_huang@novatek.com.tw>
+L:	linux-i2c@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
+F:	drivers/i2c/busses/i2c-nt726xx.c
+
 NOVATEK NVT-TS I2C TOUCHSCREEN DRIVER
 M:	Hans de Goede <hansg@kernel.org>
 L:	linux-input@vger.kernel.org
-- 
2.40.1

[PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller

[SP_ISW1_AT]

[-- Attachment #1: Type: text/html, Size: 1596 bytes --]

[-- Attachment #2: Type: text/plain, Size: 1756 bytes --]

From: Ben Huang <Ben_Huang@novatek.com.tw>

Add device tree documentation for Novatek NT726xx SoC i2c controller.

Signed-off-by: Ben Huang <Ben_Huang@novatek.com.tw>
---
 .../bindings/i2c/novatek,nt726xx-i2c.yaml     | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
new file mode 100644
index 000000000000..0826ee9ec831
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/novatek,nt726xx-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+maintainers:
+  - Ben Huang <ben_huang@novatek.com.tw>
+  - Jason JJ Wu <jason_jj_wu@novatek.com.tw>
+
+title: Novatek NT726xx SoC I2C master controller
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    const: novatek,nt726xx_i2c
+
+  nvt,hwmods:
+    description: Name of each i2c pin, must be named with "i2cX" (X is
+                 an integer starting from 0)
+    minItems: 1
+
+  bus-enable:
+    description: enable this i2c pin function or not
+    maxItems: 1
+
+  reg:
+    maxItems: 4
+
+  interrupts:
+    maxItems: 3
+
+  clock-frequency:
+    description: Operation clock frequency of i2c in kHz. Default is 100kHz.
+    default: 100
+    maxItems: 1
+
+required:
+  - compatible
+  - bus-enable
+  - nvt,hwmods
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
-- 
2.40.1

[PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs.

[SP_ISW1_AT]

[-- Attachment #1: Type: text/html, Size: 1596 bytes --]

[-- Attachment #2: Type: text/plain, Size: 22224 bytes --]

From: Ben Huang <Ben_Huang@novatek.com.tw>

This patch introduce the support of i2c bus controller of
Novatek NT726xx SoCs.

This driver performs the fundamental read/write functions as
an i2c controller and supports Standard-mode and Fast-mode.
Default operation is Stardard-mode.

Signed-off-by: Ben Huang <Ben_Huang@novatek.com.tw>
---
 drivers/i2c/busses/Kconfig       |  10 +
 drivers/i2c/busses/Makefile      |   1 +
 drivers/i2c/busses/i2c-nt726xx.c | 699 +++++++++++++++++++++++++++++++
 3 files changed, 710 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-nt726xx.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 8c935f867a37..61daeac6b042 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -950,6 +950,16 @@ config I2C_NPCM
 	  controllers.
 	  Driver can also support slave mode (select I2C_SLAVE).
 
+config I2C_NT726XX
+	tristate "Novatek NT726xx Driver"
+	default n
+	help
+	  Say Y here if you want to enable I2C bus controller on
+	  Novatek NT726xx SoCs.
+	  This driver performs fundamental read/write functions
+	  as an I2C bus controller and supports Standard-mode and
+	  Fast-mode. Default operation is Standard-mode.
+
 config I2C_OCORES
 	tristate "OpenCores I2C Controller"
 	help
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 547123ab351f..bfcd29203b35 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_I2C_MV64XXX)	+= i2c-mv64xxx.o
 obj-$(CONFIG_I2C_MXS)		+= i2c-mxs.o
 obj-$(CONFIG_I2C_NOMADIK)	+= i2c-nomadik.o
 obj-$(CONFIG_I2C_NPCM)		+= i2c-npcm7xx.o
+obj-$(CONFIG_I2C_NT726XX)	+= i2c-nt726xx.o
 obj-$(CONFIG_I2C_OCORES)	+= i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP)		+= i2c-omap.o
 obj-$(CONFIG_I2C_OWL)		+= i2c-owl.o
diff --git a/drivers/i2c/busses/i2c-nt726xx.c b/drivers/i2c/busses/i2c-nt726xx.c
new file mode 100644
index 000000000000..b0e79722a85b
--- /dev/null
+++ b/drivers/i2c/busses/i2c-nt726xx.c
@@ -0,0 +1,699 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Novatek Microelectronics Corp.
+ * Author: Ben Huang <ben_huang@novatek.com.tw>
+ */
+
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/version.h>
+
+#define I2C_INFO_LOG(fmt, ...)		\
+	pr_info("[I/SOC_I2C] " fmt, ##__VA_ARGS__)
+
+#define I2C_ERR_LOG(fmt, ...)		\
+	pr_err("[E/SOC_I2C] " fmt, ##__VA_ARGS__)
+
+#define I2C_NAME                              "NT72xxx I2C adapter"
+#define I2C_REG_CTRL                          0xC0
+#define I2C_REG_CLK                           0xC4
+#define I2C_REG_ACK                           0xC8
+#define I2C_REG_SIZE                          0xCC
+#define I2C_REG_FIFO1                         0xD0
+#define I2C_REG_SUBADDR                       0xE0
+#define I2C_REG_PINGPONG                      0xE4
+#define I2C_REG_INTR                          0xE8
+#define I2C_REG_FIFO2                         0xEC
+#define I2C_REG_DUTY                          0xFC
+#define I2C_CLR_FIFO1                         0x80
+#define I2C_CLR_FIFO2                         0x800000
+#define I2C_BUF_LITTLE_ENDIAN                 0x00002000
+#define I2C_BUSY                              0x00000002
+#define I2C_ENABLE                            0x00000004
+#define I2C_REPEAT_ENABLE                     0x00000080
+#define I2C_READ_OPERATION                    0x00000100
+#define I2C_NACK                              0x01000000
+#define I2C_CLOCK_DUTY_ENABLE                 0x00200000
+#define I2C_CLOCK_STRETCH_ENABLE              0x08000000
+#define I2C_MASTER_CLK_STRETCH_ENABLE         0x10000000
+#define I2C_TRIGGER                           0x00000001
+#define I2C_IRQ_FLAG                          0x0000FF00
+#define I2C_IRQ_ARBI_LOSS                     0x00008000
+#define I2C_IRQ_SUS                           0x00004000
+#define I2C_IRQ_ALERT                         0x00002000
+#define I2C_IRQ_CLK_STR_TIMEOUT               0x00001000
+#define I2C_IRQ_NACK                          0x00000800
+#define I2C_IRQ_RX_FULL                       0x00000400
+#define I2C_IRQ_TX_EMPTY                      0x00000200
+#define I2C_IRQ_FINISH                        0x00000100
+#define I2C_IRQ_TX_SETTING                    0x001F001B
+#define I2C_IRQ_RX_SETTING                    0x001F001D
+#define I2C_IRQ_ENABLE_SETTING                0x001F001F
+#define I2C_IRQ_DISABLE_SETTING               0x00000000
+#define I2C_SUBADDR_ENABLE                    0x00000040
+#define I2C_16BITSUBADDR_ENABLE               0x00010000
+#define I2C_24BITSUBADDR_ENABLE               0x00020000
+#define I2C_32BITSUBADDR_ENABLE               0x00040000
+#define I2C_TX_EMPTY_FIFO1                    0x40
+#define I2C_TX_EMPTY_FIFO2                    0x400000
+#define I2C_RX_FULL_FIFO1                     0x20
+#define I2C_RX_FULL_FIFO2                     0x200000
+#define FIFO_1                                0
+#define FIFO_2                                1
+#define FIFO_ALL                              2
+#define FIFO_CHUNK_SIZE                       16
+#define FIFO_WORD_BYTES                       4
+#define STBC_MASTER                           0xFC040000
+#define STBC_PASSWORD                         0xFC040204
+#define STBC_PASSWORD_DATA1                   0x72682
+#define STBC_PASSWORD_DATA2                   0x78627
+#define STBC_KEYPASS                          0xFC040208
+#define STBC_I2C_SWITCH                       0xFC040220
+
+enum {
+	SUBADDR_DISABLE,
+	SUBADDR_8BITS,
+	SUBADDR_16BITS,
+	SUBADDR_24BITS,
+	SUBADDR_32BITS,
+	SUBADDR_40BITS,
+	SUBADDR_48BITS,
+	SUBADDR_56BITS,
+	SUBADDR_64BITS
+};
+
+struct nvt_i2c_compatible_data {
+	unsigned int sys_clock; // Unit: Hz
+	unsigned int stbc_clock; // Unit: Hz
+};
+
+struct nvt_i2c_bus {
+	void __iomem *base;
+	struct i2c_adapter adapter;
+	struct device *dev;
+	struct completion msg_complete;
+	struct i2c_msg *msg;
+	unsigned int bus_enable;
+	unsigned int bus_clk_rate;
+	unsigned int stbc_i2c;
+	const struct nvt_i2c_compatible_data *comp_data;
+	int irq;
+	/* used for xfer */
+	struct i2c_msg *current_msg;
+	int remaining;
+	int write_ptr;
+	int read_ptr;
+	int fifo_idx;
+	int error_code;
+};
+
+static void nvt_i2c_use_case_feature(struct nvt_i2c_bus *i2c)
+{
+	void __iomem *reg_tmp;
+
+	if (i2c->stbc_i2c) {
+		reg_tmp = ioremap(STBC_PASSWORD, 4);
+		writel(readl(reg_tmp) | STBC_PASSWORD_DATA1, reg_tmp);
+		writel(readl(reg_tmp) | STBC_PASSWORD_DATA2, reg_tmp);
+
+		reg_tmp = ioremap(STBC_KEYPASS, 4);
+		writel(readl(reg_tmp) | 0x1, reg_tmp);
+		iounmap(reg_tmp);
+	}
+}
+
+static void nvt_i2c_reset(struct nvt_i2c_bus *i2c)
+{
+	writel(readl(i2c->base + I2C_REG_CTRL) & ~I2C_ENABLE,
+	       i2c->base + I2C_REG_CTRL);
+	writel(readl(i2c->base + I2C_REG_CTRL) | I2C_ENABLE,
+	       i2c->base + I2C_REG_CTRL);
+}
+
+static void nvt_i2c_set_clk(struct nvt_i2c_bus *i2c)
+{
+	unsigned int duty = 0;
+	unsigned int clk_div = 0;
+	unsigned int source_speed = i2c->stbc_i2c ?
+		i2c->comp_data->stbc_clock : i2c->comp_data->sys_clock;
+
+	clk_div = (source_speed / i2c->bus_clk_rate) / 1000;
+	writel(clk_div << 1, i2c->base + I2C_REG_CLK);
+
+	duty = (clk_div * 9 + 10) / 20;
+	writel(duty << 16, i2c->base + I2C_REG_DUTY);
+
+	writel(0x1e0, i2c->base + I2C_REG_ACK);
+}
+
+static void nvt_i2c_set_subaddr(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
+{
+	unsigned int reg_ctrl;
+	unsigned int subaddr = 0;
+	int i = 0;
+
+	reg_ctrl = readl(i2c->base + I2C_REG_CTRL);
+	reg_ctrl &= ~(I2C_16BITSUBADDR_ENABLE |
+		      I2C_24BITSUBADDR_ENABLE |
+		      I2C_32BITSUBADDR_ENABLE);
+
+	if (msg && msg->len > 0 && msg->len <= 4 && msg->buf) {
+		reg_ctrl |= I2C_SUBADDR_ENABLE;
+
+		switch (msg->len) {
+		case SUBADDR_8BITS:
+			break;
+		case SUBADDR_16BITS:
+			reg_ctrl |= I2C_16BITSUBADDR_ENABLE;
+			break;
+		case SUBADDR_24BITS:
+			reg_ctrl |= I2C_24BITSUBADDR_ENABLE;
+			break;
+		case SUBADDR_32BITS:
+			reg_ctrl |= I2C_32BITSUBADDR_ENABLE;
+			break;
+		default:
+			return;
+		}
+
+		for (i = 0; i < msg->len; i++)
+			subaddr |= msg->buf[i] << (8 * (msg->len - 1 - i));
+		writel(subaddr, i2c->base + I2C_REG_SUBADDR);
+	} else {
+		reg_ctrl &= ~I2C_SUBADDR_ENABLE;
+	}
+
+	writel(reg_ctrl, i2c->base + I2C_REG_CTRL);
+}
+
+static void nvt_i2c_init(struct nvt_i2c_bus *i2c)
+{
+	nvt_i2c_use_case_feature(i2c);
+	nvt_i2c_set_clk(i2c);
+	writel(I2C_BUF_LITTLE_ENDIAN, i2c->base + I2C_REG_PINGPONG);
+	writel(I2C_IRQ_ENABLE_SETTING, i2c->base + I2C_REG_INTR);
+}
+
+static int nvt_i2c_suspend(struct device *dev)
+{
+	struct nvt_i2c_bus *i2c = dev_get_drvdata(dev);
+
+	if (i2c) {
+		i2c_mark_adapter_suspended(&i2c->adapter);
+		i2c->current_msg = NULL;
+		writel(I2C_IRQ_DISABLE_SETTING, i2c->base + I2C_REG_INTR);
+	}
+
+	return 0;
+}
+
+static int nvt_i2c_resume(struct device *dev)
+{
+	struct nvt_i2c_bus *i2c = dev_get_drvdata(dev);
+
+	if (i2c) {
+		nvt_i2c_init(i2c);
+		i2c_mark_adapter_resumed(&i2c->adapter);
+	}
+
+	return 0;
+}
+
+static void nvt_i2c_clear_fifo(struct nvt_i2c_bus *i2c, unsigned int which)
+{
+	unsigned int regval = readl(i2c->base + I2C_REG_PINGPONG);
+
+	switch (which) {
+	case FIFO_1:
+		regval |= I2C_CLR_FIFO1;
+		break;
+	case FIFO_2:
+		regval |= I2C_CLR_FIFO2;
+		break;
+	case FIFO_ALL:
+		regval |= I2C_CLR_FIFO1 | I2C_CLR_FIFO2;
+		break;
+	default:
+		break;
+	}
+	writel(regval, i2c->base + I2C_REG_PINGPONG);
+}
+
+static void nvt_i2c_write_fifo(struct nvt_i2c_bus *i2c,
+			       unsigned int fifo_reg,
+			       const unsigned char *buf,
+			       unsigned int buf_offset,
+			       unsigned int length)
+{
+	unsigned int reg_idx = 0, copy_bytes = 0, j = 0, value = 0;
+
+	while (length > 0) {
+		value = 0;
+		copy_bytes = length >= FIFO_WORD_BYTES ? FIFO_WORD_BYTES : length;
+		for (j = 0; j < copy_bytes; j++)
+			value |= ((unsigned int)buf[buf_offset + j]) << (j * 8);
+
+		writel(value, i2c->base + fifo_reg + reg_idx * 4);
+		buf_offset += copy_bytes;
+		length -= copy_bytes;
+		reg_idx++;
+	}
+}
+
+static void nvt_i2c_read_fifo(struct nvt_i2c_bus *i2c,
+			      unsigned int fifo_reg,
+			      unsigned char *buf,
+			      unsigned int buf_offset,
+			      unsigned int length)
+{
+	unsigned int reg_idx = 0, copy_bytes = 0, j = 0, value = 0;
+
+	while (length > 0) {
+		value = readl(i2c->base + fifo_reg + reg_idx * 4);
+		copy_bytes = length >= FIFO_WORD_BYTES ? FIFO_WORD_BYTES : length;
+		for (j = 0; j < copy_bytes; j++)
+			buf[buf_offset + j] = (unsigned char)(value >> (j * 8));
+
+		buf_offset += copy_bytes;
+		length -= copy_bytes;
+		reg_idx++;
+	}
+}
+
+static void nvt_i2c_handle(struct nvt_i2c_bus *i2c, struct i2c_msg *msg, int is_read)
+{
+	unsigned int bytes, fiforeg;
+
+	if (!i2c || !msg || !msg->buf || msg->len == 0 || msg->len > 4096) {
+		I2C_ERR_LOG("I2C invalid msg: i2c=%p, msg=%p, buf=%p, len=%d\n",
+			    i2c, msg, msg ? msg->buf : NULL, msg ? msg->len : 0);
+		if (i2c) {
+			I2C_ERR_LOG("[%s.%d]: i2c_handle\n", dev_name(i2c->dev), i2c->adapter.nr);
+			i2c->error_code = -EINVAL;
+		}
+
+		return;
+	}
+
+	bytes = i2c->remaining > FIFO_CHUNK_SIZE ? FIFO_CHUNK_SIZE : i2c->remaining;
+	fiforeg = i2c->fifo_idx == 0 ? I2C_REG_FIFO1 : I2C_REG_FIFO2;
+
+	if (is_read) {
+		nvt_i2c_read_fifo(i2c, fiforeg, msg->buf, i2c->read_ptr, bytes);
+		i2c->read_ptr += bytes;
+	} else {
+		nvt_i2c_write_fifo(i2c, fiforeg, msg->buf, i2c->write_ptr, bytes);
+		i2c->write_ptr += bytes;
+	}
+	nvt_i2c_clear_fifo(i2c, i2c->fifo_idx);
+	i2c->remaining -= bytes;
+	i2c->fifo_idx ^= 1;
+}
+
+static irqreturn_t nvt_i2c_isr(int irq, void *dev_id)
+{
+	struct nvt_i2c_bus *i2c = dev_id;
+	struct i2c_msg *msg = i2c->current_msg;
+	unsigned int status = readl(i2c->base + I2C_REG_INTR);
+	unsigned int clr = 0;
+	int do_complete = 0;
+
+	if (!(status & I2C_IRQ_FLAG) || !i2c->current_msg)
+		return IRQ_NONE;
+
+	if (status & I2C_IRQ_NACK) {
+		i2c->error_code = -ENXIO;
+		clr |= I2C_IRQ_NACK << 8;
+	} else if (status & I2C_IRQ_RX_FULL) {
+		if (i2c->remaining > 0)
+			nvt_i2c_handle(i2c, msg, 1);
+		clr |= I2C_IRQ_RX_FULL << 8;
+	} else if (status & I2C_IRQ_TX_EMPTY) {
+		if (i2c->remaining > 0)
+			nvt_i2c_handle(i2c, msg, 0);
+		clr |= I2C_IRQ_TX_EMPTY << 8;
+	} else if (status & I2C_IRQ_FINISH) {
+		if (i2c->remaining > 0 && (msg->flags & I2C_M_RD))
+			nvt_i2c_handle(i2c, msg, 1);
+		clr |= I2C_IRQ_FINISH << 8;
+		do_complete = 1;
+	}
+	if (i2c->error_code)
+		do_complete = 1;
+
+	writel(status | clr, i2c->base + I2C_REG_INTR);
+	if (do_complete)
+		complete(&i2c->msg_complete);
+
+	return IRQ_HANDLED;
+}
+
+static void nvt_i2c_ctrl_init(struct nvt_i2c_bus *i2c)
+{
+	int i = 0;
+
+	writel(0, i2c->base + I2C_REG_CTRL);
+	for (i = 0; i < 4; i++) {
+		writel(0, i2c->base + I2C_REG_FIFO1 + i * 4);
+		writel(0, i2c->base + I2C_REG_FIFO2 + i * 4);
+	}
+	nvt_i2c_clear_fifo(i2c, FIFO_ALL);
+	reinit_completion(&i2c->msg_complete);
+}
+
+static int nvt_i2c_check_msg(const struct i2c_msg *msg)
+{
+	if (!msg || !msg->buf || msg->len == 0 || msg->len > 4096)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void nvt_i2c_prepare_xfer(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
+{
+	i2c->remaining   = msg->len;
+	i2c->current_msg = msg;
+	i2c->write_ptr   = 0;
+	i2c->read_ptr    = 0;
+	i2c->error_code  = 0;
+	i2c->fifo_idx    = 0;
+}
+
+static int nvt_i2c_write(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
+{
+	const unsigned char *buf = msg->buf;
+	int ret, offset = 0, write_bytes, fifo_num;
+	unsigned int ctrl_mask;
+
+	ret = nvt_i2c_check_msg(msg);
+	if (ret)
+		return ret;
+
+	nvt_i2c_prepare_xfer(i2c, msg);
+
+	writel((msg->len * 8) << 8, i2c->base + I2C_REG_SIZE);
+
+	/*  Write FIFO data first */
+	for (fifo_num = 0; fifo_num < FIFO_ALL && offset < msg->len; fifo_num++) {
+		write_bytes = msg->len - offset > FIFO_CHUNK_SIZE ?
+				FIFO_CHUNK_SIZE : msg->len - offset;
+		nvt_i2c_write_fifo(i2c, fifo_num == 0 ? I2C_REG_FIFO1 : I2C_REG_FIFO2,
+				   buf, offset, write_bytes);
+		offset += write_bytes;
+	}
+	i2c->write_ptr = offset;
+	i2c->remaining = msg->len - offset;
+	i2c->fifo_idx = 0;
+
+	ctrl_mask = readl(i2c->base + I2C_REG_CTRL);
+	ctrl_mask |= (((msg->addr << 1) << 8) | I2C_ENABLE |
+			I2C_CLOCK_DUTY_ENABLE | I2C_CLOCK_STRETCH_ENABLE |
+			I2C_MASTER_CLK_STRETCH_ENABLE | I2C_TRIGGER);
+	writel(ctrl_mask, i2c->base + I2C_REG_CTRL);
+
+	ret = wait_for_completion_timeout(&i2c->msg_complete, i2c->adapter.timeout);
+	if (ret == 0) {
+		i2c->error_code = -ETIMEDOUT;
+		nvt_i2c_reset(i2c);
+	}
+	if (i2c->error_code)
+		I2C_ERR_LOG("[%s.%d]: write failed (err:%d);"
+			    " SA[0x%X]\n",
+			    dev_name(i2c->dev), i2c->adapter.nr, i2c->error_code,
+			    msg->addr);
+
+	i2c->current_msg = NULL;
+
+	return i2c->error_code;
+}
+
+static int nvt_i2c_read(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
+{
+	unsigned int ctrl_mask;
+	int ret;
+
+	ret = nvt_i2c_check_msg(msg);
+	if (ret)
+		return ret;
+
+	nvt_i2c_prepare_xfer(i2c, msg);
+
+	writel((msg->len * 8) << 8, i2c->base + I2C_REG_SIZE);
+
+	ctrl_mask = readl(i2c->base + I2C_REG_CTRL);
+	ctrl_mask |= (((msg->addr << 1) << 8) | I2C_ENABLE |
+			I2C_REPEAT_ENABLE | I2C_READ_OPERATION |
+			I2C_CLOCK_DUTY_ENABLE | I2C_CLOCK_STRETCH_ENABLE |
+			I2C_MASTER_CLK_STRETCH_ENABLE | I2C_TRIGGER);
+	writel(ctrl_mask, i2c->base + I2C_REG_CTRL);
+
+	ret = wait_for_completion_timeout(&i2c->msg_complete, i2c->adapter.timeout);
+	if (ret == 0) {
+		i2c->error_code = -ETIMEDOUT;
+		nvt_i2c_reset(i2c);
+	}
+	if (i2c->error_code)
+		I2C_ERR_LOG("[%s.%d]: read failed (err:%d);"
+			    " SA[0x%X]\n",
+			    dev_name(i2c->dev), i2c->adapter.nr, i2c->error_code,
+			    msg->addr);
+
+	i2c->current_msg = NULL;
+
+	return i2c->error_code;
+}
+
+static int nvt_i2c_xfer(struct i2c_adapter *adap,
+			struct i2c_msg msgs[],
+			int num)
+{
+	struct nvt_i2c_bus *i2c = i2c_get_adapdata(adap);
+	int ret = 0, i = 0;
+	struct i2c_msg *msg = NULL;
+
+	nvt_i2c_ctrl_init(i2c);
+
+	if (num == 2) {
+		nvt_i2c_set_subaddr(i2c, &msgs[0]);
+		msg = &msgs[1];
+
+		if (msg->flags & I2C_M_RD)
+			ret = nvt_i2c_read(i2c, msg);
+		else
+			ret = nvt_i2c_write(i2c, msg);
+	} else {
+		nvt_i2c_set_subaddr(i2c, NULL);
+
+		for (i = 0; i < num; i++) {
+			msg = &msgs[i];
+			if (msg->flags & I2C_M_RD)
+				ret = nvt_i2c_read(i2c, msg);
+			else
+				ret = nvt_i2c_write(i2c, msg);
+			if (ret < 0)
+				break;
+		}
+	}
+
+	if (ret < 0)
+		return ret;
+	return num;
+}
+
+static u32 nvt_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static int nvt_i2c_get_hwmods(const char *hwmods_name)
+{
+	long val;
+
+	if (!hwmods_name || strncmp(hwmods_name, "i2c", 3) != 0)
+		return -EINVAL;
+
+	if (kstrtol(hwmods_name + 3, 10, &val) == 0 && val >= 0)
+		return (int)val;
+
+	return -ENODEV;
+}
+
+static struct i2c_algorithm nvt_i2c_algo = {
+	.master_xfer = nvt_i2c_xfer,
+	.functionality = nvt_i2c_func,
+};
+
+static int nvt_i2c_parse_dts(struct nvt_i2c_bus *i2c)
+{
+	int ret;
+	struct device *dev = i2c->dev;
+	struct device_node *np = dev->of_node;
+	const char *hwmods_val = NULL;
+
+	/* read DTS(nvt,hwmods) and set bus number */
+	ret = of_property_read_string(np, "nvt,hwmods", &hwmods_val);
+	if (ret == 0) {
+		i2c->adapter.nr = nvt_i2c_get_hwmods(hwmods_val);
+		if (i2c->adapter.nr >= 0) {
+			I2C_INFO_LOG("Get nvt,hwmods:i2c%d\n", i2c->adapter.nr);
+		} else {
+			I2C_ERR_LOG("Invalid nvt,hwmods value = %d\n", i2c->adapter.nr);
+			return -EINVAL;
+		}
+	} else {
+		I2C_ERR_LOG("Can't get nvt,hwmods\n");
+		return ret;
+	}
+
+	/* read DTS(bus-enable) */
+	ret = of_property_read_u32(np, "bus-enable", &i2c->bus_enable);
+	if (ret != 0)
+		I2C_INFO_LOG("Not set dtb bus-enable, set default disable\n");
+	else if (!i2c->bus_enable)
+		I2C_INFO_LOG("%s.%d is disabled, skipping initialization.\n",
+			     dev_name(i2c->dev), i2c->adapter.nr);
+	if (!i2c->bus_enable)
+		return 0;
+
+	i2c->comp_data = of_device_get_match_data(dev);
+
+	/* read DTS(stbc) */
+	ret = of_property_read_u32(np, "stbc", &i2c->stbc_i2c);
+	if (ret) {
+		I2C_INFO_LOG("Not set dtb stbc, set default false\n");
+		i2c->stbc_i2c = 0;
+	}
+
+	/* read DTS(clock-frequency) */
+	ret = of_property_read_u32(np, "clock-frequency", &i2c->bus_clk_rate);
+	if (ret) {
+		I2C_INFO_LOG("Not set dtb clock-frequency, set default 100kHz\n");
+		i2c->bus_clk_rate = 100;
+	}
+
+	return 0;
+}
+
+static const struct nvt_i2c_compatible_data nt726xx_data = {
+	.sys_clock = 96000000,
+	.stbc_clock = 12000000,
+};
+
+static const struct of_device_id nvt_i2c_of_match[] = {
+	{ .compatible = "novatek,nt726xx_i2c", .data = &nt726xx_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, nvt_i2c_of_match);
+
+static int nvt_i2c_probe(struct platform_device *pdev)
+{
+	struct nvt_i2c_bus *i2c;
+	struct resource *res;
+	int ret, irq;
+
+	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i2c->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2c->base)) {
+		I2C_ERR_LOG("failed to map controller\n");
+		return PTR_ERR(i2c->base);
+	}
+
+	init_completion(&i2c->msg_complete);
+	i2c->dev = &pdev->dev;
+
+	ret = nvt_i2c_parse_dts(i2c);
+	if (ret || !i2c->bus_enable)
+		return ret;
+
+	nvt_i2c_init(i2c);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		I2C_ERR_LOG("No IRQ resource\n");
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, nvt_i2c_isr,
+			       IRQF_SHARED | IRQF_TRIGGER_HIGH, I2C_NAME, i2c);
+	if (ret) {
+		I2C_ERR_LOG("devm_request_irq fail\n");
+		return ret;
+	}
+
+	/* Setup I2C adapter */
+	i2c->adapter.owner = THIS_MODULE;
+	i2c->adapter.class = I2C_CLASS_HWMON;
+	i2c->adapter.algo = &nvt_i2c_algo;
+	i2c->adapter.dev.of_node = of_node_get(pdev->dev.of_node);
+	i2c->adapter.dev.parent = &pdev->dev;
+	i2c->adapter.timeout = 3 * HZ;
+	strscpy(i2c->adapter.name, I2C_NAME, sizeof(i2c->adapter.name));
+	i2c_set_adapdata(&i2c->adapter, i2c);
+
+	ret = i2c_add_numbered_adapter(&i2c->adapter);
+	if (ret) {
+		I2C_ERR_LOG("[%s] failed to add adapter\n", dev_name(&pdev->dev));
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, i2c);
+
+	return 0;
+}
+
+static void nvt_i2c_remove(struct platform_device *pdev)
+{
+	struct nvt_i2c_bus *i2c = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c->adapter);
+}
+
+static const struct dev_pm_ops nvt_i2c_pm_ops = {
+	.resume_early = nvt_i2c_resume,
+	.suspend_late = nvt_i2c_suspend,
+};
+
+static struct platform_driver nvt_i2c_driver = {
+	.probe = nvt_i2c_probe,
+	.remove = nvt_i2c_remove,
+	.driver = {
+		.name = "nvt_i2c",
+		.owner = THIS_MODULE,
+		.pm = &nvt_i2c_pm_ops,
+		.of_match_table = of_match_ptr(nvt_i2c_of_match),
+	},
+};
+
+static int __init nvt_i2c_platform_init(void)
+{
+	return platform_driver_register(&nvt_i2c_driver);
+}
+postcore_initcall(nvt_i2c_platform_init);
+
+static void __exit nvt_i2c_platform_exit(void)
+{
+	platform_driver_unregister(&nvt_i2c_driver);
+}
+module_exit(nvt_i2c_platform_exit);
+
+MODULE_DESCRIPTION("Novatek NT72xxx SoC I2C Bus Driver");
+MODULE_AUTHOR("Ben Huang <ben_huang@novatek.com.tw>");
+MODULE_LICENSE("GPL");
-- 
2.40.1

Re: [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 16 potential issue(s) to consider:
- [Critical] Timeout handling in I2C read/write lacks synchronization, leaving a dangling pointer accessible by the interrupt handler.
- [Critical] Unbinding the driver causes a NULL pointer dereference if `bus-enable` is false or missing.
- [Critical] The `clock-frequency` DT property expects kHz and causes a divide-by-zero panic if correctly configured.
- [Critical] Control register fields are bitwise ORed without clearing previous state, allowing I2C target isolation bypass.
- [High] The `master_xfer` implementation unilaterally drops or misroutes the first message of any 2-message transfer.
- [High] Virtual memory mapping leaked during initialization due to missing `iounmap()`.
- [High] The driver advertises SMBus Quick support but explicitly rejects 0-length messages.
- [High] The ISR returns `IRQ_NONE` without clearing device interrupts if no transfer is active, causing an unhandled interrupt storm.
- [High] Device interrupts are not disabled during driver removal, risking a shared IRQ storm.
- [High] The driver interacts with hardware registers without ensuring clocks and power domains are enabled.
- [Medium] The driver hardcodes physical memory addresses to access system-level control registers.
- [Medium] The driver parses custom, non-standard Device Tree properties to handle core subsystem state.
- [Medium] Unconditional assignment of PM callbacks causes dead code when PM features are disabled.
- [Medium] The commit message contains a corporate confidentiality notice.
- [Medium] Device tree node reference acquired via `of_node_get()` is never released.
- [Medium] The driver uses `writel()`/`readl()` with manual byte-packing for FIFO access instead of stream accessors.
--

commit 57029ad0ef0636d7917520387b5868dcfc696f11
Author: SP_ISW1_AT@novatek.com.tw <SP_ISW1_AT@novatek.com.tw>
Subject: i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs.

This patch introduces the i2c bus controller driver for Novatek NT726xx SoCs,
supporting basic read/write functions in Standard and Fast modes.

> CONFIDENTIALITY NOTICE: This message, including attachments, contains information which may be confidential and privileged, and is intended only for use by the addressees designated above. Unless you are the intended recipient, any use, copying, disclosure, or distribution is prohibited. If you have received the message in error, please immediately delete the message and destroy all copies thereof and notify the sender by reply email. 本郵件及任何附件均屬機密，僅供其上指定地址之收件人使用。除非您是指定之收件人，否則請勿使用、複製、揭露或散布本郵件之任何部份。若您錯誤地收到此郵件，請立即回覆電子郵件通知寄件人，並請完全刪除且銷毀本郵件及其複本。

This isn't a regression, but the commit message contains a corporate
confidentiality notice.

> diff --git a/drivers/i2c/busses/i2c-nt726xx.c b/drivers/i2c/busses/i2c-nt726xx.c
> new file mode 100644
> index 0000000000000..b0e79722a85b0
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-nt726xx.c
[ ... ]
> +static void nvt_i2c_use_case_feature(struct nvt_i2c_bus *i2c)
> +{
> +	void __iomem *reg_tmp;
> +
> +	if (i2c->stbc_i2c) {
> +		reg_tmp = ioremap(STBC_PASSWORD, 4);

[Severity: Medium]
Is it safe to hardcode physical memory addresses like STBC_PASSWORD to access
system-level control registers, rather than using standard Device Tree
bindings or resource management?

> +		writel(readl(reg_tmp) | STBC_PASSWORD_DATA1, reg_tmp);
> +		writel(readl(reg_tmp) | STBC_PASSWORD_DATA2, reg_tmp);
> +
> +		reg_tmp = ioremap(STBC_KEYPASS, 4);

[Severity: High]
Does this code leak the first virtual memory mapping? The pointer assigned
to reg_tmp from ioremap is overwritten here without a corresponding iounmap(),
creating a persistent leak.

[ ... ]
> +static void nvt_i2c_set_clk(struct nvt_i2c_bus *i2c)
> +{
> +	unsigned int duty = 0;
> +	unsigned int clk_div = 0;
> +	unsigned int source_speed = i2c->stbc_i2c ?
> +		i2c->comp_data->stbc_clock : i2c->comp_data->sys_clock;
> +
> +	clk_div = (source_speed / i2c->bus_clk_rate) / 1000;

[Severity: Critical]
Does this assume the clock-frequency property is in kHz?

The device tree standard mandates it is specified in Hertz. If a standard
value like 400000 is provided, could this integer division result in 0,
locking up the hardware or causing a divide-by-zero kernel panic?

[ ... ]
> +static void nvt_i2c_set_subaddr(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
> +{
[ ... ]
> +		for (i = 0; i < msg->len; i++)
> +			subaddr |= msg->buf[i] << (8 * (msg->len - 1 - i));

[Severity: High]
Could this read from uninitialized memory if the first message in a
transfer is a read command?

[ ... ]
> +static void nvt_i2c_write_fifo(struct nvt_i2c_bus *i2c,
> +			       unsigned int fifo_reg,
> +			       const unsigned char *buf,
> +			       unsigned int buf_offset,
> +			       unsigned int length)
> +{
> +	unsigned int reg_idx = 0, copy_bytes = 0, j = 0, value = 0;
> +
> +	while (length > 0) {
> +		value = 0;
> +		copy_bytes = length >= FIFO_WORD_BYTES ? FIFO_WORD_BYTES : length;
> +		for (j = 0; j < copy_bytes; j++)
> +			value |= ((unsigned int)buf[buf_offset + j]) << (j * 8);
> +
> +		writel(value, i2c->base + fifo_reg + reg_idx * 4);

[Severity: Medium]
Would it be better to use stream accessors like writesl() instead of manually
byte-packing into a writel() call? The standard register accessors perform
implicit byte-swapping which may be unintended for FIFO data streams.

[ ... ]
> +static irqreturn_t nvt_i2c_isr(int irq, void *dev_id)
> +{
> +	struct nvt_i2c_bus *i2c = dev_id;
> +	struct i2c_msg *msg = i2c->current_msg;
> +	unsigned int status = readl(i2c->base + I2C_REG_INTR);
> +	unsigned int clr = 0;
> +	int do_complete = 0;
> +
> +	if (!(status & I2C_IRQ_FLAG) || !i2c->current_msg)
> +		return IRQ_NONE;

[Severity: High]
If an interrupt is active but i2c->current_msg is NULL, could returning
IRQ_NONE without clearing the hardware interrupt flag cause an unhandled
interrupt storm on a shared, level-triggered line?

[ ... ]
> +	} else if (status & I2C_IRQ_TX_EMPTY) {
> +		if (i2c->remaining > 0)
> +			nvt_i2c_handle(i2c, msg, 0);

[Severity: Critical]
Can this result in a use-after-free regression if the wait_for_completion
expires?

If wait_for_completion_timeout() times out in nvt_i2c_write(), it sets
i2c->current_msg to NULL and returns, leaving the stack-allocated msg
to be freed. If an interrupt fires at that exact moment, the asynchronous
ISR might have already copied i2c->current_msg to the local msg variable
before it was cleared, leading to nvt_i2c_handle() accessing freed
stack memory.

[ ... ]
> +static int nvt_i2c_check_msg(const struct i2c_msg *msg)
> +{
> +	if (!msg || !msg->buf || msg->len == 0 || msg->len > 4096)
> +		return -EINVAL;

[Severity: High]
Will this reject 0-length messages explicitly? The adapter advertises
I2C_FUNC_SMBUS_EMUL later on, which implies support for SMBus Quick
commands that rely on 0-length transactions.

[ ... ]
> +static int nvt_i2c_write(struct nvt_i2c_bus *i2c, struct i2c_msg *msg)
> +{
[ ... ]
> +	ctrl_mask = readl(i2c->base + I2C_REG_CTRL);
> +	ctrl_mask |= (((msg->addr << 1) << 8) | I2C_ENABLE |
> +			I2C_CLOCK_DUTY_ENABLE | I2C_CLOCK_STRETCH_ENABLE |
> +			I2C_MASTER_CLK_STRETCH_ENABLE | I2C_TRIGGER);
> +	writel(ctrl_mask, i2c->base + I2C_REG_CTRL);

[Severity: Critical]
Does this bitwise-OR the target address without first clearing the existing
address bits in the control mask?

During a sequence of multiple messages in a single transfer, the previous
address might not be cleared, causing subsequent messages to transmit a
corrupted, logically-combined address on the bus. This could potentially
bypass I2C target isolation checks.

[ ... ]
> +static int nvt_i2c_xfer(struct i2c_adapter *adap,
> +			struct i2c_msg msgs[],
> +			int num)
> +{
> +	struct nvt_i2c_bus *i2c = i2c_get_adapdata(adap);
> +	int ret = 0, i = 0;
> +	struct i2c_msg *msg = NULL;
> +
> +	nvt_i2c_ctrl_init(i2c);
> +
> +	if (num == 2) {
> +		nvt_i2c_set_subaddr(i2c, &msgs[0]);
> +		msg = &msgs[1];

[Severity: High]
Does this unilaterally drop the first message of any 2-message transfer and
treat it as a hardware subaddress? This could break standard 2-message I2C
sequences, such as interacting with two separate targets or consecutive writes.

[ ... ]
> +static int nvt_i2c_parse_dts(struct nvt_i2c_bus *i2c)
> +{
[ ... ]
> +	/* read DTS(nvt,hwmods) and set bus number */
> +	ret = of_property_read_string(np, "nvt,hwmods", &hwmods_val);
[ ... ]
> +	/* read DTS(bus-enable) */
> +	ret = of_property_read_u32(np, "bus-enable", &i2c->bus_enable);

[Severity: Medium]
Is there a reason to parse custom properties like nvt,hwmods and bus-enable
instead of relying on standard I2C core bindings like aliases and the
standard status = "disabled" flag?

[ ... ]
> +static int nvt_i2c_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	ret = nvt_i2c_parse_dts(i2c);
> +	if (ret || !i2c->bus_enable)
> +		return ret;

[Severity: Critical]
If bus-enable is false, this returns 0 (success) before calling
platform_set_drvdata(). Could this cause a NULL pointer dereference
later during unbind when nvt_i2c_remove() fetches the empty drvdata?

> +
> +	nvt_i2c_init(i2c);

[Severity: High]
Does nvt_i2c_init() access hardware registers before ensuring the system bus
clocks and I2C power domain are enabled? Accessing a clock-gated or
powered-down peripheral can trigger an imprecise external abort.

[ ... ]
> +	i2c->adapter.dev.of_node = of_node_get(pdev->dev.of_node);

[Severity: Medium]
Is this of_node reference ever released? There doesn't appear to be a
matching of_node_put() in the teardown paths, which would cause a memory
leak across bind/unbind cycles.

[ ... ]
> +static void nvt_i2c_remove(struct platform_device *pdev)
> +{
> +	struct nvt_i2c_bus *i2c = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&i2c->adapter);
> +}

[Severity: High]
Should device hardware interrupts be disabled before unregistering the
adapter and removing the driver? Since a shared IRQ line is requested,
an interrupt fired during or after teardown could cause an unhandled
interrupt storm.

> +
> +static const struct dev_pm_ops nvt_i2c_pm_ops = {
> +	.resume_early = nvt_i2c_resume,
> +	.suspend_late = nvt_i2c_suspend,
> +};

[Severity: Medium]
Would it be better to wrap these callback assignments with pm_sleep_ptr()
to avoid generating dead code when PM features are disabled?

> +
> +static struct platform_driver nvt_i2c_driver = {
> +	.probe = nvt_i2c_probe,
> +	.remove = nvt_i2c_remove,
> +	.driver = {
> +		.name = "nvt_i2c",
> +		.owner = THIS_MODULE,
> +		.pm = &nvt_i2c_pm_ops,

[Severity: Medium]
Similarly, should the pm structure pointer be wrapped with pm_ptr()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604060430.355733-1-SP_ISW1_AT@novatek.com.tw?part=3

Re: [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs.

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

Hi,

thank you for your contribution! Sadly, I have to mention a few formal
issues first.

>  NOTICE: This message, including attachments, contains information which may
>  be confidential and privileged, and is intended only for use by the
>  addressees designated above. Unless you are the intended recipient, any
>  use, copying, disclosure, or distribution is prohibited. If you have
>  received the message in error, please immediately delete the message and
>  destroy all copies thereof and notify the sender by reply
>  email.</span><span

a) Please, no HTML messages.
b) we can't apply patches with a header like this because we distribute
the code to the whole world.
c) the sender e-mail address doesn't match the author or signed-off
e-mail address. Who is the sender?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] The commit message includes a restrictive corporate confidentiality boilerplate.
- [High] Redefinition of the standard `clock-frequency` property units to kHz instead of Hz.
- [High] Custom properties `nvt,hwmods` and `bus-enable` improperly reinvent standard Devicetree bindings and framework mechanisms.
- [Medium] Missing explicit type definitions for the custom properties `nvt,hwmods` and `bus-enable`.
- [Low] The custom property `bus-enable` is missing a vendor prefix.
- [Low] The compatible string uses an underscore instead of a hyphen.
--

commit b57b0e4f8d5415ed28a61406a5c742ff25ba61db
Author: SP_ISW1_AT@novatek.com.tw <SP_ISW1_AT@novatek.com.tw>

dt-bindings: i2c: add Novatek NT726xx SoC i2c controller

This commit adds device tree binding documentation for the Novatek NT726xx
SoC I2C master controller.

>     CONFIDENTIALITY NOTICE: This message, including attachments, contains information which may be confidential and privileged, and is intended only for use by the addressees designated above. Unless you are the intended recipient, any use, copying, disclosure, or distribution is prohibited. If you have received the message in error, please immediately delete the message and destroy all copies thereof and notify the sender by reply email. 本郵件及任何附件均屬機密，僅供其上指定地址之收件人使用。除非您是指定之收件人，否則請勿使用、複製、揭露或散布本郵件之任何部份。若您錯誤地收到此郵件，請立即回覆電子郵件通知寄件人，並請完全刪除且銷毀本郵件及其複本。

[Severity: High]
Is it possible to remove this confidentiality boilerplate from the commit
message?

Such legal disclaimers conflict with the open-source nature of the Linux
kernel, the GPL-2.0 license, and the Developer's Certificate of Origin.

> diff --git a/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> new file mode 100644
> index 0000000000000..0826ee9ec8315
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml

[ ... ]

> +properties:
> +  compatible:
> +    const: novatek,nt726xx_i2c

[Severity: Low]
Does this compatible string follow the standard naming conventions?

Device tree guidelines typically suggest using hyphens rather than underscores
in compatible strings, which would make this novatek,nt726xx-i2c instead.

> +  nvt,hwmods:
> +    description: Name of each i2c pin, must be named with "i2cX" (X is
> +                 an integer starting from 0)
> +    minItems: 1

[Severity: High]
Is this custom property intended to encode the I2C adapter number?

Standard practice is to rely on the standard aliases node for this purpose,
like aliases { i2c0 = &i2c0; };, rather than bypassing it with a custom string.

[Severity: Medium]
Could we add an explicit type definition for nvt,hwmods?

Custom vendor properties require type definitions, such as referencing the
string type schema, because constraints like minItems are insufficient for
validation tools to deduce the type.

> +  bus-enable:
> +    description: enable this i2c pin function or not
> +    maxItems: 1

[Severity: High]
Does this property reinvent the standard status property?

Usually, device enablement is handled with status = "okay" or
status = "disabled" rather than creating a custom enable flag.

[Severity: Medium]
Since this is a custom property, shouldn't it include an explicit type
definition?

Additionally, using maxItems: 1 on what appears to be a scalar flag without an
array type definition will cause validation tools to fail.

[Severity: Low]
If this property is kept instead of using the standard status property, should
it include a vendor prefix like nvt,bus-enable to prevent future namespace
collisions with generic bindings?

[ ... ]

> +  clock-frequency:
> +    description: Operation clock frequency of i2c in kHz. Default is 100kHz.
> +    default: 100
> +    maxItems: 1

[Severity: High]
Is it safe to redefine the standard clock-frequency units to kHz here?

The generic i2c-controller schema and device tree specification define
clock-frequency in Hertz (Hz). If a standard device tree configures
clock-frequency = <100000> for 100 kHz, the driver might interpret it as
100,000 kHz, potentially leading to a misconfigured hardware clock divisor and
bus malfunction.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604060430.355733-1-SP_ISW1_AT@novatek.com.tw?part=2

Re: [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller

[Rob Herring (Arm)]

On Thu, 04 Jun 2026 14:04:30 +0800, SP_ISW1_AT@novatek.com.tw wrote:
> From: Ben Huang <Ben_Huang@novatek.com.tw>
> 
> Add device tree documentation for Novatek NT726xx SoC i2c controller.
> 
> Signed-off-by: Ben Huang <Ben_Huang@novatek.com.tw>
> ---
>  .../bindings/i2c/novatek,nt726xx-i2c.yaml     | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml: nvt,hwmods: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml: bus-enable: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml: properties:clock-frequency: 'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	from schema $id: http://devicetree.org/meta-schemas/cell.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260604060430.355733-1-SP_ISW1_AT@novatek.com.tw

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver.

[Krzysztof Kozlowski]

On 04/06/2026 08:04, SP_ISW1_AT@novatek.com.tw wrote:
> CONFIDENTIALITY NOTICE: This message, including attachments, contains 
> information which may be confidential and privileged, and is intended only for 
> use by the addressees designated above. Unless you are the intended recipient, 
> any use, copying, disclosure, or distribution is prohibited. If you have 
> received the message in error, please immediately delete the message and destroy 

So can I review it or not?

Patch is obviously incorrect, but not sure if I am not breaking your
confidentiality rules.

> all copies thereof and notify the sender by reply email.本郵件及任何附件均屬機 
> 密，僅供其上指定地址之收件人使用。除非您是指定之收件人，否則請勿使用、複製、揭露 
> 或散布本郵件之任何部份。若您錯誤地收到此郵件，請立即回覆電子郵件通知寄件人，並請 
> 完全刪除且銷毀本郵件及其複本。
> 
> 
> From: Ben Huang <Ben_Huang@novatek.com.tw>
> 
> Add entry for maintenance of Novatek NT726xx SoC i2c driver.
> 
> Signed-off-by: Ben Huang <Ben_Huang@novatek.com.tw>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9ec290e38b44..7a77a1690f15 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19014,6 +19014,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/nolibc/linux-nolibc.git
>  F:	tools/include/nolibc/
>  F:	tools/testing/selftests/nolibc/
>  
> +NOVATEK NT726XX I2C CONTROLLER DRIVER
> +M:	Ben Huang <ben_huang@novatek.com.tw>
> +L:	linux-i2c@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml
> +F:	drivers/i2c/busses/i2c-nt726xx.c
> +
>  NOVATEK NVT-TS I2C TOUCHSCREEN DRIVER
>  M:	Hans de Goede <hansg@kernel.org>
>  L:	linux-input@vger.kernel.org


Best regards,
Krzysztof

Re: [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver.

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

On Thu, Jun 04, 2026 at 09:26:56AM +0200, Krzysztof Kozlowski wrote:
> On 04/06/2026 08:04, SP_ISW1_AT@novatek.com.tw wrote:
> > CONFIDENTIALITY NOTICE: This message, including attachments, contains 
> > information which may be confidential and privileged, and is intended only for 
> > use by the addressees designated above. Unless you are the intended recipient, 
> > any use, copying, disclosure, or distribution is prohibited. If you have 
> > received the message in error, please immediately delete the message and destroy 
> 
> So can I review it or not?

Given the state of the binding patch, you probably don't want to review
it anyway!

> 
> Patch is obviously incorrect, but not sure if I am not breaking your
> confidentiality rules.
> 
> > all copies thereof and notify the sender by reply email.本郵件及任何附件均屬機 
> > 密，僅供其上指定地址之收件人使用。除非您是指定之收件人，否則請勿使用、複製、揭露 
> > 或散布本郵件之任何部份。若您錯誤地收到此郵件，請立即回覆電子郵件通知寄件人，並請 
> > 完全刪除且銷毀本郵件及其複本。

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver.

[Krzysztof Kozlowski]

On 04/06/2026 11:02, Ben Huang(黃士軒) wrote:
> Hi,
> 
> 	Sorry for late response.
> 
> 	Please help review the modification in MAINTAINERS.
> 	I am still looking for internal help to remove HTML-related messages.

It's confidential, no?

Best regards,
Krzysztof