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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120326144919.GP4014@phenom.ffwll.local>
Date:	Mon, 26 Mar 2012 16:49:19 +0200
From:	Daniel Vetter <daniel@...ll.ch>
To:	Daniel Kurtz <djkurtz@...omium.org>
Cc:	Keith Packard <keithp@...thp.com>, David Airlie <airlied@...ux.ie>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	Daniel Vetter <daniel@...ll.ch>,
	Chris Wilson <chris@...is-wilson.co.uk>,
	Benson Leung <bleung@...omium.org>,
	Yufeng Shen <miletus@...omium.org>
Subject: Re: [PATCH 03/11 v3] drm/i915/intel_i2c: use i2c pre/post_xfer
 functions to setup gpio xfers

On Mon, Mar 26, 2012 at 10:26:42PM +0800, Daniel Kurtz wrote:
> Instead of rolling our own custom quirk_xfer function, use the bit_algo
> pre_xfer and post_xfer functions to setup and teardown bit-banged
> i2c transactions.
> 
> gmbus_xfer uses .force_bit to determine which i2c_algorithm to use,
> either i2c_bit_algo.master_xfer or its own.  So, Similarly, let gmbus_func
> use .force_bit to determine which i2c functionalities are available,
> either i2c_bit_algo.functionality, or its own.

Please split this part of the patch into a separate patch. Furthermore I'm
not sure what this should buy us, given that we might magically changes
our i2c feature set once with gone to fallback mode. Can you please
elaborate why we need this?

The pre_xfer/post_xfer stuff looks good to me, that part along is
Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>

Yours, Daniel

> 
> Signed-off-by: Daniel Kurtz <djkurtz@...omium.org>
> ---
>  drivers/gpu/drm/i915/intel_i2c.c |   72 +++++++++++++++++++++++--------------
>  1 files changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 54f85a1..ae08a08 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -137,6 +137,35 @@ static void set_data(void *data, int state_high)
>  	POSTING_READ(bus->gpio_reg);
>  }
>  
> +static int
> +intel_gpio_pre_xfer(struct i2c_adapter *adapter)
> +{
> +	struct intel_gmbus *bus = container_of(adapter,
> +					       struct intel_gmbus,
> +					       adapter);
> +	struct drm_i915_private *dev_priv = bus->dev_priv;
> +
> +	intel_i2c_reset(dev_priv->dev);
> +	intel_i2c_quirk_set(dev_priv, true);
> +	set_data(bus, 1);
> +	set_clock(bus, 1);
> +	udelay(I2C_RISEFALL_TIME);
> +	return 0;
> +}
> +
> +static void
> +intel_gpio_post_xfer(struct i2c_adapter *adapter)
> +{
> +	struct intel_gmbus *bus = container_of(adapter,
> +					       struct intel_gmbus,
> +					       adapter);
> +	struct drm_i915_private *dev_priv = bus->dev_priv;
> +
> +	set_data(bus, 1);
> +	set_clock(bus, 1);
> +	intel_i2c_quirk_set(dev_priv, false);
> +}
> +
>  static bool
>  intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
>  {
> @@ -166,6 +195,8 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
>  	algo->setscl = set_clock;
>  	algo->getsda = get_data;
>  	algo->getscl = get_clock;
> +	algo->pre_xfer = intel_gpio_pre_xfer;
> +	algo->post_xfer = intel_gpio_post_xfer;
>  	algo->udelay = I2C_RISEFALL_TIME;
>  	algo->timeout = usecs_to_jiffies(2200);
>  	algo->data = bus;
> @@ -174,30 +205,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
>  }
>  
>  static int
> -intel_i2c_quirk_xfer(struct intel_gmbus *bus,
> -		     struct i2c_msg *msgs,
> -		     int num)
> -{
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> -	int ret;
> -
> -	intel_i2c_reset(dev_priv->dev);
> -
> -	intel_i2c_quirk_set(dev_priv, true);
> -	set_data(bus, 1);
> -	set_clock(bus, 1);
> -	udelay(I2C_RISEFALL_TIME);
> -
> -	ret = i2c_bit_algo.master_xfer(&bus->adapter, msgs, num);
> -
> -	set_data(bus, 1);
> -	set_clock(bus, 1);
> -	intel_i2c_quirk_set(dev_priv, false);
> -
> -	return ret;
> -}
> -
> -static int
>  gmbus_xfer(struct i2c_adapter *adapter,
>  	   struct i2c_msg *msgs,
>  	   int num)
> @@ -211,7 +218,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
>  	mutex_lock(&dev_priv->gmbus_mutex);
>  
>  	if (bus->force_bit) {
> -		ret = intel_i2c_quirk_xfer(bus, msgs, num);
> +		ret = i2c_bit_algo.master_xfer(adapter, msgs, num);
>  		goto out;
>  	}
>  
> @@ -325,8 +332,9 @@ timeout:
>  		ret = -EIO;
>  	} else {
>  		bus->force_bit = true;
> -		ret = intel_i2c_quirk_xfer(bus, msgs, num);
> +		ret = i2c_bit_algo.master_xfer(adapter, msgs, num);
>  	}
> +
>  out:
>  	mutex_unlock(&dev_priv->gmbus_mutex);
>  	return ret;
> @@ -334,11 +342,21 @@ out:
>  
>  static u32 gmbus_func(struct i2c_adapter *adapter)
>  {
> -	return i2c_bit_algo.functionality(adapter) &
> +	struct intel_gmbus *bus = container_of(adapter,
> +					       struct intel_gmbus,
> +					       adapter);
> +	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	u32 func;
> +
> +	mutex_lock(&dev_priv->gmbus_mutex);
> +	func = bus->force_bit ? i2c_bit_algo.functionality(adapter) :
>  		(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
>  		/* I2C_FUNC_10BIT_ADDR | */
>  		I2C_FUNC_SMBUS_READ_BLOCK_DATA |
>  		I2C_FUNC_SMBUS_BLOCK_PROC_CALL);
> +	mutex_unlock(&dev_priv->gmbus_mutex);
> +
> +	return func;
>  }
>  
>  static const struct i2c_algorithm gmbus_algorithm = {
> -- 
> 1.7.7.3
> 

-- 
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ