[PATCH 0/3] i2c: Add driver support Amlogic A9 I2C

[PATCH 0/3] i2c: Add driver support Amlogic A9 I2C

[Xianwei Zhao via B4 Relay]

Add a Device Tree binding and driver for the I2C controller found on the
Amlogic A9 SoC.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Junyi Zhao (1):
      i2c: amlogic: Add Amlogic A9 I2C controller driver

Xianwei Zhao (2):
      dt-bindings: i2c: add Amlogic A9 I2C controller
      MAINTAINERS: Add an entry for Amlogic I2C driver

 .../devicetree/bindings/i2c/amlogic,a9-i2c.yaml    |  61 +++
 MAINTAINERS                                        |   8 +
 drivers/i2c/busses/Kconfig                         |   9 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-amlogic-a9.c                | 504 +++++++++++++++++++++
 5 files changed, 583 insertions(+)
---
base-commit: e817a366cd24ddc8f986ef43c5d54cc3f60b0c21
change-id: 20260911-a9-i2c-24478e22e683

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 1/3] dt-bindings: i2c: add Amlogic A9 I2C controller

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add a Device Tree binding for the I2C controller found on the
Amlogic A9 SoC.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../devicetree/bindings/i2c/amlogic,a9-i2c.yaml    | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml
new file mode 100644
index 000000000000..681241a52680
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/amlogic,a9-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic A9 I2C Controller
+
+maintainers:
+  - Xianwei Zhao <xianwei.zhaol@amlogic.com>
+  - Junyi Zhao <junyi.zhaol@amlogic.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    const: amlogic,a9-i2c
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  dmas:
+    items:
+      - description: DMA controller phandle and request line for RX
+      - description: DMA controller phandle and request line for TX
+
+  dma-names:
+    items:
+      - const: rx
+      - const: tx
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c@3a4400 {
+        compatible = "amlogic,a9-i2c";
+        reg = <0x3a4400 0x3c>;
+        interrupts = <1 164 1>;
+        clocks = <&clkc_periphs 11>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        eeprom@52 {
+            compatible = "atmel,24c32";
+            reg = <0x52>;
+        };
+    };

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/3] i2c: amlogic: Add Amlogic A9 I2C controller driver

[Xianwei Zhao via B4 Relay]

From: Junyi Zhao <junyi.zhao@amlogic.com>

Add support for the new I2C controller used by Amlogic A9 SoCs.

Add a dedicated Kconfig option and Makefile entry for the A9 I2C
controller driver.

Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/i2c/busses/Kconfig          |   9 +
 drivers/i2c/busses/Makefile         |   1 +
 drivers/i2c/busses/i2c-amlogic-a9.c | 504 ++++++++++++++++++++++++++++++++++++
 3 files changed, 514 insertions(+)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d7b89508311f..5595a5166569 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -885,6 +885,15 @@ config I2C_MESON
 	  If you say yes to this option, support will be included for the
 	  I2C interface on the Amlogic Meson family of SoCs.
 
+config I2C_AMLOGIC_A9
+	tristate "Amlogic new I2C controller"
+	depends on ARCH_MESON || COMPILE_TEST
+	depends on COMMON_CLK
+	help
+	  If you say yes to this option, support will be included for the
+	  I2C interface on the new Amlogic family of SoCs.
+
+
 config I2C_MICROCHIP_CORE
 	tristate "Microchip FPGA I2C controller"
 	depends on ARCH_MICROCHIP_POLARFIRE || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3755c54b3d82..9ffb1082b80b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_I2C_LPC2K)		+= i2c-lpc2k.o
 obj-$(CONFIG_I2C_LS2X)		+= i2c-ls2x.o
 obj-$(CONFIG_I2C_LS2X_V2)	+= i2c-ls2x-v2.o
 obj-$(CONFIG_I2C_MESON)		+= i2c-meson.o
+obj-$(CONFIG_I2C_AMLOGIC_A9)	+= i2c-amlogic-a9.o
 obj-$(CONFIG_I2C_MICROCHIP_CORE)	+= i2c-microchip-corei2c.o
 obj-$(CONFIG_I2C_MPC)		+= i2c-mpc.o
 obj-$(CONFIG_I2C_MT65XX)	+= i2c-mt65xx.o
diff --git a/drivers/i2c/busses/i2c-amlogic-a9.c b/drivers/i2c/busses/i2c-amlogic-a9.c
new file mode 100644
index 000000000000..2fcac7ee0e72
--- /dev/null
+++ b/drivers/i2c/busses/i2c-amlogic-a9.c
@@ -0,0 +1,504 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Copyright (C) 2026 Amlogic, Inc. All rights reserved
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/delay.h>
+
+/* Amlogic I2C register map */
+#define REG_CFG_RDY		0x00
+#define REG_CFG_I2C		0x04
+#define REG_CFG_START		0x08
+#define REG_CFG_BUS		0x0c
+#define REG_TX_RD_ADDR		0x10
+#define REG_TX_WR_ADDR		0x14
+#define REG_RX_RD_ADDR		0x18
+#define REG_RX_WR_ADDR		0x1c
+#define REG_CGF_TX		0x20
+#define REG_CGF_RX		0x24
+#define REG_CGF_IRQ_STATE	0x30
+#define REG_CGF_IRQ_ENABLE	0x34
+#define REG_SHAKE_BLK_CNT	0x38
+
+/* CFG RDY fields */
+#define RDY_TEE_ONLY		BIT(1)
+#define RDY_IF			BIT(0)
+
+/* CFG I2C fields */
+#define I2C_RX_THR		GENMASK(23, 16)
+#define I2C_TX_THR		GENMASK(15, 8)
+
+/* CFG BUS fields */
+#define BUS_NO_STOP		BIT(18)
+#define BUS_SPEED_MODE		BIT(17)
+#define BUS_SLAVE_MODE		BIT(16)
+#define BUS_FILTER_MASK		GENMASK(15, 12)
+/* SCL = clk/b_ratio	if b_ratio<=8, SCL = clk/8 */
+#define BUS_RATIO_MASK		GENMASK(11, 0)
+
+/* CFG START fields */
+#define START_START		BIT(31)
+#define START_LEN		GENMASK(23, 12)
+#define START_LEN_SHIFT		12
+#define START_SLAVE_ADDR	GENMASK(10, 1)
+#define START_READ		BIT(0)
+
+/* CFG TX_RX fields */
+#define TX_RX_EMPTY		BIT(9)
+#define TX_RX_FULL		BIT(8)
+#define TX_RX_DATA		GENMASK(7, 0)
+
+/*CGF IRQ fields */
+#define IRQ_ALL_MASK		0xffff
+#define A9_NCK_ERROR		BIT(0)
+#define A9_RX_EMPTY		BIT(1)
+#define A9_RX_FULL		BIT(2)
+#define A9_TX_EMPTY		BIT(3)
+#define A9_TX_FULL		BIT(4)
+#define A9_RX_THRESH_READ	BIT(5)
+#define A9_TX_THRESH_WRITE	BIT(6)
+#define A9_PHY_DONE		BIT(7)
+#define A9_TASK_DONE		BIT(8)
+#define A9_ALL_DONE		BIT(9)
+
+#define A9_TRANS_DONE		(A9_PHY_DONE | A9_NCK_ERROR)
+#define A9_TRANS_ERROR		(A9_NCK_ERROR)
+#define A9_ENABLE_IRQ_BIT	(A9_NCK_ERROR | A9_PHY_DONE)
+#define THRESH_MODE		(A9_RX_THRESH_READ | A9_TX_THRESH_WRITE)
+
+#define A9_I2C_FIFO_DEPTH	32
+#define A9_I2C_HALF_FIFO	(A9_I2C_FIFO_DEPTH >> 1)
+
+#define I2C_TIMEOUT_MS		500
+
+enum {
+	STATE_IDLE,
+	STATE_READ,
+	STATE_WRITE,
+};
+
+enum fifo_fill_mode {
+	FIFO_FILL_FULL,
+	FIFO_FILL_HALF,
+};
+
+/**
+ * struct aml_i2c - Amlogic I2C device private data
+ *
+ * @adap:	I2C adapter instance
+ * @dev:	Pointer to device structure
+ * @regs:	Base address of the device memory mapped registers
+ * @clk:	Pointer to clock structure
+ * @msg:	Pointer to the current I2C message
+ * @state:	Current state in the driver state machine
+ * @count:	Number of bytes to be sent/received in current transfer
+ * @pos:	Current position in the send/receive buffer
+ * @error:	Flag set when an error is received
+ * @lock:	To avoid race conditions between irq handler and xfer code
+ * @done:	Completion used to wait for transfer termination
+ * @clk_rate:	Clock source rate of I2C module
+ * @frequency:	Operating frequency of I2C bus clock
+ */
+struct aml_i2c {
+	struct i2c_adapter	adap;
+	struct device		*dev;
+	void __iomem		*regs;
+	struct clk		*clk;
+
+	struct i2c_msg		*msg;
+	int			state;
+	int			count;
+	int			pos;
+	int			error;
+	/* protect reg access */
+	spinlock_t		lock;
+	struct completion	done;
+	unsigned long		clk_rate;
+	unsigned int		frequency;
+};
+
+static void aml_i2c_set_mask(struct aml_i2c *i2c, int reg, u32 mask, u32 val)
+{
+	u32 data;
+
+	data = readl(i2c->regs + reg);
+	data &= ~mask;
+	data |= val & mask;
+	writel(data, i2c->regs + reg);
+}
+
+/*
+ * SCL = clk/b_ratio. if b_ratio<=8, SCL = clk/8
+ */
+static int aml_i2c_set_clk_div(struct aml_i2c *i2c)
+{
+	unsigned long clk_rate = i2c->clk_rate;
+	unsigned int div;
+
+	div = DIV_ROUND_UP(clk_rate, i2c->frequency);
+
+	/* clock divider has 12 bits */
+	if (div >= (1 << 12)) {
+		dev_err(i2c->dev, "requested bus frequency too low\n");
+		return -EINVAL;
+	} else if (div <= 8) {
+		dev_err(i2c->dev, "requested bus frequency too high\n");
+		return -EINVAL;
+	}
+	/* CFG_BUS reg:11 - 0 bits
+	 */
+	aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_RATIO_MASK, div);
+
+	return 0;
+}
+
+static void reset_fifo_pos(struct aml_i2c *i2c)
+{
+	writel(0x00, i2c->regs + REG_TX_RD_ADDR);
+	writel(0x00, i2c->regs + REG_TX_WR_ADDR);
+	writel(0x00, i2c->regs + REG_RX_WR_ADDR);
+	writel(0x00, i2c->regs + REG_RX_RD_ADDR);
+}
+
+static int controller_is_error(struct aml_i2c *i2c)
+{
+	int ret = 0;
+
+	if (readl(i2c->regs + REG_CGF_IRQ_STATE) & A9_TRANS_ERROR)
+		ret = -ENXIO;
+
+	return ret;
+}
+
+static int controller_is_done(struct aml_i2c *i2c)
+{
+	return readl(i2c->regs + REG_CGF_IRQ_STATE) & A9_TRANS_DONE;
+}
+
+static void clear_irq_status(struct aml_i2c *i2c, u32 irq_bits)
+{
+	writel(irq_bits, i2c->regs + REG_CGF_IRQ_STATE);
+}
+
+static int aml_i2c_init(struct aml_i2c *i2c)
+{
+	int ret;
+
+	clear_irq_status(i2c, IRQ_ALL_MASK);
+	writel(A9_ENABLE_IRQ_BIT, i2c->regs + REG_CGF_IRQ_ENABLE);
+	aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_NO_STOP, BUS_NO_STOP);
+	aml_i2c_set_mask(i2c, REG_CFG_I2C, I2C_RX_THR | I2C_TX_THR,
+			 A9_I2C_FIFO_DEPTH << 16 | A9_I2C_HALF_FIFO << 8);
+	reset_fifo_pos(i2c);
+	ret = aml_i2c_set_clk_div(i2c);
+
+	return ret;
+}
+
+static void aml_i2c_add_stop(struct aml_i2c *i2c)
+{
+	aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_NO_STOP, 0);
+}
+
+static void aml_i2c_cpy_data_to_user(struct aml_i2c *i2c, u8 data)
+{
+	u8 *buf = i2c->msg->buf + i2c->pos;
+
+	i2c->count = 1;
+	*buf = data;
+}
+
+static void aml_i2c_put_data(struct aml_i2c *i2c, char *buf, int len)
+{
+	int i;
+
+	/* this i2c module when trans 0 byte, must put at least 1.
+	 */
+	if (!i2c->msg->len) {
+		writel(0x00, i2c->regs + REG_CGF_TX);
+		return;
+	}
+
+	for (i = 0; i < len; i++, buf++)
+		writel(*buf, i2c->regs + REG_CGF_TX);
+}
+
+static void aml_i2c_prepare_xfer(struct aml_i2c *i2c, enum fifo_fill_mode mode)
+{
+	bool write = !(i2c->msg->flags & I2C_M_RD);
+
+	if (write) {
+		if (mode == FIFO_FILL_FULL)
+			i2c->count = min(i2c->msg->len - i2c->pos, A9_I2C_FIFO_DEPTH);
+		else
+			i2c->count = min(i2c->msg->len - i2c->pos, A9_I2C_HALF_FIFO);
+		aml_i2c_put_data(i2c, i2c->msg->buf + i2c->pos, i2c->count);
+	}
+}
+
+static irqreturn_t aml_i2c_irq(int irqno, void *dev_id)
+{
+	struct aml_i2c *i2c = dev_id;
+	int ret;
+	u32 rd_len, i;
+	u32 rd_val;
+
+	spin_lock(&i2c->lock);
+	ret = controller_is_error(i2c);
+	if (ret) {
+		i2c->error = ret;
+		goto complete_irq_out;
+	}
+
+	switch (i2c->state) {
+	case STATE_READ:
+		rd_len = min(i2c->msg->len - i2c->pos, A9_I2C_FIFO_DEPTH);
+		for (i = 0; i < rd_len; i++) {
+			rd_val = readl(i2c->regs + REG_CGF_RX);
+			aml_i2c_cpy_data_to_user(i2c, rd_val & TX_RX_DATA);
+			i2c->pos += i2c->count;
+		}
+		/* rx must clear thresh after read fifo */
+		clear_irq_status(i2c, A9_RX_THRESH_READ);
+		break;
+	case STATE_WRITE:
+		aml_i2c_prepare_xfer(i2c, FIFO_FILL_HALF);
+		if ((i2c->pos + A9_I2C_HALF_FIFO) >= i2c->msg->len)
+			aml_i2c_set_mask(i2c, REG_CGF_IRQ_ENABLE, A9_TX_THRESH_WRITE, 0);
+		/* tx must clear thresh after write fifo */
+		clear_irq_status(i2c, A9_TX_THRESH_WRITE);
+		i2c->pos += i2c->count;
+		break;
+	case STATE_IDLE:
+		dev_warn(i2c->dev, "i2c irq triggered abnormally!!!, irq_state:0x%x,irq_en:0x%x\n",
+			 readl(i2c->regs + REG_CGF_IRQ_STATE),
+			 readl(i2c->regs + REG_CGF_IRQ_ENABLE));
+		break;
+	default:
+		break;
+	}
+
+	if (!controller_is_done(i2c)) {
+		goto irq_trans_out;
+	} else {
+		if (i2c->state == STATE_READ && (i2c->msg->len - i2c->pos))
+			goto irq_trans_out;
+	}
+
+complete_irq_out:
+	reset_fifo_pos(i2c);
+	complete(&i2c->done);
+	i2c->state = STATE_IDLE;
+	aml_i2c_set_mask(i2c, REG_CGF_IRQ_ENABLE, A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, 0);
+	clear_irq_status(i2c, IRQ_ALL_MASK);
+irq_trans_out:
+	spin_unlock(&i2c->lock);
+
+	return IRQ_HANDLED;
+}
+
+static void aml_i2c_do_start(struct aml_i2c *i2c, struct i2c_msg *msg)
+{
+	unsigned int reg_val;
+	unsigned int trans_len;
+
+	trans_len = msg->len;
+	reg_val = (unsigned int)((msg->addr << 1) |
+				((msg->flags & I2C_M_RD) ? START_READ : 0) |
+				((trans_len & 0xfff) << 12) | START_START);
+	aml_i2c_set_mask(i2c, REG_CFG_START,
+			 START_START | START_SLAVE_ADDR | START_READ | START_LEN, reg_val);
+	clear_irq_status(i2c, IRQ_ALL_MASK);
+	/* enable thresh mode */
+	if (trans_len > A9_I2C_FIFO_DEPTH) {
+		reg_val = (msg->flags & I2C_M_RD) ? A9_RX_THRESH_READ : A9_TX_THRESH_WRITE;
+		aml_i2c_set_mask(i2c, REG_CGF_IRQ_ENABLE,
+				 A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, reg_val);
+	}
+}
+
+static int aml_i2c_xfer_msg(struct aml_i2c *i2c, struct i2c_msg *msg, bool last)
+{
+	unsigned long time_left, flags;
+	int ret = 0;
+
+	i2c->msg = msg;
+	i2c->pos = 0;
+	i2c->count = 0;
+	i2c->error = 0;
+
+	i2c->state = (msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
+	aml_i2c_prepare_xfer(i2c, FIFO_FILL_FULL);
+	i2c->pos += i2c->count;
+	reinit_completion(&i2c->done);
+
+	if (last)
+		aml_i2c_add_stop(i2c);
+	/* Start the transfer */
+	aml_i2c_do_start(i2c, msg);
+
+	time_left = msecs_to_jiffies(I2C_TIMEOUT_MS);
+	time_left = wait_for_completion_timeout(&i2c->done, time_left);
+
+	/*
+	 * Protect access to i2c struct and registers from interrupt
+	 * handlers triggered by a transfer terminated after the
+	 * timeout period
+	 */
+	spin_lock_irqsave(&i2c->lock, flags);
+
+	if (!time_left) {
+		i2c->state = STATE_IDLE;
+		clear_irq_status(i2c, IRQ_ALL_MASK);
+		reset_fifo_pos(i2c);
+		ret = -ETIMEDOUT;
+	}
+
+	if (i2c->error)
+		ret = i2c->error;
+
+	spin_unlock_irqrestore(&i2c->lock, flags);
+
+	return ret;
+}
+
+static int aml_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct aml_i2c *i2c = adap->algo_data;
+	int i, ret = 0;
+
+	for (i = 0; i < num; i++) {
+		ret = aml_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
+		if (ret)
+			break;
+	}
+
+	return ret ?: i;
+}
+
+static u32 aml_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm aml_i2c_algorithm = {
+	.master_xfer	= aml_i2c_xfer,
+	.functionality	= aml_i2c_func,
+};
+
+static int aml_i2c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct aml_i2c *i2c;
+	struct i2c_timings timings;
+	int irq, ret;
+
+	i2c = devm_kzalloc(&pdev->dev, sizeof(struct aml_i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c_parse_fw_timings(&pdev->dev, &timings, true);
+	i2c->frequency = timings.bus_freq_hz;
+
+	i2c->dev = &pdev->dev;
+	platform_set_drvdata(pdev, i2c);
+
+	spin_lock_init(&i2c->lock);
+	init_completion(&i2c->done);
+
+	i2c->regs  = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(i2c->regs))
+		return PTR_ERR(i2c->regs);
+
+	i2c->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(i2c->clk)) {
+		dev_err(&pdev->dev, "can't get device clock\n");
+		return PTR_ERR(i2c->clk);
+	}
+
+	i2c->clk_rate = clk_get_rate(i2c->clk);
+	if (i2c->clk_rate == 0) {
+		dev_err(&pdev->dev, "failed to get clk rate\n");
+		return -EINVAL;
+	}
+
+	ret = aml_i2c_init(i2c);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "fail init i2c\n");
+		return ret;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "can't find IRQ\n");
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, aml_i2c_irq, 0, dev_name(&pdev->dev), i2c);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "can't request IRQ\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(i2c->clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "can't prepare clock\n");
+		return ret;
+	}
+
+	strscpy(i2c->adap.name, "Amlogic I2C adapter",
+		sizeof(i2c->adap.name));
+	i2c->adap.owner = THIS_MODULE;
+	i2c->adap.algo = &aml_i2c_algorithm;
+	i2c->adap.dev.parent = &pdev->dev;
+	i2c->adap.dev.of_node = np;
+	i2c->adap.algo_data = i2c;
+
+	ret = i2c_add_adapter(&i2c->adap);
+	if (ret < 0) {
+		clk_disable_unprepare(i2c->clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void aml_i2c_remove(struct platform_device *pdev)
+{
+	struct aml_i2c *i2c = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c->adap);
+	clk_disable_unprepare(i2c->clk);
+}
+
+static const struct of_device_id aml_i2c_match[] = {
+	{	.compatible = "amlogic,a9-i2c",},
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, aml_i2c_match);
+
+static struct platform_driver aml_i2c_driver = {
+	.probe   = aml_i2c_probe,
+	.remove  = aml_i2c_remove,
+	.driver  = {
+		.name  = "amlogic-i2c",
+		.of_match_table = aml_i2c_match,
+	},
+};
+
+module_platform_driver(aml_i2c_driver);
+
+MODULE_DESCRIPTION("Amlogic I2C Bus driver");
+MODULE_AUTHOR("Junyi Zhao <junyi.zhao@amlogic.com>");
+MODULE_LICENSE("GPL");

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 3/3] MAINTAINERS: Add an entry for Amlogic I2C driver

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add Amlogic I2C controller entry to MAINTAINERS to clarify
the maintainers.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6215fcb07770..33c39ecf535f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1371,6 +1371,14 @@ F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
 F:	drivers/perf/amlogic/
 F:	include/soc/amlogic/
 
+AMLOGIC I2C DRIVER
+M:	Junyi Zhao <junyi.zhao@amlogic.com>
+L:	linux-i2c@vger.kernel.org
+L:	linux-amlogic@lists.infradead.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml
+F:	drivers/i2c/busses/i2c-amlogic-a9.c
+
 AMLOGIC ISP DRIVER
 M:	Keke Li <keke.li@amlogic.com>
 L:	linux-media@vger.kernel.org

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/3] dt-bindings: i2c: add Amlogic A9 I2C controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: i2c: amlogic: misspelled maintainer email addresses
- [Low] dt-bindings: i2c: amlogic: invalid GIC interrupt type in example
--

commit 1ce7d70f157b5cc3e10c7e75c64287c2dbe2c1bb
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>

dt-bindings: i2c: add Amlogic A9 I2C controller
    
Add a Device Tree binding for the I2C controller found on the
Amlogic A9 SoC.

> diff --git a/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml
> new file mode 100644
> index 0000000000000..681241a526806
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/amlogic,a9-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic A9 I2C Controller
> +
> +maintainers:
> +  - Xianwei Zhao <xianwei.zhaol@amlogic.com>
> +  - Junyi Zhao <junyi.zhaol@amlogic.com>

[Severity: Low]
This isn't a functional bug, but are these email addresses misspelled?

The trailing 'l' in 'zhaol' causes emails to bounce when contributors
use get_maintainer.pl to CC the maintainers on patches modifying this
binding.

[ ... ]

> +examples:
> +  - |
> +    i2c@3a4400 {
> +        compatible = "amlogic,a9-i2c";
> +        reg = <0x3a4400 0x3c>;
> +        interrupts = <1 164 1>;

[Severity: Low]
Is this GIC interrupt specifier in the examples block correct?

The first cell value of 1 designates a GIC_PPI (Per-Processor Interrupt),
which is limited to the range 0-15. A value of 164 is highly invalid for a
PPI.

Since I2C controllers are typically shared peripherals, should this be 0
(GIC_SPI) instead?

Developers copy-pasting the example will create invalid device trees that
fail at runtime or during dtc validation.

> +        clocks = <&clkc_periphs 11>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        eeprom@52 {
> +            compatible = "atmel,24c32";
> +            reg = <0x52>;
> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-a9-i2c-v1-0-b8ad9b46f4a8@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/3] i2c: amlogic: Add Amlogic A9 I2C controller driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] i2c: amlogic: hardware registers accessed before clock is enabled
- [High] i2c: amlogic: race condition drops completion interrupts
- [High] i2c: amlogic: silent truncation of large transfers
- [High] i2c: amlogic: broken Repeated START for subsequent transfers
- [High] i2c: amlogic: data race on REG_CGF_IRQ_ENABLE
- [High] i2c: amlogic: IRQ storm on transfer timeout
- [High] i2c: amlogic: race in lockless state initialization
- [Medium] i2c: amlogic: crash due to early clock disable in remove
--

commit 5e779e1911cc216efb8179fa9e2910973c79bb74
Author: Junyi Zhao <junyi.zhao@amlogic.com>

i2c: amlogic: Add Amlogic A9 I2C controller driver
    
Add support for the new I2C controller used by Amlogic A9 SoCs.

> diff --git a/drivers/i2c/busses/i2c-amlogic-a9.c b/drivers/i2c/busses/i2c-amlogic-a9.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amlogic-a9.c

[ ... ]

> +static irqreturn_t aml_i2c_irq(int irqno, void *dev_id)
> +{
> +	struct aml_i2c *i2c = dev_id;
> +	int ret;
> +	u32 rd_len, i;
> +	u32 rd_val;
> +
> +	spin_lock(&i2c->lock);
> +	ret = controller_is_error(i2c);
> +	if (ret) {
> +		i2c->error = ret;
> +		goto complete_irq_out;
> +	}
> +
> +	switch (i2c->state) {
> +	case STATE_READ:

[ ... ]

> +	case STATE_WRITE:

[ ... ]

> +	case STATE_IDLE:
> +		dev_warn(i2c->dev, "i2c irq triggered abnormally!!!, irq_state:0x%x,irq_en:0x%x\n",
> +			 readl(i2c->regs + REG_CGF_IRQ_STATE),
> +			 readl(i2c->regs + REG_CGF_IRQ_ENABLE));
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (!controller_is_done(i2c)) {
> +		goto irq_trans_out;
> +	} else {

[Severity: High]
Could an unhandled interrupt cause an infinite IRQ storm and CPU lockup here?
When a transfer times out, aml_i2c_xfer_msg() resets the state to STATE_IDLE
but doesn't appear to disable the hardware threshold interrupts. If the
delayed interrupt fires, this handler logs a warning and returns IRQ_HANDLED
without clearing the interrupt source.

[ ... ]

> +static void aml_i2c_do_start(struct aml_i2c *i2c, struct i2c_msg *msg)
> +{
> +	unsigned int reg_val;
> +	unsigned int trans_len;
> +
> +	trans_len = msg->len;
> +	reg_val = (unsigned int)((msg->addr << 1) |
> +				((msg->flags & I2C_M_RD) ? START_READ : 0) |
> +				((trans_len & 0xfff) << 12) | START_START);

[Severity: High]
Does this silently truncate I2C message lengths greater than 4095 bytes?
The length is masked with 0xfff without returning an error to the caller,
which could result in silent data corruption for large transfers.

> +	aml_i2c_set_mask(i2c, REG_CFG_START,
> +			 START_START | START_SLAVE_ADDR | START_READ | START_LEN, reg_val);
> +	clear_irq_status(i2c, IRQ_ALL_MASK);

[Severity: High]
Could clear_irq_status() inadvertently drop a valid completion interrupt?
If the hardware responds extremely fast (for example, generating a NACK on
the address byte) immediately after START_START is written, the completion
interrupt might get cleared before the handler has a chance to run, causing
the transfer to spuriously time out.

> +	/* enable thresh mode */
> +	if (trans_len > A9_I2C_FIFO_DEPTH) {
> +		reg_val = (msg->flags & I2C_M_RD) ? A9_RX_THRESH_READ : A9_TX_THRESH_WRITE;
> +		aml_i2c_set_mask(i2c, REG_CGF_IRQ_ENABLE,
> +				 A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, reg_val);

[Severity: High]
Is it safe to perform a read-modify-write on REG_CGF_IRQ_ENABLE here without
holding i2c->lock? If a hardware interrupt asserts immediately after the
transfer starts, a concurrent aml_i2c_irq() running on another CPU might
also modify REG_CGF_IRQ_ENABLE, causing a data race that loses updates.

> +	}
> +}
> +
> +static int aml_i2c_xfer_msg(struct aml_i2c *i2c, struct i2c_msg *msg, bool last)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +
> +	i2c->msg = msg;
> +	i2c->pos = 0;
> +	i2c->count = 0;
> +	i2c->error = 0;
> +
> +	i2c->state = (msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
> +	aml_i2c_prepare_xfer(i2c, FIFO_FILL_FULL);
> +	i2c->pos += i2c->count;
> +	reinit_completion(&i2c->done);

[Severity: High]
Are these transfer state variables reinitialized without holding i2c->lock?
If a delayed interrupt from a previous timed-out transfer fires concurrently,
it might read this inconsistent state or falsely complete this new transfer,
potentially causing memory corruption or an invalid state machine transition.

> +
> +	if (last)
> +		aml_i2c_add_stop(i2c);

[Severity: High]
While this clears the BUS_NO_STOP bit for the final message in a transfer,
is there missing logic to restore it to 1 for non-last messages in new
transfers? Without setting it back, subsequent multi-message transfers might
incorrectly emit a STOP between messages, causing devices requiring Repeated
START to fail.

> +	/* Start the transfer */
> +	aml_i2c_do_start(i2c, msg);
> +
> +	time_left = msecs_to_jiffies(I2C_TIMEOUT_MS);
> +	time_left = wait_for_completion_timeout(&i2c->done, time_left);
> +
> +	/*
> +	 * Protect access to i2c struct and registers from interrupt
> +	 * handlers triggered by a transfer terminated after the
> +	 * timeout period
> +	 */
> +	spin_lock_irqsave(&i2c->lock, flags);
> +
> +	if (!time_left) {
> +		i2c->state = STATE_IDLE;
> +		clear_irq_status(i2c, IRQ_ALL_MASK);
> +		reset_fifo_pos(i2c);
> +		ret = -ETIMEDOUT;
> +	}

[Severity: High]
When a transfer times out, is it necessary to also disable the hardware
interrupts in REG_CGF_IRQ_ENABLE? As currently written, the hardware might
still assert a delayed interrupt once this function returns.

[ ... ]

> +static int aml_i2c_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct aml_i2c *i2c;
> +	struct i2c_timings timings;
> +	int irq, ret;

[ ... ]

> +	i2c->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(i2c->clk)) {
> +		dev_err(&pdev->dev, "can't get device clock\n");
> +		return PTR_ERR(i2c->clk);
> +	}
> +
> +	i2c->clk_rate = clk_get_rate(i2c->clk);
> +	if (i2c->clk_rate == 0) {
> +		dev_err(&pdev->dev, "failed to get clk rate\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = aml_i2c_init(i2c);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "fail init i2c\n");
> +		return ret;
> +	}

[Severity: High]
Does aml_i2c_init() attempt to write to hardware registers before the clock
is enabled? clk_prepare_enable() appears to be called further down in this
function. Accessing unclocked registers could trigger a synchronous external
abort on ARM and crash the system.

> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "can't find IRQ\n");
> +		return irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, irq, aml_i2c_irq, 0, dev_name(&pdev->dev), i2c);

[Severity: Medium]
Since the interrupt is requested using devres here, it will remain active
until the devres cleanup runs after aml_i2c_remove() returns. Could this
leave a vulnerable window during removal where a spurious interrupt might
attempt to read hardware registers after the clock has been disabled?

> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "can't request IRQ\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(i2c->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "can't prepare clock\n");
> +		return ret;
> +	}
> +
> +	strscpy(i2c->adap.name, "Amlogic I2C adapter",
> +		sizeof(i2c->adap.name));
> +	i2c->adap.owner = THIS_MODULE;
> +	i2c->adap.algo = &aml_i2c_algorithm;
> +	i2c->adap.dev.parent = &pdev->dev;
> +	i2c->adap.dev.of_node = np;
> +	i2c->adap.algo_data = i2c;
> +
> +	ret = i2c_add_adapter(&i2c->adap);

[Severity: High]
Could the silent truncation of large transfers observed in aml_i2c_do_start()
be avoided by setting the quirks pointer on i2c->adap here? Populating the
quirks would allow the I2C core to properly enforce the 4095-byte hardware
limit and return an error to the caller natively.

[ ... ]

> +static void aml_i2c_remove(struct platform_device *pdev)
> +{
> +	struct aml_i2c *i2c = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&i2c->adap);
> +	clk_disable_unprepare(i2c->clk);
> +}

[Severity: Medium]
Is the clock being manually disabled here before the interrupt handler is
cleaned up? As mentioned in aml_i2c_probe(), the devm-managed IRQ is still
active at this point. A spurious interrupt arriving after this line could
crash the system by accessing unclocked registers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-a9-i2c-v1-0-b8ad9b46f4a8@amlogic.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic