drm/i915/intel_i2c: always wait for IDLE before clearing NAK

The GMBUS controller can report a NAK condition while a transaction is
still active. If the driver is fast enough, and the bus is slow enough,
the driver may clear the NAK condition while the controller is still
busy, resulting in a confused GMBUS controller.  This will leave the
controller in a bad state such that the next transaction may fail.

Also, return -ENXIO if a device NAKs a transaction.

Note: this patch also refactors gmbus_xfer to remove the "done" label.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 31 insertions, 10 deletions

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index f02e52aca746..9707868881ec 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -324,26 +324,47 @@ gmbus_xfer(struct i2c_adapter *adapter,
                         goto clear_err;
         }
 
-        goto done;
+        /* Mark the GMBUS interface as disabled after waiting for idle.
+         * We will re-enable it at the start of the next xfer,
+         * till then let it sleep.
+         */
+        if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10))
+                DRM_INFO("GMBUS [%s] timed out waiting for idle\n",
+                         adapter->name);
+        I915_WRITE(GMBUS0 + reg_offset, 0);
+        ret = i;
+        goto out;
 
 clear_err:
+        /*
+         * Wait for bus to IDLE before clearing NAK.
+         * If we clear the NAK while bus is still active, then it will stay
+         * active and the next transaction may fail.
+         */
+        if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
+                     10))
+                DRM_INFO("GMBUS [%s] timed out after NAK\n", adapter->name);
+
         /* Toggle the Software Clear Interrupt bit. This has the effect
          * of resetting the GMBUS controller and so clearing the
          * BUS_ERROR raised by the slave's NAK.
          */
         I915_WRITE(GMBUS1 + reg_offset, GMBUS_SW_CLR_INT);
         I915_WRITE(GMBUS1 + reg_offset, 0);
+        I915_WRITE(GMBUS0 + reg_offset, 0);
 
-done:
-        /* Mark the GMBUS interface as disabled after waiting for idle.
-         * We will re-enable it at the start of the next xfer,
-         * till then let it sleep.
+        DRM_DEBUG_DRIVER("GMBUS [%s] NAK for addr: %04x %c(%d)\n",
+                         adapter->name, msgs[i].addr,
+                         (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len);
+
+        /*
+         * If no ACK is received during the address phase of a transaction,
+         * the adapter must report -ENXIO.
+         * It is not clear what to return if no ACK is received at other times.
+         * So, we always return -ENXIO in all NAK cases, to ensure we send
+         * it at least during the one case that is specified.
          */
-        if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, 10))
-                DRM_INFO("GMBUS [%s] timed out waiting for idle\n",
-                         bus->adapter.name);
-        I915_WRITE(GMBUS0 + reg_offset, 0);
-        ret = i;
+        ret = -ENXIO;
         goto out;
 
 timeout: