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 <[email protected]> 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/[email protected]?part=10
