| 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
| ||
|
Message-ID: <CAGS+omAB-ngre8wX9Aq1vDUjYjcxsOeT1BtB7tdJSDAB6MLbDw@mail.gmail.com>
Date: Tue, 27 Mar 2012 01:58:47 +0800
From: Daniel Kurtz <djkurtz@...omium.org>
To: Daniel Kurtz <djkurtz@...omium.org>,
Keith Packard <keithp@...thp.com>,
David Airlie <airlied@...ux.ie>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Chris Wilson <chris@...is-wilson.co.uk>,
Benson Leung <bleung@...omium.org>,
Yufeng Shen <miletus@...omium.org>
Cc: Daniel Vetter <daniel@...ll.ch>
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:49 PM, Daniel Vetter <daniel@...ll.ch> wrote:
> 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?
An i2c adapter's functionality is provided by its algorithm.
Since these gmbus adapters can [for now] change their algorithm at
runtime, I thought the functionality returned should match the
currently selected algorithm at any given moment.
Arguably, the adapter actually sort of provides the union of the two
functionalities since if a particular transfer fails using gmbus, it
gets retried using bit-banged. But then again, this is a one-shot
permanent switch, so perhaps we should return the union of the
functionalities if force_bit == 0, and then only the bit-algo
functionality after the switch?
>
> 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