* [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver @ 2026-06-05 3:55 SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller SP_ISW1_AT 0 siblings, 1 reply; 9+ messages in thread From: SP_ISW1_AT @ 2026-06-05 3:55 UTC (permalink / raw) To: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel Cc: SP_ISW1_AT, ben_huang, toby_chui, shihpei_hsu 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> Signed-off-by: Novatek i2c <SP_ISW1_AT@novatek.com.tw> --- v2: - Add Novatek i2c to Signed-off-by email list. - Remove confidential related statements and HTML messages. - Fix the potential issues in the device tree document and i2c driver source codes. - Fix typos. v1: https://lore.kernel.org/lkml/20260604060411.355675-1-SP_ISW1_AT@novatek.com.tw/T/#t --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e035a3be797c..d42f644a596d 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller 2026-06-05 3:55 [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver SP_ISW1_AT @ 2026-06-05 3:56 ` SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: SP_ISW1_AT @ 2026-06-05 3:56 UTC (permalink / raw) To: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel Cc: SP_ISW1_AT, ben_huang, toby_chui, shihpei_hsu 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> Signed-off-by: Novatek i2c <SP_ISW1_AT@novatek.com.tw> --- v2: - Add Novatek i2c to Signed-off-by email list. - Remove confidential related statements and HTML messages. - Fix the potential issues in the device tree document and i2c driver source codes. - Fix typos. v1: https://lore.kernel.org/lkml/20260604060411.355675-1-SP_ISW1_AT@novatek.com.tw/T/#t --- .../bindings/i2c/novatek,nt726xx-i2c.yaml | 48 +++++++++++++++++++ 1 file changed, 48 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..d9dfdaaec205 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml @@ -0,0 +1,48 @@ +# 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 + + reg: + maxItems: 4 + + interrupts: + maxItems: 3 + + clock-frequency: + description: Operation clock frequency of i2c in Hz. + default: 100000 + enum: [ 100000, 400000 ] + + novatek,hwmods: + $ref: /schemas/types.yaml#/definitions/string + description: Name of each i2c pin, must be named with "i2cX". (X is + an integer starting from 0) + minItems: 1 + + novatek,stbc: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Set if this i2c master controlled by stbc. + minItems: 1 + +required: + - compatible + - reg + - interrupts + - novatek,hwmods + +unevaluatedProperties: false -- 2.40.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs 2026-06-05 3:56 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller SP_ISW1_AT @ 2026-06-05 3:56 ` SP_ISW1_AT 2026-06-05 4:08 ` sashiko-bot 2026-06-05 4:04 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller sashiko-bot 2026-06-05 5:25 ` Rob Herring (Arm) 2 siblings, 1 reply; 9+ messages in thread From: SP_ISW1_AT @ 2026-06-05 3:56 UTC (permalink / raw) To: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel Cc: SP_ISW1_AT, ben_huang, toby_chui, shihpei_hsu 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> Signed-off-by: Novatek i2c <SP_ISW1_AT@novatek.com.tw> --- v2: - Add Novatek i2c to Signed-off-by email list. - Remove confidential related statements and HTML messages. - Fix the potential issues in the device tree document and i2c driver source codes. - Fix typos. v1: https://lore.kernel.org/lkml/20260604060411.355675-1-SP_ISW1_AT@novatek.com.tw/T/#t --- drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-nt726xx.c | 694 +++++++++++++++++++++++++++++++ 3 files changed, 705 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..921f47b7dd55 --- /dev/null +++ b/drivers/i2c/busses/i2c-nt726xx.c @@ -0,0 +1,694 @@ +// 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 "NT726xx 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_ACK_CTRL_COUNTER 0x1E0 +#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_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); + iounmap(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; + writel(clk_div << 1, i2c->base + I2C_REG_CLK); + + duty = (clk_div * 9 + 10) / 20; + writel(duty << 16, i2c->base + I2C_REG_DUTY); + + writel(I2C_ACK_CTRL_COUNTER, 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) & ~I2C_FUNC_SMBUS_QUICK; +} + +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(novatek,hwmods) and set bus number */ + ret = of_property_read_string(np, "novatek,hwmods", &hwmods_val); + if (ret == 0) { + i2c->adapter.nr = nvt_i2c_get_hwmods(hwmods_val); + if (i2c->adapter.nr >= 0) { + I2C_INFO_LOG("Get novatek,hwmods:i2c%d\n", i2c->adapter.nr); + } else { + I2C_ERR_LOG("Invalid novatek,hwmods value = %d\n", i2c->adapter.nr); + return -EINVAL; + } + } else { + I2C_ERR_LOG("Can't get novatek,hwmods\n"); + return ret; + } + + i2c->comp_data = of_device_get_match_data(dev); + + /* read DTS(novatek,stbc) */ + ret = of_property_read_u32(np, "novatek,stbc", &i2c->stbc_i2c); + if (ret) { + I2C_INFO_LOG("Not set dtb novatek,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->bus_clk_rate) { + I2C_INFO_LOG("Not set dtb clock-frequency, set default 100kHz\n"); + i2c->bus_clk_rate = 100000; + } + + 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) + 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); + + writel(I2C_IRQ_DISABLE_SETTING, i2c->base + I2C_REG_INTR); + writel(readl(i2c->base + I2C_REG_CTRL) & ~I2C_ENABLE, + i2c->base + I2C_REG_CTRL); + of_node_put(i2c->adapter.dev.of_node); + 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 NT726xx SoC I2C Bus Driver"); +MODULE_AUTHOR("Ben Huang <ben_huang@novatek.com.tw>"); +MODULE_LICENSE("GPL"); -- 2.40.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs 2026-06-05 3:56 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT @ 2026-06-05 4:08 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-06-05 4:08 UTC (permalink / raw) To: SP_ISW1_AT; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider: - [High] Hardcoded physical addresses mapped via ioremap() without NULL checks, and unprotected read-modify-write on shared global registers. - [High] Driver breaks generic I2C transfers by unconditionally hijacking any 2-message array. - [High] The ISR returns IRQ_NONE without clearing the hardware interrupt status when current_msg is NULL. - [High] Missing synchronization between the transfer timeout path and the ISR leads to a use-after-free. - [High] Interrupts are enabled in hardware before the IRQ handler is registered, and are leaked enabled on probe failure paths. - [High] The I2C hardware and interrupts are disabled before the adapter is unregistered in the remove function. - [Medium] Using a custom Device Tree property to statically assign the I2C bus number. - [Medium] Device Tree node reference is permanently leaked on the adapter registration error path. - [Low] Unconditional assignment of PM callbacks causes dead code when PM is disabled. -- commit 2b999343fc99d6d31a3c1ef33d691ee2f0f37f51 Author: Ben Huang <Ben_Huang@novatek.com.tw> i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs This patch introduces support for the I2C bus controller of Novatek NT726xx SoCs. It performs fundamental read/write functions and supports Standard-mode and Fast-mode. > diff --git a/drivers/i2c/busses/i2c-nt726xx.c b/drivers/i2c/busses/i2c-nt726xx.c > new file mode 100644 > index 0000000000000..921f47b7dd551 > --- /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); > + writel(readl(reg_tmp) | STBC_PASSWORD_DATA1, reg_tmp); > + writel(readl(reg_tmp) | STBC_PASSWORD_DATA2, reg_tmp); [Severity: High] Does this code map a hardcoded physical address without checking the return value of ioremap()? If the mapping fails, readl() will dereference a NULL pointer. Additionally, is it safe to perform an unprotected read-modify-write on these shared global registers, or could it corrupt state if other drivers concurrently access the Standby Controller? > + iounmap(reg_tmp); [ ... ] > +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 the hardware asserts an interrupt and i2c->current_msg is NULL, does this early return skip clearing the hardware interrupt status? The write to clear the interrupt at the end of the ISR is bypassed, which could leave the IRQ line asserted and cause an infinite interrupt loop. > + > + if (status & I2C_IRQ_NACK) { [ ... ] > +static int nvt_i2c_write(struct nvt_i2c_bus *i2c, struct i2c_msg *msg) > +{ [ ... ] > + 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; [Severity: High] Can a use-after-free happen here if the timeout path races with the ISR? If a transfer times out, nvt_i2c_reset() is called and current_msg is set to NULL, but there is no synchronization (like a spinlock or synchronize_irq()) to wait for a concurrently running ISR to finish. If the ISR checks current_msg before it is set to NULL, the thread could return and free msg->buf while the ISR continues to execute nvt_i2c_handle(). > + > + 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]; [Severity: High] Does this code unconditionally intercept all 2-message arrays? By consuming msgs[0] as a subaddress and only transmitting msgs[1], this breaks standard I2C semantics if a user-space application or generic kernel driver issues a regular 2-message transfer. If the first message is a read, could it lead to reading from an uninitialized buffer and writing it to the register? > + > + if (msg->flags & I2C_M_RD) [ ... ] > +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(novatek,hwmods) and set bus number */ > + ret = of_property_read_string(np, "novatek,hwmods", &hwmods_val); > + if (ret == 0) { > + i2c->adapter.nr = nvt_i2c_get_hwmods(hwmods_val); [Severity: Medium] Is it appropriate to use a custom Device Tree property like novatek,hwmods to statically assign the I2C adapter number? The standard Linux mechanism for assigning bus numbers relies on the aliases node in the Device Tree rather than custom properties. > + if (i2c->adapter.nr >= 0) { [ ... ] > +static int nvt_i2c_probe(struct platform_device *pdev) > +{ [ ... ] > + ret = nvt_i2c_parse_dts(i2c); > + if (ret) > + 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); [Severity: High] Does nvt_i2c_init() enable hardware interrupts before devm_request_irq() registers the software handler? If the hardware has a pending interrupt, the kernel might receive it prematurely. Furthermore, if probe fails on subsequent calls like devm_request_irq(), are these interrupts left enabled in the hardware, potentially causing an unhandled interrupt storm on a shared line? > + 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; [Severity: Medium] If i2c_add_numbered_adapter() fails, does this return path leak the OF node reference? The node was acquired just above with of_node_get(), and returning directly here skips calling of_node_put(). > + } > + > + 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); > + > + writel(I2C_IRQ_DISABLE_SETTING, i2c->base + I2C_REG_INTR); > + writel(readl(i2c->base + I2C_REG_CTRL) & ~I2C_ENABLE, > + i2c->base + I2C_REG_CTRL); > + of_node_put(i2c->adapter.dev.of_node); > + i2c_del_adapter(&i2c->adapter); [Severity: High] Is the teardown order reversed here? The code disables the I2C hardware and interrupts before calling i2c_del_adapter(). If an active transaction is running, this will forcefully break it. Additionally, i2c_del_adapter() ensures synchronization by draining and blocking transfers, so it should be called first before shutting down the hardware. > +} > + > +static const struct dev_pm_ops nvt_i2c_pm_ops = { > + .resume_early = nvt_i2c_resume, > + .suspend_late = nvt_i2c_suspend, > +}; [Severity: Low] Are the pm_sleep_ptr() wrappers missing here? Without wrapping the callbacks in the dev_pm_ops structure, this will result in dead code being compiled into the kernel when CONFIG_PM_SLEEP is 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: Low] Does this assignment need a pm_ptr() wrapper to avoid dead code when PM is disabled? > + .of_match_table = of_match_ptr(nvt_i2c_of_match), > + }, > +}; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260605035614.383387-1-SP_ISW1_AT@novatek.com.tw?part=3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller 2026-06-05 3:56 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT @ 2026-06-05 4:04 ` sashiko-bot 2026-06-05 5:25 ` Rob Herring (Arm) 2 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-06-05 4:04 UTC (permalink / raw) To: SP_ISW1_AT; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Low] The device tree binding schema is entirely missing the mandatory `examples` section. - [Low] The `novatek,hwmods` property uses incorrect type constraints, lacks pattern enforcement, and has ambiguous semantics. - [Low] The `novatek,stbc` property is defined as a `uint32` but its description indicates it is a boolean flag. - [Low] The `reg` and `interrupts` properties permit multiple items without documenting their meaning or expected order. -- commit 1cba013f2a2d800236543516887a86edb943712e Author: Ben Huang <Ben_Huang@novatek.com.tw> dt-bindings: i2c: add Novatek NT726xx SoC i2c controller Add device tree documentation for Novatek NT726xx SoC i2c controller. > 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..d9dfdaaec205e > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/novatek,nt726xx-i2c.yaml > @@ -0,0 +1,48 @@ [ ... ] > +properties: > + compatible: > + const: novatek,nt726xx-i2c > + > + reg: > + maxItems: 4 > + > + interrupts: > + maxItems: 3 [Severity: Low] These properties permit multiple items without documenting their meaning or expected order. Should we use an items list with descriptions so that the purpose and expected order of each entry is defined? > + > + clock-frequency: > + description: Operation clock frequency of i2c in Hz. > + default: 100000 > + enum: [ 100000, 400000 ] > + > + novatek,hwmods: > + $ref: /schemas/types.yaml#/definitions/string > + description: Name of each i2c pin, must be named with "i2cX". (X is > + an integer starting from 0) > + minItems: 1 [Severity: Low] This property references a scalar string definition, but minItems: 1 is an array constraint, and the description "Name of each i2c pin" suggests it accepts multiple names. Should this be defined as a string-array instead? Also, should we add a pattern regex (like ^i2c[0-9]+$) to enforce the documented naming rule? > + > + novatek,stbc: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Set if this i2c master controlled by stbc. > + minItems: 1 [Severity: Low] The description "Set if..." outlines a boolean state, but it is defined as a uint32. Should this be declared using /schemas/types.yaml#/definitions/flag instead? Applying minItems: 1 is also redundant for a single scalar type. > + > +required: > + - compatible > + - reg > + - interrupts > + - novatek,hwmods > + > +unevaluatedProperties: false [Severity: Low] Is the mandatory examples block missing from the end of this file? All new hardware device tree binding YAML schemas require an examples block to demonstrate how the device tree node should be written and to allow dt_binding_check validation testing. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260605035614.383387-1-SP_ISW1_AT@novatek.com.tw?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller 2026-06-05 3:56 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT 2026-06-05 4:04 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller sashiko-bot @ 2026-06-05 5:25 ` Rob Herring (Arm) 2 siblings, 0 replies; 9+ messages in thread From: Rob Herring (Arm) @ 2026-06-05 5:25 UTC (permalink / raw) To: SP_ISW1_AT Cc: shihpei_hsu, linux-kernel, devicetree, toby_chui, andi.shyti, krzk+dt, conor+dt, ben_huang, linux-i2c On Fri, 05 Jun 2026 11:56:14 +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> > Signed-off-by: Novatek i2c <SP_ISW1_AT@novatek.com.tw> > --- > v2: > - Add Novatek i2c to Signed-off-by email list. > - Remove confidential related statements and HTML messages. > - Fix the potential issues in the device tree document and > i2c driver source codes. > - Fix typos. > > v1: > https://lore.kernel.org/lkml/20260604060411.355675-1-SP_ISW1_AT@novatek.com.tw/T/#t > --- > .../bindings/i2c/novatek,nt726xx-i2c.yaml | 48 +++++++++++++++++++ > 1 file changed, 48 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: properties:novatek,stbc: False schema does not allow 1 hint: Scalar properties should not have array keywords from schema $id: http://devicetree.org/meta-schemas/keywords.yaml doc reference errors (make refcheckdocs): See https://patchwork.kernel.org/project/devicetree/patch/20260605035614.383387-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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller @ 2026-06-04 6:04 SP_ISW1_AT 2026-06-04 6:04 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT 0 siblings, 1 reply; 9+ messages in thread From: SP_ISW1_AT @ 2026-06-04 6:04 UTC (permalink / raw) To: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel Cc: SP_ISW1_AT, ben_huang, toby_chui, shihpei_hsu [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs. 2026-06-04 6:04 SP_ISW1_AT @ 2026-06-04 6:04 ` SP_ISW1_AT 2026-06-04 6:22 ` sashiko-bot 2026-06-04 6:38 ` Wolfram Sang 0 siblings, 2 replies; 9+ messages in thread From: SP_ISW1_AT @ 2026-06-04 6:04 UTC (permalink / raw) To: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel Cc: SP_ISW1_AT, ben_huang, toby_chui, shihpei_hsu [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs. 2026-06-04 6:04 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT @ 2026-06-04 6:22 ` sashiko-bot 2026-06-04 6:38 ` Wolfram Sang 1 sibling, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-06-04 6:22 UTC (permalink / raw) To: SP_ISW1_AT; +Cc: conor+dt, devicetree, robh 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs. 2026-06-04 6:04 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT 2026-06-04 6:22 ` sashiko-bot @ 2026-06-04 6:38 ` Wolfram Sang 1 sibling, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2026-06-04 6:38 UTC (permalink / raw) To: SP_ISW1_AT Cc: andi.shyti, robh, krzk+dt, conor+dt, linux-i2c, devicetree, linux-kernel, ben_huang, toby_chui, shihpei_hsu [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-06-05 5:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-05 3:55 [PATCH 1/3] MAINTAINERS: Add entry for Novatek NT726xx SoC i2c driver SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller SP_ISW1_AT 2026-06-05 3:56 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT 2026-06-05 4:08 ` sashiko-bot 2026-06-05 4:04 ` [PATCH 2/3] dt-bindings: i2c: add Novatek NT726xx SoC i2c controller sashiko-bot 2026-06-05 5:25 ` Rob Herring (Arm) -- strict thread matches above, loose matches on Subject: below -- 2026-06-04 6:04 SP_ISW1_AT 2026-06-04 6:04 ` [PATCH 3/3] i2c: Add i2c-nt726xx.c i2c driver for Novatek NT726xx SoCs SP_ISW1_AT 2026-06-04 6:22 ` sashiko-bot 2026-06-04 6:38 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox