* [PATCH v3 0/5] i2c: mt7621: improve support for Airoha
@ 2026-05-19 22:32 Christian Marangi
2026-05-19 22:32 ` [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant Christian Marangi
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw)
To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
linux-i2c, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek
Cc: 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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi @ 2026-05-19 22:32 ` Christian Marangi 2026-05-19 22:55 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset Christian Marangi ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw) To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek Cc: 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant 2026-05-19 22:32 ` [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant Christian Marangi @ 2026-05-19 22:55 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-05-19 22:55 UTC (permalink / raw) To: Christian Marangi; +Cc: devicetree, conor+dt, robh 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi 2026-05-19 22:32 ` [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant Christian Marangi @ 2026-05-19 22:32 ` Christian Marangi 2026-05-19 23:16 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible Christian Marangi ` (2 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw) To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek Cc: 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset 2026-05-19 22:32 ` [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset Christian Marangi @ 2026-05-19 23:16 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-05-19 23:16 UTC (permalink / raw) To: Christian Marangi; +Cc: devicetree, conor+dt, robh 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi 2026-05-19 22:32 ` [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant Christian Marangi 2026-05-19 22:32 ` [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset Christian Marangi @ 2026-05-19 22:32 ` Christian Marangi 2026-05-19 23:24 ` sashiko-bot 2026-05-20 15:49 ` Conor Dooley 2026-05-19 22:32 ` [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC Christian Marangi 2026-05-19 22:32 ` [PATCH v3 5/5] i2c: mt7621: make device reset optional Christian Marangi 4 siblings, 2 replies; 13+ messages in thread From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw) To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek Cc: 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible 2026-05-19 22:32 ` [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible Christian Marangi @ 2026-05-19 23:24 ` sashiko-bot 2026-05-20 15:49 ` Conor Dooley 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-05-19 23:24 UTC (permalink / raw) To: Christian Marangi; +Cc: devicetree, robh, conor+dt 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible 2026-05-19 22:32 ` [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible Christian Marangi 2026-05-19 23:24 ` sashiko-bot @ 2026-05-20 15:49 ` Conor Dooley 1 sibling, 0 replies; 13+ messages in thread From: Conor Dooley @ 2026-05-20 15:49 UTC (permalink / raw) To: Christian Marangi Cc: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek [-- Attachment #1: Type: text/plain, Size: 1463 bytes --] On Wed, May 20, 2026 at 12:32:45AM +0200, Christian Marangi wrote: > 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 The sashiko report looks valid here. pw-bot: changes-requested > + > unevaluatedProperties: false > > examples: > -- > 2.53.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi ` (2 preceding siblings ...) 2026-05-19 22:32 ` [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible Christian Marangi @ 2026-05-19 22:32 ` Christian Marangi 2026-05-19 23:45 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 5/5] i2c: mt7621: make device reset optional Christian Marangi 4 siblings, 1 reply; 13+ messages in thread From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw) To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek Cc: 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC 2026-05-19 22:32 ` [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC Christian Marangi @ 2026-05-19 23:45 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-05-19 23:45 UTC (permalink / raw) To: Christian Marangi; +Cc: robh, conor+dt, devicetree 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 5/5] i2c: mt7621: make device reset optional 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi ` (3 preceding siblings ...) 2026-05-19 22:32 ` [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC Christian Marangi @ 2026-05-19 22:32 ` Christian Marangi 2026-05-20 0:01 ` sashiko-bot 2026-05-20 9:41 ` Benjamin Larsson 4 siblings, 2 replies; 13+ messages in thread From: Christian Marangi @ 2026-05-19 22:32 UTC (permalink / raw) To: Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek Cc: 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] i2c: mt7621: make device reset optional 2026-05-19 22:32 ` [PATCH v3 5/5] i2c: mt7621: make device reset optional Christian Marangi @ 2026-05-20 0:01 ` sashiko-bot 2026-05-20 9:41 ` Benjamin Larsson 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-05-20 0:01 UTC (permalink / raw) To: Christian Marangi; +Cc: devicetree, robh, conor+dt 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] i2c: mt7621: make device reset optional 2026-05-19 22:32 ` [PATCH v3 5/5] i2c: mt7621: make device reset optional Christian Marangi 2026-05-20 0:01 ` sashiko-bot @ 2026-05-20 9:41 ` Benjamin Larsson 1 sibling, 0 replies; 13+ messages in thread From: Benjamin Larsson @ 2026-05-20 9:41 UTC (permalink / raw) To: Christian Marangi, Stefan Roese, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-05-20 15:49 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-19 22:32 [PATCH v3 0/5] i2c: mt7621: improve support for Airoha Christian Marangi 2026-05-19 22:32 ` [PATCH v3 1/5] i2c: mt7621: rework cmd/wait OPs to support atomic afer variant Christian Marangi 2026-05-19 22:55 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 2/5] i2c: mt7621: clear pending interrupt on i2c reset Christian Marangi 2026-05-19 23:16 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 3/5] dt-bindings: i2c: mt7621: Document an7581 compatible Christian Marangi 2026-05-19 23:24 ` sashiko-bot 2026-05-20 15:49 ` Conor Dooley 2026-05-19 22:32 ` [PATCH v3 4/5] i2c: mt7621: limit SCL_STRETCH only to Mediatek SoC Christian Marangi 2026-05-19 23:45 ` sashiko-bot 2026-05-19 22:32 ` [PATCH v3 5/5] i2c: mt7621: make device reset optional Christian Marangi 2026-05-20 0:01 ` sashiko-bot 2026-05-20 9:41 ` Benjamin Larsson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox