Re: [PATCH v2 10/12] i2c: designware: stop transfer if spurious STOP is detected

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Spurious STOP condition is missed if it occurs before the first byte of read data is received, leading to an interrupt storm and CPU lockup.
--

commit e3d26d6a69ce1efff22a0f3c5508b0815ec71b26
Author: Dmitry Guzman <Dmitry.Guzman@mobileye.com>

i2c: designware: stop transfer if spurious STOP is detected
    
In i2c_dw_process_transfer(), if STOP_DET flag is set while read or
write is in progress, the transfer should be aborted. If this flag is
handled the same interrupt as TX_EMPTY flag, the driver first handles
TX_EMPTY flag and transmits the following messages, and only afterwards
checks STOP_DET flag.

> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 76f28e1696bdc..936b9150754e8 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -651,17 +651,19 @@ static void i2c_dw_process_transfer(struct dw_i2c_dev *dev, unsigned int stat)
>  	if (stat & DW_IC_INTR_RX_FULL)
>  		i2c_dw_read(dev);
>  
> -	if (stat & DW_IC_INTR_TX_EMPTY)
> -		i2c_dw_xfer_msg(dev);
> -
>  	/* Abort if we detect a STOP in the middle of a read or a write */
>  	if ((stat & DW_IC_INTR_STOP_DET) &&
>  	    (dev->status & (STATUS_READ_IN_PROGRESS | STATUS_WRITE_IN_PROGRESS))) {

[Severity: High]
This is a pre-existing issue, but can this logic lead to an interrupt
storm and system hang if a spurious STOP occurs during a read?

If a spurious STOP condition occurs on the bus after a read command
has been fully written to the transmit FIFO, but before enough data is
received to trigger a DW_IC_INTR_RX_FULL interrupt, neither
STATUS_WRITE_IN_PROGRESS nor STATUS_READ_IN_PROGRESS will be set.

This causes the check in i2c_dw_process_transfer() to bypass the spurious
STOP handling.

Furthermore, i2c_dw_read_clear_intrbits() refuses to clear the STOP
hardware interrupt flag under these conditions:

drivers/i2c/busses/i2c-designware-master.c:i2c_dw_read_clear_intrbits() {
    ...
	if ((stat & DW_IC_INTR_STOP_DET) &&
	    ((dev->rx_outstanding == 0) || (stat & DW_IC_INTR_RX_FULL)))
		regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
    ...
}

Because dev->rx_outstanding is greater than zero and DW_IC_INTR_RX_FULL
is not set, the flag is intentionally left uncleared. Since the interrupt
is never cleared or masked, would the interrupt service routine immediately
re-trigger upon return, causing a hard system hang?

>  		dev_err(dev->dev, "spurious STOP detected\n");
>  		dev->rx_outstanding = 0;
>  		dev->msg_err = -EIO;
> +		__i2c_dw_write_intr_mask(dev, 0);
> +		goto tx_aborted;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-i2c-fault-reporting-v2-0-fedeb91792e6@mobileye.com?part=10