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

Benoît Monin
2026-05-06T08:57:29Z

Hi David,

On Wednesday, 6 May 2026 at 01:22:42 CEST, David LaPorte wrote:
> 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.
>
So Keem Bay hardware can get a STOP_DET interrupt while a read is in
progress? If that's the case then yes it is not a spurious STOP and
limiting the check to controllers that don't hold SCL on Tx FIFO underrun
should fix that.

Best regards,
--
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering