[PATCH 0/2] i2c: add I2C driver for Andes atciic100

[PATCH 0/2] i2c: add I2C driver for Andes atciic100

[Ben Zong-You Xie]

This patch series includes DT-bindings(1/2) and I2C driver(2/2).

The atciic100 is an I2C adapter/client controller. It can act as either an
I2C adapter or an I2C client depending on the control register settings.

Ben Zong-You Xie (2):
  dt-bindings: i2c: add atciic100
  i2c: atciic100: add Andes I2C driver support

 .../bindings/i2c/andestech,i2c-atciic100.yaml |  40 ++
 MAINTAINERS                                   |   6 +
 drivers/i2c/busses/Kconfig                    |  10 +
 drivers/i2c/busses/Makefile                   |   1 +
 drivers/i2c/busses/i2c-atciic100.c            | 346 ++++++++++++++++++
 5 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
 create mode 100644 drivers/i2c/busses/i2c-atciic100.c

---
2.34.1

[PATCH 1/2] dt-bindings: i2c: add atciic100

[Ben Zong-You Xie]

Document devicetree bindings for Andes I2C controller.

Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
---
 .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
 MAINTAINERS                                   |  5 +++
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml

diff --git a/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
new file mode 100644
index 000000000000..cf96a9186176
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Andes I2C Controller
+
+maintainers:
+  - Ben Zong-You Xie <ben717@andestech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    const: andestech,atciic100
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c@f0a00000 {
+        compatible = "andestech,atciic100";
+        reg = <0xf0a00000 0x1000>;
+        interrupts = <6 IRQ_TYPE_LEVEL_HIGH>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..544251a03c7d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1635,6 +1635,11 @@ S:	Supported
 F:	drivers/clk/analogbits/*
 F:	include/linux/clk/analogbits*
 
+ANDES TECHNOLOGY I2C DRIVER
+M:	Ben Zong-You Xie <ben717@andestech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml
+
 ANDROID DRIVERS
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 M:	Arve Hjønnevåg <arve@android.com>
-- 
2.34.1

[PATCH 2/2] i2c: atciic100: add Andes I2C driver support

[Ben Zong-You Xie]

Add support for Andes I2C driver.

Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
---
 MAINTAINERS                        |   1 +
 drivers/i2c/busses/Kconfig         |  10 +
 drivers/i2c/busses/Makefile        |   1 +
 drivers/i2c/busses/i2c-atciic100.c | 346 +++++++++++++++++++++++++++++
 4 files changed, 358 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-atciic100.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 544251a03c7d..939ff77eed19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1639,6 +1639,7 @@ ANDES TECHNOLOGY I2C DRIVER
 M:	Ben Zong-You Xie <ben717@andestech.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml
+F:	drivers/i2c/busses/i2c-atciic100.c
 
 ANDROID DRIVERS
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fc438f445771..93edb62c7d3f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1422,6 +1422,16 @@ config I2C_ACORN
 
 	  If you don't know, say Y.
 
+config I2C_ATCIIC100
+	tristate "Andes I2C Controller"
+	depends on OF && HAS_IOMEM
+	help
+	  If you say yes to this option, support will be included for the
+	  Andes I2C controller.
+
+	  This support is also available as a module.  If so, the module
+	  will be called i2c-atciic100.
+
 config I2C_ELEKTOR
 	tristate "Elektor ISA card"
 	depends on ISA && HAS_IOPORT_MAP && BROKEN_ON_SMP
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1c2a4510abe4..58756a80c11f 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_I2C_VIPERBOARD)	+= i2c-viperboard.o
 
 # Other I2C/SMBus bus drivers
 obj-$(CONFIG_I2C_ACORN)		+= i2c-acorn.o
+obj-$(CONFIG_I2C_ATCIIC100)	+= i2c-atciic100.o
 obj-$(CONFIG_I2C_BCM_KONA)	+= i2c-bcm-kona.o
 obj-$(CONFIG_I2C_BRCMSTB)	+= i2c-brcmstb.o
 obj-$(CONFIG_I2C_CROS_EC_TUNNEL)	+= i2c-cros-ec-tunnel.o
diff --git a/drivers/i2c/busses/i2c-atciic100.c b/drivers/i2c/busses/i2c-atciic100.c
new file mode 100644
index 000000000000..cfb18b3b506e
--- /dev/null
+++ b/drivers/i2c/busses/i2c-atciic100.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * I2C Support for Andes ATCIIC100 two-wire interface (TWI)
+ *
+ * Copyright (C) 2025 Andes Technology Corporation.
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define	ATCIIC100_CFG_REG		0x10
+#define ATCIIC100_CFG_FIFOSIZE		GENMASK(1, 0)
+
+#define	ATCIIC100_INTEN_REG		0x14
+#define	ATCIIC100_INTEN_FIFO_EMPTY	BIT(0)
+#define	ATCIIC100_INTEN_FIFO_FULL	BIT(1)
+#define	ATCIIC100_INTEN_CMPL		BIT(9)
+
+#define	ATCIIC100_STATUS_REG		0x18
+#define ATCIIC100_STATUS_FIFO_EMPTY	BIT(0)
+#define ATCIIC100_STATUS_FIFO_FULL	BIT(1)
+#define ATCIIC100_STATUS_ADDR_HIT	BIT(3)
+#define ATCIIC100_STATUS_CMPL		BIT(9)
+#define ATCIIC100_STATUS_W1C		GENMASK(9, 3)
+
+#define	ATCIIC100_ADDR_REG		0x1C
+
+#define	ATCIIC100_DATA_REG		0x20
+
+#define	ATCIIC100_CTRL_REG		0x24
+#define ATCIIC100_CTRL_DATA_CNT		GENMASK(7, 0)
+#define ATCIIC100_CTRL_DIR		BIT(8)
+#define ATCIIC100_CTRL_PHASE		GENMASK(12, 9)
+
+#define	ATCIIC100_CMD_REG		0x28
+#define ATCIIC100_CMD_ACTION		GENMASK(2, 0)
+#define ATCIIC100_CMD_ACTION_TRANS	BIT(0)
+
+#define	ATCIIC100_SETUP_REG		0x2C
+#define ATCIIC100_SETUP_IICEN		BIT(0)
+#define ATCIIC100_SETUP_REQ		BIT(2)
+
+#define ATCIIC100_TIMEOUT_US		400000
+#define ATCIIC100_TIMEOUT		usecs_to_jiffies(ATCIIC100_TIMEOUT_US)
+
+#define ATCIIC100_MAX_DATA_LEN		256
+
+struct atciic100 {
+	struct i2c_adapter	adap;
+	struct completion	complete;
+	spinlock_t		lock;
+	void __iomem		*base;
+	u8			*buf;
+	int			irq;
+	u16			buf_len;
+	u8			fifo_size;
+	bool			addr_hit;
+	bool			xfer_done;
+};
+
+static void atciic100_xfer_common(struct atciic100 *i2c, u32 status)
+{
+	unsigned long flags;
+	u32 val;
+	u8 fsize = i2c->fifo_size;
+
+	spin_lock_irqsave(&i2c->lock, flags);
+
+	if (status & ATCIIC100_STATUS_FIFO_EMPTY) { /* Data transmit */
+		/* The last write to FIFO */
+		if (i2c->buf_len <= fsize) {
+			fsize = i2c->buf_len;
+			val = readl(i2c->base + ATCIIC100_INTEN_REG) &
+			      ~ATCIIC100_INTEN_FIFO_EMPTY;
+			writel(val, i2c->base + ATCIIC100_INTEN_REG);
+		}
+
+		while (fsize--) {
+			writel(*i2c->buf++, i2c->base + ATCIIC100_DATA_REG);
+			i2c->buf_len--;
+		}
+	} else if (status & ATCIIC100_STATUS_FIFO_FULL) { /* Data receive */
+		while (fsize--) {
+			*i2c->buf++ = readl(i2c->base + ATCIIC100_DATA_REG);
+			i2c->buf_len--;
+		}
+	}
+
+	if (status & ATCIIC100_STATUS_CMPL) {
+		/* Write 1 to clear all status */
+		writel(ATCIIC100_STATUS_W1C, i2c->base + ATCIIC100_STATUS_REG);
+
+		i2c->xfer_done = true;
+		if (status & ATCIIC100_STATUS_ADDR_HIT)
+			i2c->addr_hit = true;
+
+		/* For the last read, retrieve all remaining data in FIFO. */
+		while (i2c->buf_len--)
+			*i2c->buf++ = readl(i2c->base + ATCIIC100_DATA_REG);
+
+	}
+
+	spin_unlock_irqrestore(&i2c->lock, flags);
+}
+
+static irqreturn_t atciic100_irq_handler(int irq, void *data)
+{
+	struct atciic100 *i2c = data;
+	u32 status = readl(i2c->base + ATCIIC100_STATUS_REG);
+
+	atciic100_xfer_common(i2c, status);
+	if (i2c->xfer_done) {
+		/* Disable all interrupts */
+		writel(0, i2c->base + ATCIIC100_INTEN_REG);
+		if (i2c->addr_hit)
+			complete(&i2c->complete);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int atciic100_xfer_wait(struct atciic100 *i2c, struct i2c_msg *msg)
+{
+	u32 val;
+
+	/*
+	 * Set the data count.
+	 * If there are 256 bytes to be transmitted/received, write 0 to the
+	 * data count field.
+	 */
+	val = readl(i2c->base + ATCIIC100_CTRL_REG) |
+	      (i2c->buf_len & ATCIIC100_CTRL_DATA_CNT);
+
+	/* Set the transaction direction */
+	if (msg->flags & I2C_M_RD)
+		val |= ATCIIC100_CTRL_DIR;
+	else
+		val &= ~ATCIIC100_CTRL_DIR;
+
+	/* Enable all phases choices */
+	val |= ATCIIC100_CTRL_PHASE;
+	writel(val, i2c->base + ATCIIC100_CTRL_REG);
+
+	/* Write the address of the target device to the address register */
+	writel(msg->addr, i2c->base + ATCIIC100_ADDR_REG);
+
+	/* Enable the completion and FIFO empty/full interrupts */
+	if (i2c->irq > 0) {
+		val = readl(i2c->base + ATCIIC100_INTEN_REG);
+		val |= (msg->flags & I2C_M_RD) ? ATCIIC100_INTEN_FIFO_FULL
+					       : ATCIIC100_INTEN_FIFO_EMPTY;
+		val |= ATCIIC100_INTEN_CMPL;
+		writel(val, i2c->base + ATCIIC100_INTEN_REG);
+	}
+
+	/* Issue the transaction */
+	writel(ATCIIC100_CMD_ACTION_TRANS, i2c->base + ATCIIC100_CMD_REG);
+
+	if (i2c->irq > 0) {
+		unsigned long time_left;
+
+		time_left = wait_for_completion_timeout(&i2c->complete,
+							ATCIIC100_TIMEOUT);
+		if (!time_left)
+			return -ETIMEDOUT;
+
+		reinit_completion(&i2c->complete);
+	} else {
+		u32 cond = ATCIIC100_STATUS_CMPL;
+		unsigned long timeout;
+
+		cond |= (msg->flags & I2C_M_RD) ? ATCIIC100_STATUS_FIFO_FULL
+						: ATCIIC100_STATUS_FIFO_EMPTY;
+		while (!i2c->xfer_done) {
+			timeout = readl_poll_timeout_atomic(
+					i2c->base + ATCIIC100_STATUS_REG,
+					val, val & cond,
+					10, ATCIIC100_TIMEOUT_US);
+
+			if (timeout)
+				return -ETIMEDOUT;
+
+			atciic100_xfer_common(i2c, val);
+		}
+
+		if (!i2c->addr_hit)
+			return -ENXIO;
+	}
+
+	/* Check if all data is successfully transmitted. */
+	if (readl(i2c->base + ATCIIC100_CTRL_REG) & ATCIIC100_CTRL_DATA_CNT)
+		return -EIO;
+
+	return 0;
+}
+
+static int atciic100_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
+			  int num)
+{
+	int i, ret;
+	struct atciic100 *i2c = i2c_get_adapdata(adap);
+
+	for (i = 0; i < num; i++) {
+		struct i2c_msg *m = &msg[i];
+
+		i2c->addr_hit = false;
+		i2c->buf = m->buf;
+		i2c->buf_len = m->len;
+		i2c->xfer_done = false;
+		ret = atciic100_xfer_wait(i2c, m);
+		if (ret < 0)
+			return ret;
+	}
+
+	return num;
+}
+
+static u32 atciic100_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm atciic100_algo = {
+	.xfer = atciic100_xfer,
+	.functionality = atciic100_func,
+};
+
+static const struct i2c_adapter_quirks atciic100_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN,
+	.max_write_len = ATCIIC100_MAX_DATA_LEN,
+	.max_read_len = ATCIIC100_MAX_DATA_LEN,
+};
+
+static int atciic100_probe(struct platform_device *pdev)
+{
+	int ret;
+	u32 val;
+	struct atciic100 *i2c;
+	struct i2c_adapter *adap;
+	struct device *dev = &pdev->dev;
+
+	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	i2c->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(i2c->base))
+		return PTR_ERR(i2c->base);
+
+	i2c->irq = platform_get_irq(pdev, 0);
+	if (i2c->irq > 0) {
+		ret = devm_request_irq(dev, i2c->irq, atciic100_irq_handler, 0,
+				       dev_name(dev), i2c);
+		if (ret < 0) {
+			return dev_err_probe(dev, ret,
+					     "unable to request irq %d\n",
+					     i2c->irq);
+		}
+	} else {
+		dev_warn(dev, "no irq resource, falling back to poll mode\n");
+	}
+
+	spin_lock_init(&i2c->lock);
+	init_completion(&i2c->complete);
+
+	adap = &i2c->adap;
+	strscpy(adap->name, pdev->name, sizeof(adap->name));
+	adap->algo = &atciic100_algo;
+	adap->class = I2C_CLASS_HWMON;
+	adap->dev.parent = dev;
+	adap->dev.of_node = dev->of_node;
+	adap->owner = THIS_MODULE;
+	adap->quirks = &atciic100_quirks;
+	adap->retries = 1;
+	adap->timeout = ATCIIC100_TIMEOUT;
+
+	i2c_set_adapdata(adap, i2c);
+	platform_set_drvdata(pdev, i2c);
+	ret = devm_i2c_add_adapter(dev, adap);
+	if (ret)
+		return dev_err_probe(dev, ret, "i2c add adapter failed\n");
+
+	/* Get FIFO size */
+	i2c->fifo_size = 2 << (readl(i2c->base + ATCIIC100_CFG_REG) &
+			       ATCIIC100_CFG_FIFOSIZE);
+
+	/* Configure ATCIIC100 as a controller and enable ATCIIC100 */
+	val = readl(i2c->base + ATCIIC100_SETUP_REG) | ATCIIC100_SETUP_IICEN
+						     | ATCIIC100_SETUP_REQ;
+	writel(val, i2c->base + ATCIIC100_SETUP_REG);
+	return 0;
+}
+
+static void atciic100_remove(struct platform_device *pdev)
+{
+	u32 val;
+	struct atciic100 *i2c = platform_get_drvdata(pdev);
+
+	/* Disable ATCIIC100 */
+	val = readl(i2c->base + ATCIIC100_SETUP_REG) & ~ATCIIC100_SETUP_IICEN;
+	writel(val, i2c->base + ATCIIC100_SETUP_REG);
+
+	i2c_del_adapter(&i2c->adap);
+}
+
+static const struct of_device_id atciic100_dt[] = {
+	{ .compatible = "andestech,atciic100" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, atciic100_dt);
+
+static struct platform_driver atciic100_platform_driver = {
+	.driver		= {
+		.name	= "atciic100",
+		.of_match_table	= of_match_ptr(atciic100_dt),
+	},
+	.probe		= atciic100_probe,
+	.remove		= atciic100_remove,
+};
+
+/* I2C may be needed to bring up other drivers */
+static int __init atciic100_init_driver(void)
+{
+	return platform_driver_register(&atciic100_platform_driver);
+}
+subsys_initcall(atciic100_init_driver);
+
+static void __exit atciic100_exit_driver(void)
+{
+	platform_driver_unregister(&atciic100_platform_driver);
+}
+module_exit(atciic100_exit_driver);
+
+MODULE_AUTHOR("Ben Zong-You Xie <ben717@andestech.com>");
+MODULE_DESCRIPTION("Andes ATCIIC100 I2C bus adapter");
+MODULE_LICENSE("GPL");
+
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Rob Herring (Arm)]

On Fri, 07 Feb 2025 10:19:22 +0800, Ben Zong-You Xie wrote:
> Document devicetree bindings for Andes I2C controller.
> 
> Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> ---
>  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
>  MAINTAINERS                                   |  5 +++
>  2 files changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.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/andestech,i2c-atciic100.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
 	 $id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml
MAINTAINERS: Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250207021923.2912373-2-ben717@andestech.com

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

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

pip3 install dtschema --upgrade

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

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Ben Zong-You Xie]

On Thu, Feb 06, 2025 at 09:23:44PM -0600, Rob Herring (Arm) wrote:
> [EXTERNAL MAIL]
> 
> On Fri, 07 Feb 2025 10:19:22 +0800, Ben Zong-You Xie wrote:
> > Document devicetree bindings for Andes I2C controller.
> >
> > Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> > ---
> >  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
> >  MAINTAINERS                                   |  5 +++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.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/andestech,i2c-atciic100.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
>          $id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml
>         file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> 
> doc reference errors (make refcheckdocs):
> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml
> MAINTAINERS: Documentation/devicetree/bindings/i2c/andestech,atciic100.yaml
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250207021923.2912373-2-ben717@andestech.com
> 
> 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.
> 

Hi Rob,

Thanks for your reminder. I found there are some errors in the YAML file and
MAINTAINERS file for my patch. I will fix them and check with 'make dt_binding_check'
before sending new patches.

Best regards,
Ben

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Krzysztof Kozlowski]

On Fri, Feb 07, 2025 at 10:19:22AM +0800, Ben Zong-You Xie wrote:
> Document devicetree bindings for Andes I2C controller.

Explain what is the hardware... Here is Andes I2C

> 
> Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> ---
>  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
>  MAINTAINERS                                   |  5 +++
>  2 files changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> new file mode 100644
> index 000000000000..cf96a9186176
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Andes I2C Controller

Here as well

> +
> +maintainers:
> +  - Ben Zong-You Xie <ben717@andestech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: andestech,atciic100

But here atciic100. This is all confusing. What is the SoC? What is the
name of this device?


Best regards,
Krzysztof

Re: [PATCH 2/2] i2c: atciic100: add Andes I2C driver support

[Andi Shyti]

Hi Ben,

I glanced throught the patch and looks OK, just few things with
little importance. Please, do not forget to run checkpatch.pl
before sendint the patch.

Once you send a v2, I will read it more carefully.

...

> +	if (status & ATCIIC100_STATUS_CMPL) {
> +		/* Write 1 to clear all status */
> +		writel(ATCIIC100_STATUS_W1C, i2c->base + ATCIIC100_STATUS_REG);
> +
> +		i2c->xfer_done = true;
> +		if (status & ATCIIC100_STATUS_ADDR_HIT)
> +			i2c->addr_hit = true;
> +
> +		/* For the last read, retrieve all remaining data in FIFO. */
> +		while (i2c->buf_len--)
> +			*i2c->buf++ = readl(i2c->base + ATCIIC100_DATA_REG);
> +

please, remove this blank space here

> +	}
> +
> +	spin_unlock_irqrestore(&i2c->lock, flags);
> +}

...

> +static int atciic100_xfer_wait(struct atciic100 *i2c, struct i2c_msg *msg)
> +{
> +	u32 val;
> +
> +	/*
> +	 * Set the data count.
> +	 * If there are 256 bytes to be transmitted/received, write 0 to the
> +	 * data count field.
> +	 */
> +	val = readl(i2c->base + ATCIIC100_CTRL_REG) |
> +	      (i2c->buf_len & ATCIIC100_CTRL_DATA_CNT);

please checkt the alignment here.

> +	/* Set the transaction direction */
> +	if (msg->flags & I2C_M_RD)
> +		val |= ATCIIC100_CTRL_DIR;
> +	else
> +		val &= ~ATCIIC100_CTRL_DIR;

...

> +/* I2C may be needed to bring up other drivers */
> +static int __init atciic100_init_driver(void)
> +{
> +	return platform_driver_register(&atciic100_platform_driver);
> +}
> +subsys_initcall(atciic100_init_driver);
> +
> +static void __exit atciic100_exit_driver(void)
> +{
> +	platform_driver_unregister(&atciic100_platform_driver);
> +}
> +module_exit(atciic100_exit_driver);

can you please use module_platform_driver()?

Andi

> +
> +MODULE_AUTHOR("Ben Zong-You Xie <ben717@andestech.com>");
> +MODULE_DESCRIPTION("Andes ATCIIC100 I2C bus adapter");
> +MODULE_LICENSE("GPL");
> +
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Ben Zong-You Xie]

On Sun, Feb 09, 2025 at 01:29:58PM +0100, Krzysztof Kozlowski wrote:
> On Fri, Feb 07, 2025 at 10:19:22AM +0800, Ben Zong-You Xie wrote:
> > Document devicetree bindings for Andes I2C controller.
> 
> Explain what is the hardware... Here is Andes I2C
> 
> >
> > Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> > ---
> >  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
> >  MAINTAINERS                                   |  5 +++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> > new file mode 100644
> > index 000000000000..cf96a9186176
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> > @@ -0,0 +1,40 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Andes I2C Controller
> 
> Here as well
> 
> > +
> > +maintainers:
> > +  - Ben Zong-You Xie <ben717@andestech.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: andestech,atciic100
> 
> But here atciic100. This is all confusing. What is the SoC? What is the
> name of this device?

Hi Krzysztof,

Sorry for the confusion. atciic100 is the name for the I2C IP block, and it
is integrated on QiLai SoC. That's why I added a new compatible
"andestech,qilai-i2c" in v2.

For AE350 platform, I know it has not been upstreamed yet, but it was
discussed and acknowledged in a separate SPI series [1], which is why I
included it as a fallback. Can I keep this? If not, I will drop it
and update the compatibles in v3 as follows:

properties:
  compatible:
    const: andestech,qilai-i2c

[1] https://lore.kernel.org/linux-spi/20251210-repeated-perjurer-99219893524a@spud/

Thanks,
Ben

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Krzysztof Kozlowski]

On 22/01/2026 12:18, Ben Zong-You Xie wrote:
> On Sun, Feb 09, 2025 at 01:29:58PM +0100, Krzysztof Kozlowski wrote:
>> On Fri, Feb 07, 2025 at 10:19:22AM +0800, Ben Zong-You Xie wrote:
>>> Document devicetree bindings for Andes I2C controller.
>>
>> Explain what is the hardware... Here is Andes I2C
>>
>>>
>>> Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
>>> ---
>>>  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
>>>  MAINTAINERS                                   |  5 +++
>>>  2 files changed, 45 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
>>> new file mode 100644
>>> index 000000000000..cf96a9186176
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
>>> @@ -0,0 +1,40 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Andes I2C Controller
>>
>> Here as well
>>
>>> +
>>> +maintainers:
>>> +  - Ben Zong-You Xie <ben717@andestech.com>
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/i2c/i2c-controller.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: andestech,atciic100
>>
>> But here atciic100. This is all confusing. What is the SoC? What is the
>> name of this device?
> 
> Hi Krzysztof,
> 
> Sorry for the confusion. atciic100 is the name for the I2C IP block, and it
> is integrated on QiLai SoC. That's why I added a new compatible
> "andestech,qilai-i2c" in v2.

So atciic100 is not an SoC... but then why there is I2C and SPI variant?
Is this some serial engine? Because if it is, you probably miss here
much more bindings for complete hardware description.

Plus, if this is IP block, how can it be used alone? We forbid that sort
of compatibles long time ago.

Anyway you have entire commit msg to explain that.

> 
> For AE350 platform, I know it has not been upstreamed yet, but it was
> discussed and acknowledged in a separate SPI series [1], which is why I

I see ae350-spi there, not atciic100.

> included it as a fallback. Can I keep this? If not, I will drop it
> and update the compatibles in v3 as follows:

Nothing was explained in the commit msg, so with all this being
confusing that's the review you got. You literally wrote one half baked
sentence being copy of subject, so like nothing relevant and should be
treated as almost empty commit msg.

How do you expect us to understand anything from that if you write
NOTHING in the commit msg (except copying subject)?

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: i2c: add atciic100

[Ben Zong-You Xie]

On Thu, Jan 22, 2026 at 12:27:00PM +0100, Krzysztof Kozlowski wrote:
> [EXTERNAL MAIL]
> 
> On 22/01/2026 12:18, Ben Zong-You Xie wrote:
> > On Sun, Feb 09, 2025 at 01:29:58PM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Feb 07, 2025 at 10:19:22AM +0800, Ben Zong-You Xie wrote:
> >>> Document devicetree bindings for Andes I2C controller.
> >>
> >> Explain what is the hardware... Here is Andes I2C
> >>
> >>>
> >>> Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> >>> ---
> >>>  .../bindings/i2c/andestech,i2c-atciic100.yaml | 40 +++++++++++++++++++
> >>>  MAINTAINERS                                   |  5 +++
> >>>  2 files changed, 45 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> >>> new file mode 100644
> >>> index 000000000000..cf96a9186176
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/i2c/andestech,i2c-atciic100.yaml
> >>> @@ -0,0 +1,40 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/pwm/andestech,atciic100.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Andes I2C Controller
> >>
> >> Here as well
> >>
> >>> +
> >>> +maintainers:
> >>> +  - Ben Zong-You Xie <ben717@andestech.com>
> >>> +
> >>> +allOf:
> >>> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: andestech,atciic100
> >>
> >> But here atciic100. This is all confusing. What is the SoC? What is the
> >> name of this device?
> >
> > Hi Krzysztof,
> >
> > Sorry for the confusion. atciic100 is the name for the I2C IP block, and it
> > is integrated on QiLai SoC. That's why I added a new compatible
> > "andestech,qilai-i2c" in v2.
> 
> So atciic100 is not an SoC... but then why there is I2C and SPI variant?
> Is this some serial engine? Because if it is, you probably miss here
> much more bindings for complete hardware description.
> 

There is no variant for atciic100. It's a dedicated I2C controller IP.

> Plus, if this is IP block, how can it be used alone? We forbid that sort
> of compatibles long time ago.
>

I made the mistake you mentioned above in v1. But after your review,
I know the compatibles in the bindings should be <soc/platform>-<device>.
Thus, I have removed "andestech,atciic100" in v2, and have used
"andestech,qilai-i2c" and "andestech,ae350-i2c" instead. Also, I have
removed all the occurrences of atciic100 in v2 to avoid the confusion.

v2: https://lore.kernel.org/linux-i2c/20260122-atciic100-v2-0-7559136d07cf@andestech.com/

> Anyway you have entire commit msg to explain that.
> 

> >
> > For AE350 platform, I know it has not been upstreamed yet, but it was
> > discussed and acknowledged in a separate SPI series [1], which is why I
> 
> I see ae350-spi there, not atciic100.
> 

The reason I mentioned the SPI series is I want to add "ae350-i2c" in
this binding, like "ae350-spi" in the SPI series. Again, could I keep the
compatible "ae350-i2c" as the fallback compatible in this binding? If
not, there will be only one compatible "andestech,qilai-i2c" in the
next version.

> > included it as a fallback. Can I keep this? If not, I will drop it
> > and update the compatibles in v3 as follows:
> 
> Nothing was explained in the commit msg, so with all this being
> confusing that's the review you got. You literally wrote one half baked
> sentence being copy of subject, so like nothing relevant and should be
> treated as almost empty commit msg.
> 
> How do you expect us to understand anything from that if you write
> NOTHING in the commit msg (except copying subject)?
> 

I apologize for the lack of detail, and thank you for your patience.

Thanks,
Ben