[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar C D]

This RFC patch is w.r.t multimaster bus arbitration which is already
being discussing in the mainline.
This patch provides hooks for the i2c multimaster bus arbitration
and to have the arbitration parameters.

Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/i2c/i2c-core.c |   13 ++++++++++++-
 include/linux/i2c.h    |   12 ++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..ed89fc8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1408,18 +1408,29 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 				(msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : "");
 		}
 #endif
+		if (adap->mm_arbit_algo) {
+			ret = adap->mm_arbit_algo->i2c_mm_bus_get(adap);
+			if (ret)
+				/* I2C bus under control of another master. */
+				return -EAGAIN;
+		}
 
 		if (in_atomic() || irqs_disabled()) {
 			ret = i2c_trylock_adapter(adap);
-			if (!ret)
+			if (!ret && adap->mm_arbit_algo) {
+				adap->mm_arbit_algo->i2c_mm_bus_release(adap);
+
 				/* I2C activity is ongoing. */
 				return -EAGAIN;
+			}
 		} else {
 			i2c_lock_adapter(adap);
 		}
 
 		ret = __i2c_transfer(adap, msgs, num);
 		i2c_unlock_adapter(adap);
+		if (adap->mm_arbit_algo)
+			adap->mm_arbit_algo->i2c_mm_bus_release(adap);
 
 		return ret;
 	} else {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d0c4db7..61fbfe3 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -371,6 +371,17 @@ struct i2c_algorithm {
 };
 
 /*
+ *This struct provides hooks for i2c multi-master arbitration needs.
+ */
+struct i2c_mm_arbitration {
+	void *arbitration_data;
+
+	/* Should return 0 if mastership could be successfully established */
+	int (*i2c_mm_bus_get)(struct i2c_adapter *adap);
+	void (*i2c_mm_bus_release)(struct i2c_adapter *adap);
+};
+
+/*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
  */
@@ -393,6 +404,7 @@ struct i2c_adapter {
 
 	struct mutex userspace_clients_lock;
 	struct list_head userspace_clients;
+	struct i2c_mm_arbitration *mm_arbit_algo;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
1.7.9.5

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

Below are the links on multi master bus arbitration where it has been
discussing.

 http://comments.gmane.org/gmane.linux.kernel/1410276

 http://www.kernelhub.org/?msg=179505&p=2


On Mon, Feb 4, 2013 at 2:33 PM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote:
> This RFC patch is w.r.t multimaster bus arbitration which is already
> being discussing in the mainline.
> This patch provides hooks for the i2c multimaster bus arbitration
> and to have the arbitration parameters.
>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> ---
>  drivers/i2c/i2c-core.c |   13 ++++++++++++-
>  include/linux/i2c.h    |   12 ++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..ed89fc8 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1408,18 +1408,29 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>                                 (msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : "");
>                 }
>  #endif
> +               if (adap->mm_arbit_algo) {
> +                       ret = adap->mm_arbit_algo->i2c_mm_bus_get(adap);
> +                       if (ret)
> +                               /* I2C bus under control of another master. */
> +                               return -EAGAIN;
> +               }
>
>                 if (in_atomic() || irqs_disabled()) {
>                         ret = i2c_trylock_adapter(adap);
> -                       if (!ret)
> +                       if (!ret && adap->mm_arbit_algo) {
> +                               adap->mm_arbit_algo->i2c_mm_bus_release(adap);
> +
>                                 /* I2C activity is ongoing. */
>                                 return -EAGAIN;
> +                       }
>                 } else {
>                         i2c_lock_adapter(adap);
>                 }
>
>                 ret = __i2c_transfer(adap, msgs, num);
>                 i2c_unlock_adapter(adap);
> +               if (adap->mm_arbit_algo)
> +                       adap->mm_arbit_algo->i2c_mm_bus_release(adap);
>
>                 return ret;
>         } else {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d0c4db7..61fbfe3 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -371,6 +371,17 @@ struct i2c_algorithm {
>  };
>
>  /*
> + *This struct provides hooks for i2c multi-master arbitration needs.
> + */
> +struct i2c_mm_arbitration {
> +       void *arbitration_data;
> +
> +       /* Should return 0 if mastership could be successfully established */
> +       int (*i2c_mm_bus_get)(struct i2c_adapter *adap);
> +       void (*i2c_mm_bus_release)(struct i2c_adapter *adap);
> +};
> +
> +/*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
>   * with the access algorithms necessary to access it.
>   */
> @@ -393,6 +404,7 @@ struct i2c_adapter {
>
>         struct mutex userspace_clients_lock;
>         struct list_head userspace_clients;
> +       struct i2c_mm_arbitration *mm_arbit_algo;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>
> --
> 1.7.9.5
>

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

Yavaraj,

On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote:
>
> This RFC patch is w.r.t multimaster bus arbitration which is already
> being discussing in the mainline.
> This patch provides hooks for the i2c multimaster bus arbitration
> and to have the arbitration parameters.

I may have missed something in all the threads, but my recollection
was that the request was that this should be implemented without
touching the i2c core code.  Your patch modifies the i2c core code.

My impression was that the best solution was to use the infrastructure
in place for i2c multiplexing.  ...but in your case you would only
multiplex one thing.  This was suggested by Grant Likely and seems the
cleanest...


[apologies for double-email; darn gmail and HTML email]

-Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Yavaraj,
>
> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> wrote:
>>
>> This RFC patch is w.r.t multimaster bus arbitration which is already
>> being discussing in the mainline.
>> This patch provides hooks for the i2c multimaster bus arbitration
>> and to have the arbitration parameters.
>
>
> I may have missed something in all the threads, but my recollection was that
> the request was that this should be implemented without touching the i2c
> core code.  Your patch modifies the i2c core code.
   Different i2c devices will directly talk to i2c core using i2c_transfer().
   If we put this out of the i2c core,the i2c devices like tpsxxxx and etc
   should be registered through the bridge/mux .
>
> My impression was that the best solution was to use the infrastructure in
> place for i2c multiplexing.  ...but in your case you would only multiplex
> one thing.  This was suggested by Grant Likely and seems the cleanest...
   I agree that Grants idea was the cleanest but we may end up in change in

>
> -Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

On Wed, Feb 6, 2013 at 10:58 AM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote:
> On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders@chromium.org> wrote:
>> Yavaraj,
>>
>> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com>
>> wrote:
>>>
>>> This RFC patch is w.r.t multimaster bus arbitration which is already
>>> being discussing in the mainline.
>>> This patch provides hooks for the i2c multimaster bus arbitration
>>> and to have the arbitration parameters.
>>
>>
>> I may have missed something in all the threads, but my recollection was that
>> the request was that this should be implemented without touching the i2c
>> core code.  Your patch modifies the i2c core code.

   Different i2c devices will directly talk to i2c core using i2c_transfer().
    If we put this out of the i2c core,the i2c devices like tpsxxxx and etc
    should be registered through the bridge/mux .

>>
>> My impression was that the best solution was to use the infrastructure in
>> place for i2c multiplexing.  ...but in your case you would only multiplex
>> one thing.  This was suggested by Grant Likely and seems the cleanest...
>
     I agree that Grants idea was the cleanest but we may end up in change in
      i2c device probe to register through mux/bridge.(for the
device's connected on that
     particular bus which requires arbitration).
>
>>
>> -Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

Hi,

On Tue, Feb 5, 2013 at 9:33 PM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote:
> On Wed, Feb 6, 2013 at 10:58 AM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote:
>> On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders@chromium.org> wrote:
>>> Yavaraj,
>>>
>>> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com>
>>> wrote:
>>>>
>>>> This RFC patch is w.r.t multimaster bus arbitration which is already
>>>> being discussing in the mainline.
>>>> This patch provides hooks for the i2c multimaster bus arbitration
>>>> and to have the arbitration parameters.
>>>
>>>
>>> I may have missed something in all the threads, but my recollection was that
>>> the request was that this should be implemented without touching the i2c
>>> core code.  Your patch modifies the i2c core code.
>
>    Different i2c devices will directly talk to i2c core using i2c_transfer().
>     If we put this out of the i2c core,the i2c devices like tpsxxxx and etc
>     should be registered through the bridge/mux .
>
>>>
>>> My impression was that the best solution was to use the infrastructure in
>>> place for i2c multiplexing.  ...but in your case you would only multiplex
>>> one thing.  This was suggested by Grant Likely and seems the cleanest...
>>
>      I agree that Grants idea was the cleanest but we may end up in change in
>       i2c device probe to register through mux/bridge.(for the
> device's connected on that
>      particular bus which requires arbitration).

Does the device tree not handle that case automatically? How are
muxes/bridges done in that case? It's not problem to adjust the device
tree file accordingly if Grant's approach is the cleanest method.

Regards,
Simon

>>
>>>
>>> -Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote:
> This RFC patch is w.r.t multimaster bus arbitration which is already
> being discussing in the mainline.
> This patch provides hooks for the i2c multimaster bus arbitration
> and to have the arbitration parameters.

I still haven't understood why the arbitration specified in the I2C
standard is not enough for you. Or what you would need to make use of
it.

Thanks,

   Wolfram

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

Wolfram,

On Sun, Feb 10, 2013 at 10:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote:
>> This RFC patch is w.r.t multimaster bus arbitration which is already
>> being discussing in the mainline.
>> This patch provides hooks for the i2c multimaster bus arbitration
>> and to have the arbitration parameters.
>
> I still haven't understood why the arbitration specified in the I2C
> standard is not enough for you. Or what you would need to make use of
> it.

At the moment perhaps the strongest argument for why this particular
arbitration scheme is needed is that's what the EC (embedded
controller) on the ARM Chromebook uses.  There have been several
arguments in-house about whether this was the most ideal way to
structure things, but that's what we shipped with.  Thus, if we want
to be able to talk to i2c devices on this bus (contains the keyboard,
battery, and a bunch of power switches) we need something in the
system that implements this arbitration scheme.

As a side note: I'm trying to code up a quick arbitrator now using the
muxing framework as Grant suggested.  I will play with this and post
it if I can get it to work.  It's quite short so far.

-Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

On Mon, Feb 11, 2013 at 3:53 PM, Doug Anderson <dianders@chromium.org> wrote:
> Wolfram,
>
> On Sun, Feb 10, 2013 at 10:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote:
>>> This RFC patch is w.r.t multimaster bus arbitration which is already
>>> being discussing in the mainline.
>>> This patch provides hooks for the i2c multimaster bus arbitration
>>> and to have the arbitration parameters.
>>
>> I still haven't understood why the arbitration specified in the I2C
>> standard is not enough for you. Or what you would need to make use of
>> it.
>
> At the moment perhaps the strongest argument for why this particular
> arbitration scheme is needed is that's what the EC (embedded
> controller) on the ARM Chromebook uses.  There have been several
> arguments in-house about whether this was the most ideal way to
> structure things, but that's what we shipped with.  Thus, if we want
> to be able to talk to i2c devices on this bus (contains the keyboard,
> battery, and a bunch of power switches) we need something in the
> system that implements this arbitration scheme.
>

There was some discussion about using the built-in I2C multi-master
support but no one had had a good experience getting it to work
reliably in a real system so we ended up deciding not to rely on this.

> As a side note: I'm trying to code up a quick arbitrator now using the
> muxing framework as Grant suggested.  I will play with this and post
> it if I can get it to work.  It's quite short so far.
>
> -Doug

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

Hi,

> > At the moment perhaps the strongest argument for why this particular
> > arbitration scheme is needed is that's what the EC (embedded
> > controller) on the ARM Chromebook uses.  There have been several
> > arguments in-house about whether this was the most ideal way to
> > structure things, but that's what we shipped with.  Thus, if we want
> > to be able to talk to i2c devices on this bus (contains the keyboard,
> > battery, and a bunch of power switches) we need something in the
> > system that implements this arbitration scheme.
> >
> 
> There was some discussion about using the built-in I2C multi-master
> support but no one had had a good experience getting it to work
> reliably in a real system so we ended up deciding not to rely on this.

So, were there no experiences at all or bad experiences when trying? I
am interested in hearing what I2C multi master restrictions might exist
in practice.

Thanks,

   Wolfram