linux-kernel - Re: [PATCH] drm/i915: Fix single msg gmbus_xfers writes

Open Source and information security mailing list archives

Message-ID: <20120215104837.GB4095@phenom.ffwll.local>
Date:	Wed, 15 Feb 2012 11:48:37 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Benson Leung <bleung@...omium.org>
Cc:	keithp@...thp.com, airlied@...ux.ie, chris@...is-wilson.co.uk,
	djkurtz@...omium.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/i915: Fix single msg gmbus_xfers writes

On Thu, Feb 09, 2012 at 12:03:17PM -0800, Benson Leung wrote:
> gmbus_xfer with a single message (particularly a single message write) would
> set Bus Cycle Select to 100b, the Gen Stop cycle, instead of 101b,
> No Index, Stop cycle. This would not start single message i2c transactions.
> 
> Also, gmbus_xfer done: will disable the interface without checking if
> it is idle. In the case of writes, there will be no wait on status or delay
> to ensure the write starts and completes before the interface is turned off.
> 
> Fixed the former issue by using the same cycle selection as used in the
> I2C_M_RD for the write case.
> GMBUS_CYCLE_WAIT | (i + 1 == num ? GMBUS_CYCLE_STOP : 0)
> Fixed the latter by waiting on GMBUS_ACTIVE to deassert before disable.
> 
> Signed-off-by: Benson Leung <bleung@...omium.org>

Can you clarify the commit message a bit and say that the first hunk is
just for optics and the issue is only with the write path (because the
read path is correct already). Silly me is just to easily confused ;-)

Btw, I've reworked the gmbus -> gpio bit-banging fallback code a bit and
if that passes review and all I'll reenable gmbus by default again. See

http://cgit.freedesktop.org/~danvet/drm/log/?h=gmbus

Yours, Daniel

> ---
>  drivers/gpu/drm/i915/intel_i2c.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index d30cccc..64bb9cd 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -249,7 +249,8 @@ gmbus_xfer(struct i2c_adapter *adapter,
>  
>  		if (msgs[i].flags & I2C_M_RD) {
>  			I915_WRITE(GMBUS1 + reg_offset,
> -				   GMBUS_CYCLE_WAIT | (i + 1 == num ? GMBUS_CYCLE_STOP : 0) |
> +				   GMBUS_CYCLE_WAIT |
> +				   (i + 1 == num ? GMBUS_CYCLE_STOP : 0) |
>  				   (len << GMBUS_BYTE_COUNT_SHIFT) |
>  				   (msgs[i].addr << GMBUS_SLAVE_ADDR_SHIFT) |
>  				   GMBUS_SLAVE_READ | GMBUS_SW_RDY);
> @@ -278,7 +279,8 @@ gmbus_xfer(struct i2c_adapter *adapter,
>  
>  			I915_WRITE(GMBUS3 + reg_offset, val);
>  			I915_WRITE(GMBUS1 + reg_offset,
> -				   (i + 1 == num ? GMBUS_CYCLE_STOP : GMBUS_CYCLE_WAIT) |
> +				   GMBUS_CYCLE_WAIT |
> +				   (i + 1 == num ? GMBUS_CYCLE_STOP : 0) |
>  				   (msgs[i].len << GMBUS_BYTE_COUNT_SHIFT) |
>  				   (msgs[i].addr << GMBUS_SLAVE_ADDR_SHIFT) |
>  				   GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
> @@ -317,9 +319,12 @@ clear_err:
>  	I915_WRITE(GMBUS1 + reg_offset, 0);
>  
>  done:
> -	/* Mark the GMBUS interface as disabled. We will re-enable it at the
> -	 * start of the next xfer, till then let it sleep.
> +	/* 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 timed out waiting for idle\n");
>  	I915_WRITE(GMBUS0 + reg_offset, 0);
>  	return i;
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Mail: daniel@...ll.ch
Mobile: +41 (0)79 365 57 48
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives