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

[David LaPorte <dalaport@amazon.com>]

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