[PATCH 0/2] i2c: tegra20: add target controller support

[PATCH 0/2] i2c: tegra20: add target controller support

[Marc Dietrich]

Hi,

this series adds I2C target mode support to the Tegra20 I2C controller.
The driver is very simple and only support single byte transfers. The goal
is to remove the NVEC driver from staging with which this implentation was also
tested.
I used a separate driver for the target for maintainability and extensibility
reasons, in case support for more Tegra SOCs will be added. The driver was
coded by use of an LLM with a few adaptions by me.

Marc

Marc Dietrich (2):
  dt-bindings: i2c: Add Tegra20 I2C target controller
  i2c: tegra20: Add driver for Tegra20 I2C target controller instances

 .../bindings/i2c/nvidia,tegra20-i2c.yaml      |  27 +-
 MAINTAINERS                                   |   7 +
 drivers/i2c/busses/Kconfig                    |  14 +
 drivers/i2c/busses/Makefile                   |   1 +
 drivers/i2c/busses/i2c-tegra-target.c         | 372 ++++++++++++++++++
 5 files changed, 417 insertions(+), 4 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-tegra-target.c

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

[Marc Dietrich]

Tegra20 SoCs supports 3 instances of an I2C target controller. They share
ressouces with the I2C controller instances and are introduced here via an
additional compatible.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
 .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 27 ++++++++++++++++---
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
index 51241c1293e3..2d7ee8e8750d 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
@@ -13,11 +13,15 @@ title: NVIDIA Tegra I2C controller driver
 properties:
   compatible:
     oneOf:
-      - description: Tegra20 has 4 generic I2C controller. This can support
-          master and slave mode of I2C communication. The i2c-tegra driver
-          only support master mode of I2C communication. Driver of I2C
-          controller is only compatible with "nvidia,tegra20-i2c".
+      - description: Tegra20 has 3 instances of a generic I2C controller.
+          It supports controller and target mode of I2C communication.
+          The i2c-tegra driver only supports master mode of I2C communication.
+          Driver of I2C controller is only compatible with "nvidia,tegra20-i2c".
         const: nvidia,tegra20-i2c
+      - description: The i2c-tegra-target driver supports the target mode of
+          the generic Tegra20 I2C controller. Driver of the I2C target
+          controller is only compatible with "nvidia,tegra20-i2c-target".
+        const: nvidia,tegra20-i2c-target
       - description: Tegra20 has specific I2C controller called as DVC I2C
           controller. This only support master mode of I2C communication.
           Register interface/offset and interrupts handling are different than
@@ -222,3 +226,18 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
     };
+
+  - |
+    i2c@7000c500 {
+        compatible = "nvidia,tegra20-i2c-target";
+        reg = <0x7000c500 0x100>;
+        interrupts = <0 92 0x04>;
+        clocks = <&tegra_car 67>, <&tegra_car 124>;
+        clock-names = "div-clk", "fast-clk";
+        clock-frequency = <80000>;
+        resets = <&tegra_car 67>;
+        reset-names = "i2c";
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
-- 
2.55.0

[PATCH 2/2] i2c: tegra20: Add driver for Tegra20 I2C target controller instances

[Marc Dietrich]

Tegra20 SoC support 3 instances of the I2C target controller. It shares
ressouces with the I2C controller. Only exclusive operation as an i2c
target is supported for now.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
 MAINTAINERS                           |   7 +
 drivers/i2c/busses/Kconfig            |  14 +
 drivers/i2c/busses/Makefile           |   1 +
 drivers/i2c/busses/i2c-tegra-target.c | 372 ++++++++++++++++++++++++++
 4 files changed, 394 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-tegra-target.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 95cd976a068c..7846aea22c96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26579,6 +26579,13 @@ R:	Dmitry Osipenko <digetx@gmail.com>
 S:	Supported
 F:	drivers/i2c/busses/i2c-tegra.c
 
+TEGRA I2C TARGET DRIVER
+M:	Marc Dietrich <marvin24@gmx.de>
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c-target.yaml
+F:	drivers/i2c/busses/i2c-tegra-target.c
+
 TEGRA IOMMU DRIVERS
 M:	Thierry Reding <thierry.reding@kernel.org>
 R:	Krishna Reddy <vdumpa@nvidia.com>
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d35456994280..4c5b0d15ec8e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1242,6 +1242,20 @@ config I2C_TEGRA_BPMP
 	  of the BPMP firmware, and this driver merely communicates with that
 	  real driver.
 
+config I2C_TEGRA_TARGET
+	tristate "NVIDIA Tegra20 I2C controller in target mode"
+	depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
+	select I2C_SLAVE
+	help
+	  If you say yes to this option, support will be included for the
+	  I2C controller embedded in NVIDIA Tegra20 SoCs when it is used as
+	  an I2C target (slave) on the bus. A controller instance that is
+	  described with the target compatible in the device tree cannot be
+	  used as an I2C controller at the same time.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called i2c-tegra-target.
+
 config I2C_UNIPHIER
 	tristate "UniPhier FIFO-less I2C controller"
 	depends on ARCH_UNIPHIER || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3755c54b3d82..724931114412 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_I2C_SUN6I_P2WI)	+= i2c-sun6i-p2wi.o
 obj-$(CONFIG_I2C_SYNQUACER)	+= i2c-synquacer.o
 obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
 obj-$(CONFIG_I2C_TEGRA_BPMP)	+= i2c-tegra-bpmp.o
+obj-$(CONFIG_I2C_TEGRA_TARGET)	+= i2c-tegra-target.o
 obj-$(CONFIG_I2C_UNIPHIER)	+= i2c-uniphier.o
 obj-$(CONFIG_I2C_UNIPHIER_F)	+= i2c-uniphier-f.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
diff --git a/drivers/i2c/busses/i2c-tegra-target.c b/drivers/i2c/busses/i2c-tegra-target.c
new file mode 100644
index 000000000000..6a8642983ea2
--- /dev/null
+++ b/drivers/i2c/busses/i2c-tegra-target.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for the I2C target controller on Tegra20 SoC
+ *
+ * Copyright (C) 2026 Marc Dietrich
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/reset.h>
+
+#define I2C_CNFG				0x000
+#define I2C_CNFG_DEBOUNCE_CNT			GENMASK(14, 12)
+#define I2C_CNFG_PACKET_MODE_EN			BIT(10)
+#define I2C_CNFG_NEW_MASTER_FSM			BIT(11)
+
+#define I2C_SL_CNFG				0x020
+#define I2C_SL_CNFG_NACK			BIT(1)
+#define I2C_SL_CNFG_NEWSL			BIT(2)
+
+#define I2C_SL_RCVD				0x024
+
+#define I2C_SL_STATUS				0x028
+#define I2C_SL_STATUS_RNW			BIT(1)
+#define I2C_SL_STATUS_RCVD			BIT(2)
+#define I2C_SL_STATUS_SL_IRQ			BIT(3)
+#define I2C_SL_STATUS_END_TRANS			BIT(4)
+
+#define I2C_SL_ADDR1				0x02c
+#define I2C_SL_ADDR1_7BIT_ADDR			GENMASK(6, 0)
+#define I2C_SL_ADDR2				0x030
+
+#define I2C_CLK_MULTIPLIER			8
+
+/*
+ * This delay is used for a Tegra20 specific workaround which requires the
+ * delayed acknowledge of the address byte during read access
+ */
+#define I2C_READ_ACK_DELAY_US			33
+
+/**
+ * struct tegra_i2c_target_dev - per device I2C target context
+ * @dev: device reference
+ * @adapter: core I2C layer adapter information
+ * @clocks: array of I2C controller clocks
+ * @div_clk: clock reference for div clock of I2C controller
+ * @base: ioremapped registers cookie
+ * @irq: IRQ number of the target interrupt
+ * @timings: i2c timings information like bus frequency
+ * @target: registered target client, NULL if none
+ * @nack: bytes of the current transfer are not acknowledged
+ */
+struct tegra_i2c_target_dev {
+	struct device *dev;
+	struct i2c_adapter adapter;
+
+	struct clk_bulk_data clocks[2];
+	struct clk *div_clk;
+
+	void __iomem *base;
+	unsigned int irq;
+
+	struct i2c_timings timings;
+	struct i2c_client *target;
+	bool nack;
+};
+
+static u32 i2c_readl(struct tegra_i2c_target_dev *i2c_dev, unsigned int reg)
+{
+	return readl_relaxed(i2c_dev->base + reg);
+}
+
+static void i2c_writel(struct tegra_i2c_target_dev *i2c_dev, u32 val,
+		       unsigned int reg)
+{
+	writel_relaxed(val, i2c_dev->base + reg);
+
+	/* read back register to make sure that register writes completed */
+	readl_relaxed(i2c_dev->base + reg);
+}
+
+static int tegra_i2c_target_init(struct tegra_i2c_target_dev *i2c_dev)
+{
+	u32 val;
+	int err;
+
+	err = device_reset(i2c_dev->dev);
+	if (err)
+		return err;
+
+	val = I2C_CNFG_NEW_MASTER_FSM | I2C_CNFG_PACKET_MODE_EN |
+	      FIELD_PREP(I2C_CNFG_DEBOUNCE_CNT, 2);
+	i2c_writel(i2c_dev, val, I2C_CNFG);
+
+	err = clk_set_rate(i2c_dev->div_clk,
+			   i2c_dev->timings.bus_freq_hz * I2C_CLK_MULTIPLIER);
+	if (err) {
+		dev_err(i2c_dev->dev, "failed to set div-clk rate: %d\n", err);
+		return err;
+	}
+
+	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG);
+
+	/* only 7-bit target address is supported for now */
+	val = FIELD_PREP(I2C_SL_ADDR1_7BIT_ADDR, i2c_dev->target->addr);
+	i2c_writel(i2c_dev, val, I2C_SL_ADDR1);
+	i2c_writel(i2c_dev, 0, I2C_SL_ADDR2);
+
+	return 0;
+}
+
+static void tegra_i2c_target_deinit(struct tegra_i2c_target_dev *i2c_dev)
+{
+	/* stop acknowledging the own address */
+	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL | I2C_SL_CNFG_NACK, I2C_SL_CNFG);
+}
+
+static void tegra_i2c_target_set_nack(struct tegra_i2c_target_dev *i2c_dev,
+				      bool nack)
+{
+	u32 val = I2C_SL_CNFG_NEWSL;
+
+	if (nack)
+		val |= I2C_SL_CNFG_NACK;
+
+	i2c_dev->nack = nack;
+	i2c_writel(i2c_dev, val, I2C_SL_CNFG);
+}
+
+static irqreturn_t tegra_i2c_target_isr(int irq, void *dev_id)
+{
+	struct tegra_i2c_target_dev *i2c_dev = dev_id;
+	struct i2c_client *target = i2c_dev->target;
+	bool read, new_xfer;
+	u32 status;
+	u8 val = 0xff;
+
+	status = i2c_readl(i2c_dev, I2C_SL_STATUS);
+	if (!(status & I2C_SL_STATUS_SL_IRQ))
+		return IRQ_NONE;
+
+	/* RNW is set if the controller reads from the target */
+	read = status & I2C_SL_STATUS_RNW;
+
+	/* RCVD is set if the interrupt is for the address byte */
+	new_xfer = status & I2C_SL_STATUS_RCVD;
+
+	if (!read) {
+		val = i2c_readl(i2c_dev, I2C_SL_RCVD);
+		if (new_xfer)
+			i2c_writel(i2c_dev, 0, I2C_SL_RCVD);
+	}
+
+	if (status & I2C_SL_STATUS_END_TRANS) {
+		i2c_slave_event(target, I2C_SLAVE_STOP, &val);
+
+		if (i2c_dev->nack)
+			tegra_i2c_target_set_nack(i2c_dev, false);
+
+		/* a new transfer may already have started */
+		if (!new_xfer)
+			return IRQ_HANDLED;
+	}
+
+	if (read) {
+		if (new_xfer) {
+			udelay(I2C_READ_ACK_DELAY_US);
+			i2c_slave_event(target, I2C_SLAVE_READ_REQUESTED, &val);
+		} else {
+			i2c_slave_event(target, I2C_SLAVE_READ_PROCESSED, &val);
+		}
+
+		/* send the byte and release the bus */
+		i2c_writel(i2c_dev, val, I2C_SL_RCVD);
+	} else if (new_xfer) {
+		/* the address phase is always acknowledged */
+		if (i2c_slave_event(target, I2C_SLAVE_WRITE_REQUESTED, &val))
+			tegra_i2c_target_set_nack(i2c_dev, true);
+	} else if (!i2c_dev->nack) {
+		if (i2c_slave_event(target, I2C_SLAVE_WRITE_RECEIVED, &val))
+			tegra_i2c_target_set_nack(i2c_dev, true);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int tegra_i2c_target_reg_target(struct i2c_client *target)
+{
+	struct tegra_i2c_target_dev *i2c_dev = i2c_get_adapdata(target->adapter);
+	int err;
+
+	if (i2c_dev->target)
+		return -EBUSY;
+
+	if (target->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	err = clk_bulk_prepare_enable(ARRAY_SIZE(i2c_dev->clocks),
+				      i2c_dev->clocks);
+	if (err) {
+		dev_err(i2c_dev->dev, "failed to enable clocks: %d\n", err);
+		return err;
+	}
+
+	i2c_dev->target = target;
+	i2c_dev->nack = false;
+
+	err = tegra_i2c_target_init(i2c_dev);
+	if (err) {
+		i2c_dev->target = NULL;
+		clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks),
+					   i2c_dev->clocks);
+		return err;
+	}
+
+	enable_irq(i2c_dev->irq);
+
+	return 0;
+}
+
+static int tegra_i2c_target_unreg_target(struct i2c_client *target)
+{
+	struct tegra_i2c_target_dev *i2c_dev = i2c_get_adapdata(target->adapter);
+
+	WARN_ON(!i2c_dev->target);
+
+	disable_irq(i2c_dev->irq);
+	tegra_i2c_target_deinit(i2c_dev);
+	i2c_dev->target = NULL;
+	clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks), i2c_dev->clocks);
+
+	return 0;
+}
+
+static u32 tegra_i2c_target_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SLAVE;
+}
+
+static const struct i2c_algorithm tegra_i2c_target_algo = {
+	.functionality = tegra_i2c_target_func,
+	.reg_target = tegra_i2c_target_reg_target,
+	.unreg_target = tegra_i2c_target_unreg_target,
+};
+
+static int tegra_i2c_target_probe(struct platform_device *pdev)
+{
+	struct tegra_i2c_target_dev *i2c_dev;
+	int err;
+
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, i2c_dev);
+
+	i2c_dev->dev = &pdev->dev;
+
+	i2c_dev->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(i2c_dev->base))
+		return PTR_ERR(i2c_dev->base);
+
+	err = platform_get_irq(pdev, 0);
+	if (err < 0)
+		return err;
+
+	i2c_dev->irq = err;
+
+	i2c_parse_fw_timings(&pdev->dev, &i2c_dev->timings, true);
+
+	i2c_dev->clocks[0].id = "div-clk";
+	i2c_dev->clocks[1].id = "fast-clk";
+
+	err = devm_clk_bulk_get(&pdev->dev, ARRAY_SIZE(i2c_dev->clocks),
+				i2c_dev->clocks);
+	if (err)
+		return dev_err_probe(&pdev->dev, err,
+				     "failed to get controller clocks\n");
+
+	i2c_dev->div_clk = i2c_dev->clocks[0].clk;
+
+	err = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_target_isr,
+			       IRQF_NO_AUTOEN, dev_name(&pdev->dev), i2c_dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err,
+				     "failed to request IRQ %u\n", i2c_dev->irq);
+
+	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
+	i2c_dev->adapter.dev.of_node = pdev->dev.of_node;
+	i2c_dev->adapter.dev.parent = &pdev->dev;
+	i2c_dev->adapter.owner = THIS_MODULE;
+	i2c_dev->adapter.algo = &tegra_i2c_target_algo;
+	strscpy(i2c_dev->adapter.name, dev_name(&pdev->dev),
+		sizeof(i2c_dev->adapter.name));
+
+	return devm_i2c_add_adapter(&pdev->dev, &i2c_dev->adapter);
+}
+
+static int tegra_i2c_target_suspend(struct device *dev)
+{
+	struct tegra_i2c_target_dev *i2c_dev = dev_get_drvdata(dev);
+
+	if (!i2c_dev->target)
+		return 0;
+
+	disable_irq(i2c_dev->irq);
+	tegra_i2c_target_deinit(i2c_dev);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks), i2c_dev->clocks);
+
+	return 0;
+}
+
+static int tegra_i2c_target_resume(struct device *dev)
+{
+	struct tegra_i2c_target_dev *i2c_dev = dev_get_drvdata(dev);
+	int err;
+
+	if (!i2c_dev->target)
+		return 0;
+
+	err = clk_bulk_prepare_enable(ARRAY_SIZE(i2c_dev->clocks),
+				      i2c_dev->clocks);
+	if (err) {
+		dev_err(dev, "failed to enable clocks: %d\n", err);
+		return err;
+	}
+
+	err = tegra_i2c_target_init(i2c_dev);
+	if (err) {
+		clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks),
+					   i2c_dev->clocks);
+		return err;
+	}
+
+	enable_irq(i2c_dev->irq);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(tegra_i2c_target_pm,
+				tegra_i2c_target_suspend,
+				tegra_i2c_target_resume);
+
+static const struct of_device_id tegra_i2c_target_of_match[] = {
+	{ .compatible = "nvidia,tegra20-i2c-target", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tegra_i2c_target_of_match);
+
+static struct platform_driver tegra_i2c_target_driver = {
+	.probe = tegra_i2c_target_probe,
+	.driver = {
+		.name = "tegra-i2c-target",
+		.of_match_table = tegra_i2c_target_of_match,
+		.pm = pm_sleep_ptr(&tegra_i2c_target_pm),
+	},
+};
+module_platform_driver(tegra_i2c_target_driver);
+
+MODULE_DESCRIPTION("NVIDIA Tegra20 I2C controller driver for target mode");
+MODULE_AUTHOR("Marc Dietrich <marvin24@gmx.de>");
+MODULE_LICENSE("GPL");
-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] dt-bindings: i2c: tegra20: driver-specific compatible for
  target mode
- [Low] dt-bindings: i2c: tegra20: missing clock constraints for target
  compatible
--

commit 1d779eb91a256c3147a2ce9798c783f5f76b1f50
Author: Marc Dietrich <marvin24@gmx.de>

dt-bindings: i2c: Add Tegra20 I2C target controller

This commit adds the "nvidia,tegra20-i2c-target" compatible string and an
example for the Tegra20 I2C target controller to the Tegra20 I2C binding
schema.

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> index 51241c1293e3e..2d7ee8e8750d4 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> @@ -13,11 +13,15 @@ title: NVIDIA Tegra I2C controller driver
>  properties:
>    compatible:
>      oneOf:
> -      - description: Tegra20 has 4 generic I2C controller. This can support
> -          master and slave mode of I2C communication. The i2c-tegra driver
> -          only support master mode of I2C communication. Driver of I2C
> -          controller is only compatible with "nvidia,tegra20-i2c".
> +      - description: Tegra20 has 3 instances of a generic I2C controller.
> +          It supports controller and target mode of I2C communication.
> +          The i2c-tegra driver only supports master mode of I2C communication.
> +          Driver of I2C controller is only compatible with "nvidia,tegra20-i2c".
>          const: nvidia,tegra20-i2c
> +      - description: The i2c-tegra-target driver supports the target mode of
> +          the generic Tegra20 I2C controller. Driver of the I2C target
> +          controller is only compatible with "nvidia,tegra20-i2c-target".
> +        const: nvidia,tegra20-i2c-target

[Severity: Medium]
Should target mode be represented by a separate "nvidia,tegra20-i2c-target"
compatible string?

The three generic Tegra20 I2C controllers (such as i2c@7000c500 in
arch/arm/boot/dts/nvidia/tegra20.dtsi) are the same hardware blocks already
described by "nvidia,tegra20-i2c", with controller and target registers in
the same register aperture.  Defining a second compatible string to bind a
separate i2c-tegra-target driver encodes Linux driver structure and runtime
mode into the Devicetree ABI.  In standard I2C bindings
(i2c-controller.yaml), dual-role controllers keep their hardware compatible
string and select target mode via child nodes marked with
I2C_OWN_SLAVE_ADDRESS in reg.

> @@ -222,3 +226,18 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
>      };
> +
> +  - |
> +    i2c@7000c500 {
> +        compatible = "nvidia,tegra20-i2c-target";
> +        reg = <0x7000c500 0x100>;
> +        interrupts = <0 92 0x04>;
> +        clocks = <&tegra_car 67>, <&tegra_car 124>;
> +        clock-names = "div-clk", "fast-clk";

[Severity: Low]
If "nvidia,tegra20-i2c-target" is kept, should it also be added to the
allOf conditional that constrains clocks and clock-names for Tegra20 and
Tegra30?

Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml {
  ...
  - if:
      properties:
        compatible:
          contains:
            enum:
              - nvidia,tegra20-i2c
              - nvidia,tegra30-i2c
    then:
      properties:
        clocks:
          minItems: 2
        clock-names:
          items:
            - const: div-clk
            - const: fast-clk
}

Without "nvidia,tegra20-i2c-target" in that enum, nodes using the new
compatible fall back to the top-level clocks definition (minItems: 1) and
do not validate the required "div-clk" and "fast-clk" entries.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108212458.275980-1-marvin24@gmx.de?part=1

Re: [PATCH 2/2] i2c: tegra20: Add driver for Tegra20 I2C target controller instances

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] i2c: tegra-target: stale nack state in
  tegra_i2c_target_init() and ISR
- [Medium] i2c: tegra-target: unclocked MMIO and unbalanced IRQ/clocks
  after resume error
- [Low] i2c: tegra-target: broken DT binding file path in MAINTAINERS
--

commit f38185c59a54e5417359880cac9a408a1a024e72
Author: Marc Dietrich <marvin24@gmx.de>

i2c: tegra20: Add driver for Tegra20 I2C target controller instances

This commit adds a driver for the NVIDIA Tegra20 I2C controller operating
exclusively in target (slave) mode.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3ee846e37066d..e543fde8bdae4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27534,6 +27534,13 @@ R:	Dmitry Osipenko <digetx@gmail.com>
>  S:	Supported
>  F:	drivers/i2c/busses/i2c-tegra.c
>  
> +TEGRA I2C TARGET DRIVER
> +M:	Marc Dietrich <marvin24@gmx.de>
> +L:	linux-tegra@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c-target.yaml
> +F:	drivers/i2c/busses/i2c-tegra-target.c

[Severity: Low]
Does Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c-target.yaml
exist?  The nvidia,tegra20-i2c-target compatible string is documented in
Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml, so this F:
entry points to a non-existent path and triggers a broken file pattern
warning in get_maintainer.pl.

> diff --git a/drivers/i2c/busses/i2c-tegra-target.c b/drivers/i2c/busses/i2c-tegra-target.c
> new file mode 100644
> index 0000000000000..6a8642983ea24
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-tegra-target.c

[ ... ]

> +static int tegra_i2c_target_init(struct tegra_i2c_target_dev *i2c_dev)
> +{
> +	u32 val;
> +	int err;
> +
> +	err = device_reset(i2c_dev->dev);
> +	if (err)
> +		return err;
> +
> +	val = I2C_CNFG_NEW_MASTER_FSM | I2C_CNFG_PACKET_MODE_EN |
> +	      FIELD_PREP(I2C_CNFG_DEBOUNCE_CNT, 2);
> +	i2c_writel(i2c_dev, val, I2C_CNFG);
> +
> +	err = clk_set_rate(i2c_dev->div_clk,
> +			   i2c_dev->timings.bus_freq_hz * I2C_CLK_MULTIPLIER);
> +	if (err) {
> +		dev_err(i2c_dev->dev, "failed to set div-clk rate: %d\n", err);
> +		return err;
> +	}
> +
> +	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG);
> +
> +	/* only 7-bit target address is supported for now */
> +	val = FIELD_PREP(I2C_SL_ADDR1_7BIT_ADDR, i2c_dev->target->addr);
> +	i2c_writel(i2c_dev, val, I2C_SL_ADDR1);
> +	i2c_writel(i2c_dev, 0, I2C_SL_ADDR2);
> +
> +	return 0;
> +}

[ ... ]

> +static irqreturn_t tegra_i2c_target_isr(int irq, void *dev_id)
> +{
> +	struct tegra_i2c_target_dev *i2c_dev = dev_id;
> +	struct i2c_client *target = i2c_dev->target;
> +	bool read, new_xfer;
> +	u32 status;
> +	u8 val = 0xff;
> +
> +	status = i2c_readl(i2c_dev, I2C_SL_STATUS);
> +	if (!(status & I2C_SL_STATUS_SL_IRQ))
> +		return IRQ_NONE;
> +
> +	/* RNW is set if the controller reads from the target */
> +	read = status & I2C_SL_STATUS_RNW;
> +
> +	/* RCVD is set if the interrupt is for the address byte */
> +	new_xfer = status & I2C_SL_STATUS_RCVD;
> +
> +	if (!read) {
> +		val = i2c_readl(i2c_dev, I2C_SL_RCVD);
> +		if (new_xfer)
> +			i2c_writel(i2c_dev, 0, I2C_SL_RCVD);
> +	}
> +
> +	if (status & I2C_SL_STATUS_END_TRANS) {
> +		i2c_slave_event(target, I2C_SLAVE_STOP, &val);
> +
> +		if (i2c_dev->nack)
> +			tegra_i2c_target_set_nack(i2c_dev, false);
> +
> +		/* a new transfer may already have started */
> +		if (!new_xfer)
> +			return IRQ_HANDLED;
> +	}
> +
> +	if (read) {
> +		if (new_xfer) {
> +			udelay(I2C_READ_ACK_DELAY_US);
> +			i2c_slave_event(target, I2C_SLAVE_READ_REQUESTED, &val);
> +		} else {
> +			i2c_slave_event(target, I2C_SLAVE_READ_PROCESSED, &val);
> +		}
> +
> +		/* send the byte and release the bus */
> +		i2c_writel(i2c_dev, val, I2C_SL_RCVD);
> +	} else if (new_xfer) {
> +		/* the address phase is always acknowledged */
> +		if (i2c_slave_event(target, I2C_SLAVE_WRITE_REQUESTED, &val))
> +			tegra_i2c_target_set_nack(i2c_dev, true);
> +	} else if (!i2c_dev->nack) {
> +		if (i2c_slave_event(target, I2C_SLAVE_WRITE_RECEIVED, &val))
> +			tegra_i2c_target_set_nack(i2c_dev, true);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

[Severity: Medium]
Can i2c_dev->nack remain stale across a Repeated START or across system
suspend and resume?

In tegra_i2c_target_isr(), tegra_i2c_target_set_nack(i2c_dev, false) is
only called when I2C_SL_STATUS_END_TRANS is set.  If a controller follows
a NACKed write with a Repeated START (where I2C_SL_STATUS_RCVD is set
without I2C_SL_STATUS_END_TRANS), i2c_dev->nack is never cleared, so a
new write transfer remains NACKed and skips I2C_SLAVE_WRITE_RECEIVED even
when I2C_SLAVE_WRITE_REQUESTED succeeds.

Similarly, tegra_i2c_target_init() writes I2C_SL_CNFG_NEWSL to
I2C_SL_CNFG, clearing I2C_SL_CNFG_NACK in hardware without resetting
i2c_dev->nack = false.  If the system suspends while i2c_dev->nack is
true, tegra_i2c_target_resume() -> tegra_i2c_target_init() leaves
hardware ACKing bytes on the bus while tegra_i2c_target_isr() silently
drops them because i2c_dev->nack is still true.

[ ... ]

> +static int tegra_i2c_target_suspend(struct device *dev)
> +{
> +	struct tegra_i2c_target_dev *i2c_dev = dev_get_drvdata(dev);
> +
> +	if (!i2c_dev->target)
> +		return 0;
> +
> +	disable_irq(i2c_dev->irq);
> +	tegra_i2c_target_deinit(i2c_dev);
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks), i2c_dev->clocks);
> +
> +	return 0;
> +}
> +
> +static int tegra_i2c_target_resume(struct device *dev)
> +{
> +	struct tegra_i2c_target_dev *i2c_dev = dev_get_drvdata(dev);
> +	int err;
> +
> +	if (!i2c_dev->target)
> +		return 0;
> +
> +	err = clk_bulk_prepare_enable(ARRAY_SIZE(i2c_dev->clocks),
> +				      i2c_dev->clocks);
> +	if (err) {
> +		dev_err(dev, "failed to enable clocks: %d\n", err);
> +		return err;
> +	}
> +
> +	err = tegra_i2c_target_init(i2c_dev);
> +	if (err) {
> +		clk_bulk_disable_unprepare(ARRAY_SIZE(i2c_dev->clocks),
> +					   i2c_dev->clocks);
> +		return err;
> +	}
> +
> +	enable_irq(i2c_dev->irq);
> +
> +	return 0;
> +}

[Severity: Medium]
What happens in tegra_i2c_target_unreg_target() or a later
tegra_i2c_target_suspend() if tegra_i2c_target_resume() fails?

When clk_bulk_prepare_enable() or tegra_i2c_target_init() fails in
tegra_i2c_target_resume(), the clocks are left disabled and i2c_dev->irq
remains disabled, while i2c_dev->target is still non-NULL.  A subsequent
call to tegra_i2c_target_unreg_target() or tegra_i2c_target_suspend()
will then access I2C_SL_CNFG in tegra_i2c_target_deinit() with the
controller clocks disabled (which hangs the APB bus on Tegra20), call
disable_irq() a second time, and call clk_bulk_disable_unprepare() on
already disabled clocks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108212458.275980-1-marvin24@gmx.de?part=2

Re: [PATCH 2/2] i2c: tegra20: Add driver for Tegra20 I2C target controller instances

[Uwe Kleine-König]

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

Hello Marc,

On Sun, Nov 08, 2026 at 10:24:58PM +0100, Marc Dietrich wrote:
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>

Please don't include <linux/mod_devicetable.h>.
<linux/platform_device.h> is good enough to provide struct of_device_id.
So please just drop <linux/mod_devicetable.h>. (If you insist on full
IWYU, use <linux/device-id/of.h> instead.)

> [...]
> +static const struct of_device_id tegra_i2c_target_of_match[] = {
> +	{ .compatible = "nvidia,tegra20-i2c-target", },

No trailing , before the closing } please.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tegra_i2c_target_of_match);

Best regards
Uwe

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