On Fri, Mar 30, 2012 at 07:46:39PM +0800, Daniel Kurtz wrote:
> The i915 is only able to generate a STOP cycle (i.e. finalize an i2c
> transaction) during a DATA or WAIT phase. In other words, the
> controller rejects a STOP requested as part of the first transaction in a
> sequence.
>
> Thus, for the first transaction we must always use a WAIT cycle, detect
> when the device has finished (and is in a WAIT phase), and then either
> start the next transaction, or, if there are no more transactions,
> generate a STOP cycle.
>
> Note: Theoretically, the last transaction of a multi-transaction sequence
> could initiate a STOP cycle. However, this slight optimization is left
> for another patch. We return -ETIMEDOUT if the hardware doesn't
> deactivate after the STOP cycle.
>
> Signed-off-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
I've re-read gmbus register spec and STOP seems to be allowed even in the
first cycle. Does this patch solve an issue for you? If not, I prefer we
just drop it.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_i2c.c | 30 +++++++++++++++---------------
> 1 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 9707868..25f2c9e 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -204,8 +204,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
> }
>
> static int
> -gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> - bool last)
> +gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
> {
> int reg_offset = dev_priv->gpio_mmio_base;
> u16 len = msg->len;
> @@ -213,7 +212,6 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>
> I915_WRITE(GMBUS1 + reg_offset,
> GMBUS_CYCLE_WAIT |
> - (last ? GMBUS_CYCLE_STOP : 0) |
> (len << GMBUS_BYTE_COUNT_SHIFT) |
> (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) |
> GMBUS_SLAVE_READ | GMBUS_SW_RDY);
> @@ -239,8 +237,7 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> }
>
> static int
> -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> - bool last)
> +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
> {
> int reg_offset = dev_priv->gpio_mmio_base;
> u16 len = msg->len;
> @@ -256,7 +253,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
> I915_WRITE(GMBUS3 + reg_offset, val);
> I915_WRITE(GMBUS1 + reg_offset,
> GMBUS_CYCLE_WAIT |
> - (last ? GMBUS_CYCLE_STOP : 0) |
> (msg->len << GMBUS_BYTE_COUNT_SHIFT) |
> (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) |
> GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
> @@ -289,7 +285,8 @@ gmbus_xfer(struct i2c_adapter *adapter,
> struct intel_gmbus,
> adapter);
> struct drm_i915_private *dev_priv = bus->dev_priv;
> - int i, reg_offset, ret;
> + int i, reg_offset;
> + int ret = 0;
>
> mutex_lock(&dev_priv->gmbus_mutex);
>
> @@ -303,20 +300,17 @@ gmbus_xfer(struct i2c_adapter *adapter,
> I915_WRITE(GMBUS0 + reg_offset, bus->reg0);
>
> for (i = 0; i < num; i++) {
> - bool last = i + 1 == num;
> -
> if (msgs[i].flags & I2C_M_RD)
> - ret = gmbus_xfer_read(dev_priv, &msgs[i], last);
> + ret = gmbus_xfer_read(dev_priv, &msgs[i]);
> else
> - ret = gmbus_xfer_write(dev_priv, &msgs[i], last);
> + ret = gmbus_xfer_write(dev_priv, &msgs[i]);
>
> if (ret == -ETIMEDOUT)
> goto timeout;
> if (ret == -ENXIO)
> goto clear_err;
>
> - if (!last &&
> - wait_for(I915_READ(GMBUS2 + reg_offset) &
> + if (wait_for(I915_READ(GMBUS2 + reg_offset) &
> (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE),
> 50))
> goto timeout;
> @@ -324,15 +318,21 @@ gmbus_xfer(struct i2c_adapter *adapter,
> goto clear_err;
> }
>
> + /* Generate a STOP condition on the bus */
> + I915_WRITE(GMBUS1 + reg_offset, GMBUS_CYCLE_STOP | GMBUS_SW_RDY);
> +
> /* 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))
> + if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
> + 10)) {
> DRM_INFO("GMBUS [%s] timed out waiting for idle\n",
> adapter->name);
> + ret = -ETIMEDOUT;
> + }
> I915_WRITE(GMBUS0 + reg_offset, 0);
> - ret = i;
> + ret = ret ?: i;
> goto out;
>
> clear_err:
> --
> 1.7.7.3
>
--
Daniel Vetter
Mail: daniel@xxxxxxxx
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
