[PATCH v3 0/2] Add Qualcomm I2C target controller driver

[PATCH v3 0/2] Add Qualcomm I2C target controller driver

[Viken Dadhaniya]

QDU1000 and related Qualcomm SoCs include a dedicated I2C target
controller that operates exclusively in target mode and is not supported
by the existing Qualcomm I2C master controller drivers (GENI, QUP).

This series adds DT binding and driver support for this IP. The driver
uses the standard Linux I2C slave framework (reg_target/unreg_target,
i2c_slave_event) so any slave backend (e.g. slave-24c02) can be
attached via i2c_slave_register().

The series is structured as follows:

  Patch 1: DT binding document
  Patch 2: Driver implementation including Kconfig, Makefile and
           MAINTAINERS entries

The driver has been tested on QDU1000 hardware.

Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
Changes in v3:
- Rename file to match compatible (qcom,qdu1000-i2c-target.yaml)
- Remove interconnect-names property
- Remove allOf reference to i2c-controller.yaml
- Remove #address-cells and #size-cells from example
- Add minItems: 1 to pinctrl-names
- Move I2C_S_CORE_EN out of hw_init; enable core only in reg_slave
  and hw_reset
- Add usleep_range(10, 20) after SW_RESET before reconfiguring hardware 
- Fix RESTART_DETECTED dispatch order; handle before STRCH_RD and
  Rx data phases
- Use test_and_set_bit in write_requested and handle_strch_rd
- Loop drain_rx_fifo until FIFO is empty to close post-STOP data race
- Disable core before clearing slave pointer in unreg_slave
- Clear target->status on resume
- Switch from devm_clk_get_enabled to devm_clk_get with manual
  clk_prepare_enable
- Remove unbalanced disable_irq from remove
- Update struct kdoc to note I2C_LOCK_ROOT_ADAPTER serialises
  reg/unreg_target
- Link to v2: https://patch.msgid.link/20260802-i2c-qcom-slave-v2-0-27653118fa75@oss.qualcomm.com

Changes in v2:
- Rename driver and binding from "slave" to "target" terminology
- Use SoC-specific compatible string (qcom,qdu1000-i2c-target) instead of
  generic qcom,i2c-slave; add allOf/$ref to i2c-controller.yaml
- Replace SMBus layer (smbus_xfer, I2C_FUNC_SMBUS_*) with native Linux I2C
  slave framework (reg_target/unreg_target, I2C_SLAVE_* events)
- Remove qcom,slave-addr DT property; slave address taken from the
  registered i2c_client at reg_target() time
- Remove staging buffers and spinlock; bytes delivered directly to backend
  via i2c_slave_event() per STRCH_RD/RX/STOP event
- Use SET_NOIRQ_SYSTEM_SLEEP_PM_OPS; PM core quiesces IRQs at noirq stage,
  removing need for manual disable_irq/enable_irq in suspend
- Simplify clock-names (xo/ahb) and interconnect-names (i2c)
- Replace icc_enable/icc_disable with icc_set_bw() vote/unvote
- Merge MAINTAINERS entry into the dt-bindings patch
- Link to v1: https://patch.msgid.link/20260628-i2c-qcom-slave-v1-0-8b0a5c01f9f6@oss.qualcomm.com

---
Viken Dadhaniya (2):
      dt-bindings: i2c: Add Qualcomm I2C target controller
      i2c: qcom-target: Add driver for Qualcomm I2C target controller

 .../bindings/i2c/qcom,qdu1000-i2c-target.yaml      |  75 +++
 MAINTAINERS                                        |   9 +
 drivers/i2c/busses/Kconfig                         |  15 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-qcom-target.c               | 588 +++++++++++++++++++++
 5 files changed, 688 insertions(+)
---
base-commit: 415606a7be939835db9b0d6b711887586646346d
change-id: 20260628-i2c-qcom-slave-c382ff4e8691

Best regards,
--  
Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>

[PATCH v3 1/2] dt-bindings: i2c: Add Qualcomm I2C target controller

[Viken Dadhaniya]

QDU1000 and related Qualcomm SoCs include a dedicated I2C target
controller that operates exclusively in target mode. It is a distinct
IP from the Qualcomm I2C master controllers (GENI, QUP) with a
different register interface, so it requires its own binding.

Document the MMIO region, interrupt, XO and AHB clocks, interconnect
path, and optional pinctrl states for the controller.

Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 .../bindings/i2c/qcom,qdu1000-i2c-target.yaml      | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/qcom,qdu1000-i2c-target.yaml b/Documentation/devicetree/bindings/i2c/qcom,qdu1000-i2c-target.yaml
new file mode 100644
index 000000000000..9a6b08f0ab0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,qdu1000-i2c-target.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/qcom,qdu1000-i2c-target.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm I2C Target Controller
+
+maintainers:
+  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+  - Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
+
+description:
+  Dedicated hardware IP found on Qualcomm SoCs that operates exclusively
+  as an I2C target (slave) device on the bus. Supports FIFO (PIO) mode
+  for data transfer.
+
+properties:
+  compatible:
+    enum:
+      - qcom,qdu1000-i2c-target
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: XO reference clock
+      - description: AHB bus clock
+
+  clock-names:
+    items:
+      - const: xo
+      - const: ahb
+
+  interconnects:
+    maxItems: 1
+
+  pinctrl-0: true
+  pinctrl-1: true
+
+  pinctrl-names:
+    minItems: 1
+    items:
+      - const: default
+      - const: sleep
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - interconnects
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,qdu1000-gcc.h>
+    #include <dt-bindings/interconnect/qcom,qdu1000-rpmh.h>
+
+    i2c@88ca000 {
+        compatible = "qcom,qdu1000-i2c-target";
+        reg = <0x88ca000 0x64>;
+        clocks = <&gcc GCC_SM_BUS_XO_CLK>, <&gcc GCC_SM_BUS_AHB_CLK>;
+        clock-names = "xo", "ahb";
+        interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
+        interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SMBUS_CFG 0>;
+    };
+...

-- 
2.34.1

[PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Viken Dadhaniya]

QDU1000 and related Qualcomm SoCs include a dedicated I2C target
controller that operates exclusively in target mode. The existing
Qualcomm I2C controller drivers (GENI, QUP) are master-only and cannot
serve systems where the SoC must respond as an I2C target on the bus.

Register the controller with the Linux I2C slave framework via
i2c_algorithm.reg_target and i2c_algorithm.unreg_target so that any
standard slave backend (e.g. slave-24c02) can be attached at runtime
via i2c_slave_register(). Handle IRQ events for RX FIFO service, clock
stretching during read and write phases, STOP and repeated-start
conditions, and error recovery with SW reset. Enable the required AHB
and XO clocks, vote for interconnect bandwidth, and restore hardware
state across suspend and resume using the noirq PM callbacks.

Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 MAINTAINERS                          |   9 +
 drivers/i2c/busses/Kconfig           |  15 +
 drivers/i2c/busses/Makefile          |   1 +
 drivers/i2c/busses/i2c-qcom-target.c | 588 +++++++++++++++++++++++++++++++++++
 4 files changed, 613 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fe67f7bfa44c..7a2d66385e36 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22380,6 +22380,15 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
 F:	drivers/i2c/busses/i2c-qcom-geni.c
 
+QUALCOMM I2C TARGET CONTROLLER DRIVER
+M:	Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
+M:	Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+L:	linux-i2c@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/qcom,qdu1000-i2c-target.yaml
+F:	drivers/i2c/busses/i2c-qcom-target.c
+
 QUALCOMM I2C CCI DRIVER
 M:	Loic Poulain <loic.poulain@oss.qualcomm.com>
 M:	Robert Foss <rfoss@kernel.org>
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d7b89508311f..2e18238e0ae6 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1070,6 +1070,21 @@ config I2C_QCOM_GENI
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-qcom-geni.
 
+config I2C_QCOM_TARGET
+	tristate "Qualcomm I2C target controller"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on COMMON_CLK
+	depends on INTERCONNECT
+	select I2C_SLAVE
+	help
+	  This driver supports I2C target mode on Qualcomm Technologies
+	  SoCs. If you say yes to this option, support will be included
+	  for the built-in I2C target controller on QDU1000 and other
+	  compatible Qualcomm SoCs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called i2c-qcom-target.
+
 config I2C_QUP
 	tristate "Qualcomm QUP based I2C controller"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3755c54b3d82..ab3070cdb144 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_I2C_PXA)		+= i2c-pxa.o
 obj-$(CONFIG_I2C_PXA_PCI)	+= i2c-pxa-pci.o
 obj-$(CONFIG_I2C_QCOM_CCI)	+= i2c-qcom-cci.o
 obj-$(CONFIG_I2C_QCOM_GENI)	+= i2c-qcom-geni.o
+obj-$(CONFIG_I2C_QCOM_TARGET)	+= i2c-qcom-target.o
 obj-$(CONFIG_I2C_QUP)		+= i2c-qup.o
 obj-$(CONFIG_I2C_RIIC)		+= i2c-riic.o
 obj-$(CONFIG_I2C_RK3X)		+= i2c-rk3x.o
diff --git a/drivers/i2c/busses/i2c-qcom-target.c b/drivers/i2c/busses/i2c-qcom-target.c
new file mode 100644
index 000000000000..277ea944eedb
--- /dev/null
+++ b/drivers/i2c/busses/i2c-qcom-target.c
@@ -0,0 +1,588 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interconnect.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+/* Register offsets */
+#define I2C_S_DEVICE_ADDR			0x00
+#define I2C_S_IRQ_STATUS			0x08
+#define I2C_S_IRQ_CLR				0x0C
+#define I2C_S_IRQ_EN				0x10
+#define I2C_S_CONFIG				0x18
+#define I2C_S_CONTROL				0x1C
+#define I2C_S_FIFOS_STATUS			0x20
+#define I2C_S_TX_FIFO				0x24
+#define I2C_S_RX_FIFO				0x28
+#define I2C_S_DEBUG_REG1			0x3C
+#define I2C_S_DEBUG_REG2			0x40
+#define I2C_S_SW_RESET_REG			0x4C
+#define I2C_S_CLK_LOW_TIMEOUT			0x50
+#define I2C_S_CLK_RELEASE_DELAY_CNT_VAL	0x54
+#define I2C_S_SDA_HOLD_CNT_VAL			0x58
+
+/* I2C_S_CONFIG register fields */
+#define I2C_S_CORE_EN				BIT(0)
+
+/* I2C_S_CONTROL register fields */
+#define CLEAR_RX_FIFO				BIT(0)
+#define CLEAR_TX_FIFO				BIT(1)
+#define NACK					BIT(2)
+#define ACK_RESUME				BIT(3)
+
+/* I2C_S_SW_RESET_REG register fields */
+#define SW_RESET				BIT(0)
+
+/* I2C_S_FIFOS_STATUS register fields */
+#define RX_FIFO_COUNT_MASK			GENMASK(31, 16)
+
+/* Interconnect bandwidth vote in bytes per second */
+#define APPS_PROC_TO_I2C_TARGET_VOTE		1190000
+
+/**
+ * enum qcom_i2c_target_irq - IRQ bit positions in I2C_S_IRQ_STATUS
+ * @STOP_DETECTED:	I2C stop condition detected on the bus
+ * @RX_FIFO_FULL:	receive FIFO has reached capacity
+ * @TX_FIFO_EMPTY:	transmit FIFO is empty
+ * @RX_DATA_AVAIL:	receive data is available in the RX FIFO
+ * @CLOCK_LOW_TIMEOUT:	SCL held low longer than the configured timeout
+ * @STRCH_WR:		clock stretching during a write (Rx) phase
+ * @STRCH_RD:		clock stretching during a read (Tx) phase
+ * @GCA_DETECTED:	general call address detected (not used)
+ * @ERR_CONDITION:	unexpected start or stop bit detected (error)
+ * @RESTART_DETECTED:	repeated start condition detected
+ */
+enum qcom_i2c_target_irq {
+	STOP_DETECTED,
+	RX_FIFO_FULL,
+	TX_FIFO_EMPTY,
+	RX_DATA_AVAIL,
+	CLOCK_LOW_TIMEOUT,
+	STRCH_WR,
+	STRCH_RD,
+	GCA_DETECTED,
+	ERR_CONDITION,
+	RESTART_DETECTED,
+};
+
+/*
+ * TX_FIFO_EMPTY is excluded: this driver fills the TX FIFO one byte at a
+ * time in response to STRCH_RD (clock-stretch during read phase), so
+ * TX_FIFO_EMPTY adds no value and would double the interrupt rate.
+ * GCA (general call address) is unsupported.
+ */
+#define QCOM_I2C_TARGET_ALL_IRQ	(GENMASK(RESTART_DETECTED, STOP_DETECTED) \
+				 & ~(BIT(GCA_DETECTED) | BIT(TX_FIFO_EMPTY)))
+
+/* Bit indices for the status bitmask, used with set_bit/test_bit/clear_bit */
+enum qcom_i2c_target_status {
+	READ_IN_PROGRESS,
+	WRITE_IN_PROGRESS,
+};
+
+/**
+ * struct qcom_i2c_target - Qualcomm I2C target controller private data
+ * @dev:		driver model device node
+ * @base:		base address of HW registers
+ * @adap:		I2C adapter (slave mode)
+ * @ahb_clk:		AHB bus clock
+ * @xo_clk:		XO reference clock
+ * @icc_path:		interconnect bandwidth path
+ * @slave:		currently registered slave backend client; written only
+ *			under disable_irq() in reg_slave()/unreg_slave() so the
+ *			ISR sees a stable pointer for its entire execution
+ * @status:		bitmask of enum qcom_i2c_target_status flags; must be
+ *			unsigned long for set_bit/test_bit/clear_bit; accessed
+ *			only from the ISR and from reg_slave()/unreg_slave()
+ *			under disable_irq(), so no additional lock is needed
+ * @irq:		interrupt line number
+ */
+struct qcom_i2c_target {
+	struct device		*dev;
+	void __iomem		*base;
+	struct i2c_adapter	adap;
+	struct clk		*ahb_clk;
+	struct clk		*xo_clk;
+	struct icc_path		*icc_path;
+	struct i2c_client	*slave;
+	unsigned long		status;
+	int			irq;
+};
+
+static void qcom_i2c_target_dump_regs(struct qcom_i2c_target *target)
+{
+	dev_dbg(target->dev, "I2C_S_DEVICE_ADDR:               0x%x\n",
+		readl_relaxed(target->base + I2C_S_DEVICE_ADDR));
+	dev_dbg(target->dev, "I2C_S_IRQ_STATUS:                0x%x\n",
+		readl_relaxed(target->base + I2C_S_IRQ_STATUS));
+	dev_dbg(target->dev, "I2C_S_CONFIG:                    0x%x\n",
+		readl_relaxed(target->base + I2C_S_CONFIG));
+	dev_dbg(target->dev, "I2C_S_IRQ_EN:                    0x%x\n",
+		readl_relaxed(target->base + I2C_S_IRQ_EN));
+	dev_dbg(target->dev, "I2C_S_FIFOS_STATUS:              0x%x\n",
+		readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
+	dev_dbg(target->dev, "I2C_S_DEBUG_REG1:                0x%x\n",
+		readl_relaxed(target->base + I2C_S_DEBUG_REG1));
+	dev_dbg(target->dev, "I2C_S_DEBUG_REG2:                0x%x\n",
+		readl_relaxed(target->base + I2C_S_DEBUG_REG2));
+	dev_dbg(target->dev, "I2C_S_CLK_LOW_TIMEOUT:           0x%x\n",
+		readl_relaxed(target->base + I2C_S_CLK_LOW_TIMEOUT));
+	dev_dbg(target->dev, "I2C_S_CLK_RELEASE_DELAY_CNT_VAL: 0x%x\n",
+		readl_relaxed(target->base + I2C_S_CLK_RELEASE_DELAY_CNT_VAL));
+	dev_dbg(target->dev, "I2C_S_SDA_HOLD_CNT_VAL:          0x%x\n",
+		readl_relaxed(target->base + I2C_S_SDA_HOLD_CNT_VAL));
+}
+
+static void qcom_i2c_target_hw_init(struct qcom_i2c_target *target)
+{
+	dev_dbg(target->dev, "HW init: resetting FIFOs, enabling IRQs\n");
+	writel(CLEAR_TX_FIFO | CLEAR_RX_FIFO, target->base + I2C_S_CONTROL);
+	writel(QCOM_I2C_TARGET_ALL_IRQ, target->base + I2C_S_IRQ_EN);
+}
+
+static void qcom_i2c_target_write_requested(struct qcom_i2c_target *target)
+{
+	u8 val = 0;
+
+	if (!test_and_set_bit(WRITE_IN_PROGRESS, &target->status)) {
+		dev_dbg(target->dev, "Write phase started\n");
+		i2c_slave_event(target->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+	}
+}
+
+static int qcom_i2c_target_drain_rx_fifo(struct qcom_i2c_target *target)
+{
+	unsigned int rx_count;
+	int ret = 0;
+	u8 val;
+
+	while (!ret) {
+		rx_count = FIELD_GET(RX_FIFO_COUNT_MASK,
+				     readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
+		if (!rx_count)
+			break;
+
+		while (rx_count--) {
+			val = (u8)readl_relaxed(target->base + I2C_S_RX_FIFO);
+			dev_dbg(target->dev, "Data from RX FIFO: 0x%x\n", val);
+			ret = i2c_slave_event(target->slave, I2C_SLAVE_WRITE_RECEIVED, &val);
+			if (ret)
+				break;
+		}
+	}
+
+	return ret;
+}
+
+static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
+{
+	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
+	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
+	       target->base + I2C_S_IRQ_CLR);
+	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);
+	/*
+	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
+	 * Use a conservative delay to allow the reset to finish before
+	 * reconfiguring the controller.
+	 */
+	usleep_range(10, 20);
+	qcom_i2c_target_hw_init(target);
+	writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
+	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
+}
+
+static irqreturn_t qcom_i2c_target_handle_error(struct qcom_i2c_target *target,
+						u32 irq_stat)
+{
+	u8 val = 0;
+
+	if (irq_stat & BIT(ERR_CONDITION))
+		dev_err(target->dev, "Error condition: unexpected Start/Stop bits\n");
+	else
+		dev_err(target->dev, "Clock low timeout\n");
+	qcom_i2c_target_dump_regs(target);
+	qcom_i2c_target_hw_reset(target);
+	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
+	target->status = 0;
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t qcom_i2c_target_handle_stop(struct qcom_i2c_target *target,
+					       u32 irq_stat)
+{
+	u8 val = 0;
+
+	dev_dbg(target->dev, "Stop bit detected\n");
+	/*
+	 * Short-write corner case: WRITE_IN_PROGRESS was never set because
+	 * no RX_DATA_AVAIL or STRCH_WR fired before STOP. Do not call
+	 * qcom_i2c_target_write_requested() here — STOP ends the transaction
+	 * so setting WRITE_IN_PROGRESS now would leave a stale bit after
+	 * target->status is cleared below.
+	 */
+	if (!test_bit(WRITE_IN_PROGRESS, &target->status)) {
+		unsigned int rx_count;
+
+		rx_count = FIELD_GET(RX_FIFO_COUNT_MASK,
+				     readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
+		if (rx_count)
+			i2c_slave_event(target->slave, I2C_SLAVE_WRITE_REQUESTED,
+					&val);
+	}
+	qcom_i2c_target_drain_rx_fifo(target);
+	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
+	target->status = 0;
+	writel(CLEAR_RX_FIFO, target->base + I2C_S_CONTROL);
+	/*
+	 * STOP terminates the transaction, so any other Rx or clock
+	 * stretch bits latched in this same status read have already
+	 * been serviced by the drain above or are now stale. Ack the
+	 * whole word rather than just STOP_DETECTED; clearing only STOP
+	 * would leave those bits set and immediately re-enter the
+	 * handler, which for a co-asserted STRCH_RD would push a stale
+	 * byte into the TX FIFO.
+	 */
+	writel(irq_stat, target->base + I2C_S_IRQ_CLR);
+	return IRQ_HANDLED;
+}
+
+static void qcom_i2c_target_handle_rx_data(struct qcom_i2c_target *target,
+					   u32 rx_irq_bits)
+{
+	int ret;
+
+	dev_dbg(target->dev, "Rx data event (rx_irq_bits=0x%x)\n", rx_irq_bits);
+	qcom_i2c_target_write_requested(target);
+	ret = qcom_i2c_target_drain_rx_fifo(target);
+	if (ret)
+		dev_dbg(target->dev, "Backend requested NACK\n");
+	writel(ret ? NACK | CLEAR_RX_FIFO : ACK_RESUME,
+	       target->base + I2C_S_CONTROL);
+	writel(rx_irq_bits, target->base + I2C_S_IRQ_CLR);
+}
+
+static void qcom_i2c_target_handle_strch_rd(struct qcom_i2c_target *target)
+{
+	enum i2c_slave_event event = I2C_SLAVE_READ_PROCESSED;
+	u8 val = 0;
+
+	dev_dbg(target->dev, "Clock stretching during read (Tx) phase\n");
+	if (!test_and_set_bit(READ_IN_PROGRESS, &target->status)) {
+		/* Repeated-start: master switched direction from write to read */
+		clear_bit(WRITE_IN_PROGRESS, &target->status);
+		event = I2C_SLAVE_READ_REQUESTED;
+	}
+	i2c_slave_event(target->slave, event, &val);
+	dev_dbg(target->dev, "Data to TX FIFO: 0x%x\n", val);
+	writel(val, target->base + I2C_S_TX_FIFO);
+	writel(ACK_RESUME, target->base + I2C_S_CONTROL);
+	writel(BIT(STRCH_RD), target->base + I2C_S_IRQ_CLR);
+}
+
+static irqreturn_t qcom_i2c_target_irq(int irq, void *dev)
+{
+	struct qcom_i2c_target *target = dev;
+	u32 irq_stat, rx_bits;
+
+	/*
+	 * Dispatch priority (highest first):
+	 *   ERR_CONDITION / CLOCK_LOW_TIMEOUT  — hardware error, triggers SW reset
+	 *   STOP_DETECTED                      — end of transaction, clears all state
+	 *   RESTART_DETECTED                   — repeated start, resets state before
+	 *                                        any data phase in the same snapshot
+	 *   STRCH_RD                           — read-phase data supply
+	 *   RX_FIFO_FULL / RX_DATA_AVAIL /
+	 *   STRCH_WR                           — write-phase Rx, coalesced into one drain
+	 */
+	irq_stat = readl_relaxed(target->base + I2C_S_IRQ_STATUS);
+	if (!irq_stat)
+		return IRQ_NONE;
+
+	dev_dbg(target->dev, "IRQ status: 0x%x\n", irq_stat);
+
+	/*
+	 * Load target->slave once. Both reg_slave() and unreg_slave() disable
+	 * the IRQ before writing the pointer, so it cannot change while this
+	 * handler runs. Sub-handlers may dereference target->slave directly.
+	 *
+	 * The core is enabled only in reg_slave() and disabled in unreg_slave(),
+	 * so no bus activity is expected here. Clear and discard any stale IRQ.
+	 */
+	if (!READ_ONCE(target->slave)) {
+		writel(irq_stat, target->base + I2C_S_IRQ_CLR);
+		return IRQ_HANDLED;
+	}
+
+	if (irq_stat & (BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT)))
+		return qcom_i2c_target_handle_error(target, irq_stat);
+
+	if (irq_stat & BIT(STOP_DETECTED))
+		return qcom_i2c_target_handle_stop(target, irq_stat);
+
+	if (irq_stat & BIT(RESTART_DETECTED)) {
+		dev_dbg(target->dev, "Repeated start bit detected\n");
+		target->status = 0;
+		writel(ACK_RESUME, target->base + I2C_S_CONTROL);
+		writel(BIT(RESTART_DETECTED), target->base + I2C_S_IRQ_CLR);
+	}
+
+	if (irq_stat & BIT(STRCH_RD))
+		qcom_i2c_target_handle_strch_rd(target);
+
+	/*
+	 * Coalesce all write-phase Rx bits into a single drain+ACK. When the
+	 * RX FIFO fills at threshold, RX_FIFO_FULL, RX_DATA_AVAIL and STRCH_WR
+	 * can all assert in the same irq_stat snapshot. Pass the combined mask
+	 * so ACK_RESUME is written exactly once and all bits are cleared together.
+	 */
+	rx_bits = irq_stat & (BIT(RX_FIFO_FULL) | BIT(RX_DATA_AVAIL) |
+			      BIT(STRCH_WR));
+	if (rx_bits)
+		qcom_i2c_target_handle_rx_data(target, rx_bits);
+
+	return IRQ_HANDLED;
+}
+
+static int qcom_i2c_target_reg_slave(struct i2c_client *slave)
+{
+	struct qcom_i2c_target *target = i2c_get_adapdata(slave->adapter);
+
+	if (target->slave)
+		return -EBUSY;
+
+	if (slave->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	disable_irq(target->irq);
+	WRITE_ONCE(target->slave, slave);
+	writel(slave->addr, target->base + I2C_S_DEVICE_ADDR);
+	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
+	enable_irq(target->irq);
+
+	return 0;
+}
+
+static int qcom_i2c_target_unreg_slave(struct i2c_client *slave)
+{
+	struct qcom_i2c_target *target = i2c_get_adapdata(slave->adapter);
+
+	if (!target->slave)
+		return -EINVAL;
+	disable_irq(target->irq);
+	writel(0, target->base + I2C_S_CONFIG);
+	WRITE_ONCE(target->slave, NULL);
+	target->status = 0;
+	writel(0, target->base + I2C_S_DEVICE_ADDR);
+	enable_irq(target->irq);
+
+	return 0;
+}
+
+static int qcom_i2c_target_icc_init(struct qcom_i2c_target *target)
+{
+	int ret;
+
+	target->icc_path = devm_of_icc_get(target->dev, NULL);
+	if (IS_ERR(target->icc_path))
+		return dev_err_probe(target->dev, PTR_ERR(target->icc_path),
+				     "failed to get ICC path\n");
+
+	/*
+	 * The controller only needs interconnect bandwidth while it is
+	 * powered. The vote is placed here and across resume with
+	 * icc_set_bw(), and dropped with icc_set_bw(path, 0, 0) on suspend
+	 * and remove, so the vote and unvote are always symmetric.
+	 */
+	ret = icc_set_bw(target->icc_path, APPS_PROC_TO_I2C_TARGET_VOTE,
+			 APPS_PROC_TO_I2C_TARGET_VOTE);
+	if (ret)
+		return dev_err_probe(target->dev, ret, "icc_set_bw failed\n");
+
+	return 0;
+}
+
+static u32 qcom_i2c_target_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SLAVE;
+}
+
+static const struct i2c_algorithm qcom_i2c_target_algo = {
+	.reg_target	= qcom_i2c_target_reg_slave,
+	.unreg_target	= qcom_i2c_target_unreg_slave,
+	.functionality	= qcom_i2c_target_func,
+};
+
+static int qcom_i2c_target_adap_init(struct qcom_i2c_target *target)
+{
+	target->adap.algo = &qcom_i2c_target_algo;
+	target->adap.dev.parent = target->dev;
+	target->adap.dev.of_node = target->dev->of_node;
+	strscpy(target->adap.name, "qcom-i2c-target",
+		sizeof(target->adap.name));
+	i2c_set_adapdata(&target->adap, target);
+
+	return i2c_add_adapter(&target->adap);
+}
+
+static int qcom_i2c_target_probe(struct platform_device *pdev)
+{
+	struct qcom_i2c_target *target;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	target = devm_kzalloc(dev, sizeof(*target), GFP_KERNEL);
+	if (!target)
+		return -ENOMEM;
+
+	target->dev = dev;
+
+	target->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(target->base))
+		return dev_err_probe(dev, PTR_ERR(target->base),
+				     "failed to map registers\n");
+
+	target->xo_clk = devm_clk_get(dev, "xo");
+	if (IS_ERR(target->xo_clk))
+		return dev_err_probe(dev, PTR_ERR(target->xo_clk),
+				     "failed to get XO clock\n");
+
+	target->ahb_clk = devm_clk_get(dev, "ahb");
+	if (IS_ERR(target->ahb_clk))
+		return dev_err_probe(dev, PTR_ERR(target->ahb_clk),
+				     "failed to get AHB clock\n");
+
+	ret = clk_prepare_enable(target->xo_clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
+
+	ret = clk_prepare_enable(target->ahb_clk);
+	if (ret) {
+		clk_disable_unprepare(target->xo_clk);
+		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
+	}
+
+	target->irq = platform_get_irq(pdev, 0);
+	if (target->irq < 0)
+		return target->irq;
+
+	ret = qcom_i2c_target_icc_init(target);
+	if (ret)
+		return ret;
+
+	ret = devm_request_irq(dev, target->irq, qcom_i2c_target_irq, 0,
+			       dev_name(dev), target);
+	if (ret)
+		return dev_err_probe(dev, ret, "request_irq failed for IRQ %d\n",
+				     target->irq);
+
+	qcom_i2c_target_hw_init(target);
+
+	platform_set_drvdata(pdev, target);
+
+	ret = qcom_i2c_target_adap_init(target);
+	if (ret)
+		return dev_err_probe(dev, ret, "i2c_add_adapter failed\n");
+
+	return 0;
+}
+
+static void qcom_i2c_target_remove(struct platform_device *pdev)
+{
+	struct qcom_i2c_target *target = platform_get_drvdata(pdev);
+
+	writel(0, target->base + I2C_S_CONFIG);
+	i2c_del_adapter(&target->adap);
+	icc_set_bw(target->icc_path, 0, 0);
+	clk_disable_unprepare(target->xo_clk);
+	clk_disable_unprepare(target->ahb_clk);
+}
+
+static int qcom_i2c_target_suspend(struct device *dev)
+{
+	struct qcom_i2c_target *target = dev_get_drvdata(dev);
+	int ret;
+
+	ret = icc_set_bw(target->icc_path, 0, 0);
+	if (ret)
+		dev_err(dev, "icc_set_bw failed on suspend: %d\n", ret);
+
+	clk_disable_unprepare(target->xo_clk);
+	clk_disable_unprepare(target->ahb_clk);
+
+	return 0;
+}
+
+static int qcom_i2c_target_resume(struct device *dev)
+{
+	struct qcom_i2c_target *target = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(target->ahb_clk);
+	if (ret) {
+		dev_err(dev, "failed to enable AHB clock\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(target->xo_clk);
+	if (ret) {
+		dev_err(dev, "failed to enable XO clock\n");
+		goto err_disable_ahb;
+	}
+
+	ret = icc_set_bw(target->icc_path, APPS_PROC_TO_I2C_TARGET_VOTE,
+			 APPS_PROC_TO_I2C_TARGET_VOTE);
+	if (ret) {
+		dev_err(dev, "icc_set_bw failed\n");
+		goto err_disable_xo;
+	}
+
+	qcom_i2c_target_hw_init(target);
+	target->status = 0;
+	if (target->slave) {
+		writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
+		writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
+	}
+
+	return 0;
+
+err_disable_xo:
+	clk_disable_unprepare(target->xo_clk);
+err_disable_ahb:
+	clk_disable_unprepare(target->ahb_clk);
+	return ret;
+}
+
+static const struct dev_pm_ops qcom_i2c_target_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_i2c_target_suspend,
+				      qcom_i2c_target_resume)
+};
+
+static const struct of_device_id qcom_i2c_target_dt_match[] = {
+	{ .compatible = "qcom,qdu1000-i2c-target" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_i2c_target_dt_match);
+
+static struct platform_driver qcom_i2c_target_driver = {
+	.driver = {
+		.name		= "qcom-i2c-target",
+		.pm		= &qcom_i2c_target_pm_ops,
+		.of_match_table	= qcom_i2c_target_dt_match,
+	},
+	.probe	= qcom_i2c_target_probe,
+	.remove	= qcom_i2c_target_remove,
+};
+module_platform_driver(qcom_i2c_target_driver);
+
+MODULE_DESCRIPTION("Qualcomm I2C target controller driver");
+MODULE_AUTHOR("Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v3 1/2] dt-bindings: i2c: Add Qualcomm I2C target controller

[Krzysztof Kozlowski]

On Thu, Aug 13, 2026 at 09:22:32PM +0530, Viken Dadhaniya wrote:
> QDU1000 and related Qualcomm SoCs include a dedicated I2C target
> controller that operates exclusively in target mode. It is a distinct
> IP from the Qualcomm I2C master controllers (GENI, QUP) with a
> different register interface, so it requires its own binding.
> 
> Document the MMIO region, interrupt, XO and AHB clocks, interconnect
> path, and optional pinctrl states for the controller.
> 
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---
>  .../bindings/i2c/qcom,qdu1000-i2c-target.yaml      | 75 ++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 

I think comments of Sashiko are wrong.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Konrad Dybcio]

On 8/13/26 5:52 PM, Viken Dadhaniya wrote:
> QDU1000 and related Qualcomm SoCs include a dedicated I2C target
> controller that operates exclusively in target mode. The existing
> Qualcomm I2C controller drivers (GENI, QUP) are master-only and cannot
> serve systems where the SoC must respond as an I2C target on the bus.

[...]

> +	ret = clk_prepare_enable(target->xo_clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
> +
> +	ret = clk_prepare_enable(target->ahb_clk);
> +	if (ret) {
> +		clk_disable_unprepare(target->xo_clk);
> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
> +	}

Does this need an OPP table? Presumably we need a power vote for each bus
frequency

Konrad

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Viken Dadhaniya]

On 8/25/2026 3:11 PM, Konrad Dybcio wrote:
> On 8/13/26 5:52 PM, Viken Dadhaniya wrote:
>> QDU1000 and related Qualcomm SoCs include a dedicated I2C target
>> controller that operates exclusively in target mode. The existing
>> Qualcomm I2C controller drivers (GENI, QUP) are master-only and cannot
>> serve systems where the SoC must respond as an I2C target on the bus.
> 
> [...]
> 
>> +	ret = clk_prepare_enable(target->xo_clk);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
>> +
>> +	ret = clk_prepare_enable(target->ahb_clk);
>> +	if (ret) {
>> +		clk_disable_unprepare(target->xo_clk);
>> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
>> +	}
> 
> Does this need an OPP table? Presumably we need a power vote for each bus
> frequency

The xo and ahb are simple gate clocks with no rate states —
xo runs at a fixed 19.2 MHz and the ahb rate is fabric-managed.
Since this is a target-only controller the master drives bus speed,
so there is nothing for us to scale. OPP is not needed here

> 
> Konrad

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Konrad Dybcio]

On 8/26/26 2:52 PM, Viken Dadhaniya wrote:
> 
> 
> On 8/25/2026 3:11 PM, Konrad Dybcio wrote:
>> On 8/13/26 5:52 PM, Viken Dadhaniya wrote:
>>> QDU1000 and related Qualcomm SoCs include a dedicated I2C target
>>> controller that operates exclusively in target mode. The existing
>>> Qualcomm I2C controller drivers (GENI, QUP) are master-only and cannot
>>> serve systems where the SoC must respond as an I2C target on the bus.
>>
>> [...]
>>
>>> +	ret = clk_prepare_enable(target->xo_clk);
>>> +	if (ret)
>>> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
>>> +
>>> +	ret = clk_prepare_enable(target->ahb_clk);
>>> +	if (ret) {
>>> +		clk_disable_unprepare(target->xo_clk);
>>> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
>>> +	}
>>
>> Does this need an OPP table? Presumably we need a power vote for each bus
>> frequency
> 
> The xo and ahb are simple gate clocks with no rate states —
> xo runs at a fixed 19.2 MHz and the ahb rate is fabric-managed.
> Since this is a target-only controller the master drives bus speed,
> so there is nothing for us to scale. OPP is not needed here

OK, thanks for confirming.

Konrad

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Andi Shyti]

Hi Viken,

just a quick look here.

...

> +static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
> +{
> +	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
> +	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
> +	       target->base + I2C_S_IRQ_CLR);
> +	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);
> +	/*
> +	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
> +	 * Use a conservative delay to allow the reset to finish before
> +	 * reconfiguring the controller.
> +	 */
> +	usleep_range(10, 20);

This is also called in atomic context from the irq handler.
Please avoid using usleep_range(), perhaps you can put this in a
thread.

> +	qcom_i2c_target_hw_init(target);
> +	writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
> +	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
> +}
> +
> +static irqreturn_t qcom_i2c_target_handle_error(struct qcom_i2c_target *target,
> +						u32 irq_stat)
> +{
> +	u8 val = 0;
> +
> +	if (irq_stat & BIT(ERR_CONDITION))
> +		dev_err(target->dev, "Error condition: unexpected Start/Stop bits\n");
> +	else
> +		dev_err(target->dev, "Clock low timeout\n");
> +	qcom_i2c_target_dump_regs(target);
> +	qcom_i2c_target_hw_reset(target);
> +	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
> +	target->status = 0;
> +	return IRQ_HANDLED;
> +}

...

> +static irqreturn_t qcom_i2c_target_irq(int irq, void *dev)
> +{
> +	struct qcom_i2c_target *target = dev;
> +	u32 irq_stat, rx_bits;
> +
> +	/*
> +	 * Dispatch priority (highest first):
> +	 *   ERR_CONDITION / CLOCK_LOW_TIMEOUT  — hardware error, triggers SW reset
> +	 *   STOP_DETECTED                      — end of transaction, clears all state
> +	 *   RESTART_DETECTED                   — repeated start, resets state before
> +	 *                                        any data phase in the same snapshot
> +	 *   STRCH_RD                           — read-phase data supply
> +	 *   RX_FIFO_FULL / RX_DATA_AVAIL /
> +	 *   STRCH_WR                           — write-phase Rx, coalesced into one drain
> +	 */
> +	irq_stat = readl_relaxed(target->base + I2C_S_IRQ_STATUS);
> +	if (!irq_stat)
> +		return IRQ_NONE;
> +
> +	dev_dbg(target->dev, "IRQ status: 0x%x\n", irq_stat);
> +
> +	/*
> +	 * Load target->slave once. Both reg_slave() and unreg_slave() disable
> +	 * the IRQ before writing the pointer, so it cannot change while this
> +	 * handler runs. Sub-handlers may dereference target->slave directly.
> +	 *
> +	 * The core is enabled only in reg_slave() and disabled in unreg_slave(),
> +	 * so no bus activity is expected here. Clear and discard any stale IRQ.
> +	 */
> +	if (!READ_ONCE(target->slave)) {
> +		writel(irq_stat, target->base + I2C_S_IRQ_CLR);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (irq_stat & (BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT)))
> +		return qcom_i2c_target_handle_error(target, irq_stat);
> +
> +	if (irq_stat & BIT(STOP_DETECTED))
> +		return qcom_i2c_target_handle_stop(target, irq_stat);

...

> +	target->xo_clk = devm_clk_get(dev, "xo");
> +	if (IS_ERR(target->xo_clk))
> +		return dev_err_probe(dev, PTR_ERR(target->xo_clk),
> +				     "failed to get XO clock\n");
> +
> +	target->ahb_clk = devm_clk_get(dev, "ahb");
> +	if (IS_ERR(target->ahb_clk))
> +		return dev_err_probe(dev, PTR_ERR(target->ahb_clk),
> +				     "failed to get AHB clock\n");
> +
> +	ret = clk_prepare_enable(target->xo_clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
> +
> +	ret = clk_prepare_enable(target->ahb_clk);
> +	if (ret) {
> +		clk_disable_unprepare(target->xo_clk);
> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
> +	}

The problem is that these clocks are not disabled in the returns
below.

Thanks,
Andi

> +
> +	target->irq = platform_get_irq(pdev, 0);
> +	if (target->irq < 0)
> +		return target->irq;
> +
> +	ret = qcom_i2c_target_icc_init(target);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_request_irq(dev, target->irq, qcom_i2c_target_irq, 0,
> +			       dev_name(dev), target);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "request_irq failed for IRQ %d\n",
> +				     target->irq);
> +
> +	qcom_i2c_target_hw_init(target);
> +
> +	platform_set_drvdata(pdev, target);
> +
> +	ret = qcom_i2c_target_adap_init(target);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "i2c_add_adapter failed\n");
> +
> +	return 0;
> +}

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Mukesh Savaliya]

On 8/13/2026 9:22 PM, Viken Dadhaniya wrote:
> QDU1000 and related Qualcomm SoCs include a dedicated I2C target
> controller that operates exclusively in target mode. The existing
> Qualcomm I2C controller drivers (GENI, QUP) are master-only and cannot
> serve systems where the SoC must respond as an I2C target on the bus.
> 
> Register the controller with the Linux I2C slave framework via
> i2c_algorithm.reg_target and i2c_algorithm.unreg_target so that any
> standard slave backend (e.g. slave-24c02) can be attached at runtime
> via i2c_slave_register(). Handle IRQ events for RX FIFO service, clock
> stretching during read and write phases, STOP and repeated-start
> conditions, and error recovery with SW reset. Enable the required AHB
> and XO clocks, vote for interconnect bandwidth, and restore hardware
> state across suspend and resume using the noirq PM callbacks.
Neat:
Though second paragraph gives good information, but i would suggest to 
keep  new lines to make it easier and simpler.>
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---
>   MAINTAINERS                          |   9 +
>   drivers/i2c/busses/Kconfig           |  15 +
>   drivers/i2c/busses/Makefile          |   1 +
>   drivers/i2c/busses/i2c-qcom-target.c | 588 +++++++++++++++++++++++++++++++++++
>   4 files changed, 613 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe67f7bfa44c..7a2d66385e36 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22380,6 +22380,15 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>   F:	drivers/i2c/busses/i2c-qcom-geni.c
>   
> +QUALCOMM I2C TARGET CONTROLLER DRIVER
> +M:	Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> +M:	Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> +L:	linux-i2c@vger.kernel.org
> +L:	linux-arm-msm@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/i2c/qcom,qdu1000-i2c-target.yaml
> +F:	drivers/i2c/busses/i2c-qcom-target.c
> +
>   QUALCOMM I2C CCI DRIVER
>   M:	Loic Poulain <loic.poulain@oss.qualcomm.com>
>   M:	Robert Foss <rfoss@kernel.org>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index d7b89508311f..2e18238e0ae6 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1070,6 +1070,21 @@ config I2C_QCOM_GENI
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called i2c-qcom-geni.
>   
> +config I2C_QCOM_TARGET
> +	tristate "Qualcomm I2C target controller"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on COMMON_CLK
> +	depends on INTERCONNECT
> +	select I2C_SLAVE
> +	help
> +	  This driver supports I2C target mode on Qualcomm Technologies
> +	  SoCs. If you say yes to this option, support will be included
> +	  for the built-in I2C target controller on QDU1000 and other
> +	  compatible Qualcomm SoCs.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called i2c-qcom-target.
> +
>   config I2C_QUP
>   	tristate "Qualcomm QUP based I2C controller"
>   	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3755c54b3d82..ab3070cdb144 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_I2C_PXA)		+= i2c-pxa.o
>   obj-$(CONFIG_I2C_PXA_PCI)	+= i2c-pxa-pci.o
>   obj-$(CONFIG_I2C_QCOM_CCI)	+= i2c-qcom-cci.o
>   obj-$(CONFIG_I2C_QCOM_GENI)	+= i2c-qcom-geni.o
> +obj-$(CONFIG_I2C_QCOM_TARGET)	+= i2c-qcom-target.o
>   obj-$(CONFIG_I2C_QUP)		+= i2c-qup.o
>   obj-$(CONFIG_I2C_RIIC)		+= i2c-riic.o
>   obj-$(CONFIG_I2C_RK3X)		+= i2c-rk3x.o
> diff --git a/drivers/i2c/busses/i2c-qcom-target.c b/drivers/i2c/busses/i2c-qcom-target.c
> new file mode 100644
> index 000000000000..277ea944eedb
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-qcom-target.c
> @@ -0,0 +1,588 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interconnect.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +/* Register offsets */
> +#define I2C_S_DEVICE_ADDR			0x00
> +#define I2C_S_IRQ_STATUS			0x08
> +#define I2C_S_IRQ_CLR				0x0C
> +#define I2C_S_IRQ_EN				0x10
> +#define I2C_S_CONFIG				0x18
> +#define I2C_S_CONTROL				0x1C
> +#define I2C_S_FIFOS_STATUS			0x20
> +#define I2C_S_TX_FIFO				0x24
> +#define I2C_S_RX_FIFO				0x28
> +#define I2C_S_DEBUG_REG1			0x3C
> +#define I2C_S_DEBUG_REG2			0x40
> +#define I2C_S_SW_RESET_REG			0x4C
> +#define I2C_S_CLK_LOW_TIMEOUT			0x50
> +#define I2C_S_CLK_RELEASE_DELAY_CNT_VAL	0x54
> +#define I2C_S_SDA_HOLD_CNT_VAL			0x58
> +
> +/* I2C_S_CONFIG register fields */
> +#define I2C_S_CORE_EN				BIT(0)
> +
> +/* I2C_S_CONTROL register fields */
> +#define CLEAR_RX_FIFO				BIT(0)
> +#define CLEAR_TX_FIFO				BIT(1)
> +#define NACK					BIT(2)
> +#define ACK_RESUME				BIT(3)
> +
> +/* I2C_S_SW_RESET_REG register fields */
> +#define SW_RESET				BIT(0)
> +
> +/* I2C_S_FIFOS_STATUS register fields */
> +#define RX_FIFO_COUNT_MASK			GENMASK(31, 16)
> +
> +/* Interconnect bandwidth vote in bytes per second */
> +#define APPS_PROC_TO_I2C_TARGET_VOTE		1190000
> +
> +/**
> + * enum qcom_i2c_target_irq - IRQ bit positions in I2C_S_IRQ_STATUS
> + * @STOP_DETECTED:	I2C stop condition detected on the bus
> + * @RX_FIFO_FULL:	receive FIFO has reached capacity
> + * @TX_FIFO_EMPTY:	transmit FIFO is empty
> + * @RX_DATA_AVAIL:	receive data is available in the RX FIFO
> + * @CLOCK_LOW_TIMEOUT:	SCL held low longer than the configured timeout
> + * @STRCH_WR:		clock stretching during a write (Rx) phase
> + * @STRCH_RD:		clock stretching during a read (Tx) phase
> + * @GCA_DETECTED:	general call address detected (not used)
> + * @ERR_CONDITION:	unexpected start or stop bit detected (error)
> + * @RESTART_DETECTED:	repeated start condition detected
> + */
> +enum qcom_i2c_target_irq {
> +	STOP_DETECTED,
> +	RX_FIFO_FULL,
> +	TX_FIFO_EMPTY,
> +	RX_DATA_AVAIL,
> +	CLOCK_LOW_TIMEOUT,
> +	STRCH_WR,
> +	STRCH_RD,
> +	GCA_DETECTED,
> +	ERR_CONDITION,
> +	RESTART_DETECTED,
> +};
> +
> +/*
> + * TX_FIFO_EMPTY is excluded: this driver fills the TX FIFO one byte at a
> + * time in response to STRCH_RD (clock-stretch during read phase), so
> + * TX_FIFO_EMPTY adds no value and would double the interrupt rate.
> + * GCA (general call address) is unsupported.
> + */
> +#define QCOM_I2C_TARGET_ALL_IRQ	(GENMASK(RESTART_DETECTED, STOP_DETECTED) \
> +				 & ~(BIT(GCA_DETECTED) | BIT(TX_FIFO_EMPTY)))
> +
> +/* Bit indices for the status bitmask, used with set_bit/test_bit/clear_bit */
> +enum qcom_i2c_target_status {
> +	READ_IN_PROGRESS,
> +	WRITE_IN_PROGRESS,
> +};
> +
> +/**
> + * struct qcom_i2c_target - Qualcomm I2C target controller private data
> + * @dev:		driver model device node
> + * @base:		base address of HW registers
> + * @adap:		I2C adapter (slave mode)
> + * @ahb_clk:		AHB bus clock
> + * @xo_clk:		XO reference clock
> + * @icc_path:		interconnect bandwidth path
> + * @slave:		currently registered slave backend client; written only
> + *			under disable_irq() in reg_slave()/unreg_slave() so the
> + *			ISR sees a stable pointer for its entire execution
> + * @status:		bitmask of enum qcom_i2c_target_status flags; must be
> + *			unsigned long for set_bit/test_bit/clear_bit; accessed
> + *			only from the ISR and from reg_slave()/unreg_slave()
> + *			under disable_irq(), so no additional lock is needed
> + * @irq:		interrupt line number
> + */
> +struct qcom_i2c_target {
> +	struct device		*dev;
> +	void __iomem		*base;
> +	struct i2c_adapter	adap;
> +	struct clk		*ahb_clk;
> +	struct clk		*xo_clk;
> +	struct icc_path		*icc_path;
> +	struct i2c_client	*slave;
> +	unsigned long		status;
> +	int			irq;
> +};
> +
> +static void qcom_i2c_target_dump_regs(struct qcom_i2c_target *target)
> +{
> +	dev_dbg(target->dev, "I2C_S_DEVICE_ADDR:               0x%x\n",
> +		readl_relaxed(target->base + I2C_S_DEVICE_ADDR));
> +	dev_dbg(target->dev, "I2C_S_IRQ_STATUS:                0x%x\n",
> +		readl_relaxed(target->base + I2C_S_IRQ_STATUS));
> +	dev_dbg(target->dev, "I2C_S_CONFIG:                    0x%x\n",
> +		readl_relaxed(target->base + I2C_S_CONFIG));
> +	dev_dbg(target->dev, "I2C_S_IRQ_EN:                    0x%x\n",
> +		readl_relaxed(target->base + I2C_S_IRQ_EN));
> +	dev_dbg(target->dev, "I2C_S_FIFOS_STATUS:              0x%x\n",
> +		readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
> +	dev_dbg(target->dev, "I2C_S_DEBUG_REG1:                0x%x\n",
> +		readl_relaxed(target->base + I2C_S_DEBUG_REG1));
> +	dev_dbg(target->dev, "I2C_S_DEBUG_REG2:                0x%x\n",
> +		readl_relaxed(target->base + I2C_S_DEBUG_REG2));
> +	dev_dbg(target->dev, "I2C_S_CLK_LOW_TIMEOUT:           0x%x\n",
> +		readl_relaxed(target->base + I2C_S_CLK_LOW_TIMEOUT));
> +	dev_dbg(target->dev, "I2C_S_CLK_RELEASE_DELAY_CNT_VAL: 0x%x\n",
> +		readl_relaxed(target->base + I2C_S_CLK_RELEASE_DELAY_CNT_VAL));
> +	dev_dbg(target->dev, "I2C_S_SDA_HOLD_CNT_VAL:          0x%x\n",
> +		readl_relaxed(target->base + I2C_S_SDA_HOLD_CNT_VAL));
> +}
> +
> +static void qcom_i2c_target_hw_init(struct qcom_i2c_target *target)
> +{
> +	dev_dbg(target->dev, "HW init: resetting FIFOs, enabling IRQs\n");
> +	writel(CLEAR_TX_FIFO | CLEAR_RX_FIFO, target->base + I2C_S_CONTROL);
> +	writel(QCOM_I2C_TARGET_ALL_IRQ, target->base + I2C_S_IRQ_EN);
> +}
> +
> +static void qcom_i2c_target_write_requested(struct qcom_i2c_target *target)
> +{
> +	u8 val = 0;
> +
> +	if (!test_and_set_bit(WRITE_IN_PROGRESS, &target->status)) {
> +		dev_dbg(target->dev, "Write phase started\n");
> +		i2c_slave_event(target->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
> +	}
> +}
> +
> +static int qcom_i2c_target_drain_rx_fifo(struct qcom_i2c_target *target)
> +{
> +	unsigned int rx_count;
> +	int ret = 0;
> +	u8 val;
> +
> +	while (!ret) {
> +		rx_count = FIELD_GET(RX_FIFO_COUNT_MASK,
> +				     readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
> +		if (!rx_count)
> +			break;
> +
> +		while (rx_count--) {
> +			val = (u8)readl_relaxed(target->base + I2C_S_RX_FIFO);
> +			dev_dbg(target->dev, "Data from RX FIFO: 0x%x\n", val);
> +			ret = i2c_slave_event(target->slave, I2C_SLAVE_WRITE_RECEIVED, &val);
> +			if (ret)
> +				break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
> +{
> +	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
> +	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
> +	       target->base + I2C_S_IRQ_CLR);
> +	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);

line space > +	/*
> +	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
> +	 * Use a conservative delay to allow the reset to finish before
> +	 * reconfiguring the controller.
> +	 */
> +	usleep_range(10, 20);
> +	qcom_i2c_target_hw_init(target);
> +	writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
> +	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
> +}
> +
> +static irqreturn_t qcom_i2c_target_handle_error(struct qcom_i2c_target *target,
> +						u32 irq_stat)
> +{
> +	u8 val = 0;
> +
> +	if (irq_stat & BIT(ERR_CONDITION))
> +		dev_err(target->dev, "Error condition: unexpected Start/Stop bits\n");
> +	else
> +		dev_err(target->dev, "Clock low timeout\n");
> +	qcom_i2c_target_dump_regs(target);
> +	qcom_i2c_target_hw_reset(target);
> +	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
> +	target->status = 0;
line space before return> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t qcom_i2c_target_handle_stop(struct qcom_i2c_target *target,
> +					       u32 irq_stat)
> +{
> +	u8 val = 0;
> +
> +	dev_dbg(target->dev, "Stop bit detected\n");
> +	/*
> +	 * Short-write corner case: WRITE_IN_PROGRESS was never set because
> +	 * no RX_DATA_AVAIL or STRCH_WR fired before STOP. Do not call
> +	 * qcom_i2c_target_write_requested() here — STOP ends the transaction
> +	 * so setting WRITE_IN_PROGRESS now would leave a stale bit after
> +	 * target->status is cleared below.
> +	 */
> +	if (!test_bit(WRITE_IN_PROGRESS, &target->status)) {
> +		unsigned int rx_count;
> +
> +		rx_count = FIELD_GET(RX_FIFO_COUNT_MASK,
> +				     readl_relaxed(target->base + I2C_S_FIFOS_STATUS));
> +		if (rx_count)
> +			i2c_slave_event(target->slave, I2C_SLAVE_WRITE_REQUESTED,
> +					&val);
> +	}
minor as cleanup: give a line space > + 
qcom_i2c_target_drain_rx_fifo(target);
> +	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
> +	target->status = 0;
> +	writel(CLEAR_RX_FIFO, target->base + I2C_S_CONTROL);
> +	/*
> +	 * STOP terminates the transaction, so any other Rx or clock
> +	 * stretch bits latched in this same status read have already
> +	 * been serviced by the drain above or are now stale. Ack the
> +	 * whole word rather than just STOP_DETECTED; clearing only STOP
> +	 * would leave those bits set and immediately re-enter the
> +	 * handler, which for a co-asserted STRCH_RD would push a stale
> +	 * byte into the TX FIFO.
> +	 */
> +	writel(irq_stat, target->base + I2C_S_IRQ_CLR);

give a line space before returning.> +	return IRQ_HANDLED;
> +}
> +
> +static void qcom_i2c_target_handle_rx_data(struct qcom_i2c_target *target,
> +					   u32 rx_irq_bits)
> +{
> +	int ret;
> +
> +	dev_dbg(target->dev, "Rx data event (rx_irq_bits=0x%x)\n", rx_irq_bits);
> +	qcom_i2c_target_write_requested(target);
> +	ret = qcom_i2c_target_drain_rx_fifo(target);
> +	if (ret)
> +		dev_dbg(target->dev, "Backend requested NACK\n");
line space for neat> +	writel(ret ? NACK | CLEAR_RX_FIFO : ACK_RESUME,
> +	       target->base + I2C_S_CONTROL);
> +	writel(rx_irq_bits, target->base + I2C_S_IRQ_CLR);
> +}
> +
> +static void qcom_i2c_target_handle_strch_rd(struct qcom_i2c_target *target)
> +{
> +	enum i2c_slave_event event = I2C_SLAVE_READ_PROCESSED;
> +	u8 val = 0;
> +
> +	dev_dbg(target->dev, "Clock stretching during read (Tx) phase\n");
> +	if (!test_and_set_bit(READ_IN_PROGRESS, &target->status)) {
> +		/* Repeated-start: master switched direction from write to read */
> +		clear_bit(WRITE_IN_PROGRESS, &target->status);
> +		event = I2C_SLAVE_READ_REQUESTED;
> +	}
line space> +	i2c_slave_event(target->slave, event, &val);
> +	dev_dbg(target->dev, "Data to TX FIFO: 0x%x\n", val);
> +	writel(val, target->base + I2C_S_TX_FIFO);
> +	writel(ACK_RESUME, target->base + I2C_S_CONTROL);
> +	writel(BIT(STRCH_RD), target->base + I2C_S_IRQ_CLR);
> +}
> +
> +static irqreturn_t qcom_i2c_target_irq(int irq, void *dev)
> +{
> +	struct qcom_i2c_target *target = dev;
> +	u32 irq_stat, rx_bits;
> +
> +	/*
> +	 * Dispatch priority (highest first):
> +	 *   ERR_CONDITION / CLOCK_LOW_TIMEOUT  — hardware error, triggers SW reset
> +	 *   STOP_DETECTED                      — end of transaction, clears all state
> +	 *   RESTART_DETECTED                   — repeated start, resets state before
> +	 *                                        any data phase in the same snapshot
> +	 *   STRCH_RD                           — read-phase data supply
> +	 *   RX_FIFO_FULL / RX_DATA_AVAIL /
> +	 *   STRCH_WR                           — write-phase Rx, coalesced into one drain
> +	 */
> +	irq_stat = readl_relaxed(target->base + I2C_S_IRQ_STATUS);
> +	if (!irq_stat)
> +		return IRQ_NONE;
> +
> +	dev_dbg(target->dev, "IRQ status: 0x%x\n", irq_stat);
> +
> +	/*
> +	 * Load target->slave once. Both reg_slave() and unreg_slave() disable
> +	 * the IRQ before writing the pointer, so it cannot change while this
> +	 * handler runs. Sub-handlers may dereference target->slave directly.
> +	 *
> +	 * The core is enabled only in reg_slave() and disabled in unreg_slave(),
> +	 * so no bus activity is expected here. Clear and discard any stale IRQ.
> +	 */
> +	if (!READ_ONCE(target->slave)) {
> +		writel(irq_stat, target->base + I2C_S_IRQ_CLR);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (irq_stat & (BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT)))
> +		return qcom_i2c_target_handle_error(target, irq_stat);
> +
> +	if (irq_stat & BIT(STOP_DETECTED))
> +		return qcom_i2c_target_handle_stop(target, irq_stat);
> +
> +	if (irq_stat & BIT(RESTART_DETECTED)) {
> +		dev_dbg(target->dev, "Repeated start bit detected\n");
> +		target->status = 0;
> +		writel(ACK_RESUME, target->base + I2C_S_CONTROL);
> +		writel(BIT(RESTART_DETECTED), target->base + I2C_S_IRQ_CLR);
> +	}
> +
> +	if (irq_stat & BIT(STRCH_RD))
> +		qcom_i2c_target_handle_strch_rd(target);
> +
> +	/*
> +	 * Coalesce all write-phase Rx bits into a single drain+ACK. When the
> +	 * RX FIFO fills at threshold, RX_FIFO_FULL, RX_DATA_AVAIL and STRCH_WR
> +	 * can all assert in the same irq_stat snapshot. Pass the combined mask
> +	 * so ACK_RESUME is written exactly once and all bits are cleared together.
> +	 */
> +	rx_bits = irq_stat & (BIT(RX_FIFO_FULL) | BIT(RX_DATA_AVAIL) |
> +			      BIT(STRCH_WR));
> +	if (rx_bits)
> +		qcom_i2c_target_handle_rx_data(target, rx_bits);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int qcom_i2c_target_reg_slave(struct i2c_client *slave)
> +{
> +	struct qcom_i2c_target *target = i2c_get_adapdata(slave->adapter);
> +
> +	if (target->slave)
> +		return -EBUSY;
> +
> +	if (slave->flags & I2C_CLIENT_TEN)
> +		return -EAFNOSUPPORT;
> +
> +	disable_irq(target->irq);
> +	WRITE_ONCE(target->slave, slave);
> +	writel(slave->addr, target->base + I2C_S_DEVICE_ADDR);
> +	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
> +	enable_irq(target->irq);
> +
> +	return 0;
> +}
> +
> +static int qcom_i2c_target_unreg_slave(struct i2c_client *slave)
> +{
> +	struct qcom_i2c_target *target = i2c_get_adapdata(slave->adapter);
> +
> +	if (!target->slave)
> +		return -EINVAL;
minor comment: line space > +	disable_irq(target->irq);
> +	writel(0, target->base + I2C_S_CONFIG);
> +	WRITE_ONCE(target->slave, NULL);
> +	target->status = 0;
> +	writel(0, target->base + I2C_S_DEVICE_ADDR);
> +	enable_irq(target->irq);
> +
> +	return 0;
> +}
> +
> +static int qcom_i2c_target_icc_init(struct qcom_i2c_target *target)
> +{
> +	int ret;
> +
> +	target->icc_path = devm_of_icc_get(target->dev, NULL);
> +	if (IS_ERR(target->icc_path))
> +		return dev_err_probe(target->dev, PTR_ERR(target->icc_path),
> +				     "failed to get ICC path\n");
> +
> +	/*
> +	 * The controller only needs interconnect bandwidth while it is
> +	 * powered. The vote is placed here and across resume with
> +	 * icc_set_bw(), and dropped with icc_set_bw(path, 0, 0) on suspend
> +	 * and remove, so the vote and unvote are always symmetric.
> +	 */
> +	ret = icc_set_bw(target->icc_path, APPS_PROC_TO_I2C_TARGET_VOTE,
> +			 APPS_PROC_TO_I2C_TARGET_VOTE);
> +	if (ret)
> +		return dev_err_probe(target->dev, ret, "icc_set_bw failed\n");
> +
> +	return 0;
> +}
> +
> +static u32 qcom_i2c_target_func(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_SLAVE;
> +}
> +
> +static const struct i2c_algorithm qcom_i2c_target_algo = {
> +	.reg_target	= qcom_i2c_target_reg_slave,
> +	.unreg_target	= qcom_i2c_target_unreg_slave,
> +	.functionality	= qcom_i2c_target_func,
> +};
> +
> +static int qcom_i2c_target_adap_init(struct qcom_i2c_target *target)
> +{
> +	target->adap.algo = &qcom_i2c_target_algo;
> +	target->adap.dev.parent = target->dev;
> +	target->adap.dev.of_node = target->dev->of_node;
> +	strscpy(target->adap.name, "qcom-i2c-target",
> +		sizeof(target->adap.name));
> +	i2c_set_adapdata(&target->adap, target);
> +
> +	return i2c_add_adapter(&target->adap);
> +}
> +
> +static int qcom_i2c_target_probe(struct platform_device *pdev)
> +{
> +	struct qcom_i2c_target *target;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	target = devm_kzalloc(dev, sizeof(*target), GFP_KERNEL);
> +	if (!target)
> +		return -ENOMEM;
> +
> +	target->dev = dev;
> +
> +	target->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(target->base))
> +		return dev_err_probe(dev, PTR_ERR(target->base),
> +				     "failed to map registers\n");
> +
> +	target->xo_clk = devm_clk_get(dev, "xo");
> +	if (IS_ERR(target->xo_clk))
> +		return dev_err_probe(dev, PTR_ERR(target->xo_clk),
> +				     "failed to get XO clock\n");
> +
> +	target->ahb_clk = devm_clk_get(dev, "ahb");
> +	if (IS_ERR(target->ahb_clk))
> +		return dev_err_probe(dev, PTR_ERR(target->ahb_clk),
> +				     "failed to get AHB clock\n");
> +
> +	ret = clk_prepare_enable(target->xo_clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
> +
> +	ret = clk_prepare_enable(target->ahb_clk);
> +	if (ret) {
> +		clk_disable_unprepare(target->xo_clk);
> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
> +	}
> +
> +	target->irq = platform_get_irq(pdev, 0);
> +	if (target->irq < 0)
> +		return target->irq;
> +
> +	ret = qcom_i2c_target_icc_init(target);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_request_irq(dev, target->irq, qcom_i2c_target_irq, 0,
> +			       dev_name(dev), target);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "request_irq failed for IRQ %d\n",
> +				     target->irq);
> +
> +	qcom_i2c_target_hw_init(target);
> +
> +	platform_set_drvdata(pdev, target);
> +
> +	ret = qcom_i2c_target_adap_init(target);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "i2c_add_adapter failed\n");
> +
> +	return 0;
> +}
> +
> +static void qcom_i2c_target_remove(struct platform_device *pdev)
> +{
> +	struct qcom_i2c_target *target = platform_get_drvdata(pdev);
> +
> +	writel(0, target->base + I2C_S_CONFIG);
> +	i2c_del_adapter(&target->adap);
> +	icc_set_bw(target->icc_path, 0, 0);
> +	clk_disable_unprepare(target->xo_clk);
> +	clk_disable_unprepare(target->ahb_clk);
> +}
> +
> +static int qcom_i2c_target_suspend(struct device *dev)
> +{
> +	struct qcom_i2c_target *target = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = icc_set_bw(target->icc_path, 0, 0);
> +	if (ret)
> +		dev_err(dev, "icc_set_bw failed on suspend: %d\n", ret);
> +
> +	clk_disable_unprepare(target->xo_clk);
> +	clk_disable_unprepare(target->ahb_clk);
> +
> +	return 0;
> +}
> +
> +static int qcom_i2c_target_resume(struct device *dev)
> +{
> +	struct qcom_i2c_target *target = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(target->ahb_clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable AHB clock\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(target->xo_clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable XO clock\n");
> +		goto err_disable_ahb;
> +	}
> +
> +	ret = icc_set_bw(target->icc_path, APPS_PROC_TO_I2C_TARGET_VOTE,
> +			 APPS_PROC_TO_I2C_TARGET_VOTE);
> +	if (ret) {
> +		dev_err(dev, "icc_set_bw failed\n");
> +		goto err_disable_xo;
> +	}
> +
> +	qcom_i2c_target_hw_init(target);
> +	target->status = 0;
if the status is 0 here after target init, what about other places ? 
does it need to change status ? OR is it inherent to framework ?

Any comment can be help while changing status ?> +	if (target->slave) {
> +		writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
> +		writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
> +	}
> +
> +	return 0;
> +
> +err_disable_xo:
> +	clk_disable_unprepare(target->xo_clk);
> +err_disable_ahb:
> +	clk_disable_unprepare(target->ahb_clk);
line space before return> +	return ret;
> +}
> +
> +static const struct dev_pm_ops qcom_i2c_target_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_i2c_target_suspend,
> +				      qcom_i2c_target_resume)
> +};
> +

can we also add reason here to add NOIRQ callbacks ? My generic question 
would be why not system PM or Runtime PM callbacks ?
This will help later stage for any futurstic changes.

> +static const struct of_device_id qcom_i2c_target_dt_match[] = {
> +	{ .compatible = "qcom,qdu1000-i2c-target" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_i2c_target_dt_match);
> +
> +static struct platform_driver qcom_i2c_target_driver = {
> +	.driver = {
> +		.name		= "qcom-i2c-target",
> +		.pm		= &qcom_i2c_target_pm_ops,
> +		.of_match_table	= qcom_i2c_target_dt_match,
> +	},
> +	.probe	= qcom_i2c_target_probe,
> +	.remove	= qcom_i2c_target_remove,
> +};
> +module_platform_driver(qcom_i2c_target_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm I2C target controller driver");
> +MODULE_AUTHOR("Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Viken Dadhaniya]

On 8/26/2026 7:27 PM, Andi Shyti wrote:
> Hi Viken,
> 
> just a quick look here.
> 
> ...
> 
>> +static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
>> +{
>> +	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
>> +	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
>> +	       target->base + I2C_S_IRQ_CLR);
>> +	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);
>> +	/*
>> +	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
>> +	 * Use a conservative delay to allow the reset to finish before
>> +	 * reconfiguring the controller.
>> +	 */
>> +	usleep_range(10, 20);
> 
> This is also called in atomic context from the irq handler.
> Please avoid using usleep_range(), perhaps you can put this in a
> thread.
> 

Thanks for the review. The delay is only 20 µs, so we were
thinking of replacing usleep_range() with udelay() instead of
converting to a threaded IRQ, to avoid the added complexity on
all the normal fast paths.

>> +	qcom_i2c_target_hw_init(target);
>> +	writel(target->slave->addr, target->base + I2C_S_DEVICE_ADDR);
>> +	writel(I2C_S_CORE_EN, target->base + I2C_S_CONFIG);
>> +}
>> +
>> +static irqreturn_t qcom_i2c_target_handle_error(struct qcom_i2c_target *target,
>> +						u32 irq_stat)
>> +{
>> +	u8 val = 0;
>> +
>> +	if (irq_stat & BIT(ERR_CONDITION))
>> +		dev_err(target->dev, "Error condition: unexpected Start/Stop bits\n");
>> +	else
>> +		dev_err(target->dev, "Clock low timeout\n");
>> +	qcom_i2c_target_dump_regs(target);
>> +	qcom_i2c_target_hw_reset(target);
>> +	i2c_slave_event(target->slave, I2C_SLAVE_STOP, &val);
>> +	target->status = 0;
>> +	return IRQ_HANDLED;
>> +}
> 
> ...
> 
>> +static irqreturn_t qcom_i2c_target_irq(int irq, void *dev)
>> +{
>> +	struct qcom_i2c_target *target = dev;
>> +	u32 irq_stat, rx_bits;
>> +
>> +	/*
>> +	 * Dispatch priority (highest first):
>> +	 *   ERR_CONDITION / CLOCK_LOW_TIMEOUT  — hardware error, triggers SW reset
>> +	 *   STOP_DETECTED                      — end of transaction, clears all state
>> +	 *   RESTART_DETECTED                   — repeated start, resets state before
>> +	 *                                        any data phase in the same snapshot
>> +	 *   STRCH_RD                           — read-phase data supply
>> +	 *   RX_FIFO_FULL / RX_DATA_AVAIL /
>> +	 *   STRCH_WR                           — write-phase Rx, coalesced into one drain
>> +	 */
>> +	irq_stat = readl_relaxed(target->base + I2C_S_IRQ_STATUS);
>> +	if (!irq_stat)
>> +		return IRQ_NONE;
>> +
>> +	dev_dbg(target->dev, "IRQ status: 0x%x\n", irq_stat);
>> +
>> +	/*
>> +	 * Load target->slave once. Both reg_slave() and unreg_slave() disable
>> +	 * the IRQ before writing the pointer, so it cannot change while this
>> +	 * handler runs. Sub-handlers may dereference target->slave directly.
>> +	 *
>> +	 * The core is enabled only in reg_slave() and disabled in unreg_slave(),
>> +	 * so no bus activity is expected here. Clear and discard any stale IRQ.
>> +	 */
>> +	if (!READ_ONCE(target->slave)) {
>> +		writel(irq_stat, target->base + I2C_S_IRQ_CLR);
>> +		return IRQ_HANDLED;
>> +	}
>> +
>> +	if (irq_stat & (BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT)))
>> +		return qcom_i2c_target_handle_error(target, irq_stat);
>> +
>> +	if (irq_stat & BIT(STOP_DETECTED))
>> +		return qcom_i2c_target_handle_stop(target, irq_stat);
> 
> ...
> 
>> +	target->xo_clk = devm_clk_get(dev, "xo");
>> +	if (IS_ERR(target->xo_clk))
>> +		return dev_err_probe(dev, PTR_ERR(target->xo_clk),
>> +				     "failed to get XO clock\n");
>> +
>> +	target->ahb_clk = devm_clk_get(dev, "ahb");
>> +	if (IS_ERR(target->ahb_clk))
>> +		return dev_err_probe(dev, PTR_ERR(target->ahb_clk),
>> +				     "failed to get AHB clock\n");
>> +
>> +	ret = clk_prepare_enable(target->xo_clk);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to enable XO clock\n");
>> +
>> +	ret = clk_prepare_enable(target->ahb_clk);
>> +	if (ret) {
>> +		clk_disable_unprepare(target->xo_clk);
>> +		return dev_err_probe(dev, ret, "failed to enable AHB clock\n");
>> +	}
> 
> The problem is that these clocks are not disabled in the returns
> below.

Sure, I will add the missing clock cleanup paths in the next revision.
> 
> Thanks,
> Andi
> 
>> +
>> +	target->irq = platform_get_irq(pdev, 0);
>> +	if (target->irq < 0)
>> +		return target->irq;
>> +
>> +	ret = qcom_i2c_target_icc_init(target);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = devm_request_irq(dev, target->irq, qcom_i2c_target_irq, 0,
>> +			       dev_name(dev), target);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "request_irq failed for IRQ %d\n",
>> +				     target->irq);
>> +
>> +	qcom_i2c_target_hw_init(target);
>> +
>> +	platform_set_drvdata(pdev, target);
>> +
>> +	ret = qcom_i2c_target_adap_init(target);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "i2c_add_adapter failed\n");
>> +
>> +	return 0;
>> +}

Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Andi Shyti]

Hi Viken,

On Thu, Aug 27, 2026 at 06:44:21PM +0530, Viken Dadhaniya wrote:
> On 8/26/2026 7:27 PM, Andi Shyti wrote:
> > ...
> > 
> >> +static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
> >> +{
> >> +	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
> >> +	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
> >> +	       target->base + I2C_S_IRQ_CLR);
> >> +	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);
> >> +	/*
> >> +	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
> >> +	 * Use a conservative delay to allow the reset to finish before
> >> +	 * reconfiguring the controller.
> >> +	 */
> >> +	usleep_range(10, 20);
> > 
> > This is also called in atomic context from the irq handler.
> > Please avoid using usleep_range(), perhaps you can put this in a
> > thread.
> > 
> 
> Thanks for the review. The delay is only 20 µs, so we were
> thinking of replacing usleep_range() with udelay() instead of
> converting to a threaded IRQ, to avoid the added complexity on
> all the normal fast paths.

perhaps before sleeping you could check the context you are
executing (with an explaining comment).

Thanks,
Andi