Re: [PATCH v6 3/3] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

David LaPorte
2026-05-05T23:22:53Z

Hi all,

I don't have Keem Bay hardware, but reviewing this against the existing driver I think this hunk may regress the multi-master support added by commit f4e0ba52a89f ("i2c: designware: Do not complete i2c read without RX_FULL interrupt"):

/* 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))) {
dev_err(dev->dev, "spurious STOP detected\n");
dev->rx_outstanding = 0;
dev->msg_err = -EIO;
}

That commit handles the case where STOP_DET is observed before the final RX_FULL. The new check aborts on that intermediate state.

The sibling TX-side underrun check in i2c_dw_xfer_msg() is gated on !emptyfifo_hold_master; this one isn't. I couldn't find discussion of the interaction in the v1–v6 review history so flagging in case it was an oversight.

A minimal fix matching the TX-side gating:

- /* Abort if we detect a STOP in the middle of a read or a write */
- if ((stat & DW_IC_INTR_STOP_DET) &&
+ /*
+ * Abort if we detect a STOP in the middle of a read or a write on
+ * controllers that do not hold SCL on FIFO underrun.
+ */
+ if (!dev->emptyfifo_hold_master &&
+ (stat & DW_IC_INTR_STOP_DET) &&
(dev->status & (STATUS_READ_IN_PROGRESS | STATUS_WRITE_IN_PROGRESS))) {
dev_err(dev->dev, "spurious STOP detected\n");
dev->rx_outstanding = 0;

Happy to send as a proper patch if that's the right direction, or to be corrected.

Thanks,
David