lkml.org 
[lkml]   [2012]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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@ffwll.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@ffwll.ch>
    >
    > Yours, Daniel
    >
    >>
    >> Signed-off-by: Daniel Kurtz <djkurtz@chromium.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@ffwll.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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-26 20:01    [W:0.039 / U:61.232 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site