Re: [PATCH 5/5] drm/i915/gmbus: Reset the controller on initialisation

[Ben Widawsky <ben@bwidawsk.net>]

On Tue, Apr 05, 2011 at 10:24:18AM +0100, Chris Wilson wrote:
> Toggle the Software Clear Interrupt bit which resets the controller to
> clear any prior BUS_ERROR condition before we begin to use the
> controller in earnest.
> 
> Suggested-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_i2c.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index d3b903b..f9f0c42 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -55,10 +55,18 @@ void
>  intel_i2c_reset(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int reg_offset;
> +
> +	reg_offset = 0;
>  	if (HAS_PCH_SPLIT(dev))
> -		I915_WRITE(PCH_GMBUS0, 0);
> -	else
> -		I915_WRITE(GMBUS0, 0);
> +		reg_offset = PCH_GMBUS0 - GMBUS0;
> +
> +	/* First reset the controller by toggling the Sw Clear Interrupt. */
> +	I915_WRITE(GMBUS1 + reg_offset, GMBUS_SW_CLR_INT);
> +	I915_WRITE(GMBUS1 + reg_offset, 0);
> +
> +	/* Then mark the controller as disabled. */
> +	I915_WRITE(GMBUS0 + reg_offset, 0);
>  }
>  

I think in addition to this we should try to send a STOP condition.
That way any devices in an unknown state from a failed reset (or
something similar), should go back to their default state after seeing a
stop. As long as the host is always the master, and the devices are
fairly compliant, it should work...  Alas, I'm just looking at the GMBUS
interface for the first time really, and it appears there is no straight
forward way to do what I want outside of bitbanging (you want to avoid
toggling SCL until you've issues the STOP).

Outside of being able to do that directly, I would add a call to
intel_i2c_reset() from intel_teardown_gmbus().

Ben