[PATCH v3 0/5] i2c: mt7621: improve support for Airoha

[PATCH v3 0/5] i2c: mt7621: improve support for Airoha

[Christian Marangi]

This small series improve support for Airoha SoC that use the
same MT7621 implementation. Some additional tweak are required
to better support it.

Also we add support for atomic variant of .xfer required for
some attached pheriperals on the Airoha SoC.

Changes v3:
- Rebase on top of linux-next
Changes v2:
- Fix compatible order for schema patch

Christian Marangi (5):
  i2c: mt7621: rework cmd/wait OPs to support atomic afer variant
  i2c: mt7621: clear pending interrupt on i2c reset
  dt-bindings: i2c: mt7621: Document an7581 compatible
  i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC
  i2c: mt7621: make device reset optional

Christian Marangi (5):
  i2c: mt7621: rework cmd/wait OPs to support atomic afer variant
  i2c: mt7621: clear pending interrupt on i2c reset
  dt-bindings: i2c: mt7621: Document an7581 compatible
  i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC
  i2c: mt7621: make device reset optional

 .../bindings/i2c/mediatek,mt7621-i2c.yaml     | 14 +++-
 drivers/i2c/busses/i2c-mt7621.c               | 70 +++++++++++++------
 2 files changed, 62 insertions(+), 22 deletions(-)

-- 
2.53.0

[PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant

[Christian Marangi]

It was reported the need for atomic operation on some Airoha SoC that
makes use of I2C bus. Rework the cmd/wait OPs to suppor the xfer_atomic
variant. To support this it's mainlin needed to do the readl poll in
atomic context.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/i2c/busses/i2c-mt7621.c | 56 ++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
index 0a288c998419..700beb9e7b1a 100644
--- a/drivers/i2c/busses/i2c-mt7621.c
+++ b/drivers/i2c/busses/i2c-mt7621.c
@@ -67,14 +67,19 @@ struct mtk_i2c {
 	struct clk *clk;
 };
 
-static int mtk_i2c_wait_idle(struct mtk_i2c *i2c)
+static int mtk_i2c_wait_idle(struct mtk_i2c *i2c, bool atomic)
 {
 	int ret;
 	u32 val;
 
-	ret = readl_relaxed_poll_timeout(i2c->base + REG_SM0CTL1_REG,
-					 val, !(val & SM0CTL1_TRI),
-					 10, TIMEOUT_MS * 1000);
+	if (atomic)
+		ret = readl_relaxed_poll_timeout_atomic(i2c->base + REG_SM0CTL1_REG,
+							val, !(val & SM0CTL1_TRI),
+							10, TIMEOUT_MS * 1000);
+	else
+		ret = readl_relaxed_poll_timeout(i2c->base + REG_SM0CTL1_REG,
+						 val, !(val & SM0CTL1_TRI),
+						 10, TIMEOUT_MS * 1000);
 	if (ret)
 		dev_dbg(i2c->dev, "idle err(%d)\n", ret);
 
@@ -117,27 +122,28 @@ static int mtk_i2c_check_ack(struct mtk_i2c *i2c, u32 expected)
 	return ((ack & ack_expected) == ack_expected) ? 0 : -ENXIO;
 }
 
-static int mtk_i2c_start(struct mtk_i2c *i2c)
+static int mtk_i2c_start(struct mtk_i2c *i2c, bool atomic)
 {
 	iowrite32(SM0CTL1_START | SM0CTL1_TRI, i2c->base + REG_SM0CTL1_REG);
-	return mtk_i2c_wait_idle(i2c);
+	return mtk_i2c_wait_idle(i2c, atomic);
 }
 
-static int mtk_i2c_stop(struct mtk_i2c *i2c)
+static int mtk_i2c_stop(struct mtk_i2c *i2c, bool atomic)
 {
 	iowrite32(SM0CTL1_STOP | SM0CTL1_TRI, i2c->base + REG_SM0CTL1_REG);
-	return mtk_i2c_wait_idle(i2c);
+	return mtk_i2c_wait_idle(i2c, atomic);
 }
 
-static int mtk_i2c_cmd(struct mtk_i2c *i2c, u32 cmd, int page_len)
+static int mtk_i2c_cmd(struct mtk_i2c *i2c, u32 cmd, int page_len,
+		       bool atomic)
 {
 	iowrite32(cmd | SM0CTL1_TRI | SM0CTL1_PGLEN(page_len),
 		  i2c->base + REG_SM0CTL1_REG);
-	return mtk_i2c_wait_idle(i2c);
+	return mtk_i2c_wait_idle(i2c, atomic);
 }
 
-static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
-			int num)
+static int mtk_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			       int num, bool atomic)
 {
 	struct mtk_i2c *i2c;
 	struct i2c_msg *pmsg;
@@ -152,12 +158,12 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		pmsg = &msgs[i];
 
 		/* wait hardware idle */
-		ret = mtk_i2c_wait_idle(i2c);
+		ret = mtk_i2c_wait_idle(i2c, atomic);
 		if (ret)
 			goto err_timeout;
 
 		/* start sequence */
-		ret = mtk_i2c_start(i2c);
+		ret = mtk_i2c_start(i2c, atomic);
 		if (ret)
 			goto err_timeout;
 
@@ -173,7 +179,8 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			len = 1;
 		}
 		iowrite32(addr, i2c->base + REG_SM0D0_REG);
-		ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, len);
+		ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, len,
+				  atomic);
 		if (ret)
 			goto err_timeout;
 
@@ -198,7 +205,7 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 				cmd = SM0CTL1_WRITE;
 			}
 
-			ret = mtk_i2c_cmd(i2c, cmd, page_len);
+			ret = mtk_i2c_cmd(i2c, cmd, page_len, atomic);
 			if (ret)
 				goto err_timeout;
 
@@ -218,7 +225,7 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		}
 	}
 
-	ret = mtk_i2c_stop(i2c);
+	ret = mtk_i2c_stop(i2c, atomic);
 	if (ret)
 		goto err_timeout;
 
@@ -226,7 +233,7 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	return i;
 
 err_ack:
-	ret = mtk_i2c_stop(i2c);
+	ret = mtk_i2c_stop(i2c, atomic);
 	if (ret)
 		goto err_timeout;
 	return -ENXIO;
@@ -237,6 +244,18 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	return ret;
 }
 
+static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			int num)
+{
+	return mtk_i2c_xfer_common(adap, msgs, num, false);
+}
+
+static int mtk_i2c_xfer_atomic(struct i2c_adapter *adap,
+			       struct i2c_msg *msgs, int num)
+{
+	return mtk_i2c_xfer_common(adap, msgs, num, true);
+}
+
 static u32 mtk_i2c_func(struct i2c_adapter *a)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
@@ -244,6 +263,7 @@ static u32 mtk_i2c_func(struct i2c_adapter *a)
 
 static const struct i2c_algorithm mtk_i2c_algo = {
 	.xfer = mtk_i2c_xfer,
+	.xfer_atomic = mtk_i2c_xfer_atomic,
 	.functionality = mtk_i2c_func,
 };
 
-- 
2.53.0

[PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset

[Christian Marangi]

On resetting the i2c bus, clear any pending interrupt to have a more
consistent state on the next operation.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/i2c/busses/i2c-mt7621.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
index 700beb9e7b1a..d8fa29e7e0fa 100644
--- a/drivers/i2c/busses/i2c-mt7621.c
+++ b/drivers/i2c/busses/i2c-mt7621.c
@@ -101,6 +101,8 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
 	iowrite32(((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN |
 		  SM0CTL0_SCL_STRETCH, i2c->base + REG_SM0CTL0_REG);
 	iowrite32(0, i2c->base + REG_SM0CFG2_REG);
+	/* Clear any pending interrupt */
+	iowrite32(1, i2c->base + REG_PINTEN_REG);
 }
 
 static void mtk_i2c_dump_reg(struct mtk_i2c *i2c)
-- 
2.53.0

[PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible

[Christian Marangi]

Airoha SoC implement the same Mediatek logic for I2C bus with the only
difference of not having a dedicated reset line to reset it.

Add a dedicated compatible for the Airoha AN7581 SoC and reject the
unsupported property.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../bindings/i2c/mediatek,mt7621-i2c.yaml          | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
index 118ec00fc190..8223fbc74f14 100644
--- a/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
@@ -14,7 +14,9 @@ allOf:
 
 properties:
   compatible:
-    const: mediatek,mt7621-i2c
+    enum:
+      - airoha,an7581-i2c
+      - mediatek,mt7621-i2c
 
   reg:
     maxItems: 1
@@ -38,6 +40,16 @@ required:
   - "#address-cells"
   - "#size-cells"
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: airoha,an7581-i2c
+then:
+  properties:
+    resets: false
+    reset-names: false
+
 unevaluatedProperties: false
 
 examples:
-- 
2.53.0

[PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC

[Christian Marangi]

The same I2C driver is also used for Airoha SoC with the only difference
that the i2c_reset should not enable SCL_STRETCH for Airoha SoC.

Introduce a new compatible for Airoha and limit the SCL_STRETCH only to
mediatek SoC.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/i2c/busses/i2c-mt7621.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
index d8fa29e7e0fa..3cde43c57a2b 100644
--- a/drivers/i2c/busses/i2c-mt7621.c
+++ b/drivers/i2c/busses/i2c-mt7621.c
@@ -88,6 +88,7 @@ static int mtk_i2c_wait_idle(struct mtk_i2c *i2c, bool atomic)
 
 static void mtk_i2c_reset(struct mtk_i2c *i2c)
 {
+	u32 reg;
 	int ret;
 
 	ret = device_reset(i2c->adap.dev.parent);
@@ -98,8 +99,12 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
 	 * Don't set SM0CTL0_ODRAIN as its bit meaning is inverted. To
 	 * configure open-drain mode, this bit needs to be cleared.
 	 */
-	iowrite32(((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN |
-		  SM0CTL0_SCL_STRETCH, i2c->base + REG_SM0CTL0_REG);
+	reg = ((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN;
+	/* Set SCL_STRETCH only for Mediatek SoC */
+	if (device_is_compatible(i2c->dev, "mediatek,mt7621-i2c"))
+		reg |= SM0CTL0_SCL_STRETCH;
+
+	iowrite32(reg, i2c->base + REG_SM0CTL0_REG);
 	iowrite32(0, i2c->base + REG_SM0CFG2_REG);
 	/* Clear any pending interrupt */
 	iowrite32(1, i2c->base + REG_PINTEN_REG);
@@ -271,6 +276,7 @@ static const struct i2c_algorithm mtk_i2c_algo = {
 
 static const struct of_device_id i2c_mtk_dt_ids[] = {
 	{ .compatible = "mediatek,mt7621-i2c" },
+	{ .compatible = "airoha,an7581-i2c" },
 	{ /* sentinel */ }
 };
 
-- 
2.53.0

[PATCH v3 5/5] i2c: mt7621: make device reset optional

[Christian Marangi]

Airoha SoC that makes use of the same Mediatek I2C driver/logic doesn't
have reset line for I2C so use optional device_reset variant.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/i2c/busses/i2c-mt7621.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
index 3cde43c57a2b..fb9d9701bb10 100644
--- a/drivers/i2c/busses/i2c-mt7621.c
+++ b/drivers/i2c/busses/i2c-mt7621.c
@@ -91,7 +91,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
 	u32 reg;
 	int ret;
 
-	ret = device_reset(i2c->adap.dev.parent);
+	ret = device_reset_optional(i2c->adap.dev.parent);
 	if (ret)
 		dev_err(i2c->dev, "I2C reset failed!\n");
 
-- 
2.53.0

Re: [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The atomic transfer error path incorrectly calls `device_reset()`, which uses mutexes and sleepable functions, leading to a sleeping-in-atomic bug.
- [High] The atomic hardware polling loop spins for up to 1 second, which can cause severe CPU stalls and trigger hardware watchdogs.

Pre-existing issues:
- [High] Uninitialized kernel stack memory is written to the I2C data hardware registers for transfers shorter than 8 bytes.
--

commit 23969b8ae7fc1346d150b2481900eddb6e43c094
Author: Christian Marangi <ansuelsmth@gmail.com>

i2c: mt7621: rework cmd/wait OPs to support atomic afer variant

This commit reworks the command and wait operations in the mt7621 i2c driver
to support atomic transfers. It adds an atomic parameter to the common transfer
path and uses readl_relaxed_poll_timeout_atomic for hardware polling when
executing in an atomic context.

> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 0a288c998419b..700beb9e7b1a8 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -67,14 +67,19 @@ struct mtk_i2c {
>  	struct clk *clk;
>  };
>  
> -static int mtk_i2c_wait_idle(struct mtk_i2c *i2c)
> +static int mtk_i2c_wait_idle(struct mtk_i2c *i2c, bool atomic)
>  {
>  	int ret;
>  	u32 val;
>  
> -	ret = readl_relaxed_poll_timeout(i2c->base + REG_SM0CTL1_REG,
> -					 val, !(val & SM0CTL1_TRI),
> -					 10, TIMEOUT_MS * 1000);
> +	if (atomic)
> +		ret = readl_relaxed_poll_timeout_atomic(i2c->base + REG_SM0CTL1_REG,
> +							val, !(val & SM0CTL1_TRI),
> +							10, TIMEOUT_MS * 1000);

[Severity: High]
Is this timeout too long for an atomic polling loop?

TIMEOUT_MS is defined as 1000, which means this loop will spin using udelay()
for up to 1,000,000 microseconds (1 full second).

Spinning for a full second in an atomic context stops other critical tasks
from executing, which could trigger RCU stall warnings or the hardware
hard-lockup watchdog timer.

[ ... ]

> @@ -198,7 +205,7 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  				cmd = SM0CTL1_WRITE;
>  			}
>  
> -			ret = mtk_i2c_cmd(i2c, cmd, page_len);
> +			ret = mtk_i2c_cmd(i2c, cmd, page_len, atomic);

[Severity: High]
This is a pre-existing issue, but does this write uninitialized kernel stack
memory to the hardware registers?

In mtk_i2c_xfer_common(), the data array is allocated on the stack:

	u32 data[2];

During a write transfer with a page_len less than 8, memcpy() only initializes
the first page_len bytes of the data array. The code then unconditionally
writes the full 8 bytes to the IO registers:

	iowrite32(data[0], i2c->base + REG_SM0D0_REG);
	iowrite32(data[1], i2c->base + REG_SM0D1_REG);

Could this leak uninitialized stack memory to the I2C peripheral?

[ ... ]

> @@ -237,6 +244,18 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	return ret;
>  }
>  
> +static int mtk_i2c_xfer_atomic(struct i2c_adapter *adap,
> +			       struct i2c_msg *msgs, int num)
> +{
> +	return mtk_i2c_xfer_common(adap, msgs, num, true);
> +}

[Severity: High]
Can the error path in this atomic transfer trigger a sleep in atomic context?

If an error occurs in mtk_i2c_xfer_common() (such as a hardware timeout), the
code jumps to err_timeout and resets the device:

mtk_i2c_xfer_atomic()
  mtk_i2c_xfer_common()
    goto err_timeout;

err_timeout:
  mtk_i2c_reset(i2c);
    device_reset(i2c->adap.dev.parent);

The device_reset() function maps to __device_reset(), which calls
__reset_control_get(). This sequence acquires multiple mutexes
(reset_list_mutex and rcdev->lock) and can allocate memory with GFP_KERNEL.

Since mtk_i2c_xfer_atomic() can be executed with IRQs disabled or during a
kernel panic, won't these blocking operations trigger a scheduling while atomic
system panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519223253.1093-1-ansuelsmth@gmail.com?part=1

Re: [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch mistakenly writes to the Interrupt Enable register (`REG_PINTEN_REG`) instead of the Interrupt Clear register (`REG_PINTCL_REG`), which fails to clear the interrupt and can trigger an unhandled interrupt storm.

Pre-existing issues:
- [High] The function `mtk_i2c_reset()` calls `device_reset()`, which may sleep, but it can be called from an atomic context.
--

commit d97627074d52407e96e5474c13276e0a4ad61014
Author: Christian Marangi <ansuelsmth@gmail.com>

i2c: mt7621: clear pending interrupt on i2c reset

This commit attempts to clear pending interrupts during an I2C bus reset
to ensure a consistent state for subsequent operations.

> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 700beb9e7b1a8..d8fa29e7e0fa3 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -101,6 +101,8 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)

[Severity: High]
This is a pre-existing issue, but can this function be safely called from an
atomic context?

Earlier in this patch series, mtk_i2c_xfer_atomic() was introduced, which
can trigger a reset from an atomic context on transfer timeout:

mtk_i2c_xfer_atomic()
  mtk_i2c_xfer_common(..., atomic = true)
    mtk_i2c_reset()
      device_reset()
        __fwnode_reset_control_get()

The __fwnode_reset_control_get() function attempts to acquire a mutex.
Acquiring a mutex in an atomic context is a lock context violation that
can lead to a kernel panic.

>  	iowrite32(((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN |
>  		  SM0CTL0_SCL_STRETCH, i2c->base + REG_SM0CTL0_REG);
>  	iowrite32(0, i2c->base + REG_SM0CFG2_REG);
> +	/* Clear any pending interrupt */
> +	iowrite32(1, i2c->base + REG_PINTEN_REG);

[Severity: High]
Does this code inadvertently unmask the interrupt instead of clearing it?

The comment indicates the intent is to clear pending interrupts, but
REG_PINTEN_REG is the interrupt enable register, whereas REG_PINTCL_REG is
typically used to clear interrupts.

Writing to the enable register unmasks the interrupt at the hardware block
level. Since the driver operates entirely in polling mode and does not
register an IRQ handler, asserting the hardware IRQ line can lead to an
unhandled interrupt storm. If the interrupt line is shared and unmasked at
the GIC, the spurious interrupt detector might eventually disable the shared
IRQ line, potentially causing a denial of service for other devices.

Additionally, writing 1 only targets bit 0. To safely clear all pending
interrupts, should a full bitmask be written to the clear register instead?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519223253.1093-1-ansuelsmth@gmail.com?part=2

Re: [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `resets` property is simultaneously required globally and forbidden for `airoha,an7581-i2c`, creating an unfulfillable schema constraint.
--

commit 3adf72aadbf7e46de9e7f59a27a4659de5c618d9
Author: Christian Marangi <ansuelsmth@gmail.com>

dt-bindings: i2c: mt7621: Document an7581 compatible

This commit adds the airoha,an7581-i2c compatible to the mt7621-i2c binding.
It notes that the Airoha SoC lacks a dedicated reset line and adds a
conditional block to reject the resets property for this new compatible.

> diff --git a/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
> --- a/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
[ ... ]
> @@ -38,6 +40,16 @@ required:
>    - "#address-cells"
>    - "#size-cells"
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: airoha,an7581-i2c
> +then:
> +  properties:
> +    resets: false

[Severity: Medium]
Since the resets property is listed in the global required block for this
schema, does setting resets: false here create an unfulfillable constraint
for the airoha,an7581-i2c compatible? 

It appears a device tree node will fail validation if it omits resets due to
the global requirement, and also fail if it includes it due to this conditional
constraint. 

Should resets be removed from the global required list and instead be required
conditionally for the other compatibles?

> +    reset-names: false
> +
>  unevaluatedProperties: false
>  
>  examples:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519223253.1093-1-ansuelsmth@gmail.com?part=3

Re: [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamic evaluation of `device_is_compatible()` in `mtk_i2c_reset()` causes a sleep-in-atomic bug.
- [High] The hardware reset routine acquires a mutex while executing in atomic context.
--

commit 5cf3b78e15e95214810bba784c999fc11f06780f
Author: Christian Marangi <ansuelsmth@gmail.com>

i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC

This commit introduces a new compatible string for Airoha SoCs and limits
the SCL_STRETCH feature to Mediatek devices. It enforces this by checking
the compatible string during the I2C reset routine.

> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index d8fa29e7e0fa3..3cde43c57a2bc 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -88,6 +88,7 @@ static int mtk_i2c_wait_idle(struct mtk_i2c *i2c, bool atomic)
>  
>  static void mtk_i2c_reset(struct mtk_i2c *i2c)
>  {
> +	u32 reg;
>  	int ret;
>  
>  	ret = device_reset(i2c->adap.dev.parent);

[Severity: High]
Does this device_reset() call sleep in atomic context?

When an I2C transfer times out during mtk_i2c_xfer_atomic(), the driver
branches to err_timeout and calls mtk_i2c_reset(). This reset routine calls
device_reset(), which eventually invokes __reset_control_get(). That
function unconditionally acquires the reset_list_mutex.

Acquiring a mutex in atomic context (such as during panic or reboot, when
mtk_i2c_xfer_atomic() is used) triggers a 'schedule while atomic' bug and
causes a kernel panic. Commit "i2c: mt7621: rework cmd/wait OPs to support
atomic afer variant" (Patch 1 of this series) directly exposed this sleeping
reset path to atomic execution contexts.

>  	if (ret)
>  		dev_err(i2c->dev, "I2C reset failed!\n");
>  
> @@ -98,8 +99,12 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
>  	 * Don't set SM0CTL0_ODRAIN as its bit meaning is inverted. To
>  	 * configure open-drain mode, this bit needs to be cleared.
>  	 */
> -	iowrite32(((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN |
> -		  SM0CTL0_SCL_STRETCH, i2c->base + REG_SM0CTL0_REG);
> +	reg = ((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN;
> +	/* Set SCL_STRETCH only for Mediatek SoC */
> +	if (device_is_compatible(i2c->dev, "mediatek,mt7621-i2c"))

[Severity: High]
Does evaluating device_is_compatible() here cause a sleep-in-atomic bug?

The mtk_i2c_reset() function is called from the I2C transfer timeout handler,
which runs in an atomic context when the driver executes mtk_i2c_xfer_atomic().

The device_is_compatible() function internally invokes
fwnode_property_match_string(), which unconditionally allocates memory using
kcalloc() with GFP_KERNEL. Allocating memory with GFP_KERNEL in atomic
context blocks and triggers a 'schedule while atomic' kernel panic.

Should the driver parse the compatible properties once during probe() and
cache the configuration (e.g., as a has_scl_stretch boolean) in struct mtk_i2c
instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519223253.1093-1-ansuelsmth@gmail.com?part=4

Re: [PATCH v3 5/5] i2c: mt7621: make device reset optional

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling `device_reset_optional()` dynamically retrieves the reset control, acquiring mutexes, which causes a sleep-in-atomic panic during atomic I2C transfers.
- [High] Probe deferrals (-EPROBE_DEFER) and fatal reset errors are ignored, causing the driver to initialize unreset hardware.
--

commit 9cb04554e65951eee40dcf78d9e980702bbacb5d
Author: Christian Marangi <ansuelsmth@gmail.com>

i2c: mt7621: make device reset optional

This commit changes the I2C reset logic in the Mediatek I2C driver to use
device_reset_optional() instead of device_reset(). This accommodates Airoha
SoCs that share the driver but lack a dedicated reset line.

> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 3cde43c57a2bc..fb9d9701bb10d 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -91,7 +91,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
>  	u32 reg;
>  	int ret;
>  
> -	ret = device_reset(i2c->adap.dev.parent);
> +	ret = device_reset_optional(i2c->adap.dev.parent);

[Severity: High]
This is a pre-existing issue, but does this dynamic reset lookup cause a
sleep-in-atomic panic during atomic transfers?

If a transaction times out in mtk_i2c_xfer_atomic(), the code branches to
the err_timeout label and calls mtk_i2c_reset(). This function invokes
device_reset_optional(), which dynamically looks up the reset line via
__device_reset() and __fwnode_reset_control_get().

This lookup acquires the reset_list_mutex and performs memory allocations,
which could trigger a scheduling-while-atomic panic since
mtk_i2c_xfer_atomic() runs in an atomic context.

Would it be safer to fetch and cache the reset control at probe time using
devm_reset_control_get_optional_exclusive() and only call
reset_control_reset() during runtime?

>  	if (ret)
>  		dev_err(i2c->dev, "I2C reset failed!\n");

[Severity: High]
This is also a pre-existing issue, but does swallowing the error code here
break probe deferral?

During driver initialization, mtk_i2c_probe() calls mtk_i2c_init(), which
in turn calls mtk_i2c_reset(). If the reset controller is defined in the
device tree but its driver has not yet probed, device_reset_optional()
will return -EPROBE_DEFER.

Since mtk_i2c_reset() returns void and only logs the error, the I2C driver
incorrectly succeeds in probing instead of propagating -EPROBE_DEFER to the
driver core.

Does this leave the hardware in an unpredictable state from the bootloader?

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519223253.1093-1-ansuelsmth@gmail.com?part=5

Re: [PATCH v3 5/5] i2c: mt7621: make device reset optional

[Benjamin Larsson]

Hi.

On 5/20/26 00:32, Christian Marangi wrote:
> Airoha SoC that makes use of the same Mediatek I2C driver/logic doesn't
> have reset line for I2C so use optional device_reset variant.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>   drivers/i2c/busses/i2c-mt7621.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 3cde43c57a2b..fb9d9701bb10 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -91,7 +91,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
>   	u32 reg;
>   	int ret;
>   
> -	ret = device_reset(i2c->adap.dev.parent);
> +	ret = device_reset_optional(i2c->adap.dev.parent);
>   	if (ret)
>   		dev_err(i2c->dev, "I2C reset failed!\n");
>   


Can you elaborate on this one? I get this:

root@XGX-B-00e092000160:~# devmem 0x1fbf8040
0x00C7800C
root@XGX-B-00e092000160:~# devmem 0x1FB00834 32 0x10000
root@XGX-B-00e092000160:~# devmem 0x1fbf8040
[  396.658742] pbus timeout interrupt ERR ADDR=1fbf8040
[  396.663845] CPU: 0 PID: 5622 Comm: sleep Tainted: P  O      5.4.55 #0
[  396.671117] Hardware name: XGX-B (DT)
[  396.674884] Call trace:
[  396.677394]  dump_backtrace+0x0/0x120
[  396.681111]  show_stack+0x14/0x20
[  396.684478]  dump_stack+0xac/0xec
[  396.687900]  bus_timeout_interrupt+0x54/0x70
[  396.692223]  __handle_irq_event_percpu+0x3c/0x140
[  396.696978]  handle_irq_event+0x4c/0xec
[  396.700920]  handle_fasteoi_irq+0xbc/0x21c
[  396.705069]  __handle_domain_irq+0x6c/0xd0
[  396.709218]  gic_handle_irq+0x8c/0x190
[  396.713019]  el1_irq+0xf0/0x1c0
[  396.716217]  __do_softirq+0x98/0x264
[  396.719847]  irq_exit+0x98/0xe0
[  396.723118]  __handle_domain_irq+0x74/0xd0
[  396.727268]  gic_handle_irq+0x8c/0x190
[  396.731070]  el1_irq+0xf0/0x1c0
[  396.734320]  tlb_flush+0xf8/0x260
[  396.737693]  tlb_finish_mmu+0x48/0xe0
[  396.741417]  exit_mmap+0xc0/0x170
[  396.744841]  mmput+0x44/0x120
[  396.747872]  do_exit+0x2b4/0x8ec
[  396.751161]  do_group_exit+0x34/0x9c
[  396.754794]  __wake_up_parent+0x0/0x2c
[  396.758651]  el0_svc_handler+0x8c/0x150
[  396.762545]  el0_svc+0x8/0x208
0xDEADBEEF
root@XGX-B-00e092000160:~# devmem 0x1FB00834 32 0x00000
root@XGX-B-00e092000160:~# devmem 0x1fbf8040
0x0000800C


and

root@XGX-B-00e092000160:~# devmem 0x1fbf8140
0x00318013
root@XGX-B-00e092000160:~# devmem 0x1FB00830 32 0x00040
root@XGX-B-00e092000160:~# devmem 0x1fbf8140
[  611.730070] pbus timeout interrupt ERR ADDR=1fbf8140
[  611.735197] CPU: 0 PID: 2651 Comm: ux-manager Tainted: P    O      
5.4.55 #0
[  611.742925] Hardware name: XGX-B (DT)
[  611.746697] Call trace:
[  611.749222]  dump_backtrace+0x0/0x120
[  611.752960]  show_stack+0x14/0x20
[  611.756424]  dump_stack+0xac/0xec
[  611.759801]  bus_timeout_interrupt+0x54/0x70
[  611.764145]  __handle_irq_event_percpu+0x3c/0x140
[  611.769001]  handle_irq_event+0x4c/0xec
[  611.772912]  handle_fasteoi_irq+0xbc/0x21c
[  611.777077]  __handle_domain_irq+0x6c/0xd0
[  611.781312]  gic_handle_irq+0x8c/0x190
[  611.785117]  el1_irq+0xf0/0x1c0
[  611.788335]  __do_softirq+0x98/0x264
[  611.792032]  irq_exit+0x98/0xe0
[  611.795249]  __handle_domain_irq+0x74/0xd0
[  611.799411]  gic_handle_irq+0x8c/0x190
[  611.803303]  el1_irq+0xf0/0x1c0
[  611.806518]  bgpio_read32+0x4/0x20
[  611.809995]  gpiod_get_value_cansleep+0x44/0x100
[  611.814742]  value_show+0x2c/0x64
[  611.818114]  dev_attr_show+0x1c/0x54
[  611.821760]  sysfs_kf_read+0x54/0xc0
[  611.825396]  kernfs_fop_read+0xac/0x300
[  611.829355]  __vfs_read+0x18/0x3c
[  611.832741]  vfs_read+0xc8/0x150
[  611.836028]  ksys_read+0x58/0xd4
[  611.839369]  __arm64_sys_read+0x18/0x20
[  611.843264]  el0_svc_handler+0x8c/0x150
[  611.847166]  el0_svc+0x8/0x208
0xDEADBEEF
root@XGX-B-00e092000160:~# devmem 0x1FB00830 32 0x00000
root@XGX-B-00e092000160:~# devmem 0x1fbf8140
0x00008000

When I look at the current dts:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/airoha/en7581.dtsi?h=v7.1-rc4#n322

it looks like the resets are just crossed with regards to the nodes.


MvH

Benjamin Larsson