[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-28 Thread Jean Delvare 
On Mon, 27 Feb 2012 23:52:23 +0100, Daniel Vetter wrote:
> On Mon, Feb 27, 2012 at 11:20:40PM +0100, Jean Delvare wrote:
> > If you need to hot-switch between hardware and bit-banged I2C, I suggest
> > that you lock the bus while doing so, to avoid switching while a
> > transaction is in progress. This can be achieved with
> > i2c_lock_adapter() and i2c_unlock_adapter().
> 
> The drm/i915 xfer function is currently protected by a single mutex
> (because the hw i2c controller can only be used on one bus at a time). So
> I think we're covered. Also we do the fallback in our xfer function when
> we notice that things don't quite work as they should, so we actually want
> to switch while a transfer is in progress. Dunno whether that's the best
> approach, but the current code is structured like this.

This seems perfectly sane.

> (...)
> I've noticed that the the bit-banging algo does some test upon
> registration but figured that it's not worth the fuss to add a new init
> function that avoids the registration.

Note that thanks to the way things are implemented in i2c-algo-bit,
you'd really only have to write a new wrapper around
__i2c_bit_add_bus(), so this is really only 1 line of code. That being
said, I am not insisting, it's really up to you.

-- 
Jean Delvare

Re: [PATCH 3/7] i2c: export bit-banging algo functions

2012-02-28 Thread Jean Delvare 
On Mon, 27 Feb 2012 23:52:23 +0100, Daniel Vetter wrote:
 On Mon, Feb 27, 2012 at 11:20:40PM +0100, Jean Delvare wrote:
  If you need to hot-switch between hardware and bit-banged I2C, I suggest
  that you lock the bus while doing so, to avoid switching while a
  transaction is in progress. This can be achieved with
  i2c_lock_adapter() and i2c_unlock_adapter().
 
 The drm/i915 xfer function is currently protected by a single mutex
 (because the hw i2c controller can only be used on one bus at a time). So
 I think we're covered. Also we do the fallback in our xfer function when
 we notice that things don't quite work as they should, so we actually want
 to switch while a transfer is in progress. Dunno whether that's the best
 approach, but the current code is structured like this.

This seems perfectly sane.

 (...)
 I've noticed that the the bit-banging algo does some test upon
 registration but figured that it's not worth the fuss to add a new init
 function that avoids the registration.

Note that thanks to the way things are implemented in i2c-algo-bit,
you'd really only have to write a new wrapper around
__i2c_bit_add_bus(), so this is really only 1 line of code. That being
said, I am not insisting, it's really up to you.

-- 
Jean Delvare
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-27 Thread Daniel Vetter 
On Mon, Feb 27, 2012 at 11:20:40PM +0100, Jean Delvare wrote:
> Hi Daniel,
> 
> Sorry for the late reply.
> 
> On Tue, 14 Feb 2012 22:37:21 +0100, Daniel Vetter wrote:
> > i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
> > we need to be able to fall back to the bit-banging algo on gpio pins.
> > 
> > The current code sets up a 2nd i2c controller for the same i2c bus using
> > the bit-banging algo. This has a bunch of issues, the major one being
> > that userspace can directly access this fallback i2c adaptor behind
> > the drivers back.
> > 
> > But we need to frob a few registers before and after using fallback
> > gpio bit-banging, so this horribly fails.
> 
> You could use the pre_xfer and post_xfer hooks together with a shared
> mutex to solve the problem. But I agree its much cleaner to expose a
> single adapter in the first place.

Yeah, I've seen these and I think we could use them. Still it's cleaner to
only expose one algo for a single bus.

> If you need to hot-switch between hardware and bit-banged I2C, I suggest
> that you lock the bus while doing so, to avoid switching while a
> transaction is in progress. This can be achieved with
> i2c_lock_adapter() and i2c_unlock_adapter().

The drm/i915 xfer function is currently protected by a single mutex
(because the hw i2c controller can only be used on one bus at a time). So
I think we're covered. Also we do the fallback in our xfer function when
we notice that things don't quite work as they should, so we actually want
to switch while a transfer is in progress. Dunno whether that's the best
approach, but the current code is structured like this.

> > The new plan is to only set up one i2c adaptor and transparently fall
> > back to bit-banging by directly calling the xfer function of the bit-
> > banging algo in the i2c core.
> > 
> > To make that possible, export the 2 i2c algo functions.
> > 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
> >  include/linux/i2c-algo-bit.h |4 
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/i2c/algos/i2c-algo-bit.c 
> > b/drivers/i2c/algos/i2c-algo-bit.c
> > index 525c734..ec1651a 100644
> > --- a/drivers/i2c/algos/i2c-algo-bit.c
> > +++ b/drivers/i2c/algos/i2c-algo-bit.c
> > @@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
> > struct i2c_msg *msg)
> > return 0;
> >  }
> >  
> > -static int bit_xfer(struct i2c_adapter *i2c_adap,
> > -   struct i2c_msg msgs[], int num)
> > +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> > +struct i2c_msg msgs[], int num)
> >  {
> > struct i2c_msg *pmsg;
> > struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
> > @@ -598,21 +598,23 @@ bailout:
> > adap->post_xfer(i2c_adap);
> > return ret;
> >  }
> > +EXPORT_SYMBOL(i2c_bit_xfer);
> >  
> > -static u32 bit_func(struct i2c_adapter *adap)
> > +u32 i2c_bit_func(struct i2c_adapter *adap)
> >  {
> > return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> >I2C_FUNC_SMBUS_READ_BLOCK_DATA |
> >I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
> >I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> >  }
> > +EXPORT_SYMBOL(i2c_bit_func);
> >  
> >  
> >  /* -exported algorithm data: - 
> > */
> >  
> >  static const struct i2c_algorithm i2c_bit_algo = {
> > -   .master_xfer= bit_xfer,
> > -   .functionality  = bit_func,
> > +   .master_xfer= i2c_bit_xfer,
> > +   .functionality  = i2c_bit_func,
> >  };
> >  
> >  /*
> > diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
> > index 4f98148..cdd6336 100644
> > --- a/include/linux/i2c-algo-bit.h
> > +++ b/include/linux/i2c-algo-bit.h
> > @@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
> > int timeout;/* in jiffies */
> >  };
> >  
> > +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> > +struct i2c_msg msgs[], int num);
> > +u32 i2c_bit_func(struct i2c_adapter *adap);
> > +
> >  int i2c_bit_add_bus(struct i2c_adapter *);
> >  int i2c_bit_add_numbered_bus(struct i2c_adapter *);
> 
> I have no problem with this in the principle. In the details, I don't
> understand why you don't simply export i2c_bit_algo? This is one fewer
> export and should serve the same purpose - even allowing faster
> initializations in some cases.

I've figured that hunting for magic functions in a struct is a bit to
intransparent and hence exported the 2 functions I needed instead of the
struct. But if you prefer me to export the vtable I'll gladly do so - that
way I can drop a patch to rename some functions in drm/nouveau that
conflict with these here.

> Looking a bit more to the i2c-algo-bit code, I also notice that
> bypassing i2c_bit_add_bus() means you'll miss some of its features,
> namely bus testing, default retries value and warning on non-compliant
> buses. While none of these are mandatory, it may

[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-27 Thread Jean Delvare 
Hi Daniel,

Sorry for the late reply.

On Tue, 14 Feb 2012 22:37:21 +0100, Daniel Vetter wrote:
> i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
> we need to be able to fall back to the bit-banging algo on gpio pins.
> 
> The current code sets up a 2nd i2c controller for the same i2c bus using
> the bit-banging algo. This has a bunch of issues, the major one being
> that userspace can directly access this fallback i2c adaptor behind
> the drivers back.
> 
> But we need to frob a few registers before and after using fallback
> gpio bit-banging, so this horribly fails.

You could use the pre_xfer and post_xfer hooks together with a shared
mutex to solve the problem. But I agree its much cleaner to expose a
single adapter in the first place.

If you need to hot-switch between hardware and bit-banged I2C, I suggest
that you lock the bus while doing so, to avoid switching while a
transaction is in progress. This can be achieved with
i2c_lock_adapter() and i2c_unlock_adapter().

> The new plan is to only set up one i2c adaptor and transparently fall
> back to bit-banging by directly calling the xfer function of the bit-
> banging algo in the i2c core.
> 
> To make that possible, export the 2 i2c algo functions.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
>  include/linux/i2c-algo-bit.h |4 
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/algos/i2c-algo-bit.c 
> b/drivers/i2c/algos/i2c-algo-bit.c
> index 525c734..ec1651a 100644
> --- a/drivers/i2c/algos/i2c-algo-bit.c
> +++ b/drivers/i2c/algos/i2c-algo-bit.c
> @@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
> struct i2c_msg *msg)
>   return 0;
>  }
>  
> -static int bit_xfer(struct i2c_adapter *i2c_adap,
> - struct i2c_msg msgs[], int num)
> +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> +  struct i2c_msg msgs[], int num)
>  {
>   struct i2c_msg *pmsg;
>   struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
> @@ -598,21 +598,23 @@ bailout:
>   adap->post_xfer(i2c_adap);
>   return ret;
>  }
> +EXPORT_SYMBOL(i2c_bit_xfer);
>  
> -static u32 bit_func(struct i2c_adapter *adap)
> +u32 i2c_bit_func(struct i2c_adapter *adap)
>  {
>   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
>  I2C_FUNC_SMBUS_READ_BLOCK_DATA |
>  I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
>  I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>  }
> +EXPORT_SYMBOL(i2c_bit_func);
>  
>  
>  /* -exported algorithm data: -   
> */
>  
>  static const struct i2c_algorithm i2c_bit_algo = {
> - .master_xfer= bit_xfer,
> - .functionality  = bit_func,
> + .master_xfer= i2c_bit_xfer,
> + .functionality  = i2c_bit_func,
>  };
>  
>  /*
> diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
> index 4f98148..cdd6336 100644
> --- a/include/linux/i2c-algo-bit.h
> +++ b/include/linux/i2c-algo-bit.h
> @@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
>   int timeout;/* in jiffies */
>  };
>  
> +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> +  struct i2c_msg msgs[], int num);
> +u32 i2c_bit_func(struct i2c_adapter *adap);
> +
>  int i2c_bit_add_bus(struct i2c_adapter *);
>  int i2c_bit_add_numbered_bus(struct i2c_adapter *);

I have no problem with this in the principle. In the details, I don't
understand why you don't simply export i2c_bit_algo? This is one fewer
export and should serve the same purpose - even allowing faster
initializations in some cases.

Looking a bit more to the i2c-algo-bit code, I also notice that
bypassing i2c_bit_add_bus() means you'll miss some of its features,
namely bus testing, default retries value and warning on non-compliant
buses. While none of these are mandatory, it may make sense to export a
new function i2c_bit_add_numbered_bus() which would call
__i2c_bit_add_bus() with no callback function. If you do that, you may
not have to export anything else.

I leave it up to you which way you want to implement it, all are fine
with me, and we can always change later if more use cases show up which
would work better with a different implementation.

-- 
Jean Delvare

[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-27 Thread Daniel Vetter 
Hi Jean,

Can you please review and if you don't have any objectsion, ack this patch
for merging through drm-intel-next? Imo that's the easiest way to merge
this series.

Thanks, Daniel

On Tue, Feb 14, 2012 at 10:37:21PM +0100, Daniel Vetter wrote:
> i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
> we need to be able to fall back to the bit-banging algo on gpio pins.
> 
> The current code sets up a 2nd i2c controller for the same i2c bus using
> the bit-banging algo. This has a bunch of issues, the major one being
> that userspace can directly access this fallback i2c adaptor behind
> the drivers back.
> 
> But we need to frob a few registers before and after using fallback
> gpio bit-banging, so this horribly fails.
> 
> The new plan is to only set up one i2c adaptor and transparently fall
> back to bit-banging by directly calling the xfer function of the bit-
> banging algo in the i2c core.
> 
> To make that possible, export the 2 i2c algo functions.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
>  include/linux/i2c-algo-bit.h |4 
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/algos/i2c-algo-bit.c 
> b/drivers/i2c/algos/i2c-algo-bit.c
> index 525c734..ec1651a 100644
> --- a/drivers/i2c/algos/i2c-algo-bit.c
> +++ b/drivers/i2c/algos/i2c-algo-bit.c
> @@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
> struct i2c_msg *msg)
>   return 0;
>  }
>  
> -static int bit_xfer(struct i2c_adapter *i2c_adap,
> - struct i2c_msg msgs[], int num)
> +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> +  struct i2c_msg msgs[], int num)
>  {
>   struct i2c_msg *pmsg;
>   struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
> @@ -598,21 +598,23 @@ bailout:
>   adap->post_xfer(i2c_adap);
>   return ret;
>  }
> +EXPORT_SYMBOL(i2c_bit_xfer);
>  
> -static u32 bit_func(struct i2c_adapter *adap)
> +u32 i2c_bit_func(struct i2c_adapter *adap)
>  {
>   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
>  I2C_FUNC_SMBUS_READ_BLOCK_DATA |
>  I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
>  I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>  }
> +EXPORT_SYMBOL(i2c_bit_func);
>  
>  
>  /* -exported algorithm data: -   
> */
>  
>  static const struct i2c_algorithm i2c_bit_algo = {
> - .master_xfer= bit_xfer,
> - .functionality  = bit_func,
> + .master_xfer= i2c_bit_xfer,
> + .functionality  = i2c_bit_func,
>  };
>  
>  /*
> diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
> index 4f98148..cdd6336 100644
> --- a/include/linux/i2c-algo-bit.h
> +++ b/include/linux/i2c-algo-bit.h
> @@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
>   int timeout;/* in jiffies */
>  };
>  
> +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
> +  struct i2c_msg msgs[], int num);
> +u32 i2c_bit_func(struct i2c_adapter *adap);
> +
>  int i2c_bit_add_bus(struct i2c_adapter *);
>  int i2c_bit_add_numbered_bus(struct i2c_adapter *);
>  
> -- 
> 1.7.7.5
> 

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 3/7] i2c: export bit-banging algo functions

2012-02-27 Thread Jean Delvare 
Hi Daniel,

Sorry for the late reply.

On Tue, 14 Feb 2012 22:37:21 +0100, Daniel Vetter wrote:
 i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
 we need to be able to fall back to the bit-banging algo on gpio pins.
 
 The current code sets up a 2nd i2c controller for the same i2c bus using
 the bit-banging algo. This has a bunch of issues, the major one being
 that userspace can directly access this fallback i2c adaptor behind
 the drivers back.
 
 But we need to frob a few registers before and after using fallback
 gpio bit-banging, so this horribly fails.

You could use the pre_xfer and post_xfer hooks together with a shared
mutex to solve the problem. But I agree its much cleaner to expose a
single adapter in the first place.

If you need to hot-switch between hardware and bit-banged I2C, I suggest
that you lock the bus while doing so, to avoid switching while a
transaction is in progress. This can be achieved with
i2c_lock_adapter() and i2c_unlock_adapter().

 The new plan is to only set up one i2c adaptor and transparently fall
 back to bit-banging by directly calling the xfer function of the bit-
 banging algo in the i2c core.
 
 To make that possible, export the 2 i2c algo functions.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
  include/linux/i2c-algo-bit.h |4 
  2 files changed, 11 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/i2c/algos/i2c-algo-bit.c 
 b/drivers/i2c/algos/i2c-algo-bit.c
 index 525c734..ec1651a 100644
 --- a/drivers/i2c/algos/i2c-algo-bit.c
 +++ b/drivers/i2c/algos/i2c-algo-bit.c
 @@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
 struct i2c_msg *msg)
   return 0;
  }
  
 -static int bit_xfer(struct i2c_adapter *i2c_adap,
 - struct i2c_msg msgs[], int num)
 +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
 +  struct i2c_msg msgs[], int num)
  {
   struct i2c_msg *pmsg;
   struct i2c_algo_bit_data *adap = i2c_adap-algo_data;
 @@ -598,21 +598,23 @@ bailout:
   adap-post_xfer(i2c_adap);
   return ret;
  }
 +EXPORT_SYMBOL(i2c_bit_xfer);
  
 -static u32 bit_func(struct i2c_adapter *adap)
 +u32 i2c_bit_func(struct i2c_adapter *adap)
  {
   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
  I2C_FUNC_SMBUS_READ_BLOCK_DATA |
  I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
  I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
  }
 +EXPORT_SYMBOL(i2c_bit_func);
  
  
  /* -exported algorithm data: -   
 */
  
  static const struct i2c_algorithm i2c_bit_algo = {
 - .master_xfer= bit_xfer,
 - .functionality  = bit_func,
 + .master_xfer= i2c_bit_xfer,
 + .functionality  = i2c_bit_func,
  };
  
  /*
 diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
 index 4f98148..cdd6336 100644
 --- a/include/linux/i2c-algo-bit.h
 +++ b/include/linux/i2c-algo-bit.h
 @@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
   int timeout;/* in jiffies */
  };
  
 +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
 +  struct i2c_msg msgs[], int num);
 +u32 i2c_bit_func(struct i2c_adapter *adap);
 +
  int i2c_bit_add_bus(struct i2c_adapter *);
  int i2c_bit_add_numbered_bus(struct i2c_adapter *);

I have no problem with this in the principle. In the details, I don't
understand why you don't simply export i2c_bit_algo? This is one fewer
export and should serve the same purpose - even allowing faster
initializations in some cases.

Looking a bit more to the i2c-algo-bit code, I also notice that
bypassing i2c_bit_add_bus() means you'll miss some of its features,
namely bus testing, default retries value and warning on non-compliant
buses. While none of these are mandatory, it may make sense to export a
new function i2c_bit_add_numbered_bus() which would call
__i2c_bit_add_bus() with no callback function. If you do that, you may
not have to export anything else.

I leave it up to you which way you want to implement it, all are fine
with me, and we can always change later if more use cases show up which
would work better with a different implementation.

-- 
Jean Delvare
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] i2c: export bit-banging algo functions

2012-02-27 Thread Daniel Vetter 
On Mon, Feb 27, 2012 at 11:20:40PM +0100, Jean Delvare wrote:
 Hi Daniel,
 
 Sorry for the late reply.
 
 On Tue, 14 Feb 2012 22:37:21 +0100, Daniel Vetter wrote:
  i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
  we need to be able to fall back to the bit-banging algo on gpio pins.
  
  The current code sets up a 2nd i2c controller for the same i2c bus using
  the bit-banging algo. This has a bunch of issues, the major one being
  that userspace can directly access this fallback i2c adaptor behind
  the drivers back.
  
  But we need to frob a few registers before and after using fallback
  gpio bit-banging, so this horribly fails.
 
 You could use the pre_xfer and post_xfer hooks together with a shared
 mutex to solve the problem. But I agree its much cleaner to expose a
 single adapter in the first place.

Yeah, I've seen these and I think we could use them. Still it's cleaner to
only expose one algo for a single bus.

 If you need to hot-switch between hardware and bit-banged I2C, I suggest
 that you lock the bus while doing so, to avoid switching while a
 transaction is in progress. This can be achieved with
 i2c_lock_adapter() and i2c_unlock_adapter().

The drm/i915 xfer function is currently protected by a single mutex
(because the hw i2c controller can only be used on one bus at a time). So
I think we're covered. Also we do the fallback in our xfer function when
we notice that things don't quite work as they should, so we actually want
to switch while a transfer is in progress. Dunno whether that's the best
approach, but the current code is structured like this.

  The new plan is to only set up one i2c adaptor and transparently fall
  back to bit-banging by directly calling the xfer function of the bit-
  banging algo in the i2c core.
  
  To make that possible, export the 2 i2c algo functions.
  
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
   include/linux/i2c-algo-bit.h |4 
   2 files changed, 11 insertions(+), 5 deletions(-)
  
  diff --git a/drivers/i2c/algos/i2c-algo-bit.c 
  b/drivers/i2c/algos/i2c-algo-bit.c
  index 525c734..ec1651a 100644
  --- a/drivers/i2c/algos/i2c-algo-bit.c
  +++ b/drivers/i2c/algos/i2c-algo-bit.c
  @@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
  struct i2c_msg *msg)
  return 0;
   }
   
  -static int bit_xfer(struct i2c_adapter *i2c_adap,
  -   struct i2c_msg msgs[], int num)
  +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
  +struct i2c_msg msgs[], int num)
   {
  struct i2c_msg *pmsg;
  struct i2c_algo_bit_data *adap = i2c_adap-algo_data;
  @@ -598,21 +598,23 @@ bailout:
  adap-post_xfer(i2c_adap);
  return ret;
   }
  +EXPORT_SYMBOL(i2c_bit_xfer);
   
  -static u32 bit_func(struct i2c_adapter *adap)
  +u32 i2c_bit_func(struct i2c_adapter *adap)
   {
  return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
 I2C_FUNC_SMBUS_READ_BLOCK_DATA |
 I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
 I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
   }
  +EXPORT_SYMBOL(i2c_bit_func);
   
   
   /* -exported algorithm data: - 
  */
   
   static const struct i2c_algorithm i2c_bit_algo = {
  -   .master_xfer= bit_xfer,
  -   .functionality  = bit_func,
  +   .master_xfer= i2c_bit_xfer,
  +   .functionality  = i2c_bit_func,
   };
   
   /*
  diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
  index 4f98148..cdd6336 100644
  --- a/include/linux/i2c-algo-bit.h
  +++ b/include/linux/i2c-algo-bit.h
  @@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
  int timeout;/* in jiffies */
   };
   
  +int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
  +struct i2c_msg msgs[], int num);
  +u32 i2c_bit_func(struct i2c_adapter *adap);
  +
   int i2c_bit_add_bus(struct i2c_adapter *);
   int i2c_bit_add_numbered_bus(struct i2c_adapter *);
 
 I have no problem with this in the principle. In the details, I don't
 understand why you don't simply export i2c_bit_algo? This is one fewer
 export and should serve the same purpose - even allowing faster
 initializations in some cases.

I've figured that hunting for magic functions in a struct is a bit to
intransparent and hence exported the 2 functions I needed instead of the
struct. But if you prefer me to export the vtable I'll gladly do so - that
way I can drop a patch to rename some functions in drm/nouveau that
conflict with these here.

 Looking a bit more to the i2c-algo-bit code, I also notice that
 bypassing i2c_bit_add_bus() means you'll miss some of its features,
 namely bus testing, default retries value and warning on non-compliant
 buses. While none of these are mandatory, it may make sense to export a
 new function i2c_bit_add_numbered_bus() which would call
 __i2c_bit_add_bus() with no callback function. If you do that, you may
 not have to

[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-14 Thread Daniel Vetter 
i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
we need to be able to fall back to the bit-banging algo on gpio pins.

The current code sets up a 2nd i2c controller for the same i2c bus using
the bit-banging algo. This has a bunch of issues, the major one being
that userspace can directly access this fallback i2c adaptor behind
the drivers back.

But we need to frob a few registers before and after using fallback
gpio bit-banging, so this horribly fails.

The new plan is to only set up one i2c adaptor and transparently fall
back to bit-banging by directly calling the xfer function of the bit-
banging algo in the i2c core.

To make that possible, export the 2 i2c algo functions.

Signed-off-by: Daniel Vetter 
---
 drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
 include/linux/i2c-algo-bit.h |4 
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 525c734..ec1651a 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
struct i2c_msg *msg)
return 0;
 }

-static int bit_xfer(struct i2c_adapter *i2c_adap,
-   struct i2c_msg msgs[], int num)
+int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
+struct i2c_msg msgs[], int num)
 {
struct i2c_msg *pmsg;
struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
@@ -598,21 +598,23 @@ bailout:
adap->post_xfer(i2c_adap);
return ret;
 }
+EXPORT_SYMBOL(i2c_bit_xfer);

-static u32 bit_func(struct i2c_adapter *adap)
+u32 i2c_bit_func(struct i2c_adapter *adap)
 {
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
   I2C_FUNC_SMBUS_READ_BLOCK_DATA |
   I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
   I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
 }
+EXPORT_SYMBOL(i2c_bit_func);


 /* -exported algorithm data: - */

 static const struct i2c_algorithm i2c_bit_algo = {
-   .master_xfer= bit_xfer,
-   .functionality  = bit_func,
+   .master_xfer= i2c_bit_xfer,
+   .functionality  = i2c_bit_func,
 };

 /*
diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
index 4f98148..cdd6336 100644
--- a/include/linux/i2c-algo-bit.h
+++ b/include/linux/i2c-algo-bit.h
@@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
int timeout;/* in jiffies */
 };

+int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
+struct i2c_msg msgs[], int num);
+u32 i2c_bit_func(struct i2c_adapter *adap);
+
 int i2c_bit_add_bus(struct i2c_adapter *);
 int i2c_bit_add_numbered_bus(struct i2c_adapter *);

-- 
1.7.7.5

[PATCH 3/7] i2c: export bit-banging algo functions

2012-02-14 Thread Daniel Vetter 
i915 has a hw i2c controller (gmbus) but for a bunch of stupid reasons
we need to be able to fall back to the bit-banging algo on gpio pins.

The current code sets up a 2nd i2c controller for the same i2c bus using
the bit-banging algo. This has a bunch of issues, the major one being
that userspace can directly access this fallback i2c adaptor behind
the drivers back.

But we need to frob a few registers before and after using fallback
gpio bit-banging, so this horribly fails.

The new plan is to only set up one i2c adaptor and transparently fall
back to bit-banging by directly calling the xfer function of the bit-
banging algo in the i2c core.

To make that possible, export the 2 i2c algo functions.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/i2c/algos/i2c-algo-bit.c |   12 +++-
 include/linux/i2c-algo-bit.h |4 
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 525c734..ec1651a 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -531,8 +531,8 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, 
struct i2c_msg *msg)
return 0;
 }
 
-static int bit_xfer(struct i2c_adapter *i2c_adap,
-   struct i2c_msg msgs[], int num)
+int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
+struct i2c_msg msgs[], int num)
 {
struct i2c_msg *pmsg;
struct i2c_algo_bit_data *adap = i2c_adap-algo_data;
@@ -598,21 +598,23 @@ bailout:
adap-post_xfer(i2c_adap);
return ret;
 }
+EXPORT_SYMBOL(i2c_bit_xfer);
 
-static u32 bit_func(struct i2c_adapter *adap)
+u32 i2c_bit_func(struct i2c_adapter *adap)
 {
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
   I2C_FUNC_SMBUS_READ_BLOCK_DATA |
   I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
   I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
 }
+EXPORT_SYMBOL(i2c_bit_func);
 
 
 /* -exported algorithm data: - */
 
 static const struct i2c_algorithm i2c_bit_algo = {
-   .master_xfer= bit_xfer,
-   .functionality  = bit_func,
+   .master_xfer= i2c_bit_xfer,
+   .functionality  = i2c_bit_func,
 };
 
 /*
diff --git a/include/linux/i2c-algo-bit.h b/include/linux/i2c-algo-bit.h
index 4f98148..cdd6336 100644
--- a/include/linux/i2c-algo-bit.h
+++ b/include/linux/i2c-algo-bit.h
@@ -47,6 +47,10 @@ struct i2c_algo_bit_data {
int timeout;/* in jiffies */
 };
 
+int i2c_bit_xfer(struct i2c_adapter *i2c_adap,
+struct i2c_msg msgs[], int num);
+u32 i2c_bit_func(struct i2c_adapter *adap);
+
 int i2c_bit_add_bus(struct i2c_adapter *);
 int i2c_bit_add_numbered_bus(struct i2c_adapter *);
 
-- 
1.7.7.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel