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

[sashiko-bot@kernel.org]

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